diff --git a/nhrpd/nhrp_peer.c b/nhrpd/nhrp_peer.c index d2c1a8c40154..ace456f5243a 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.c b/nhrpd/nhrp_protocol.c new file mode 100644 index 000000000000..e69de29bb2d1 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..ffda6dbc7618 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,8 @@ 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 +98,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 +121,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 +157,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 +280,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(): @@ -262,7 +316,7 @@ def verify_mismatched_password(): """) relink_session() verify_mismatched_password() - + ### Passwords are the same - again logger.info("Recover password and verify conectivity is back") hubrouter.vtysh_cmd(""" @@ -305,6 +359,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()