From 552684fc0818e59e2591585e9da8929da1180305 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Mon, 3 Jun 2019 14:26:45 -0700 Subject: [PATCH] [dhcp_relay] Add support for DHCP client(s) on one VLAN and DHCP server(s) on another (#2946) --- .../docker-dhcp-relay.supervisord.conf.j2 | 35 ++- rules/docker-dhcp-relay.mk | 2 +- rules/isc-dhcp.mk | 11 +- src/isc-dhcp/Makefile | 7 +- ...ockets-to-configure-flags-in-debian-.patch | 26 ++ ...VE_SO_BINDTODEVICE-has-a-chance-to-b.patch | 275 ++++++++++++++++++ ...f-BOOTREQUEST-is-directed-broadcast-.patch | 267 +++++++++++++++++ src/isc-dhcp/patch/0008-CVE-2017-3144.patch | 47 +++ src/isc-dhcp/patch/0009-CVE-2018-5733.patch | 131 +++++++++ src/isc-dhcp/patch/0010-CVE-2018-5732.patch | 144 +++++++++ src/isc-dhcp/patch/series | 7 +- .../docker-dhcp-relay.supervisord.conf | 2 +- 12 files changed, 924 insertions(+), 30 deletions(-) create mode 100644 src/isc-dhcp/patch/0005-Add-enable-use-sockets-to-configure-flags-in-debian-.patch create mode 100644 src/isc-dhcp/patch/0006-Bugfix-Ensure-HAVE_SO_BINDTODEVICE-has-a-chance-to-b.patch create mode 100644 src/isc-dhcp/patch/0007-If-destination-of-BOOTREQUEST-is-directed-broadcast-.patch create mode 100644 src/isc-dhcp/patch/0008-CVE-2017-3144.patch create mode 100644 src/isc-dhcp/patch/0009-CVE-2018-5733.patch create mode 100644 src/isc-dhcp/patch/0010-CVE-2018-5732.patch diff --git a/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 b/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 index 747f65a3aa..cd18dfa131 100644 --- a/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 +++ b/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 @@ -32,28 +32,33 @@ stderr_logfile=syslog {% if num_relays.count > 0 %} [group:isc-dhcp-relay] programs= -{%- set add_preceding_comma = { 'flag': False } -%} -{%- for vlan_name in VLAN -%} -{%- if VLAN[vlan_name]['dhcp_servers'] -%} -{%- if add_preceding_comma.flag %},{% endif -%} -{%- set _dummy = add_preceding_comma.update({'flag': True}) -%} +{%- set add_preceding_comma = { 'flag': False } %} +{% for vlan_name in VLAN %} +{% if VLAN[vlan_name]['dhcp_servers'] %} +{% if add_preceding_comma.flag %},{% endif %} +{% set _dummy = add_preceding_comma.update({'flag': True}) %} isc-dhcp-relay-{{ vlan_name }} {%- endif %} {% endfor %} {# Create a program entry for each DHCP relay agent instance #} -{% for vlan_name in VLAN -%} -{%- if VLAN[vlan_name]['dhcp_servers'] -%} +{% for vlan_name in VLAN %} +{% if VLAN[vlan_name]['dhcp_servers'] %} [program:isc-dhcp-relay-{{ vlan_name }}] -command=/usr/sbin/dhcrelay -d -m discard -a %%h:%%p %%P --name-alias-map-file /tmp/port-name-alias-map.txt -i {{ vlan_name }} -{%- for (name, prefix) in INTERFACE -%} -{%- if prefix | ipv4 %} -i {{ name }}{% endif -%} -{%- endfor -%} -{%- for (name, prefix) in PORTCHANNEL_INTERFACE -%} -{%- if prefix | ipv4 %} -i {{ name }}{% endif -%} -{%- endfor -%} -{%- for dhcp_server in VLAN[vlan_name]['dhcp_servers'] %} {{ dhcp_server }}{% endfor %} +{# We treat this VLAN as a downstream interface (-id), as we only want to listen for requests #} +command=/usr/sbin/dhcrelay -d -m discard -a %%h:%%p %%P --name-alias-map-file /tmp/port-name-alias-map.txt -id {{ vlan_name }} +{#- We treat all other interfaces as upstream interfaces (-iu), as we only want to listen for replies #} +{% for (name, prefix) in VLAN_INTERFACE %} +{% if prefix | ipv4 and name != vlan_name %} -iu {{ name }}{% endif -%} +{% endfor %} +{% for (name, prefix) in INTERFACE %} +{% if prefix | ipv4 %} -iu {{ name }}{% endif -%} +{% endfor %} +{% for (name, prefix) in PORTCHANNEL_INTERFACE %} +{% if prefix | ipv4 %} -iu {{ name }}{% endif -%} +{% endfor %} +{% for dhcp_server in VLAN[vlan_name]['dhcp_servers'] %} {{ dhcp_server }}{% endfor %} priority=3 autostart=false diff --git a/rules/docker-dhcp-relay.mk b/rules/docker-dhcp-relay.mk index d15bdbf84d..f2432cdc00 100644 --- a/rules/docker-dhcp-relay.mk +++ b/rules/docker-dhcp-relay.mk @@ -6,7 +6,7 @@ DOCKER_DHCP_RELAY_DBG = $(DOCKER_DHCP_RELAY_STEM)-$(DBG_IMAGE_MARK).gz $(DOCKER_DHCP_RELAY)_PATH = $(DOCKERS_PATH)/$(DOCKER_DHCP_RELAY_STEM) -$(DOCKER_DHCP_RELAY)_DEPENDS += $(ISC_DHCP_COMMON) $(ISC_DHCP_RELAY) $(REDIS_TOOLS) +$(DOCKER_DHCP_RELAY)_DEPENDS += $(ISC_DHCP_RELAY) $(REDIS_TOOLS) $(DOCKER_DHCP_RELAY)_DBG_DEPENDS = $($(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_DEPENDS) $(DOCKER_DHCP_RELAY)_DBG_IMAGE_PACKAGES = $($(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_IMAGE_PACKAGES) diff --git a/rules/isc-dhcp.mk b/rules/isc-dhcp.mk index 16b14ac5c0..67d7ed7564 100644 --- a/rules/isc-dhcp.mk +++ b/rules/isc-dhcp.mk @@ -1,14 +1,11 @@ # isc-dhcp packages -ISC_DHCP_VERSION = 4.3.5-3.1 +ISC_DHCP_VERSION = 4.3.5-2 export ISC_DHCP_VERSION -ISC_DHCP_COMMON = isc-dhcp-common_$(ISC_DHCP_VERSION)_amd64.deb -$(ISC_DHCP_COMMON)_SRC_PATH = $(SRC_PATH)/isc-dhcp -SONIC_MAKE_DEBS += $(ISC_DHCP_COMMON) - ISC_DHCP_RELAY = isc-dhcp-relay_$(ISC_DHCP_VERSION)_amd64.deb -$(eval $(call add_derived_package,$(ISC_DHCP_COMMON),$(ISC_DHCP_RELAY))) +$(ISC_DHCP_RELAY)_SRC_PATH = $(SRC_PATH)/isc-dhcp +SONIC_MAKE_DEBS += $(ISC_DHCP_RELAY) -SONIC_STRETCH_DEBS += $(ISC_DHCP_COMMON) $(ISC_DHCP_RELAY) +SONIC_STRETCH_DEBS += $(ISC_DHCP_RELAY) diff --git a/src/isc-dhcp/Makefile b/src/isc-dhcp/Makefile index 1227b3a0c3..3994f7e4a4 100644 --- a/src/isc-dhcp/Makefile +++ b/src/isc-dhcp/Makefile @@ -2,8 +2,7 @@ SHELL = /bin/bash .SHELLFLAGS += -e -MAIN_TARGET = isc-dhcp-common_$(ISC_DHCP_VERSION)_amd64.deb -DERIVED_TARGETS = isc-dhcp-relay_$(ISC_DHCP_VERSION)_amd64.deb +MAIN_TARGET = isc-dhcp-relay_$(ISC_DHCP_VERSION)_amd64.deb $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% : # Remove any stale files @@ -27,6 +26,4 @@ $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% : popd # Move the newly-built .deb packages to the destination directory - mv $* $(DERIVED_TARGETS) $(DEST)/ - -$(addprefix $(DEST)/, $(DERIVED_TARGETS)): $(DEST)/% : $(DEST)/$(MAIN_TARGET) + mv $* $(DEST)/ diff --git a/src/isc-dhcp/patch/0005-Add-enable-use-sockets-to-configure-flags-in-debian-.patch b/src/isc-dhcp/patch/0005-Add-enable-use-sockets-to-configure-flags-in-debian-.patch new file mode 100644 index 0000000000..7c39f977bb --- /dev/null +++ b/src/isc-dhcp/patch/0005-Add-enable-use-sockets-to-configure-flags-in-debian-.patch @@ -0,0 +1,26 @@ +From 5c64cb06e3ac50a1cbca85669625fe16439064ad Mon Sep 17 00:00:00 2001 +From: Joe LeVeque +Date: Fri, 17 May 2019 21:49:00 +0000 +Subject: [PATCH 1/3] Add --enable-use-sockets to configure flags in + debian/rules + +--- + debian/rules | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/debian/rules b/debian/rules +index 114606b..9919237 100755 +--- a/debian/rules ++++ b/debian/rules +@@ -23,7 +23,7 @@ CFLAGS+=-D_PATH_DHCLIENT_CONF='\"/etc/dhcp/dhclient.conf\"' + CFLAGS+=-D_PATH_DHCLIENT_DB='\"$(LEASE_PATH)/dhclient.leases\"' + CFLAGS+=-D_PATH_DHCLIENT6_DB='\"$(LEASE_PATH)/dhclient6.leases\"' + +-CONFFLAGS=--prefix=/usr --enable-log-pid --enable-paranoia ++CONFFLAGS=--prefix=/usr --enable-log-pid --enable-paranoia --enable-use-sockets + + # cross-architecture building + ifneq ($(DEB_HOST_GNU_TYPE),$(DEB_BUILD_GNU_TYPE)) +-- +2.17.1 + diff --git a/src/isc-dhcp/patch/0006-Bugfix-Ensure-HAVE_SO_BINDTODEVICE-has-a-chance-to-b.patch b/src/isc-dhcp/patch/0006-Bugfix-Ensure-HAVE_SO_BINDTODEVICE-has-a-chance-to-b.patch new file mode 100644 index 0000000000..5553180f40 --- /dev/null +++ b/src/isc-dhcp/patch/0006-Bugfix-Ensure-HAVE_SO_BINDTODEVICE-has-a-chance-to-b.patch @@ -0,0 +1,275 @@ +From 6620d4778fe36c89ce2e95d6932338cefc90df7d Mon Sep 17 00:00:00 2001 +From: Joe LeVeque +Date: Fri, 17 May 2019 21:53:52 +0000 +Subject: [PATCH 2/3] Bugfix: Ensure HAVE_SO_BINDTODEVICE has a chance to be + defined before it is referenced + +--- + includes/osdep.h | 239 ++++++++++++++++++++++++----------------------- + 1 file changed, 120 insertions(+), 119 deletions(-) + +diff --git a/includes/osdep.h b/includes/osdep.h +index cfae90b..f07c43c 100644 +--- a/includes/osdep.h ++++ b/includes/osdep.h +@@ -48,37 +48,6 @@ + #define BYTE_ORDER DHCP_BYTE_ORDER + #endif /* BYTE_ORDER */ + +-/* Porting:: +- +- If you add a new network API, you must add a check for it below: */ +- +-#if !defined (USE_SOCKETS) && \ +- !defined (USE_SOCKET_SEND) && \ +- !defined (USE_SOCKET_RECEIVE) && \ +- !defined (USE_RAW_SOCKETS) && \ +- !defined (USE_RAW_SEND) && \ +- !defined (USE_SOCKET_RECEIVE) && \ +- !defined (USE_BPF) && \ +- !defined (USE_BPF_SEND) && \ +- !defined (USE_BPF_RECEIVE) && \ +- !defined (USE_LPF) && \ +- !defined (USE_LPF_SEND) && \ +- !defined (USE_LPF_RECEIVE) && \ +- !defined (USE_NIT) && \ +- !defined (USE_NIT_SEND) && \ +- !defined (USE_NIT_RECEIVE) && \ +- !defined (USE_DLPI_SEND) && \ +- !defined (USE_DLPI_RECEIVE) +-/* Determine default socket API to USE. */ +-# if defined(HAVE_BPF) +-# define USE_BPF 1 +-# elif defined(HAVE_LPF) +-# define USE_LPF 1 +-# elif defined(HAVE_DLPI) +-# define USE_DLPI 1 +-# endif +-#endif +- + #if !defined (TIME_MAX) + # define TIME_MAX 2147483647 + #endif +@@ -91,94 +60,6 @@ + # define vsnprintf isc_print_vsnprintf + #endif + +-/* Porting:: +- +- If you add a new network API, and have it set up so that it can be +- used for sending or receiving, but doesn't have to be used for both, +- then set up an ifdef like the ones below: */ +- +-#ifdef USE_SOCKETS +-# define USE_SOCKET_SEND +-# define USE_SOCKET_RECEIVE +-# if defined(HAVE_DLPI) && !defined(sun) && !defined(USE_V4_PKTINFO) +-# define USE_DLPI_HWADDR +-# elif defined(HAVE_LPF) +-# define USE_LPF_HWADDR +-# elif defined(HAVE_BPF) +-# define USE_BPF_HWADDR +-# endif +-#endif +- +-#ifdef USE_RAW_SOCKETS +-# define USE_RAW_SEND +-# define USE_SOCKET_RECEIVE +-#endif +- +-#ifdef USE_BPF +-# define USE_BPF_SEND +-# define USE_BPF_RECEIVE +-#endif +- +-#ifdef USE_LPF +-# define USE_LPF_SEND +-# define USE_LPF_RECEIVE +-#endif +- +-#ifdef USE_NIT +-# define USE_NIT_SEND +-# define USE_NIT_RECEIVE +-#endif +- +-#ifdef USE_DLPI +-# define USE_DLPI_SEND +-# define USE_DLPI_RECEIVE +-#endif +- +-#ifdef USE_UPF +-# define USE_UPF_SEND +-# define USE_UPF_RECEIVE +-#endif +- +-/* Porting:: +- +- If you add support for sending packets directly out an interface, +- and your support does not do ARP or routing, you must use a fallback +- mechanism to deal with packets that need to be sent to routers. +- Currently, all low-level packet interfaces use BSD sockets as a +- fallback. */ +- +-#if defined (USE_BPF_SEND) || defined (USE_NIT_SEND) || \ +- defined (USE_DLPI_SEND) || defined (USE_UPF_SEND) || \ +- defined (USE_LPF_SEND) || \ +- (defined (USE_SOCKET_SEND) && defined (HAVE_SO_BINDTODEVICE)) +-# define USE_SOCKET_FALLBACK +-# define USE_FALLBACK +-#endif +- +-/* Porting:: +- +- If you add support for sending packets directly out an interface +- and need to be able to assemble packets, add the USE_XXX_SEND +- definition for your interface to the list tested below. */ +- +-#if defined (USE_RAW_SEND) || defined (USE_BPF_SEND) || \ +- defined (USE_NIT_SEND) || defined (USE_UPF_SEND) || \ +- defined (USE_DLPI_SEND) || defined (USE_LPF_SEND) +-# define PACKET_ASSEMBLY +-#endif +- +-/* Porting:: +- +- If you add support for receiving packets directly from an interface +- and need to be able to decode raw packets, add the USE_XXX_RECEIVE +- definition for your interface to the list tested below. */ +- +-#if defined (USE_RAW_RECEIVE) || defined (USE_BPF_SEND) || \ +- defined (USE_NIT_RECEIVE) || defined (USE_UPF_RECEIVE) || \ +- defined (USE_DLPI_RECEIVE) || defined (USE_LPF_RECEIVE) +-# define PACKET_DECODING +-#endif +- + /* If we don't have a DLPI packet filter, we have to filter in userland. + Probably not worth doing, actually. */ + #if defined (USE_DLPI_RECEIVE) && !defined (USE_DLPI_PFMOD) +@@ -288,4 +169,124 @@ + # define STDERR_FILENO 2 + #endif + ++/* Porting:: ++ ++ If you add a new network API, you must add a check for it below: */ ++ ++#if !defined (USE_SOCKETS) && \ ++ !defined (USE_SOCKET_SEND) && \ ++ !defined (USE_SOCKET_RECEIVE) && \ ++ !defined (USE_RAW_SOCKETS) && \ ++ !defined (USE_RAW_SEND) && \ ++ !defined (USE_SOCKET_RECEIVE) && \ ++ !defined (USE_BPF) && \ ++ !defined (USE_BPF_SEND) && \ ++ !defined (USE_BPF_RECEIVE) && \ ++ !defined (USE_LPF) && \ ++ !defined (USE_LPF_SEND) && \ ++ !defined (USE_LPF_RECEIVE) && \ ++ !defined (USE_NIT) && \ ++ !defined (USE_NIT_SEND) && \ ++ !defined (USE_NIT_RECEIVE) && \ ++ !defined (USE_DLPI_SEND) && \ ++ !defined (USE_DLPI_RECEIVE) ++/* Determine default socket API to USE. */ ++# if defined(HAVE_BPF) ++# define USE_BPF 1 ++# elif defined(HAVE_LPF) ++# define USE_LPF 1 ++# elif defined(HAVE_DLPI) ++# define USE_DLPI 1 ++# endif ++#endif ++ ++/* Porting:: ++ ++ If you add a new network API, and have it set up so that it can be ++ used for sending or receiving, but doesn't have to be used for both, ++ then set up an ifdef like the ones below: */ ++ ++#ifdef USE_SOCKETS ++# define USE_SOCKET_SEND ++# define USE_SOCKET_RECEIVE ++# if defined(HAVE_DLPI) && !defined(sun) && !defined(USE_V4_PKTINFO) ++# define USE_DLPI_HWADDR ++# elif defined(HAVE_LPF) ++# define USE_LPF_HWADDR ++# elif defined(HAVE_BPF) ++# define USE_BPF_HWADDR ++# endif ++#endif ++ ++#ifdef USE_RAW_SOCKETS ++# define USE_RAW_SEND ++# define USE_SOCKET_RECEIVE ++#endif ++ ++#ifdef USE_BPF ++# define USE_BPF_SEND ++# define USE_BPF_RECEIVE ++#endif ++ ++#ifdef USE_LPF ++# define USE_LPF_SEND ++# define USE_LPF_RECEIVE ++#endif ++ ++#ifdef USE_NIT ++# define USE_NIT_SEND ++# define USE_NIT_RECEIVE ++#endif ++ ++#ifdef USE_DLPI ++# define USE_DLPI_SEND ++# define USE_DLPI_RECEIVE ++#endif ++ ++#ifdef USE_UPF ++# define USE_UPF_SEND ++# define USE_UPF_RECEIVE ++#endif ++ ++/* Porting:: ++ ++ If you add support for sending packets directly out an interface, ++ and your support does not do ARP or routing, you must use a fallback ++ mechanism to deal with packets that need to be sent to routers. ++ Currently, all low-level packet interfaces use BSD sockets as a ++ fallback. */ ++ ++#if defined (USE_BPF_SEND) || defined (USE_NIT_SEND) || \ ++ defined (USE_DLPI_SEND) || defined (USE_UPF_SEND) || \ ++ defined (USE_LPF_SEND) || \ ++ (defined (USE_SOCKET_SEND) && defined (HAVE_SO_BINDTODEVICE)) ++# define USE_SOCKET_FALLBACK ++# define USE_FALLBACK ++#endif ++ ++/* Porting:: ++ ++ If you add support for sending packets directly out an interface ++ and need to be able to assemble packets, add the USE_XXX_SEND ++ definition for your interface to the list tested below. */ ++ ++#if defined (USE_RAW_SEND) || defined (USE_BPF_SEND) || \ ++ defined (USE_NIT_SEND) || defined (USE_UPF_SEND) || \ ++ defined (USE_DLPI_SEND) || defined (USE_LPF_SEND) ++# define PACKET_ASSEMBLY ++#endif ++ ++/* Porting:: ++ ++ If you add support for receiving packets directly from an interface ++ and need to be able to decode raw packets, add the USE_XXX_RECEIVE ++ definition for your interface to the list tested below. */ ++ ++#if defined (USE_RAW_RECEIVE) || defined (USE_BPF_SEND) || \ ++ defined (USE_NIT_RECEIVE) || defined (USE_UPF_RECEIVE) || \ ++ defined (USE_DLPI_RECEIVE) || defined (USE_LPF_RECEIVE) ++# define PACKET_DECODING ++#endif ++ ++ + #endif /* __ISC_DHCP_OSDEP_H__ */ +-- +2.17.1 + diff --git a/src/isc-dhcp/patch/0007-If-destination-of-BOOTREQUEST-is-directed-broadcast-.patch b/src/isc-dhcp/patch/0007-If-destination-of-BOOTREQUEST-is-directed-broadcast-.patch new file mode 100644 index 0000000000..e64f8439e2 --- /dev/null +++ b/src/isc-dhcp/patch/0007-If-destination-of-BOOTREQUEST-is-directed-broadcast-.patch @@ -0,0 +1,267 @@ +From fe50ed9721d79be0bb5045a219f5f5bb4cb8868e Mon Sep 17 00:00:00 2001 +From: Joe LeVeque +Date: Fri, 17 May 2019 22:24:02 +0000 +Subject: [PATCH 3/3] If destination of BOOTREQUEST is directed broadcast, + forward on that interface. Otherwise forward on fallback or all upstream + interfaces + +--- + common/discover.c | 46 +++++++++++++++++++--- + includes/dhcpd.h | 3 ++ + relay/dhcrelay.c | 98 +++++++++++++++++++++++++++++++++++++++++------ + 3 files changed, 131 insertions(+), 16 deletions(-) + +diff --git a/common/discover.c b/common/discover.c +index 8e7f632..73eb8a9 100644 +--- a/common/discover.c ++++ b/common/discover.c +@@ -227,6 +227,7 @@ struct iface_conf_list { + struct iface_info { + char name[IF_NAMESIZE+1]; /* name of the interface, e.g. "bge0" */ + struct sockaddr_storage addr; /* address information */ ++ struct sockaddr_storage netmask; /* netmask information */ + isc_uint64_t flags; /* interface flags, e.g. IFF_LOOPBACK */ + }; + +@@ -401,6 +402,7 @@ struct iface_conf_list { + struct iface_info { + char name[IFNAMSIZ]; /* name of the interface, e.g. "eth0" */ + struct sockaddr_storage addr; /* address information */ ++ struct sockaddr_storage netmask; /* netmask information */ + isc_uint64_t flags; /* interface flags, e.g. IFF_LOOPBACK */ + }; + +@@ -576,6 +578,17 @@ next_iface4(struct iface_info *info, int *err, struct iface_conf_list *ifaces) { + } + memcpy(&info->addr, &tmp.ifr_addr, sizeof(tmp.ifr_addr)); + ++ if (ioctl(ifaces->sock, SIOCGIFNETMASK, &tmp) < 0) { ++ if (errno == EADDRNOTAVAIL) { ++ continue; ++ } ++ log_error("Error getting netmask " ++ "for '%s'; %m", name); ++ *err = 1; ++ return 0; ++ } ++ memcpy(&info->netmask, &tmp.ifr_netmask, sizeof(tmp.ifr_netmask)); ++ + memset(&tmp, 0, sizeof(tmp)); + strncpy(tmp.ifr_name, name, sizeof(tmp.ifr_name) - 1); + if (ioctl(ifaces->sock, SIOCGIFFLAGS, &tmp) < 0) { +@@ -780,6 +793,7 @@ struct iface_conf_list { + struct iface_info { + char name[IFNAMSIZ]; /* name of the interface, e.g. "bge0" */ + struct sockaddr_storage addr; /* address information */ ++ struct sockaddr_storage netmask; /* netmask information */ + isc_uint64_t flags; /* interface flags, e.g. IFF_LOOPBACK */ + }; + +@@ -840,7 +854,8 @@ end_iface_scan(struct iface_conf_list *ifaces) { + /* XXX: perhaps create drealloc() rather than do it manually */ + void + add_ipv4_addr_to_interface(struct interface_info *iface, +- const struct in_addr *addr) { ++ const struct in_addr *addr, ++ const struct in_addr *netmask) { + /* + * We don't expect a lot of addresses per IPv4 interface, so + * we use 4, as our "chunk size" for collecting addresses. +@@ -851,6 +866,12 @@ add_ipv4_addr_to_interface(struct interface_info *iface, + log_fatal("Out of memory saving IPv4 address " + "on interface."); + } ++ ++ iface->netmasks = dmalloc(4 * sizeof(struct in_addr), MDL); ++ if (iface->netmasks == NULL) { ++ log_fatal("Out of memory saving IPv4 netmask " ++ "on interface."); ++ } + iface->address_count = 0; + iface->address_max = 4; + } else if (iface->address_count >= iface->address_max) { +@@ -863,14 +884,28 @@ add_ipv4_addr_to_interface(struct interface_info *iface, + log_fatal("Out of memory saving IPv4 address " + "on interface."); + } +- memcpy(tmp, +- iface->addresses, ++ memcpy(tmp, ++ iface->addresses, + iface->address_max * sizeof(struct in_addr)); + dfree(iface->addresses, MDL); + iface->addresses = tmp; ++ ++ tmp = dmalloc(new_max * sizeof(struct in_addr), MDL); ++ if (tmp == NULL) { ++ log_fatal("Out of memory saving IPv4 netmask " ++ "on interface."); ++ } ++ memcpy(tmp, ++ iface->netmasks, ++ iface->address_max * sizeof(struct in_addr)); ++ dfree(iface->netmasks, MDL); ++ iface->netmasks = tmp; ++ + iface->address_max = new_max; + } +- iface->addresses[iface->address_count++] = *addr; ++ iface->addresses[iface->address_count] = *addr; ++ iface->netmasks[iface->address_count] = *netmask; ++ iface->address_count++; + } + + #ifdef DHCPv6 +@@ -1005,6 +1040,7 @@ discover_interfaces(int state) { + if ((info.addr.ss_family == AF_INET) && + (local_family == AF_INET)) { + struct sockaddr_in *a = (struct sockaddr_in*)&info.addr; ++ struct sockaddr_in *n = (struct sockaddr_in*)&info.netmask; + struct iaddr addr; + + /* We don't want the loopback interface. */ +@@ -1019,7 +1055,7 @@ discover_interfaces(int state) { + if (a->sin_addr.s_addr != htonl(INADDR_ANY)) + tmp->configured = 1; + +- add_ipv4_addr_to_interface(tmp, &a->sin_addr); ++ add_ipv4_addr_to_interface(tmp, &a->sin_addr, &n->sin_addr); + + /* invoke the setup hook */ + addr.len = 4; +diff --git a/includes/dhcpd.h b/includes/dhcpd.h +index 261714d..89bfe82 100644 +--- a/includes/dhcpd.h ++++ b/includes/dhcpd.h +@@ -1347,6 +1347,9 @@ struct interface_info { + struct in_addr *addresses; /* Addresses associated with this + * interface. + */ ++ struct in_addr *netmasks; /* Netmask associated with this ++ * interface. ++ */ + int address_count; /* Number of addresses stored. */ + int address_max; /* Size of addresses buffer. */ + struct in6_addr *v6addresses; /* IPv6 addresses associated with +diff --git a/relay/dhcrelay.c b/relay/dhcrelay.c +index c9b6d8e..8aac4b3 100644 +--- a/relay/dhcrelay.c ++++ b/relay/dhcrelay.c +@@ -30,6 +30,7 @@ + #include + #include + #include ++#include + #include + + TIME default_lease_time = 43200; /* 12 hours... */ +@@ -881,20 +882,95 @@ do_relay4(struct interface_info *ip, struct dhcp_packet *packet, + /* Otherwise, it's a BOOTREQUEST, so forward it to all the + servers. */ + for (sp = servers; sp; sp = sp->next) { +- if (send_packet((fallback_interface +- ? fallback_interface : interfaces), +- NULL, packet, length, ip->addresses[0], +- &sp->to, NULL) < 0) { +- ++client_packet_errors; ++ int packet_relay_attempted = 0; ++ ++ log_debug("Server IP: %s", inet_ntoa(sp->to.sin_addr)); ++ ++ /* If the server's IP address is the broadcast IP of one ++ of our interfaces, we send it directly on that interface's ++ socket, because the kernel will drop directed broadcast ++ packets if we send on the fallback. */ ++ for (out = interfaces; out; out = out->next) { ++ int i = 0; ++ ++ // Only relay BOOTREQUEST on upstream interfaces ++ if (!(out->flags & INTERFACE_UPSTREAM)) ++ continue; ++ ++ if (!out->addresses || !out->netmasks) ++ continue; ++ ++ for (i = 0; i < out->address_count; i++) { ++ struct in_addr bcast_addr; ++ ++ log_debug("Iface %s addr: %s", out->name, inet_ntoa(out->addresses[i])); ++ log_debug("Iface %s netmask: %s", out->name, inet_ntoa(out->netmasks[i])); ++ ++ // Broadcast = ip_addr | ~netmask ++ bcast_addr.s_addr = out->addresses[i].s_addr | ~out->netmasks[i].s_addr; ++ log_debug("Iface %s broadcast: %s", out->name, inet_ntoa(bcast_addr)); ++ ++ if (sp->to.sin_addr.s_addr == bcast_addr.s_addr) { ++ log_debug("Packet destined for broadcast IP of %s", out->name); ++ if (send_packet(out, NULL, packet, ++ length, ip->addresses[0],&sp->to, NULL) < 0) { ++ ++client_packet_errors; ++ } else { ++ log_debug("Forwarded BOOTREQUEST for %s to %s on interface %s", ++ print_hw_addr(packet->htype, packet->hlen, ++ packet->chaddr), ++ inet_ntoa(sp->to.sin_addr), out->name); ++ ++ ++client_packets_relayed; ++ } ++ ++ packet_relay_attempted = 1; ++ ++ break; ++ } ++ } ++ ++ if (packet_relay_attempted) ++ break; ++ } ++ ++ if (packet_relay_attempted) ++ continue; ++ ++ /* Otherwise, if we have a fallback interface, we send the packet ++ on it. If not, we send the packet out all interfaces.*/ ++ if (fallback_interface) { ++ if (send_packet(fallback_interface, NULL, packet, ++ length, ip->addresses[0],&sp->to, NULL) < 0) { ++ ++client_packet_errors; ++ } else { ++ log_debug("Forwarded BOOTREQUEST for %s to %s on fallback interface", ++ print_hw_addr(packet->htype, packet->hlen, ++ packet->chaddr), ++ inet_ntoa(sp->to.sin_addr)); ++ ++ ++client_packets_relayed; ++ } + } else { +- log_debug("Forwarded BOOTREQUEST for %s to %s", +- print_hw_addr(packet->htype, packet->hlen, +- packet->chaddr), +- inet_ntoa(sp->to.sin_addr)); +- ++client_packets_relayed; ++ for (out = interfaces; out; out = out->next) { ++ // Only relay BOOTREQUEST on upstream interfaces ++ if (!(out->flags & INTERFACE_UPSTREAM)) ++ continue; ++ ++ if (send_packet(out, NULL, packet, ++ length, ip->addresses[0],&sp->to, NULL) < 0) { ++ ++client_packet_errors; ++ } else { ++ log_debug("Forwarded BOOTREQUEST for %s to %s on interface %s", ++ print_hw_addr(packet->htype, packet->hlen, ++ packet->chaddr), ++ inet_ntoa(sp->to.sin_addr), out->name); ++ ++ ++client_packets_relayed; ++ } ++ } + } + } +- + } + + /* Strip any Relay Agent Information options from the DHCP packet +-- +2.17.1 + diff --git a/src/isc-dhcp/patch/0008-CVE-2017-3144.patch b/src/isc-dhcp/patch/0008-CVE-2017-3144.patch new file mode 100644 index 0000000000..fe066e177a --- /dev/null +++ b/src/isc-dhcp/patch/0008-CVE-2017-3144.patch @@ -0,0 +1,47 @@ +From: Thomas Markwalder +Date: Thu, 7 Dec 2017 11:23:36 -0500 +Subject: [master] Plugs a socket descriptor leak in OMAPI +Origin: https://source.isc.org/cgi-bin/gitweb.cgi?p=dhcp.git;a=commit;h=1a6b62fe17a42b00fa234d06b6dfde3d03451894 +Bug: https://bugs.isc.org/Public/Bug/Display.html?id=46767 +Bug-Debian: https://bugs.debian.org/887413 +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-3144 + + Merges in rt46767. +--- + +diff --git a/omapip/buffer.c b/omapip/buffer.c +index 6e0621b5..a21f0a80 100644 +--- a/omapip/buffer.c ++++ b/omapip/buffer.c +@@ -565,6 +565,15 @@ isc_result_t omapi_connection_writer (omapi_object_t *h) + omapi_buffer_dereference (&buffer, MDL); + } + } ++ ++ /* If we had data left to write when we're told to disconnect, ++ * we need recall disconnect, now that we're done writing. ++ * See rt46767. */ ++ if (c->out_bytes == 0 && c->state == omapi_connection_disconnecting) { ++ omapi_disconnect (h, 1); ++ return ISC_R_SHUTTINGDOWN; ++ } ++ + return ISC_R_SUCCESS; + } + +diff --git a/omapip/message.c b/omapip/message.c +index ee15d821..37abbd25 100644 +--- a/omapip/message.c ++++ b/omapip/message.c +@@ -339,7 +339,7 @@ isc_result_t omapi_message_unregister (omapi_object_t *mo) + } + + #ifdef DEBUG_PROTOCOL +-static const char *omapi_message_op_name(int op) { ++const char *omapi_message_op_name(int op) { + switch (op) { + case OMAPI_OP_OPEN: return "OMAPI_OP_OPEN"; + case OMAPI_OP_REFRESH: return "OMAPI_OP_REFRESH"; +-- +2.16.2 + diff --git a/src/isc-dhcp/patch/0009-CVE-2018-5733.patch b/src/isc-dhcp/patch/0009-CVE-2018-5733.patch new file mode 100644 index 0000000000..99017fc983 --- /dev/null +++ b/src/isc-dhcp/patch/0009-CVE-2018-5733.patch @@ -0,0 +1,131 @@ +From: Thomas Markwalder +Date: Fri, 9 Feb 2018 14:46:08 -0500 +Subject: [master] Corrected refcnt loss in option parsing +Origin: https://source.isc.org/cgi-bin/gitweb.cgi?p=dhcp.git;a=commit;h=197b26f25309f947b97a83b8fdfc414b767798f8 +Bug: https://bugs.isc.org/Public/Bug/Display.html?id=47140 +Bug-Debian: https://bugs.debian.org/891785 +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2018-5733 + + Merges in 47140. +--- + +--- a/common/options.c ++++ b/common/options.c +@@ -177,6 +177,8 @@ int parse_option_buffer (options, buffer + + /* If the length is outrageous, the options are bad. */ + if (offset + len > length) { ++ /* Avoid reference count overflow */ ++ option_dereference(&option, MDL); + reason = "option length exceeds option buffer length"; + bogus: + log_error("parse_option_buffer: malformed option " +--- a/common/tests/Makefile.am ++++ b/common/tests/Makefile.am +@@ -10,7 +10,8 @@ ATF_TESTS = + + if HAVE_ATF + +-ATF_TESTS += alloc_unittest dns_unittest misc_unittest ns_name_unittest ++ATF_TESTS += alloc_unittest dns_unittest misc_unittest ns_name_unittest \ ++ option_unittest + + alloc_unittest_SOURCES = test_alloc.c $(top_srcdir)/tests/t_api_dhcp.c + alloc_unittest_LDADD = $(ATF_LDFLAGS) +@@ -36,6 +37,14 @@ ns_name_unittest_LDADD += ../libdhcp.a + ../../omapip/libomapi.a $(BINDLIBDIR)/libirs.a \ + $(BINDLIBDIR)/libdns.a $(BINDLIBDIR)/libisccfg.a $(BINDLIBDIR)/libisc.a + ++option_unittest_SOURCES = option_unittest.c $(top_srcdir)/tests/t_api_dhcp.c ++option_unittest_LDADD = $(ATF_LDFLAGS) ++option_unittest_LDADD += ../libdhcp.@A@ ../../omapip/libomapi.@A@ \ ++ @BINDLIBIRSDIR@/libirs.@A@ \ ++ @BINDLIBDNSDIR@/libdns.@A@ \ ++ @BINDLIBISCCFGDIR@/libisccfg.@A@ \ ++ @BINDLIBISCDIR@/libisc.@A@ ++ + check: $(ATF_TESTS) + @if test $(top_srcdir) != ${top_builddir}; then \ + cp $(top_srcdir)/common/tests/Atffile Atffile; \ +--- /dev/null ++++ b/common/tests/option_unittest.c +@@ -0,0 +1,79 @@ ++/* ++ * Copyright (C) 2018 Internet Systems Consortium, Inc. ("ISC") ++ * ++ * This Source Code Form is subject to the terms of the Mozilla Public ++ * License, v. 2.0. If a copy of the MPL was not distributed with this ++ * file, You can obtain one at http://mozilla.org/MPL/2.0/. ++ * ++ * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH ++ * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY ++ * AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, ++ * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM ++ * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE ++ * OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR ++ * PERFORMANCE OF THIS SOFTWARE. ++ */ ++ ++#include ++#include ++#include "dhcpd.h" ++ ++ATF_TC(option_refcnt); ++ ++ATF_TC_HEAD(option_refcnt, tc) ++{ ++ atf_tc_set_md_var(tc, "descr", ++ "Verify option reference count does not overflow."); ++} ++ ++/* This test does a simple check to see if option reference count is ++ * decremented even an error path exiting parse_option_buffer() ++ */ ++ATF_TC_BODY(option_refcnt, tc) ++{ ++ struct option_state *options; ++ struct option *option; ++ unsigned code; ++ int refcnt; ++ unsigned char buffer[3] = { 15, 255, 0 }; ++ ++ initialize_common_option_spaces(); ++ ++ options = NULL; ++ if (!option_state_allocate(&options, MDL)) { ++ atf_tc_fail("can't allocate option state"); ++ } ++ ++ option = NULL; ++ code = 15; /* domain-name */ ++ if (!option_code_hash_lookup(&option, dhcp_universe.code_hash, ++ &code, 0, MDL)) { ++ atf_tc_fail("can't find option 15"); ++ } ++ if (option == NULL) { ++ atf_tc_fail("option is NULL"); ++ } ++ refcnt = option->refcnt; ++ ++ buffer[0] = 15; ++ buffer[1] = 255; /* invalid */ ++ buffer[2] = 0; ++ ++ if (parse_option_buffer(options, buffer, 3, &dhcp_universe)) { ++ atf_tc_fail("parse_option_buffer is expected to fail"); ++ } ++ ++ if (refcnt != option->refcnt) { ++ atf_tc_fail("refcnt changed from %d to %d", refcnt, option->refcnt); ++ } ++} ++ ++/* This macro defines main() method that will call specified ++ test cases. tp and simple_test_case names can be whatever you want ++ as long as it is a valid variable identifier. */ ++ATF_TP_ADD_TCS(tp) ++{ ++ ATF_TP_ADD_TC(tp, option_refcnt); ++ ++ return (atf_no_error()); ++} diff --git a/src/isc-dhcp/patch/0010-CVE-2018-5732.patch b/src/isc-dhcp/patch/0010-CVE-2018-5732.patch new file mode 100644 index 0000000000..d6c10e2e65 --- /dev/null +++ b/src/isc-dhcp/patch/0010-CVE-2018-5732.patch @@ -0,0 +1,144 @@ +From: Thomas Markwalder +Date: Sat, 10 Feb 2018 12:15:27 -0500 +Subject: [master] Correct buffer overrun in pretty_print_option +Origin: https://source.isc.org/cgi-bin/gitweb.cgi?p=dhcp.git;a=commit;h=c5931725b48b121d232df4ba9e45bc41e0ba114d +Bug: https://bugs.isc.org/Public/Bug/Display.html?id=47139 +Bug-Debian: https://bugs.debian.org/891786 +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2018-5732 + + Merges in rt47139. +--- + +diff --git a/common/options.c b/common/options.c +index 6f23bc15..fc0e0889 100644 +--- a/common/options.c ++++ b/common/options.c +@@ -1776,7 +1776,8 @@ format_min_length(format, oc) + + + /* Format the specified option so that a human can easily read it. */ +- ++/* Maximum pretty printed size */ ++#define MAX_OUTPUT_SIZE 32*1024 + const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) + struct option *option; + const unsigned char *data; +@@ -1784,8 +1785,9 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) + int emit_commas; + int emit_quotes; + { +- static char optbuf [32768]; /* XXX */ +- static char *endbuf = &optbuf[sizeof(optbuf)]; ++ /* We add 128 byte pad so we don't have to add checks everywhere. */ ++ static char optbuf [MAX_OUTPUT_SIZE + 128]; /* XXX */ ++ static char *endbuf = optbuf + MAX_OUTPUT_SIZE; + int hunksize = 0; + int opthunk = 0; + int hunkinc = 0; +@@ -2211,7 +2213,14 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes) + log_error ("Unexpected format code %c", + fmtbuf [j]); + } ++ + op += strlen (op); ++ if (op >= endbuf) { ++ log_error ("Option data exceeds" ++ " maximum size %d", MAX_OUTPUT_SIZE); ++ return (""); ++ } ++ + if (dp == data + len) + break; + if (j + 1 < numelem && comma != ':') +diff --git a/common/tests/option_unittest.c b/common/tests/option_unittest.c +index 36236b84..cd52cfb4 100644 +--- a/common/tests/option_unittest.c ++++ b/common/tests/option_unittest.c +@@ -43,7 +43,7 @@ ATF_TC_BODY(option_refcnt, tc) + if (!option_state_allocate(&options, MDL)) { + atf_tc_fail("can't allocate option state"); + } +- ++ + option = NULL; + code = 15; /* domain-name */ + if (!option_code_hash_lookup(&option, dhcp_universe.code_hash, +@@ -68,12 +68,75 @@ ATF_TC_BODY(option_refcnt, tc) + } + } + ++ATF_TC(pretty_print_option); ++ ++ATF_TC_HEAD(pretty_print_option, tc) ++{ ++ atf_tc_set_md_var(tc, "descr", ++ "Verify pretty_print_option does not overrun its buffer."); ++} ++ ++ ++/* ++ * This test verifies that pretty_print_option() will not overrun its ++ * internal, static buffer when given large 'x/X' format options. ++ * ++ */ ++ATF_TC_BODY(pretty_print_option, tc) ++{ ++ struct option *option; ++ unsigned code; ++ unsigned char bad_data[32*1024]; ++ unsigned char good_data[] = { 1,2,3,4,5,6 }; ++ int emit_commas = 1; ++ int emit_quotes = 1; ++ const char *output_buf; ++ ++ /* Initialize whole thing to non-printable chars */ ++ memset(bad_data, 0x1f, sizeof(bad_data)); ++ ++ initialize_common_option_spaces(); ++ ++ /* We'll use dhcp_client_identitifer because it happens to be format X */ ++ code = 61; ++ option = NULL; ++ if (!option_code_hash_lookup(&option, dhcp_universe.code_hash, ++ &code, 0, MDL)) { ++ atf_tc_fail("can't find option %d", code); ++ } ++ ++ if (option == NULL) { ++ atf_tc_fail("option is NULL"); ++ } ++ ++ /* First we will try a good value we know should fit. */ ++ output_buf = pretty_print_option (option, good_data, sizeof(good_data), ++ emit_commas, emit_quotes); ++ ++ /* Make sure we get what we expect */ ++ if (!output_buf || strcmp(output_buf, "1:2:3:4:5:6")) { ++ atf_tc_fail("pretty_print_option did not return \"\""); ++ } ++ ++ ++ /* Now we'll try a data value that's too large */ ++ output_buf = pretty_print_option (option, bad_data, sizeof(bad_data), ++ emit_commas, emit_quotes); ++ ++ /* Make sure we safely get an error */ ++ if (!output_buf || strcmp(output_buf, "")) { ++ atf_tc_fail("pretty_print_option did not return \"\""); ++ } ++} ++ ++ + /* This macro defines main() method that will call specified + test cases. tp and simple_test_case names can be whatever you want + as long as it is a valid variable identifier. */ + ATF_TP_ADD_TCS(tp) + { + ATF_TP_ADD_TC(tp, option_refcnt); ++ ATF_TP_ADD_TC(tp, pretty_print_option); + + return (atf_no_error()); + } +-- +2.16.2 + diff --git a/src/isc-dhcp/patch/series b/src/isc-dhcp/patch/series index 9f7164f5c4..4da1b494ae 100644 --- a/src/isc-dhcp/patch/series +++ b/src/isc-dhcp/patch/series @@ -3,4 +3,9 @@ 0002-Customizable-Option-82-circuit-ID-and-remote-ID-fiel.patch 0003-Support-for-obtaining-name-of-physical-interface-tha.patch 0004-Support-for-loading-port-alias-map-file-to-replace-p.patch - +0005-Add-enable-use-sockets-to-configure-flags-in-debian-.patch +0006-Bugfix-Ensure-HAVE_SO_BINDTODEVICE-has-a-chance-to-b.patch +0007-If-destination-of-BOOTREQUEST-is-directed-broadcast-.patch +0008-CVE-2017-3144.patch +0009-CVE-2018-5733.patch +0010-CVE-2018-5732.patch diff --git a/src/sonic-config-engine/tests/sample_output/docker-dhcp-relay.supervisord.conf b/src/sonic-config-engine/tests/sample_output/docker-dhcp-relay.supervisord.conf index ed14f2ca0f..d285fbfc78 100644 --- a/src/sonic-config-engine/tests/sample_output/docker-dhcp-relay.supervisord.conf +++ b/src/sonic-config-engine/tests/sample_output/docker-dhcp-relay.supervisord.conf @@ -23,7 +23,7 @@ stderr_logfile=syslog programs=isc-dhcp-relay-Vlan1000 [program:isc-dhcp-relay-Vlan1000] -command=/usr/sbin/dhcrelay -d -m discard -a %%h:%%p %%P --name-alias-map-file /tmp/port-name-alias-map.txt -i Vlan1000 -i PortChannel01 -i PortChannel02 -i PortChannel03 -i PortChannel04 192.0.0.1 192.0.0.2 +command=/usr/sbin/dhcrelay -d -m discard -a %%h:%%p %%P --name-alias-map-file /tmp/port-name-alias-map.txt -id Vlan1000 -iu PortChannel01 -iu PortChannel02 -iu PortChannel03 -iu PortChannel04 192.0.0.1 192.0.0.2 priority=3 autostart=false autorestart=false