]> The Tcpdump Group git mirrors - tcpdump/commitdiff
Clean up code a bit.
authorGuy Harris <[email protected]>
Sun, 31 Mar 2019 01:08:02 +0000 (18:08 -0700)
committerGuy Harris <[email protected]>
Sun, 31 Mar 2019 01:08:02 +0000 (18:08 -0700)
"ivoff" is a pointer to the IV, not the offset of the IV; call it ivptr.

Have a variable that points to the beginning of the ciphertext, and use
that.

Fix the check that makes sure the authentication data/integrity check
value length isn't too big - it needs to make sure that it doesn't go
before the beginning of the ciphertext, i.e. doesn't overlap with the
IV.

Don't bother with a variable pointing to the secret, just pass
sa->secret.

Fix the check that makes sure the padding length isn't too big - make
sure it, plus 2 for the padding length and next header bytes, isn't
bigger than the ciphertext length.

Update a test to reflect the stricter length checks.

print-esp.c
tests/espudp1.out

index ae6dc27bc5d60da588f4d346f7cc3a0863fa32d5..1b440c0837bc7b1bbf1ecd928f568c6342338db5 100644 (file)
@@ -688,12 +688,10 @@ esp_print(netdissect_options *ndo,
        const struct ip *ip;
        struct sa_list *sa = NULL;
        const struct ip6_hdr *ip6 = NULL;
-       int advance;
-       u_int ctlen;
-       u_char *secret;
+       const u_char *ivptr;
        u_int ivlen;
-       const u_char *ivoff;
-       const u_char *p;
+       const u_char *ctptr;
+       u_int ctlen;
        EVP_CIPHER_CTX *ctx;
        unsigned int block_size, buffer_size;
        u_char *input_buffer, *output_buffer;
@@ -704,16 +702,6 @@ esp_print(netdissect_options *ndo,
        ndo->ndo_protocol = "esp";
        esp = (const struct newesp *)bp;
 
-#ifdef HAVE_LIBCRYPTO
-       secret = NULL;
-       advance = 0;
-#endif
-
-#if 0
-       /* keep secret out of a register */
-       p = (u_char *)&secret;
-#endif
-
        /* 'ep' points to the end of available data. */
        ep = ndo->ndo_snapend;
 
@@ -787,21 +775,38 @@ esp_print(netdissect_options *ndo,
                return;
 
        /* pointer to the IV, if there is one */
-       ivoff = (const u_char *)(esp + 1) + 0;
+       ivptr = (const u_char *)(esp + 1) + 0;
        /* length of the IV, if there is one; 0, if there isn't */
        ivlen = sa->ivlen;
-       secret = sa->secret;
+
+       /*
+        * Get a pointer to the ciphertext.
+        *
+        * p points to the beginning of the payload, i.e. to the
+        * initialization vector, so if we skip past the initialization
+        * vector, it points to the beginning of the ciphertext.
+        */
+       ctptr = ivptr + ivlen;
+
        /*
         * Make sure the authentication data/integrity check value length
-        * isn't bigger than the total amount of data available and, if
-        * not, slice that off.
+        * isn't bigger than the total amount of data available after
+        * the ESP header and initialization vector is removed and,
+        * if not, slice the authentication data/ICV off.
         */
-       if (ep - bp < sa->authlen) {
+       if (ep - ctptr < sa->authlen) {
                nd_print_trunc(ndo);
                return;
        }
        ep = ep - sa->authlen;
 
+       /*
+        * Calculate the length of the ciphertext.  ep points to
+        * the beginning of the authentication data/integrity check
+        * value, i.e. right past the end of the ciphertext; 
+        */
+       ctlen = ep - ctptr;
+
        if (sa->evp == NULL)
                return;
 
@@ -830,27 +835,16 @@ esp_print(netdissect_options *ndo,
                        "esp_print: can't allocate memory for cipher context");
        }
 
-       if (set_cipher_parameters(ctx, sa->evp, secret, NULL, 0) < 0) {
+       if (set_cipher_parameters(ctx, sa->evp, sa->secret, NULL, 0) < 0) {
                (*ndo->ndo_warning)(ndo, "espkey init failed");
                return;
        }
 
-       p = ivoff;
-       if (set_cipher_parameters(ctx, NULL, NULL, p, 0) < 0) {
+       if (set_cipher_parameters(ctx, NULL, NULL, ivptr, 0) < 0) {
                (*ndo->ndo_warning)(ndo, "IV init failed");
                return;
        }
 
-       /*
-        * Calculate the length of the ciphertext.  ep points to
-        * the beginning of the authentication data/integrity check
-        * value, i.e. right past the end of the ciphertext; p points
-        * to the beginning of the payload, i.e. to the initialization
-        * vector, so if we skip past the initialization vector, it
-        * points to the beginning of the ciphertext.
-        */
-       ctlen = ep - (p + ivlen);
-
        /*
         * Allocate buffers for the encrypted and decrypted
         * data.  Both buffers' sizes must be a multiple of
@@ -873,7 +867,7 @@ esp_print(netdissect_options *ndo,
         * Copy the input data to the encrypted data buffer,
         * and pad it with zeroes.
         */
-       memcpy(input_buffer, p + ivlen, ctlen);
+       memcpy(input_buffer, ctptr, ctlen);
        memset(input_buffer + ctlen, 0, buffer_size - ctlen);
 
        /*
@@ -899,37 +893,23 @@ esp_print(netdissect_options *ndo,
         * const buffer, but changing this would require a
         * more complicated fix.
         */
-       memcpy(p + ivlen, output_buffer, ctlen);
+       memcpy(ctptr, output_buffer, ctlen);
        free(output_buffer);
-       advance = ivoff - (const u_char *)esp + ivlen;
 
        /*
-        * Sanity check for pad length.
+        * Sanity check for pad length; if it, plus 2 for the pad
+        * length and next header fields, is bigger than the ciphertext
+        * length (which is also the plaintext length), it's too big.
         *
         * XXX - the check can fail if the packet is corrupt *or* if
         * it was not decrypted with the correct key, so that the
         * "plaintext" is not what was being sent.
         */
        padlen = GET_U_1(ep - 2);
-       if (ep - bp < padlen) {
-               nd_print_trunc(ndo);
-               return;
-       }
-
-       /*
-        * Sanity check for payload length; +2 is for the pad length
-        * and next header fields.
-        *
-        * XXX - the check can fail if the packet is corrupt *or* if
-        * it was not decrypted with the correct key, so that the
-        * "plaintext" is not what was being sent.
-        */
-       if (length <= advance + padlen + 2) {
+       if (padlen + 2 > ctlen) {
                nd_print_trunc(ndo);
                return;
        }
-       bp += advance;
-       length -= advance + padlen + 2;
 
        /* Get the next header */
        nh = GET_U_1(ep - 1);
@@ -937,7 +917,8 @@ esp_print(netdissect_options *ndo,
        ND_PRINT(": ");
 
        /* Now print the payload. */
-       ip_print_demux(ndo, bp, length, ver, fragmented, ttl_hl, nh, bp2);
+       ip_print_demux(ndo, ctptr, ctlen - (padlen + 2),  ver, fragmented,
+           ttl_hl, nh, bp2);
 #endif
 }
 #ifdef HAVE_LIBCRYPTO
index ad710b983713c51090961df4d20db174ef1f663d..b585cc8c92935ff6c24767ff8330d67bd768ffaf 100644 (file)
@@ -1,8 +1,8 @@
     1  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x1), length 116 [|esp]
-    2  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x2), length 116:  ip-proto-227 49
-    3  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x3), length 116: PIMv13, length 10
+    2  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x2), length 116:  ip-proto-227 37
+    3  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x3), length 116 [|esp]
     4  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x4), length 116 [|esp]
     5  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x5), length 116 [|esp]
-    6  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x6), length 116:  ip-proto-183 28
-    7  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x7), length 116:  ip-proto-72 34
-    8  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x8), length 116:  ip-proto-224 59
+    6  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x6), length 116:  ip-proto-183 16
+    7  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x7), length 116:  ip-proto-72 22
+    8  00:00:00.000000 IP 192.1.2.23.4500 > 192.1.2.45.4500: UDP-encap: ESP(spi=0x12345678,seq=0x8), length 116:  ip-proto-224 47