Don't hand un-decrypted data to the IP demuxer.

If we can't decrypt the payload, we can't dissect it, so don't try -
just give up immediately.

While we're at it:

If EVP_CIPHER_CTX_new() fails, it means a memory allocation failed;
treat that as such.

Use some of the arguments we're passed rather than re-fetching them from
the IP header.

Add some comments.

Call nd_print_trunc() for failed length sanity checks, and note that
they can fail due to the decryption being done with the wrong key.

Update one test's output; it is, I think, being decrypted with the wrong
key.

diff --git a/print-esp.c b/print-esp.c
index d1231070e05b0a304b95b7edd4ab49ce272902d0..85a5d5ac4af6600df1e7d9d03444865479653dce 100644 (file)
--- a/print-esp.c
+++ b/print-esp.c
@@ -729,12 +729,13 @@ esp_print(netdissect_options *ndo,
                return;
 
        ip = (const struct ip *)bp2;
                return;
 
        ip = (const struct ip *)bp2;

-       switch (IP_V(ip)) {
+       switch (ver) {

        case 6:
                ip6 = (const struct ip6_hdr *)bp2;
                /* we do not attempt to decrypt jumbograms */
                if (!GET_BE_U_2(ip6->ip6_plen))
                        return;
        case 6:
                ip6 = (const struct ip6_hdr *)bp2;
                /* we do not attempt to decrypt jumbograms */
                if (!GET_BE_U_2(ip6->ip6_plen))
                        return;

+               /* XXX - check whether it's fragmented? */

                /* if we can't get nexthdr, we do not need to decrypt it */
                len = sizeof(struct ip6_hdr) + GET_BE_U_2(ip6->ip6_plen);
 
                /* if we can't get nexthdr, we do not need to decrypt it */
                len = sizeof(struct ip6_hdr) + GET_BE_U_2(ip6->ip6_plen);
 


@@ -750,7 +751,7 @@ esp_print(netdissect_options *ndo,
                break;
        case 4:
                /* nexthdr & padding are in the last fragment */
                break;
        case 4:
                /* nexthdr & padding are in the last fragment */

-               if (GET_BE_U_2(ip->ip_off) & IP_MF)
+               if (fragmented)

                        return;
                len = GET_BE_U_2(ip->ip_len);
 
                        return;
                len = GET_BE_U_2(ip->ip_len);
 


@@ -800,82 +801,105 @@ esp_print(netdissect_options *ndo,
                return;
        ep = ep - sa->authlen;
 
                return;
        ep = ep - sa->authlen;
 

-       if (sa->evp) {
-               ctx = EVP_CIPHER_CTX_new();
-               if (ctx != NULL) {
-                       if (set_cipher_parameters(ctx, sa->evp, secret, NULL, 0) < 0)
-                               (*ndo->ndo_warning)(ndo, "espkey init failed");
-
-                       p = ivoff;
-                       set_cipher_parameters(ctx, NULL, NULL, p, 0);
-                       len = ep - (p + ivlen);
-
-                       /*
-                        * Allocate buffers for the encrypted and decrypted
-                        * data.  Both buffers' sizes must be a multiple of
-                        * the cipher block size, and the output buffer must
-                        * be separate from the input buffer.
-                        */
-                       block_size = (unsigned int)EVP_CIPHER_CTX_block_size(ctx);
-                       buffer_size = len + (block_size - len % block_size);
-
-                       /*
-                        * Attempt to allocate the input buffer.
-                        */
-                       input_buffer = (u_char *)malloc(buffer_size);
-                       if (input_buffer == NULL) {
-                               EVP_CIPHER_CTX_free(ctx);
-                               (*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
-                                       "esp_print: can't allocate memory for encrypted data buffer");
-                       }
-                       /*
-                        * Copy the input data to the encrypted data buffer,
-                        * and pad it with zeroes.
-                        */
-                       memcpy(input_buffer, p + ivlen, len);
-                       memset(input_buffer + len, 0, buffer_size - len);
-
-                       /*
-                        * Attempt to allocate the output buffer.
-                        */
-                       output_buffer = (u_char *)malloc(buffer_size);
-                       if (output_buffer == NULL) {
-                               free(input_buffer);
-                               EVP_CIPHER_CTX_free(ctx);
-                               (*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
-                                       "esp_print: can't allocate memory for decryption buffer");
-                       }
+       if (sa->evp == NULL)
+               return;
+       ctx = EVP_CIPHER_CTX_new();
+       if (ctx == NULL) {
+               /*
+                * Failed to initialize the cipher context.
+                * From a look at the OpenSSL code, this appears to
+                * mean "couldn't allocate memory for the cipher context";
+                * note that we're not passing any parameters, so there's
+                * not much else it can mean.
+                */
+               (*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
+                       "esp_print: can't allocate memory for cipher context");
+       }

 
 

-                       EVP_Cipher(ctx, output_buffer, input_buffer, len);
-                       free(input_buffer);
-                       EVP_CIPHER_CTX_free(ctx);
-                       /*
-                        * XXX - of course this is wrong, because buf is a
-                        * const buffer, but changing this would require a
-                        * more complicated fix.
-                        */
-                       memcpy(p + ivlen, output_buffer, len);
-                       free(output_buffer);
-                       advance = ivoff - (const u_char *)esp + ivlen;
-               } else
-                       advance = sizeof(struct newesp);
-       } else
-               advance = sizeof(struct newesp);
+       if (set_cipher_parameters(ctx, sa->evp, secret, NULL, 0) < 0)
+               (*ndo->ndo_warning)(ndo, "espkey init failed");
+
+       p = ivoff;
+       set_cipher_parameters(ctx, NULL, NULL, p, 0);
+       len = ep - (p + ivlen);

 
 

-       /* sanity check for pad length */
+       /*
+        * Allocate buffers for the encrypted and decrypted
+        * data.  Both buffers' sizes must be a multiple of
+        * the cipher block size, and the output buffer must
+        * be separate from the input buffer.
+        */
+       block_size = (unsigned int)EVP_CIPHER_CTX_block_size(ctx);
+       buffer_size = len + (block_size - len % block_size);
+
+       /*
+        * Attempt to allocate the input buffer.
+        */
+       input_buffer = (u_char *)malloc(buffer_size);
+       if (input_buffer == NULL) {
+               EVP_CIPHER_CTX_free(ctx);
+               (*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
+                       "esp_print: can't allocate memory for encrypted data buffer");
+       }
+       /*
+        * Copy the input data to the encrypted data buffer,
+        * and pad it with zeroes.
+        */
+       memcpy(input_buffer, p + ivlen, len);
+       memset(input_buffer + len, 0, buffer_size - len);
+
+       /*
+        * Attempt to allocate the output buffer.
+        */
+       output_buffer = (u_char *)malloc(buffer_size);
+       if (output_buffer == NULL) {
+               free(input_buffer);
+               EVP_CIPHER_CTX_free(ctx);
+               (*ndo->ndo_error)(ndo, S_ERR_ND_MEM_ALLOC,
+                       "esp_print: can't allocate memory for decryption buffer");
+       }
+
+       EVP_Cipher(ctx, output_buffer, input_buffer, len);
+       free(input_buffer);
+       EVP_CIPHER_CTX_free(ctx);
+       /*
+        * XXX - of course this is wrong, because buf is a
+        * const buffer, but changing this would require a
+        * more complicated fix.
+        */
+       memcpy(p + ivlen, output_buffer, len);
+       free(output_buffer);
+       advance = ivoff - (const u_char *)esp + ivlen;
+
+       /*
+        * Sanity check for pad length.
+        *
+        * 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);
        padlen = GET_U_1(ep - 2);

-       if (ep - bp < padlen)
+       if (ep - bp < padlen) {
+               nd_print_trunc(ndo);

                return;
                return;

+       }

 
        /*
         * Sanity check for payload length; +2 is for the pad length
         * and next header fields.
 
        /*
         * 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 (length <= advance + padlen + 2) {
+               nd_print_trunc(ndo);

                return;
                return;

+       }

        bp += advance;
        length -= advance + padlen + 2;
 
        bp += advance;
        length -= advance + padlen + 2;
 

+       /* Get the next header */

        nh = GET_U_1(ep - 1);
 
        ND_PRINT(": ");
        nh = GET_U_1(ep - 1);
 
        ND_PRINT(": ");

diff --git a/tests/espudp1.out b/tests/espudp1.out
index ffc745161a6641f3aae74a1b53e56b60a88c3299..ad710b983713c51090961df4d20db174ef1f663d 100644 (file)
--- a/tests/espudp1.out
+++ b/tests/espudp1.out
@@ -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
+    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 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

-    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
-    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
+    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 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