Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DHCP relay]: Fix bug which could cause incorrect interface name association #1233

Merged
merged 4 commits into from
Dec 13, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,266 @@
From 42319f1b56ba6362c874cd64383a055ba6498bee Mon Sep 17 00:00:00 2001
From: Joe LeVeque <[email protected]>
Date: Mon, 11 Dec 2017 23:21:08 +0000
Subject: [PATCH 1/3] Customizable Option 82 circuit ID and remote ID fields

---
relay/dhcrelay.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 147 insertions(+), 25 deletions(-)

diff --git a/relay/dhcrelay.c b/relay/dhcrelay.c
index 15b4997..a26efca 100644
--- a/relay/dhcrelay.c
+++ b/relay/dhcrelay.c
@@ -73,6 +73,8 @@ int bad_circuit_id = 0; /* Circuit ID option in matching RAI option
did not match any known circuit ID. */
int missing_circuit_id = 0; /* Circuit ID option in matching RAI option
was missing. */
+const char *agent_circuit_id_fmt = NULL; /* Circuit ID custom format string. */
+const char *agent_remote_id_fmt = NULL; /* Remote ID custom format string. */
int max_hop_count = 10; /* Maximum hop count */

#ifdef DHCPv6
@@ -140,9 +142,19 @@ static const char message[] =
static const char url[] =
"For info, please visit https://www.isc.org/software/dhcp/";

+#define DHCRELAY_OPTION82_USAGE \
+"circuit_id/remote_id interpreted sequences are:\n" \
+"\n" \
+" %%%% A single %%\n" \
+" %%h Hostname of device\n" \
+" %%p Name of interface that generated the request\n" \
+" %%P Hardware address of interface that generated the request\n" \
+" %%C Client hardware address\n" \
+" %%I DHCP relay agent IP Address\n" \
+
#ifdef DHCPv6
#define DHCRELAY_USAGE \
-"Usage: dhcrelay [-4] [-d] [-q] [-a] [-D]\n"\
+"Usage: dhcrelay [-4] [-d] [-q] [-a <circuit_id> <remote_id>] [-D]\n"\
" [-A <length>] [-c <hops>] [-p <port>]\n" \
" [-pf <pid-file>] [--no-pid]\n"\
" [-m append|replace|forward|discard]\n" \
@@ -154,14 +166,15 @@ static const char url[] =
" -l lower0 [ ... -l lowerN]\n" \
" -u upper0 [ ... -u upperN]\n" \
" lower (client link): [address%%]interface[#index]\n" \
-" upper (server link): [address%%]interface"
+" upper (server link): [address%%]interface\n\n" DHCRELAY_OPTION82_USAGE
#else
#define DHCRELAY_USAGE \
-"Usage: dhcrelay [-d] [-q] [-a] [-D] [-A <length>] [-c <hops>] [-p <port>]\n" \
-" [-pf <pid-file>] [--no-pid]\n" \
+"Usage: dhcrelay [-d] [-q] [-a <circuit_id> <remote_id>] [-D]\n" \
+" [-A <length>] [-c <hops>] [-p <port>]\n" \
+" [-pf <pid-file>] [--no-pid]\n"\
" [-m append|replace|forward|discard]\n" \
" [-i interface0 [ ... -i interfaceN]\n" \
-" server0 [ ... serverN]\n\n"
+" server0 [ ... serverN]\n\n" DHCRELAY_OPTION82_USAGE
#endif

static void usage() {
@@ -287,6 +300,15 @@ main(int argc, char **argv) {
local_family_set = 1;
local_family = AF_INET;
#endif
+ if (++i == argc)
+ usage();
+
+ if (argv[i] != NULL && argv[i][0] != '-')
+ agent_circuit_id_fmt = argv[i++];
+
+ if (argv[i] != NULL && argv[i][0] != '-')
+ agent_remote_id_fmt = argv[i];
+
add_agent_options = 1;
} else if (!strcmp(argv[i], "-A")) {
#ifdef DHCPv6
@@ -938,6 +960,80 @@ find_interface_by_agent_option(struct dhcp_packet *packet,
}

/*
+ * Format the message that will be used by circuit_id and remote_id
+ */
+static int
+format_relay_agent_rfc3046_msg(const struct interface_info *ip, struct dhcp_packet *packet,
+ const char *format, char *msg, size_t msgn) {
+ size_t len = 0;
+ char hostname[HOST_NAME_MAX] = { 0 };
+ char ifname[IFNAMSIZ] = { 0 };
+ char *buf = msg;
+
+ for ( ; format && *format && len < msgn; ++format) {
+ size_t strn = 0;
+ const char *str = NULL;
+
+ if (*format == '%') {
+ switch (*++format) {
+ case '\0':
+ --format;
+ break;
+
+ case '%': /* A literal '%' */
+ str = "%";
+ break;
+
+ case 'h': /* Hostname */
+ gethostname(hostname, HOST_NAME_MAX);
+ hostname[HOST_NAME_MAX - 1] = '\0';
+ str = hostname;
+ break;
+
+ case 'p': /* Name of interface that we received the request from */
+ strncpy(ifname, ip->name, IFNAMSIZ);
+ str = ifname;
+ break;
+
+ case 'P': /* Physical address of interface that we received the request from */
+ str = print_hw_addr(ip->hw_address.hbuf[0], ip->hw_address.hlen - 1, &ip->hw_address.hbuf[1]);
+ break;
+
+ case 'C': /* 24: Client hardware address */
+ str = print_hw_addr(packet->htype, packet->hlen, packet->chaddr);
+ break;
+
+ case 'I': /* 20: DHCP relay agent IP address */
+ str = inet_ntoa(packet->giaddr);
+ break;
+
+ default:
+ log_error("Option %%%c is unrecognized and will not be formatted!", *format);
+ continue;
+ }
+
+ if (str)
+ strn = strlen(str);
+ } else {
+ str = format;
+ strn = 1;
+ }
+
+ // Do we have room?
+ if ((strn+len) >= msgn) {
+ return 0;
+ }
+
+ if (str && strn > 0) {
+ memcpy(buf+len, str, strn);
+ len += strn;
+ }
+ }
+
+ return len;
+}
+
+/*
* Examine a packet to see if it's a candidate to have a Relay
* Agent Information option tacked onto its tail. If it is, tack
* the option on.
@@ -948,6 +1044,9 @@ add_relay_agent_options(struct interface_info *ip, struct dhcp_packet *packet,
int is_dhcp = 0, mms;
unsigned optlen;
u_int8_t *op, *nextop, *sp, *max, *end_pad = NULL;
+ char circuit_id_buf[256] = { '\0' };
+ char remote_id_buf[256] = { '\0' };
+ size_t circuit_id_len = 0, remote_id_len = 0;

/* If we're not adding agent options to packets, we can skip
this. */
@@ -1077,24 +1176,47 @@ add_relay_agent_options(struct interface_info *ip, struct dhcp_packet *packet,
op = sp;
#endif

- /* Sanity check. Had better not ever happen. */
- if ((ip->circuit_id_len > 255) ||(ip->circuit_id_len < 1))
- log_fatal("Circuit ID length %d out of range [1-255] on "
- "%s\n", ip->circuit_id_len, ip->name);
- optlen = ip->circuit_id_len + 2; /* RAI_CIRCUIT_ID + len */
+ /* option82: custom string for circuit_id */
+ if (agent_circuit_id_fmt) {
+ circuit_id_len = format_relay_agent_rfc3046_msg(ip, packet, agent_circuit_id_fmt,
+ circuit_id_buf, sizeof(circuit_id_buf));
+
+ if (circuit_id_len == 0)
+ strncpy(circuit_id_buf, ip->name, sizeof(ip->name));
+
+ //log_debug("Sending on %s option82:circuit_id='%s' (%d)",
+ // ip->name, circuit_id_buf, circuit_id_len);
+ }
+
+ /* option82: custom string for remote_id */
+ if (agent_remote_id_fmt) {
+ remote_id_len = format_relay_agent_rfc3046_msg(ip, packet, agent_remote_id_fmt,
+ remote_id_buf, sizeof(remote_id_buf));
+
+ //log_debug("Sending on %s option82:remote_id='%s' (%d)",
+ // ip->name, remote_id_buf, remote_id_len);
+ }
+
+ /* Sanity check. Had better not ever happen. */
+ if (circuit_id_len > 255 || circuit_id_len < 1)
+ log_fatal("Circuit ID length %d out of range [1-255] on %s\n",
+ circuit_id_len, ip->name);
+
+ optlen = circuit_id_len + 2; // RAI_CIRCUIT_ID + len

if (ip->remote_id) {
- if (ip->remote_id_len > 255 || ip->remote_id_len < 1)
- log_fatal("Remote ID length %d out of range [1-255] "
- "on %s\n", ip->circuit_id_len, ip->name);
- optlen += ip->remote_id_len + 2; /* RAI_REMOTE_ID + len */
+ if (remote_id_len > 255 || remote_id_len < 1)
+ log_fatal("Remote ID length %d out of range [1-255] on %s\n",
+ remote_id_len, ip->name);
+
+ optlen += remote_id_len + 2; // RAI_REMOTE_ID + len
}

- /* We do not support relay option fragmenting(multiple options to
- * support an option data exceeding 255 bytes).
+ /* We do not support relay option fragmenting (multiple options to
+ * support an option data exceeding 255 bytes)
*/
- if ((optlen < 3) ||(optlen > 255))
- log_fatal("Total agent option length(%u) out of range "
+ if (optlen < 3 || optlen > 255)
+ log_fatal("Total agent option length (%u) out of range "
"[3 - 255] on %s\n", optlen, ip->name);

/*
@@ -1102,7 +1224,7 @@ add_relay_agent_options(struct interface_info *ip, struct dhcp_packet *packet,
* If not, forward without adding the option.
*/
if (max - sp >= optlen + 3) {
- log_debug("Adding %d-byte relay agent option", optlen + 3);
+ //log_debug("Adding %d-byte relay agent option", optlen + 3);

/* Okay, cons up *our* Relay Agent Information option. */
*sp++ = DHO_DHCP_AGENT_OPTIONS;
@@ -1110,16 +1232,16 @@ add_relay_agent_options(struct interface_info *ip, struct dhcp_packet *packet,

/* Copy in the circuit id... */
*sp++ = RAI_CIRCUIT_ID;
- *sp++ = ip->circuit_id_len;
- memcpy(sp, ip->circuit_id, ip->circuit_id_len);
- sp += ip->circuit_id_len;
+ *sp++ = circuit_id_len;
+ memcpy(sp, circuit_id_buf, circuit_id_len);
+ sp += circuit_id_len;

/* Copy in remote ID... */
if (ip->remote_id) {
*sp++ = RAI_REMOTE_ID;
- *sp++ = ip->remote_id_len;
- memcpy(sp, ip->remote_id, ip->remote_id_len);
- sp += ip->remote_id_len;
+ *sp++ = remote_id_len;
+ memcpy(sp, remote_id_buf, remote_id_len);
+ sp += remote_id_len;
}
} else {
++agent_option_errors;
--
2.1.4

Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
From 18ca48b7c307e1644f653df4c8503f4ce2677f62 Mon Sep 17 00:00:00 2001
From: Joe LeVeque <[email protected]>
Date: Mon, 11 Dec 2017 23:39:10 +0000
Subject: [PATCH 2/3] Support for obtaining name of physical interface that is
a member of a bridge interface

---
relay/dhcrelay.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 67 insertions(+), 2 deletions(-)

diff --git a/relay/dhcrelay.c b/relay/dhcrelay.c
index a26efca..84ec79d 100644
--- a/relay/dhcrelay.c
+++ b/relay/dhcrelay.c
@@ -758,6 +758,7 @@ do_relay4(struct interface_info *ip, struct dhcp_packet *packet,
print_hw_addr(packet->htype, packet->hlen,
packet->chaddr),
inet_ntoa(sp->to.sin_addr));
+
++client_packets_relayed;
}
}
@@ -917,6 +918,7 @@ find_interface_by_agent_option(struct dhcp_packet *packet,
++corrupt_agent_options;
return (-1);
}
+
switch(buf[i]) {
/* Remember where the circuit ID is... */
case RAI_CIRCUIT_ID:
@@ -959,6 +961,47 @@ find_interface_by_agent_option(struct dhcp_packet *packet,
return (-1);
}

+static int
+_bridgefdbquery(const char *hwAddr, char *interface, int *vlanid) {
+
+#define xstr(s) str(s)
+#define str(s) #s
+#define FDB_STRING_LEN 100
+#define FDB_BUFFER_LEN (FDB_STRING_LEN + 1)
+
+/*
+ * Format for sscanf() to read the 1st, 3th, and 5th
+ * space-delimited fields
+ *
+ * bridge fdb show output
+ * 6c:64:1a:00:06:13 dev swp35 vlan 0 master bridge permanent
+ */
+#define FDB_LINE_FORMAT "%" xstr(FDB_STRING_LEN) "s %*s " \
+ "%" xstr(FDB_STRING_LEN) "s %*s %d %*s"
+
+ char cmdstr[FDB_BUFFER_LEN];
+ char buf[FDB_BUFFER_LEN];
+ char macAddr[FDB_BUFFER_LEN];
+
+ if ((interface == NULL) || (vlanid == NULL)) {
+ return 0;
+ }
+ sprintf(cmdstr, "bridge fdb show | grep -m 1 %s", hwAddr);
+ FILE *cmd = popen(cmdstr, "r");
+
+ if (cmd != NULL) {
+ while (fgets(buf, sizeof(buf), cmd)) {
+ sscanf(buf, FDB_LINE_FORMAT, macAddr, interface, vlanid);
+ //log_debug("bridgefdbquery: macAddr:%s interface: %s vlanid %d",
+ // macAddr, interface, *vlanid);
+ }
+ pclose(cmd);
+ return 0;
+ }
+
+ return -1;
+}
+
/*
* Format the message that will be used by circuit_id and remote_id
*/
@@ -991,8 +1034,30 @@ format_relay_agent_rfc3046_msg(const struct interface_info *ip, struct dhcp_pack
break;

case 'p': /* Name of interface that we received the request from */
- strncpy(ifname, ip->name, IFNAMSIZ);
- str = ifname;
+ /*
+ * Query FDB to identify the exact physical interface only when source MAC address
+ * is present and '20: DHCP relay agent IP address' (giaddr) is not present
+ */
+ if (packet->htype && !packet->giaddr.s_addr) {
+ int ret = 0, vlanid = 0;
+
+ ret = _bridgefdbquery(print_hw_addr(packet->htype, packet->hlen, packet->chaddr),
+ ifname,
+ &vlanid);
+
+ // If we failed to find a physical interface using the source mac, default
+ // to the interface name we received it on.
+ if (ret < 0) {
+ //log_debug("MAC Address: %s (interface:%s vlan:%d) not found in bridge fdb show",
+ // print_hw_addr (packet->htype, packet->hlen, packet->chaddr),
+ // ip->name,
+ // vlanid);
+
+ strncpy(ifname, ip->name, IFNAMSIZ);
+ }
+
+ str = ifname;
+ }
break;

case 'P': /* Physical address of interface that we received the request from */
--
2.1.4

Loading