From a3fb903cb724c5446146c437c747a413e8056016 Mon Sep 17 00:00:00 2001 From: Joshua Muthii Date: Wed, 18 Sep 2024 16:13:40 -0400 Subject: [PATCH] nhrpd: Add topotest for retrying resolution request Modified nhrp_topo topotest to test for newly added resolution request retry feature. Changes to the topotest include adding a spoke to the existing nhrp_topo topotest so that a topology with two spokes and hub can be used to create shortcuts and test the sending/resending of resolution requests and responses between spoke and hub.The resolution request retry feature was tested by blocking incoming resolution requests on a receiving nodes to stop the creation of a successful shortcut - which then triggered the sending spoke to retry sending resolution requests Also modified nhrp_connection_authorized(). Initially, when writing debug informaton about incoming NHRP packets with authentication enabled, the nhrp_connection_authorized() function would print the passphrase of the incoming packet as if it were a null terminated string. This meant that if the passphrase on the incoming packet had non ASCII-complient bytes in it, it would attempt to print those bytes anyway. There was also no check that the size of the passphrase in the incoming packet matched the size of the passphrase on the interface. Moved CISCO_PASS_LENGTH_LEN from nhrp_vty.c to nhrp_protocol.h for easier access to the macro in other files Signed-off-by: Joshua Muthii --- nhrpd/nhrp_peer.c | 49 +++- nhrpd/nhrp_protocol.h | 1 + nhrpd/nhrp_vty.c | 1 - .../nhrp_topo/r1/nhrp_shortcut_present.json | 14 ++ tests/topotests/nhrp_topo/r1/zebra.conf | 1 + tests/topotests/nhrp_topo/r2/nhrp4_cache.json | 13 +- tests/topotests/nhrp_topo/r2/nhrp_route4.json | 26 ++- tests/topotests/nhrp_topo/r2/zebra.conf | 3 + tests/topotests/nhrp_topo/r4/nhrp4_cache.json | 30 +++ tests/topotests/nhrp_topo/r4/nhrp_route4.json | 26 +++ tests/topotests/nhrp_topo/r4/nhrpd.conf | 11 + tests/topotests/nhrp_topo/r4/zebra.conf | 13 ++ tests/topotests/nhrp_topo/test_nhrp_topo.py | 210 +++++++++++++++++- 13 files changed, 376 insertions(+), 22 deletions(-) create mode 100644 tests/topotests/nhrp_topo/r1/nhrp_shortcut_present.json create mode 100644 tests/topotests/nhrp_topo/r4/nhrp4_cache.json create mode 100644 tests/topotests/nhrp_topo/r4/nhrp_route4.json create mode 100644 tests/topotests/nhrp_topo/r4/nhrpd.conf create mode 100644 tests/topotests/nhrp_topo/r4/zebra.conf diff --git a/nhrpd/nhrp_peer.c b/nhrpd/nhrp_peer.c index d2c1a8c40154..fa11980c18ff 100644 --- a/nhrpd/nhrp_peer.c +++ b/nhrpd/nhrp_peer.c @@ -1169,22 +1169,55 @@ static bool nhrp_connection_authorized(struct nhrp_packet_parser *pp) struct nhrp_extension_header *ext; struct zbuf *extensions, pl; int cmp = 1; + int pl_pass_length, auth_pass_length; + size_t auth_size, pl_size; extensions = zbuf_alloc(zbuf_used(&pp->extensions)); zbuf_copy_peek(extensions, &pp->extensions, zbuf_used(&pp->extensions)); while ((ext = nhrp_ext_pull(extensions, &pl)) != NULL) { switch (htons(ext->type) & ~NHRP_EXTENSION_FLAG_COMPULSORY) { case NHRP_EXTENSION_AUTHENTICATION: - cmp = memcmp(auth->buf, pl.buf, zbuf_size(auth)); + /* Size of authentication extensions + * (varies based on password length) + */ + auth_size = zbuf_size(auth); + pl_size = zbuf_size(&pl); auth_ext = (struct nhrp_cisco_authentication_extension *) auth->buf; - debugf(NHRP_DEBUG_COMMON, - "Processing Authentication Extension for (%s:%s|%d)", - auth_ext->secret, - ((struct nhrp_cisco_authentication_extension *) - pl.buf) - ->secret, - cmp); + + if (auth_size == pl_size) + cmp = memcmp(auth_ext, pl.buf, auth_size); + else + cmp = 1; + + if (unlikely(debug_flags & NHRP_DEBUG_COMMON)) { + /* 4 bytes in nhrp_cisco_authentication_extension are allocated + * toward the authentication type. The remaining bytes are used for the + * password - so the password length is just the length of the extension - 4 + */ + auth_pass_length = (auth_size - 4); + pl_pass_length = (pl_size - 4); + /* Because characters are to be printed in HEX, (2* the max pass length) + 1 + * is needed for the string representation + */ + char auth_pass[(2 * NHRP_CISCO_PASS_LEN) + 1] = { 0 }, + pl_pass[(2 * NHRP_CISCO_PASS_LEN) + 1] = { 0 }; + /* Converting bytes in buffer to HEX and saving output as a string - + * Passphrase is converted to HEX in order to avoid printing + * non ACII-compliant characters + */ + for (int i = 0; i < (auth_pass_length); i++) + snprintf(auth_pass + (i * 2), 3, "%02X", + auth_ext->secret[i]); + for (int i = 0; i < (pl_pass_length); i++) + snprintf(pl_pass + (i * 2), 3, "%02X", + ((struct nhrp_cisco_authentication_extension *)pl.buf) + ->secret[i]); + + debugf(NHRP_DEBUG_COMMON, + "Processing Authentication Extension for (%s:%s|%d)", + auth_pass, pl_pass, cmp); + } break; default: /* Ignoring all received extensions except Authentication*/ diff --git a/nhrpd/nhrp_protocol.h b/nhrpd/nhrp_protocol.h index 8cf1ebbcd60e..a4fb315b0e76 100644 --- a/nhrpd/nhrp_protocol.h +++ b/nhrpd/nhrp_protocol.h @@ -73,6 +73,7 @@ /* NHRP Authentication extension types (ala Cisco) */ #define NHRP_AUTHENTICATION_PLAINTEXT 0x00000001 +#define NHRP_CISCO_PASS_LEN 8 /* NHRP Packet Structures */ struct nhrp_packet_header { diff --git a/nhrpd/nhrp_vty.c b/nhrpd/nhrp_vty.c index f2025769609e..199f4d75d4ec 100644 --- a/nhrpd/nhrp_vty.c +++ b/nhrpd/nhrp_vty.c @@ -467,7 +467,6 @@ DEFUN(if_no_nhrp_holdtime, if_no_nhrp_holdtime_cmd, return CMD_SUCCESS; } -#define NHRP_CISCO_PASS_LEN 8 DEFPY(if_nhrp_authentication, if_nhrp_authentication_cmd, AFI_CMD "nhrp authentication PASSWORD$password", AFI_STR diff --git a/tests/topotests/nhrp_topo/r1/nhrp_shortcut_present.json b/tests/topotests/nhrp_topo/r1/nhrp_shortcut_present.json new file mode 100644 index 000000000000..96632d84634e --- /dev/null +++ b/tests/topotests/nhrp_topo/r1/nhrp_shortcut_present.json @@ -0,0 +1,14 @@ +{ + "attr":{ + "entriesCount":1 + }, + "table":[ + { + "type":"dynamic", + "prefix":"192.168.4.0\/24", + "via":"10.255.255.4", + "identity":"" + } + ] +} + diff --git a/tests/topotests/nhrp_topo/r1/zebra.conf b/tests/topotests/nhrp_topo/r1/zebra.conf index b45670fcb25f..c8a216335fc8 100644 --- a/tests/topotests/nhrp_topo/r1/zebra.conf +++ b/tests/topotests/nhrp_topo/r1/zebra.conf @@ -10,3 +10,4 @@ exit interface r1-eth1 ip address 192.168.1.1/24 ! +ip route 0.0.0.0/0 10.255.255.2 diff --git a/tests/topotests/nhrp_topo/r2/nhrp4_cache.json b/tests/topotests/nhrp_topo/r2/nhrp4_cache.json index 34558e0c2883..ee122c59e5ae 100644 --- a/tests/topotests/nhrp_topo/r2/nhrp4_cache.json +++ b/tests/topotests/nhrp_topo/r2/nhrp4_cache.json @@ -1,8 +1,19 @@ { "attr":{ - "entriesCount":2 + "entriesCount":3 }, "table":[ + { + "interface":"r2-gre0", + "type":"dynamic", + "protocol":"10.255.255.4", + "nbma":"10.1.1.4", + "claimed_nbma":"10.1.1.4", + "used":false, + "timeout":true, + "auth":false, + "identity":"" + }, { "interface":"r2-gre0", "type":"local", diff --git a/tests/topotests/nhrp_topo/r2/nhrp_route4.json b/tests/topotests/nhrp_topo/r2/nhrp_route4.json index 7393cba89369..876b24a9b169 100644 --- a/tests/topotests/nhrp_topo/r2/nhrp_route4.json +++ b/tests/topotests/nhrp_topo/r2/nhrp_route4.json @@ -12,7 +12,31 @@ "installed":true, "internalNextHopNum":1, "internalNextHopActiveNum":1, - "nexthops":[ + "nexthops": [ + { + "fib":true, + "directlyConnected":true, + "interfaceName":"r2-gre0", + "active":true + } + ] + } + ], + "10.255.255.4\/32": [ + { + "prefix":"10.255.255.4\/32", + "prefixLen":32, + "protocol":"nhrp", + "vrfId":0, + "vrfName":"default", + "selected":true, + "destSelected":true, + "distance":10, + "metric":0, + "installed":true, + "internalNextHopNum":1, + "internalNextHopActiveNum":1, + "nexthops": [ { "fib":true, "directlyConnected":true, diff --git a/tests/topotests/nhrp_topo/r2/zebra.conf b/tests/topotests/nhrp_topo/r2/zebra.conf index 9f40d4d72e43..756cc6d8c88d 100644 --- a/tests/topotests/nhrp_topo/r2/zebra.conf +++ b/tests/topotests/nhrp_topo/r2/zebra.conf @@ -1,3 +1,4 @@ +ip forwarding interface r2-eth0 ip address 10.2.1.2/24 ! @@ -10,3 +11,5 @@ interface r2-gre0 interface r2-eth1 ip address 192.168.2.2/24 ! +ip route 192.168.4.4/24 10.255.255.4 +ip route 192.168.1.1/24 10.255.255.1 diff --git a/tests/topotests/nhrp_topo/r4/nhrp4_cache.json b/tests/topotests/nhrp_topo/r4/nhrp4_cache.json new file mode 100644 index 000000000000..19074e4e6d71 --- /dev/null +++ b/tests/topotests/nhrp_topo/r4/nhrp4_cache.json @@ -0,0 +1,30 @@ +{ + "attr":{ + "entriesCount":2 + }, + "table":[ + { + "interface":"r4-gre0", + "type":"local", + "protocol":"10.255.255.4", + "nbma":"10.1.1.4", + "claimed_nbma":"10.1.1.4", + "used":false, + "timeout":false, + "auth":false, + "identity":"-" + }, + { + "interface":"r4-gre0", + "type":"nhs", + "protocol":"10.255.255.2", + "nbma":"10.2.1.2", + "claimed_nbma":"10.2.1.2", + "used":false, + "timeout":true, + "auth":false, + "identity":"" + } + ] +} + diff --git a/tests/topotests/nhrp_topo/r4/nhrp_route4.json b/tests/topotests/nhrp_topo/r4/nhrp_route4.json new file mode 100644 index 000000000000..01d627c97738 --- /dev/null +++ b/tests/topotests/nhrp_topo/r4/nhrp_route4.json @@ -0,0 +1,26 @@ +{ + "10.255.255.2\/32": [ + { + "prefix": "10.255.255.2\/32", + "prefixLen": 32, + "protocol": "nhrp", + "vrfId": 0, + "vrfName": "default", + "selected": true, + "destSelected": true, + "distance": 10, + "metric": 0, + "installed": true, + "internalNextHopNum": 1, + "internalNextHopActiveNum": 1, + "nexthops": [ + { + "fib": true, + "directlyConnected": true, + "interfaceName": "r4-gre0", + "active": true + } + ] + } + ] +} diff --git a/tests/topotests/nhrp_topo/r4/nhrpd.conf b/tests/topotests/nhrp_topo/r4/nhrpd.conf new file mode 100644 index 000000000000..df9700c22b33 --- /dev/null +++ b/tests/topotests/nhrp_topo/r4/nhrpd.conf @@ -0,0 +1,11 @@ +log stdout debugging +debug nhrp all +interface r4-gre0 + ip nhrp authentication secret + ip nhrp holdtime 10 + ip nhrp shortcut + ip nhrp network-id 42 + ip nhrp nhs dynamic nbma 10.2.1.2 + ip nhrp registration no-unique + tunnel source r4-eth0 +exit diff --git a/tests/topotests/nhrp_topo/r4/zebra.conf b/tests/topotests/nhrp_topo/r4/zebra.conf new file mode 100644 index 000000000000..b517dbb05ebf --- /dev/null +++ b/tests/topotests/nhrp_topo/r4/zebra.conf @@ -0,0 +1,13 @@ +interface r4-eth0 + ip address 10.1.1.4/24 +! +ip route 10.2.1.0/24 10.1.1.3 +interface r4-gre0 + ip address 10.255.255.4/32 + no link-detect + ipv6 nd suppress-ra +exit +interface r4-eth1 + ip address 192.168.4.4/24 +! +ip route 0.0.0.0/0 10.255.255.2 diff --git a/tests/topotests/nhrp_topo/test_nhrp_topo.py b/tests/topotests/nhrp_topo/test_nhrp_topo.py index 883300310773..90e793f273f9 100644 --- a/tests/topotests/nhrp_topo/test_nhrp_topo.py +++ b/tests/topotests/nhrp_topo/test_nhrp_topo.py @@ -33,18 +33,52 @@ # Required to instantiate the topology builder class. pytestmark = [pytest.mark.nhrpd] +TOPOLOGY = """ + 192.168.2.0/24 + -----+----- + | + | + | + +----------+ + | | + | R2 | + | NHS | + +----------+ + | .2 + | + | + | + GRE P2MP Between + 10.2.1.0/24 + Between Spokes and Hub | + | + 10.255.255.x/32 +----+-----+ + | | + | R3 | + | | + +----+-----+ + |.3 + | + | + +----------+ | +---------+ + | | | | | | | + | |R1 | | | R4 | | +192.168.1.0/24 +-------------|NHC +---------+----------| NHC | ------+ 192.168.4.0/24 + | | |.1 .4| | | + | +----------+ 10.1.1.0/24 +---------+ | +""" def build_topo(tgen): "Build function" - # Create 3 routers. - for routern in range(1, 4): + # Create 4 routers. + for routern in range(1, 5): tgen.add_router("r{}".format(routern)) switch = tgen.add_switch("s1") switch.add_link(tgen.gears["r1"]) switch.add_link(tgen.gears["r3"]) + switch.add_link(tgen.gears["r4"]) switch = tgen.add_switch("s2") switch.add_link(tgen.gears["r2"]) switch.add_link(tgen.gears["r3"]) @@ -53,6 +87,9 @@ def build_topo(tgen): switch = tgen.add_switch("s4") switch.add_link(tgen.gears["r1"]) + switch = tgen.add_switch("s5") + switch.add_link(tgen.gears["r4"]) + def _populate_iface(): tgen = get_topogen() @@ -62,6 +99,7 @@ def _populate_iface(): "echo 0 > /proc/sys/net/ipv4/ip_forward_use_pmtu", "echo 1 > /proc/sys/net/ipv6/conf/{0}-eth0/disable_ipv6", "echo 1 > /proc/sys/net/ipv6/conf/{0}-gre0/disable_ipv6", + "iptables -A FORWARD -i {0}-gre0 -o {0}-gre0 -m hashlimit --hashlimit-upto 4/minute --hashlimit-burst 1 --hashlimit-mode srcip,dstip --hashlimit-srcmask 24 --hashlimit-dstmask 24 --hashlimit-name loglimit-0 -j NFLOG --nflog-group 1 --nflog-range 128", ] cmds_tot = [ @@ -84,10 +122,27 @@ def _populate_iface(): output = tgen.net["r1"].cmd(input) logger.info("output: " + output) + input = cmd.format("r4", "4") + logger.info("input: " + input) + output = tgen.net["r4"].cmd(input) + logger.info("output: " + output) + + +def _verify_iptables(): + tgen = get_topogen() + # Verify iptables is installed + # This is needed for creating shortcuts + for rname in ("r1", "r4"): + rc, _, _ = tgen.net[rname].cmd_status("iptables --version") + if rc == 127: + return False + return True + def setup_module(mod): "Sets up the pytest environment" + logger.info("NHRP Topology : \n {}".format(TOPOLOGY)) result = required_linux_kernel_version("5.0") if result is not True: pytest.skip("Kernel requirements are not met") @@ -103,7 +158,7 @@ def setup_module(mod): TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)), ) - if rname in ("r1", "r2"): + if rname in ("r1", "r2", "r4"): router.load_config( TopoRouter.RD_NHRP, os.path.join(CWD, "{}/nhrpd.conf".format(rname)) ) @@ -226,10 +281,10 @@ def ping_helper(): # force session to reinitialize def relink_session(): - for r in ["r1", "r2"]: + for r in ["r1", "r2", "r4"]: tgen.gears[r].vtysh_cmd("clear ip nhrp cache") - tgen.net[r].cmd("ip l del {}-gre0".format(r)); - _populate_iface(); + tgen.net[r].cmd("ip l del {}-gre0".format(r)) + _populate_iface() @retry(retry_timeout=40, initial_wait=5) def verify_same_password(): @@ -255,24 +310,29 @@ def verify_mismatched_password(): ### Passwords are different logger.info("Modify password and send ping again, should drop") - hubrouter.vtysh_cmd(""" + hubrouter.vtysh_cmd( + """ configure interface r2-gre0 ip nhrp authentication secret12 - """) + """ + ) relink_session() verify_mismatched_password() - + ### Passwords are the same - again logger.info("Recover password and verify conectivity is back") - hubrouter.vtysh_cmd(""" + hubrouter.vtysh_cmd( + """ configure interface r2-gre0 ip nhrp authentication secret - """) + """ + ) relink_session() verify_same_password() + def test_route_install(): "Test use of NHRP routes by other protocols (sharpd here)." tgen = get_topogen() @@ -305,6 +365,134 @@ def test_route_install(): assert result is None, assertmsg +# Initial wait of 30 second because that is +# what the default purge time is for nhrp - +# here we are testing that all of the expected +# retries are sent and logged before a +# shortcut is purged +@retry(retry_timeout=10, initial_wait=30) +def check_retry_debug_info(pingspoke=None): + tgen = get_topogen() + r1 = tgen.gears["r1"] + if pingspoke == None: + pingspoke = r1 + logger.info(f"Check retries are being sent from {pingspoke.name}") + output = pingspoke.cmd("grep -c 'Retrying Resolution Request' nhrpd.log") + # Making sure that we see all expected retries for a 30 second purge time + assertmsg = f"Did not see all expected retries on {pingspoke.name}" + assert output.strip() == "6", assertmsg + logger.info("Check retries are being sent OK") + + +# Helper function to ping between spokes and +# check for either complete or incomplete shortcut +# based on whichever one you are expecting - +# expect_succesful_shortcut inidcates whether +# you are expecting to find a complete shortcut +# (True) or incomplete shortcut (False) as a +# result of the ping +@retry(retry_timeout=10, initial_wait=10) +def create_shortcut(expect_successful_shortcut=True, pingspoke=None, peer_addr=None): + tgen = get_topogen() + r1 = tgen.gears["r1"] + if pingspoke == None: + pingspoke = r1 + if peer_addr == None: + peer_addr = "192.168.4.4" + # Pinging the other spoke in an attempt to create specified type of shortcut + output = pingspoke.cmd(f"ping -c 10 -i .5 {peer_addr}") + print(output) + output = pingspoke.vtysh_cmd("show ip nhrp shortcut") + if expect_successful_shortcut: + logger.info(f"Check shortcut creation from {pingspoke.name} to {peer_addr}") + else: + logger.info( + f"Check incomplete shortcut creation from {pingspoke.name} to {peer_addr}" + ) + + output = pingspoke.vtysh_cmd("show ip nhrp shortcut") + print(output) + if expect_successful_shortcut: + json_file = "{}/{}/nhrp_shortcut_present.json".format(CWD, pingspoke.name) + expected = json.loads(open(json_file).read()) + test_func = partial( + topotest.router_json_cmp, pingspoke, "show ip nhrp shortcut json", expected + ) + _, result = topotest.run_and_expect(test_func, None, count=40, wait=0.5) + + if result is not None: + assertmsg = ( + "Shortcut is not being made between spoke {} and peer {}".format( + pingspoke.name, peer_addr + ) + ) + assert 0, assertmsg + else: + logger.info("Shortcut creation between spokes OK") + else: + # Currentlly, 'show ip nhrp shortcut json' does not show incomplete shortcuts + # so an explicit check for for the 'incompete' keyword needed here + if "incomplete" not in output: + assertmsg = ( + "Incomplete shortcut between spoke {} and peer {} is not seen".format( + pingspoke.name, peer_addr + ) + ) + assert 0, assertmsg + else: + logger.info("Incomplete shortcut creation between spokes OK") + + +# This function tests the NHRP resolution request retries by dropping +# incoming packets (including the NHRP resolution request packets) +# from a receiving spoke in order to stop the NHRP resolution +# responses from ever being sent from that receiving spoke - and in turn +# resolution responses will not reach the sending spoke. +# This will trigger the NHRP resolution request retries which +# can be viewed through log messages. +def test_nhrp_retry_resolution(): + """ " + Verify resolution requests are retried when resolution responses + are not received by a spoke + """ + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + # iptables used to create shortcuts + # and subsequent resolution request retries + if not _verify_iptables(): + pytest.skip("iptables is not installed") + + r1 = tgen.gears["r1"] + r4 = tgen.gears["r4"] + + logger.info("Testing retrying resolution request functionality") + # Make sure that shortcut creation between spokes work + create_shortcut(expect_successful_shortcut=True) + # Clearing shortcut information for spokes + r1.vtysh_cmd("clear ip nhrp shortcut") + r4.vtysh_cmd("clear ip nhrp shortcut") + + # Setting iptables rules to stop incoming packets on r4 + # This should stop resolution requests from reaching + # the receiving router (r4) and hence stop the + # creation of a complete shortcut + r4.cmd("iptables -A INPUT -i r4-eth0 -j DROP") + + # Make sure that nhrp debugging is enabled to read the retry logs + r1.vtysh_cmd( + """ + configure + debug nhrp all + """ + ) + create_shortcut(expect_successful_shortcut=False) + # Look for retry logging output for resolution request retries + check_retry_debug_info() + # Undo iptables rule + r4.cmd("iptables -D INPUT -i r4-eth0 -j DROP") + + def test_memory_leak(): "Run the memory leak test and report results." tgen = get_topogen()