nhrpd: Add topotest for retrying resolution request

Modified nhrp_topo topotest to test for newly added resolution request retry feature Also modified nhrp_connection_authorized() function so that debugging messages don't read into NHRP authentication extension as if it were a null-terminated string - which can lead to printing of non ASCII-compliant bytes. 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 <jmuthii@labn.net>
FRRouting · Oct 15, 2024 · 32ddfd1 · 32ddfd1
1 parent 0aef4e4
commit 32ddfd1
Show file tree

Hide file tree

Showing 14 changed files with 365 additions and 18 deletions.
diff --git a/nhrpd/nhrp_peer.c b/nhrpd/nhrp_peer.c
@@ -1169,22 +1169,54 @@ 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++)
+					sprintf(auth_pass + (i * 2), "%02X", auth_ext->secret[i]);
+				for (int i = 0; i < (pl_pass_length); i++)
+					sprintf(pl_pass + (i * 2), "%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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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