Commit 0a2f9a62 authored by Thomas Markwalder's avatar Thomas Markwalder

[#71] Fix dhcrelay agent option buffer pointer logic

relay/dhcrelay.c
    Added UNIT_TEST conditionals and such for unit test support

    strip_relay_agent_options()
    strip_relay_agent_options()
    - corrected buffer pointer logic

relay/tests/Atffile
relay/tests/Kyuafile
relay/tests/Makefile.am
relay/tests/relay_unittests.c
    - new unit test files

configure.ac-base
    added relay/tests/Makefile

regenerated configure files
	configure
    configure.ac
    configure.ac+lt
    configure.ac-lt
parent 5da634c5
......@@ -736,6 +736,7 @@ infodir
docdir
oldincludedir
includedir
runstatedir
localstatedir
sharedstatedir
sysconfdir
......@@ -848,6 +849,7 @@ datadir='${datarootdir}'
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
......@@ -1100,6 +1102,15 @@ do
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
-runstatedir | --runstatedir | --runstatedi | --runstated \
| --runstate | --runstat | --runsta | --runst | --runs \
| --run | --ru | --r)
ac_prev=runstatedir ;;
-runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
| --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
| --run=* | --ru=* | --r=*)
runstatedir=$ac_optarg ;;
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
......@@ -1237,7 +1248,7 @@ fi
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
libdir localedir mandir
libdir localedir mandir runstatedir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
......@@ -1390,6 +1401,7 @@ Fine tuning of the installation directories:
--sysconfdir=DIR read-only single-machine data [PREFIX/etc]
--sharedstatedir=DIR modifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
--runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIR object code libraries [EPREFIX/lib]
--includedir=DIR C header files [PREFIX/include]
--oldincludedir=DIR C header files for non-gcc [/usr/include]
......@@ -7540,7 +7552,7 @@ $as_echo "#define FLEXIBLE_ARRAY_MEMBER /**/" >>confdefs.h
fi
ac_config_files="$ac_config_files Makefile client/Makefile client/tests/Makefile common/Makefile.am common/Makefile common/tests/Makefile dhcpctl/Makefile.am dhcpctl/Makefile includes/Makefile keama/Makefile omapip/Makefile.am omapip/Makefile relay/Makefile server/Makefile tests/Makefile.am tests/Makefile tests/unittest.sh server/tests/Makefile doc/devel/doxyfile"
ac_config_files="$ac_config_files Makefile client/Makefile client/tests/Makefile common/Makefile.am common/Makefile common/tests/Makefile dhcpctl/Makefile.am dhcpctl/Makefile includes/Makefile keama/Makefile omapip/Makefile.am omapip/Makefile relay/Makefile relay/tests/Makefile server/Makefile tests/Makefile.am tests/Makefile tests/unittest.sh server/tests/Makefile doc/devel/doxyfile"
cat >confcache <<\_ACEOF
# This file is a shell script that caches the results of configure
......@@ -8304,6 +8316,7 @@ do
"omapip/Makefile.am") CONFIG_FILES="$CONFIG_FILES omapip/Makefile.am" ;;
"omapip/Makefile") CONFIG_FILES="$CONFIG_FILES omapip/Makefile" ;;
"relay/Makefile") CONFIG_FILES="$CONFIG_FILES relay/Makefile" ;;
"relay/tests/Makefile") CONFIG_FILES="$CONFIG_FILES relay/tests/Makefile" ;;
"server/Makefile") CONFIG_FILES="$CONFIG_FILES server/Makefile" ;;
"tests/Makefile.am") CONFIG_FILES="$CONFIG_FILES tests/Makefile.am" ;;
"tests/Makefile") CONFIG_FILES="$CONFIG_FILES tests/Makefile" ;;
......
......@@ -1004,6 +1004,7 @@ AC_CONFIG_FILES([
omapip/Makefile.am
omapip/Makefile
relay/Makefile
relay/tests/Makefile
server/Makefile
tests/Makefile.am
tests/Makefile
......
......@@ -1005,6 +1005,7 @@ AC_CONFIG_FILES([
omapip/Makefile.am
omapip/Makefile
relay/Makefile
relay/tests/Makefile
server/Makefile
tests/Makefile.am
tests/Makefile
......
......@@ -1039,6 +1039,7 @@ AC_CONFIG_FILES([
omapip/Makefile.am
omapip/Makefile
relay/Makefile
relay/tests/Makefile
server/Makefile
tests/Makefile.am
tests/Makefile
......
......@@ -1004,6 +1004,7 @@ AC_CONFIG_FILES([
omapip/Makefile.am
omapip/Makefile
relay/Makefile
relay/tests/Makefile
server/Makefile
tests/Makefile.am
tests/Makefile
......
......@@ -114,9 +114,11 @@ struct stream_list {
int id;
} *downstreams, *upstreams;
#ifndef UNIT_TEST
static struct stream_list *parse_downstream(char *);
static struct stream_list *parse_upstream(char *);
static void setup_streams(void);
#endif /* UNIT_TEST */
/*
* A pointer to a subscriber id to add to the message we forward.
......@@ -127,18 +129,23 @@ static void setup_streams(void);
char *dhcrelay_sub_id = NULL;
#endif
#ifndef UNIT_TEST
static void do_relay4(struct interface_info *, struct dhcp_packet *,
unsigned int, unsigned int, struct iaddr,
struct hardware *);
static int add_relay_agent_options(struct interface_info *,
struct dhcp_packet *, unsigned,
struct in_addr);
static int find_interface_by_agent_option(struct dhcp_packet *,
struct interface_info **, u_int8_t *, int);
static int strip_relay_agent_options(struct interface_info *,
struct interface_info **,
struct dhcp_packet *, unsigned);
#endif /* UNIT_TEST */
extern int add_relay_agent_options(struct interface_info *,
struct dhcp_packet *, unsigned,
struct in_addr);
extern int find_interface_by_agent_option(struct dhcp_packet *,
struct interface_info **, u_int8_t *, int);
extern int strip_relay_agent_options(struct interface_info *,
struct interface_info **,
struct dhcp_packet *, unsigned);
#ifndef UNIT_TEST
static void request_v4_interface(const char* name, int flags);
static const char copyright[] =
......@@ -269,7 +276,7 @@ usage(const char *sfmt, const char *sarg) {
isc_file_basename(progname));
}
int
int
main(int argc, char **argv) {
isc_result_t status;
struct servent *ent;
......@@ -310,7 +317,7 @@ main(int argc, char **argv) {
#if !defined(DEBUG)
setlogmask(LOG_UPTO(LOG_INFO));
#endif
#endif
/* Parse arguments changing no_daemon */
for (i = 1; i < argc; i++) {
......@@ -524,7 +531,7 @@ main(int argc, char **argv) {
log_fatal("%s: uplink interface_allocate: %s",
argv[i], isc_result_totext(status));
}
if (strlen(argv[i]) >= sizeof(uplink->name)) {
log_fatal("%s: uplink name too long,"
" it cannot exceed: %ld characters",
......@@ -665,7 +672,7 @@ main(int argc, char **argv) {
log_info(copyright);
log_info(arr);
log_info(url);
} else
} else
log_perror = 0;
/* Set default port */
......@@ -789,7 +796,7 @@ main(int argc, char **argv) {
else {
fprintf(pf, "%ld\n",(long)getpid());
fclose(pf);
}
}
}
}
......@@ -968,14 +975,16 @@ do_relay4(struct interface_info *ip, struct dhcp_packet *packet,
++client_packets_relayed;
}
}
}
#endif /* UNIT_TEST */
/* Strip any Relay Agent Information options from the DHCP packet
option buffer. If there is a circuit ID suboption, look up the
outgoing interface based upon it. */
static int
int
strip_relay_agent_options(struct interface_info *in,
struct interface_info **out,
struct dhcp_packet *packet,
......@@ -1055,8 +1064,13 @@ strip_relay_agent_options(struct interface_info *in,
return (0);
if (sp != op) {
memmove(sp, op, op[1] + 2);
sp += op[1] + 2;
size_t mlen = op[1] + 2;
memmove(sp, op, mlen);
sp += mlen;
if (sp > max) {
return (0);
}
op = nextop;
} else
op = sp = nextop;
......@@ -1107,7 +1121,7 @@ strip_relay_agent_options(struct interface_info *in,
we find a circuit ID that matches an existing interface do we tell
the caller to go ahead and process the packet. */
static int
int
find_interface_by_agent_option(struct dhcp_packet *packet,
struct interface_info **out,
u_int8_t *buf, int len) {
......@@ -1171,7 +1185,7 @@ find_interface_by_agent_option(struct dhcp_packet *packet,
* Agent Information option tacked onto its tail. If it is, tack
* the option on.
*/
static int
int
add_relay_agent_options(struct interface_info *ip, struct dhcp_packet *packet,
unsigned length, struct in_addr giaddr) {
int is_dhcp = 0, mms;
......@@ -1284,8 +1298,13 @@ add_relay_agent_options(struct interface_info *ip, struct dhcp_packet *packet,
end_pad = NULL;
if (sp != op) {
memmove(sp, op, op[1] + 2);
sp += op[1] + 2;
size_t mlen = op[1] + 2;
memmove(sp, op, mlen);
sp += mlen;
if (sp > max) {
return (0);
}
op = nextop;
} else
op = sp = nextop;
......@@ -1414,6 +1433,8 @@ add_relay_agent_options(struct interface_info *ip, struct dhcp_packet *packet,
return (length);
}
#ifndef UNIT_TEST
#ifdef DHCPv6
/*
* Parse a downstream argument: [address%]interface[#index].
......@@ -1726,7 +1747,7 @@ process_up6(struct packet *packet, struct stream_list *dp) {
if (!option_state_allocate(&opts, MDL)) {
log_fatal("No memory for upwards options.");
}
/* Add an interface-id (if used). */
if (use_if_id) {
int if_id;
......@@ -1763,7 +1784,7 @@ process_up6(struct packet *packet, struct stream_list *dp) {
return;
}
}
#if defined(RELAY_PORT)
/*
......@@ -1802,7 +1823,7 @@ process_up6(struct packet *packet, struct stream_list *dp) {
/* Finish the relay-forward message. */
cursor += store_options6(forw_data + cursor,
sizeof(forw_data) - cursor,
opts, packet,
opts, packet,
required_forw_opts, NULL);
option_state_dereference(&opts, MDL);
......@@ -1812,7 +1833,7 @@ process_up6(struct packet *packet, struct stream_list *dp) {
(size_t) cursor, &up->link);
}
}
/*
* Process a packet downwards, i.e., from server to client.
*/
......@@ -2126,3 +2147,4 @@ void request_v4_interface(const char* name, int flags) {
interface_snorf(tmp, (INTERFACE_REQUESTED | flags));
interface_dereference(&tmp, MDL);
}
#endif /* UNIT_TEST */
Content-Type: application/X-atf-atffile; version="1"
prop: test-suite = dhcrelay
tp-glob: *_unittests
syntax(2)
test_suite('dhcrelay')
atf_test_program{name='dhcrelay_unittests'}
SUBDIRS = .
AM_CPPFLAGS = $(ATF_CFLAGS) -DUNIT_TEST -I$(top_srcdir)/includes
AM_CPPFLAGS += -I@BINDDIR@/include -I$(top_srcdir)
AM_CPPFLAGS += -DLOCALSTATEDIR='"."'
EXTRA_DIST = Atffile Kyuafile
# for autotools debugging only
info:
@echo "ATF_CFLAGS=$(ATF_CFLAGS)"
@echo "ATF_LDFLAGS=$(ATF_LDFLAGS)"
@echo "ATF_LIBS=$(ATF_LIBS)"
DHCPSRC = ../dhcrelay.c
DHCPLIBS = $(top_builddir)/common/libdhcp.@A@ \
$(top_builddir)/omapip/libomapi.@A@ \
@BINDLIBIRSDIR@/libirs.@A@ \
@BINDLIBDNSDIR@/libdns.@A@ \
@BINDLIBISCCFGDIR@/libisccfg.@A@ \
@BINDLIBISCDIR@/libisc.@A@
ATF_TESTS =
if HAVE_ATF
ATF_TESTS += relay_unittests
relay_unittests_SOURCES = $(DHCPSRC)
relay_unittests_SOURCES += relay_unittests.c
relay_unittests_LDADD = $(ATF_LDFLAGS)
relay_unittests_LDADD += $(DHCPLIBS)
check: $(ATF_TESTS)
@if test $(top_srcdir) != ${top_builddir}; then \
cp $(top_srcdir)/relay/tests/Atffile Atffile; \
cp $(top_srcdir)/relay/tests/Kyuafile Kyuafile; \
fi
sh ${top_builddir}/tests/unittest.sh
distclean-local:
@if test $(top_srcdir) != ${top_builddir}; then \
rm -f Atffile Kyuafile;
fi
endif
check_PROGRAMS = $(ATF_TESTS)
/*
* Copyright (c) 2019 by 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.
*
* Internet Systems Consortium, Inc.
* 950 Charter Street
* Redwood City, CA 94063
* <info@isc.org>
* https://www.isc.org/
*
*/
#include "config.h"
#include <atf-c.h>
#include <omapip/omapip_p.h>
#include "dhcpd.h"
/* @brief Externs for dhcrelay.c functions under test */
extern int add_agent_options;
extern int add_relay_agent_options(struct interface_info *,
struct dhcp_packet *, unsigned,
struct in_addr);
extern int find_interface_by_agent_option(struct dhcp_packet *,
struct interface_info **,
u_int8_t *, int);
extern int strip_relay_agent_options(struct interface_info *,
struct interface_info **,
struct dhcp_packet *, unsigned);
/* @brief Add the given option data to a DHCPv4 packet
*
* It first fills the packet.options buffer with the given pad character.
* Next it copies the DHCP magic cookie value into the beginning of the
* options buffer. Finally it appends the given data after the cookie.
*
* @param packet pointer to the packet
* @param data pointer to the option data to copy into the packet's options
* buffer
* @param len length of the option data to copy
* @param pad byte value with which to initialize the packet's options buffer
*
* @return returns the new length of the packet
*/
unsigned set_packet_options(struct dhcp_packet *packet,
unsigned char* data, unsigned len,
unsigned char pad) {
unsigned new_len;
memset(packet->options, pad, DHCP_MAX_OPTION_LEN);
// Add the COOKIE
new_len = 4;
memcpy(packet->options, DHCP_OPTIONS_COOKIE, new_len);
new_len += len;
if (new_len > DHCP_MAX_OPTION_LEN) {
return(0);
}
memcpy(&packet->options[4], data, len);
return(new_len + DHCP_FIXED_NON_UDP);
}
/* @brief Checks two sets of option data for equalit
*
* It constructs the expected options content by creating an options buffer
* filled with the pad value. Next it copies the DHCP magic cookie value
* into the beginning of the buffer and then appends the expected data after
* the cookie. It the compares this buffer to the actual buffer passed in
* for equality and returns the result.
*
* @param actual_options pointer to the packet::options to be checked
* @param expected_data pointer to the expected options data (everything after
* the DHCP cookie)
* @param data_len length of the expected options data
* @param pad byte value with which to initialize the packet's options buffer
*
* @return zero it the sets of data match, non-zero otherwise
*/
int check_with_pad(unsigned char* actual_options,
unsigned char *expected_data,
unsigned data_len, unsigned char pad) {
unsigned char exp_options[DHCP_MAX_OPTION_LEN];
unsigned new_len;
memset(exp_options, pad, DHCP_MAX_OPTION_LEN);
new_len = 4;
memcpy(exp_options, DHCP_OPTIONS_COOKIE, new_len);
new_len += data_len;
if (new_len > DHCP_MAX_OPTION_LEN) {
return(-1);
}
memcpy(&exp_options[4], expected_data, data_len);
return (memcmp(actual_options, exp_options, DHCP_MAX_OPTION_LEN));
}
ATF_TC(strip_relay_agent_options_test);
ATF_TC_HEAD(strip_relay_agent_options_test, tc) {
atf_tc_set_md_var(tc, "descr", "tests strip_relay-agent_options");
}
/* This Test exercises strip_relay_agent_options() function */
ATF_TC_BODY(strip_relay_agent_options_test, tc) {
struct interface_info ifaces;
struct interface_info *matched;
struct dhcp_packet packet;
unsigned len;
int ret;
memset(&ifaces, 0x0, sizeof(ifaces));
matched = 0;
memset(&packet, 0x0, sizeof(packet));
len = 0;
/* Make sure an empty packet is harmless */
ret = strip_relay_agent_options(&ifaces, &matched, &packet, len);
if (ret != 0) {
atf_tc_fail("empty packet failed");
}
{
/*
* Uses valid input option data to verify that:
* - When add_agent_options is false, the output option data is the
* the same as the input option data (i.e. nothing removed)
* - When add_agent_options is true, the relay agent option has
* been removed from the output option data
* - When add_agent_options is true, 0 length relay agent option has
* been removed from the output option data
*
*/
unsigned char input_data[] = {
0x35, 0x00, /* DHCP_TYPE = DISCOVER */
0x52, 0x00, /* Relay Agent Option, len = 0 */
0x52, 0x08, 0x01, 0x06, 0x65, /* Relay Agent Option, len = 8 */
0x6e, 0x70, 0x30, 0x73, 0x4f,
0x42, 0x02, 0x00, 0x10, /* Opt 0x42, len = 2 */
0x43, 0x00 /* Opt 0x43, len = 0 */
};
unsigned char input_data2[] = {
0x35, 0x00, /* DHCP_TYPE = DISCOVER */
0x52, 0x00, /* Relay Agent Option, len = 0 */
0x42, 0x02, 0x00, 0x10, /* Opt 0x42, len = 2 */
0x43, 0x00 /* Opt 0x43, len = 0 */
};
unsigned char output_data[] = {
0x35, 0x00, /* DHCP_TYPE = DISCOVER */
0x42, 0x02, 0x00, 0x10, /* Opt 0x42, len = 2 */
0x43, 0x00 /* Opt 0x43, len = 0 */
};
unsigned char pad = 0x0;
len = set_packet_options(&packet, input_data, sizeof(input_data), pad);
if (len == 0) {
atf_tc_fail("input_data: set_packet_options failed");
}
/* When add_agent_options = 0, no change should occur */
add_agent_options = 0;
ret = strip_relay_agent_options(&ifaces, &matched, &packet, len);
if (ret != len) {
atf_tc_fail("expected unchanged len %d, returned %d", len, ret);
}
if (check_with_pad(packet.options, input_data, sizeof(input_data),
pad) != 0) {
atf_tc_fail("expected unchanged data, does not match");
}
/* When add_agent_options = 1, it should remove the eight byte */
/* relay agent option. */
add_agent_options = 1;
/* Because the option data is less is small, the packet should be */
/* padded out to BOOTP_MIN_LEN */
ret = strip_relay_agent_options(&ifaces, &matched, &packet, len);
if (ret != BOOTP_MIN_LEN) {
atf_tc_fail("input_data: len of %d, returned %d",
BOOTP_MIN_LEN, ret);
}
if (check_with_pad(packet.options, output_data, sizeof(output_data),
pad) != 0) {
atf_tc_fail("input_data: expected data does not match");
}
// Now let's repeat it with a relay agent option 0 bytes in length.
len = set_packet_options(&packet, input_data2, sizeof(input_data2), pad);
if (len == 0) {
atf_tc_fail("input_data2 set_packet_options failed");
}
/* Because the option data is less is small, the packet should be */
/* padded out to BOOTP_MIN_LEN */
ret = strip_relay_agent_options(&ifaces, &matched, &packet, len);
if (ret != BOOTP_MIN_LEN) {
atf_tc_fail("input_data2: len of %d, returned %d",
BOOTP_MIN_LEN, ret);
}
if (check_with_pad(packet.options, output_data, sizeof(output_data),
pad) != 0) {
atf_tc_fail("input_data2: expected output does not match");
}
}
{
/* Verify that oversized pack filled with long options does not */
/* cause overrun */
/* We borrowed this union from discover.c:got_one() */
union {
unsigned char packbuf [4095]; /* Packet input buffer.
Must be as large as largest
possible MTU. */
struct dhcp_packet packet;
} u;
unsigned char input_data[] = {
0x35, 0x00, // DHCP_TYPE = DISCOVER
0x52, 0x00, // Relay Agent Option, len = 0
0x42, 0x02, 0x00, 0x10, // Opt 0x42, len = 2
0x43, 0x00 // Opt 0x43, len = 0
};
/* We'll pad it 0xFE, that way wherever we hit for "length" we'll */
/* have length of 254. Increasing the odds we'll push over the end. */
unsigned char pad = 0xFE;
memset(u.packbuf, pad, 4095);
len = set_packet_options(&u.packet, input_data, sizeof(input_data), pad);
if (len == 0) {
atf_tc_fail("overrun: set_packet_options failed");
}
// With add_agent_options = 1, it should remove the two byte
// relay agent option.
add_agent_options = 1;
ret = strip_relay_agent_options(&ifaces, &matched, &u.packet, 4095);
if (ret != 0) {
atf_tc_fail("overrun expected return len = 0, we got %d", ret);
}
}
}
ATF_TC(add_relay_agent_options_test);
ATF_TC_HEAD(add_relay_agent_options_test, tc) {
atf_tc_set_md_var(tc, "descr", "tests agent_relay-agent_options");
}
/* This Test exercises add_relay_agent_options() function */
ATF_TC_BODY(add_relay_agent_options_test, tc) {
struct interface_info ifc;
struct dhcp_packet packet;
unsigned len;
int ret;
struct in_addr giaddr;
giaddr.s_addr = inet_addr("192.0.1.1");
u_int8_t circuit_id[] = { 0x01,0x02,0x03,0x04,0x05,0x06 };
u_int8_t remote_id[] = { 0x11,0x22,0x33,0x44,0x55,0x66 };
memset(&ifc, 0x0, sizeof(ifc));
ifc.circuit_id = circuit_id;
ifc.circuit_id_len = sizeof(circuit_id);
ifc.remote_id = remote_id;
ifc.remote_id_len = sizeof(remote_id);
memset(&packet, 0x0, sizeof(packet));
len = 0;
/* Make sure an empty packet is harmless */
ret = add_relay_agent_options(&ifc, &packet, len, giaddr);
if (ret != 0) {