]> git.ipfire.org Git - thirdparty/dhcp.git/commitdiff
[#71] Fix dhcrelay agent option buffer pointer logic
authorThomas Markwalder <tmark@isc.org>
Fri, 20 Dec 2019 15:11:54 +0000 (10:11 -0500)
committerThomas Markwalder <tmark@isc.org>
Tue, 14 Jan 2020 19:28:42 +0000 (14:28 -0500)
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

configure
configure.ac
configure.ac+lt
configure.ac-base
configure.ac-lt
relay/dhcrelay.c
relay/tests/Atffile [new file with mode: 0644]
relay/tests/Kyuafile [new file with mode: 0644]
relay/tests/Makefile.am [new file with mode: 0644]
relay/tests/relay_unittests.c [new file with mode: 0644]

index fb60de60944aaa3f7592125461e2dd72583b09b5..24bb2f6e2f2b799d79f868fb43c35a34d686455e 100755 (executable)
--- a/configure
+++ b/configure
@@ -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" ;;
index c5d88f702296c111eddc1c05914e6ea96ce16b87..56381ea0e252c31360856f98457a680b2e2b3db6 100644 (file)
@@ -1004,6 +1004,7 @@ AC_CONFIG_FILES([
   omapip/Makefile.am
   omapip/Makefile
   relay/Makefile
+  relay/tests/Makefile
   server/Makefile
   tests/Makefile.am
   tests/Makefile
index 2dc3231e00bac703cff7ca49bcfaaa96c78588e1..3709b7ba90408b4218af02e26629d01efa5ca5e5 100644 (file)
@@ -1005,6 +1005,7 @@ AC_CONFIG_FILES([
   omapip/Makefile.am
   omapip/Makefile
   relay/Makefile
+  relay/tests/Makefile
   server/Makefile
   tests/Makefile.am
   tests/Makefile
index 864c1cdb1b9e490fc3bb1fbf12d38f55387bff5f..1ba620fcf6ae81ec7d8524d04e73b51969abb531 100644 (file)
@@ -1039,6 +1039,7 @@ AC_CONFIG_FILES([
   omapip/Makefile.am
   omapip/Makefile
   relay/Makefile
+  relay/tests/Makefile
   server/Makefile
   tests/Makefile.am
   tests/Makefile
index c5d88f702296c111eddc1c05914e6ea96ce16b87..56381ea0e252c31360856f98457a680b2e2b3db6 100644 (file)
@@ -1004,6 +1004,7 @@ AC_CONFIG_FILES([
   omapip/Makefile.am
   omapip/Makefile
   relay/Makefile
+  relay/tests/Makefile
   server/Makefile
   tests/Makefile.am
   tests/Makefile
index 896e1e2e611e68e9815c0d3efbd5bb8e54bdd604..980dacaeb1745ea83945259b6d04c1ed30bd10f8 100644 (file)
@@ -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 */
diff --git a/relay/tests/Atffile b/relay/tests/Atffile
new file mode 100644 (file)
index 0000000..0572f31
--- /dev/null
@@ -0,0 +1,5 @@
+Content-Type: application/X-atf-atffile; version="1"
+
+prop: test-suite = dhcrelay 
+
+tp-glob: *_unittests
diff --git a/relay/tests/Kyuafile b/relay/tests/Kyuafile
new file mode 100644 (file)
index 0000000..b78192f
--- /dev/null
@@ -0,0 +1,4 @@
+syntax(2)
+test_suite('dhcrelay')
+
+atf_test_program{name='dhcrelay_unittests'}
diff --git a/relay/tests/Makefile.am b/relay/tests/Makefile.am
new file mode 100644 (file)
index 0000000..0da6e29
--- /dev/null
@@ -0,0 +1,49 @@
+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)
diff --git a/relay/tests/relay_unittests.c b/relay/tests/relay_unittests.c
new file mode 100644 (file)
index 0000000..5932098
--- /dev/null
@@ -0,0 +1,407 @@
+/*
+ * 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) {
+       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 */
+            0x52, 0x10,                   /* Relay Agent, len = 16 */
+            0x1, 0x6,                     /* Circuit id, len = 6 */
+            0x1, 0x2, 0x3, 0x4, 0x5, 0x6,
+            0x2, 0x6,                     /* Remete id, len = 6 */
+            0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0xff
+        };
+
+        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 = add_relay_agent_options(&ifc, &packet, len, giaddr);
+        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 = add_relay_agent_options(&ifc, &packet, len, giaddr);
+        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 = add_relay_agent_options(&ifc, &packet, len, giaddr);
+        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");
+        }
+    }
+}
+
+ATF_TP_ADD_TCS(tp) {
+    ATF_TP_ADD_TC(tp, strip_relay_agent_options_test);
+    ATF_TP_ADD_TC(tp, add_relay_agent_options_test);
+
+    return (atf_no_error());
+}