TCP: put TCP-AO option decoding right

As it was correctly pointed out in GitHub issue #516, the TCPOPT_TCPAO
(formerly TCPOPT_AUTH) case had an issue with option length processing,
though without significant consequences thanks to a check elsewhere.
Besides that, the old code (introduced in 2005) decoded a structure
similar to a proposed encoding variant of the early (first published in
2007) revisions of the Internet-Draft but different from the encoding
of RFC 5925 (published in 2010). These issues are now addressed and the
TCP option renamed to TCP-AO.

diff --git a/print-tcp.c b/print-tcp.c
index 0ed158fa41fe0371c669496c1a24f909279c9713..e0d0531b36b26eed49ff08449cc6c880cb0524e0 100644 (file)
--- a/print-tcp.c
+++ b/print-tcp.c
@@ -129,7 +129,7 @@ static const struct tok tcp_option_values[] = {
         { TCPOPT_SIGNATURE, "md5" },
         { TCPOPT_SCPS, "scps" },
         { TCPOPT_UTO, "uto" },
-        { TCPOPT_AUTH, "enhanced auth" },
+        { TCPOPT_TCPAO, "tcp-ao" },
         { TCPOPT_MPTCP, "mptcp" },
         { TCPOPT_FASTOPEN, "tfo" },
         { TCPOPT_EXPERIMENT2, "exp" },
@@ -544,17 +544,30 @@ tcp_print(netdissect_options *ndo,
                                 ND_PRINT((ndo, " cap %02x id %u", cp[0], cp[1]));
                                 break;
 
-                        case TCPOPT_AUTH:
-                                LENCHECK(1);
-                                ND_PRINT((ndo, " keyid %d", *cp++));
-                                datalen = len - 3;
-                                for (i = 0; i < datalen; ++i) {
-                                        LENCHECK(i + 1);
-                                        ND_PRINT((ndo, "%02x", cp[i]));
+                        case TCPOPT_TCPAO:
+                                datalen = len - 2;
+                                /* RFC 5925 Section 2.2:
+                                 * "The Length value MUST be greater than or equal to 4."
+                                 * (This includes the Kind and Length fields already processed
+                                 * at this point.)
+                                 */
+                                if (datalen < 2) {
+                                        ND_PRINT((ndo, " invalid"));
+                                } else {
+                                        LENCHECK(1);
+                                        ND_PRINT((ndo, " keyid %u", cp[0]));
+                                        LENCHECK(2);
+                                        ND_PRINT((ndo, " rnextkeyid %u", cp[1]));
+                                        if (datalen > 2) {
+                                                ND_PRINT((ndo, " mac 0x"));
+                                                for (i = 2; i < datalen; i++) {
+                                                        LENCHECK(i + 1);
+                                                        ND_PRINT((ndo, "%02x", cp[i]));
+                                                }
+                                        }
                                 }
                                 break;
 
-
                         case TCPOPT_EOL:
                         case TCPOPT_NOP:
                         case TCPOPT_SACKOK:

diff --git a/tcp.h b/tcp.h
index 1084db9a2250dcd8ab631319b998b7d0c7c335ac..912b5e820ca6de542d51b4f5ba0f90086eb40188 100644 (file)
--- a/tcp.h
+++ b/tcp.h
@@ -84,7 +84,7 @@ struct tcphdr {
 #define TCPOPT_SCPS            20      /* SCPS-TP (CCSDS 714.0-B-2) */
 #define        TCPOPT_UTO              28      /* tcp user timeout (rfc5482) */
 #define           TCPOLEN_UTO                  4
-#define TCPOPT_AUTH            29      /* Enhanced AUTH option (rfc5925) */
+#define TCPOPT_TCPAO           29      /* TCP authentication option (rfc5925) */
 #define        TCPOPT_MPTCP            30      /* MPTCP options */
 #define TCPOPT_FASTOPEN                34      /* TCP Fast Open (rfc7413) */
 #define TCPOPT_EXPERIMENT2     254     /* experimental headers (rfc4727) */

diff --git a/tests/tcp-auth-heapoverflow.out b/tests/tcp-auth-heapoverflow.out
index bbf12a088d7cffde7d3700943b35d4b8cce898fc..b7ff7f7ab3b1891a70ed27cbbf00fcae96739f9c 100644 (file)
--- a/tests/tcp-auth-heapoverflow.out
+++ b/tests/tcp-auth-heapoverflow.out
@@ -1,2 +1,2 @@
 IP (tos 0x30, ttl 48, id 12336, offset 0, flags [DF], proto TCP (6), length 12336, bad cksum 3030 (->29a8)!)
-    48.48.48.48.12336 > 48.48.48.48.12336: Flags [.U], seq 808464432:808476696, ack 808464432, win 12336, urg 12336, options [enhanced auth keyid 4830303030303030[|tcp]
+    48.48.48.48.12336 > 48.48.48.48.12336: Flags [.U], seq 808464432:808476696, ack 808464432, win 12336, urg 12336, options [tcp-ao keyid 48 rnextkeyid 48 mac 0x303030303030[|tcp]