ipsec: fix udp-encap in transport mode 49/26149/3
authorAlexander Chernavin <achernavin@netgate.com>
Wed, 25 Mar 2020 14:56:52 +0000 (10:56 -0400)
committerNeale Ranns <nranns@cisco.com>
Tue, 31 Mar 2020 09:06:43 +0000 (09:06 +0000)
Now UDP enacapsulation doesn't work in transport mode with crypto
algorithms that have iv_sz=8 like AES GCM or 3DES CBC. That happens
because the inserted UDP header overlaps with the old IP header and
gets filled before the information from the old IP header can be
copied to a new IP header. The result is a broken packet:

00:03:39:620863: esp4-encrypt-tun
  esp: sa-index 3 spi 3464048590 (0xce792fce) seq 31 sa-seq-hi 0
    crypto aes-gcm-128 integrity none udp-encap-enabled
00:03:39:620867: adj-midchain-tx
  ...
00:03:39:620868: ip4-rewrite
  ...
00:03:39:620869: GigabitEthernet0/8/0-output
  GigabitEthernet0/8/0
  IP4: 08:00:27:a9:6b:d6 -> 08:00:27:5a:dd:0c
  UDP: 10.255.0.10 -> 10.255.0.20
    version 0, header length 0
    tos 0x80, ttl 63, length 0, checksum 0x653e (should be 0xffff)
      dscp CS4 ecn NON_ECN
    fragment id 0x0000
  UDP: 128 -> 0
    length 0, checksum 0x0000
00:03:39:620870: GigabitEthernet0/8/0-tx
  GigabitEthernet0/8/0 tx queue 0
  ...
  IP4: 08:00:27:a9:6b:d6 -> 08:00:27:5a:dd:0c
  UDP: 10.255.0.10 -> 10.255.0.20
    version 0, header length 0
    tos 0x80, ttl 63, length 0, checksum 0x653e (should be 0xffff)
      dscp CS4 ecn NON_ECN
    fragment id 0x0000
  UDP: 128 -> 0
    length 0, checksum 0x0000

With this commit, fill UDP header after copying the IP headers in
transport mode.

Type: fix

Change-Id: Ie9a6e562aa05a8378114329d6a9ff395189fa6a8
Signed-off-by: Alexander Chernavin <achernavin@netgate.com>
src/vnet/ipsec/esp_encrypt.c
test/test_ipsec_tun_if_esp.py

index e06babd..4793fdd 100644 (file)
@@ -463,6 +463,7 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
          u8 *l2_hdr, l2_len, *ip_hdr, ip_len;
          ip6_ext_header_t *ext_hdr;
          udp_header_t *udp = 0;
+         u16 udp_len = 0;
          u8 *old_ip_hdr = vlib_buffer_get_current (b[0]);
 
          ip_len = is_ip6 ?
@@ -537,7 +538,7 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
              if (udp)
                {
                  esp_update_ip4_hdr (ip4, len, /* is_transport */ 1, 1);
-                 esp_fill_udp_hdr (sa0, udp, len - ip_len);
+                 udp_len = len - ip_len;
                }
              else
                esp_update_ip4_hdr (ip4, len, /* is_transport */ 1, 0);
@@ -545,6 +546,11 @@ esp_encrypt_inline (vlib_main_t * vm, vlib_node_runtime_t * node,
 
          clib_memcpy_le64 (ip_hdr, old_ip_hdr, ip_len);
 
+         if (udp)
+           {
+             esp_fill_udp_hdr (sa0, udp, udp_len);
+           }
+
          if (!is_tun)
            next[0] = ESP_ENCRYPT_NEXT_INTERFACE_OUTPUT;
        }
index 55e85b1..3cd2521 100644 (file)
@@ -133,16 +133,42 @@ class TemplateIpsec4TunIfEspUdp(TemplateIpsec):
     def tearDownClass(cls):
         super(TemplateIpsec4TunIfEspUdp, cls).tearDownClass()
 
+    def verify_encrypted(self, p, sa, rxs):
+        for rx in rxs:
+            try:
+                # ensure the UDP ports are correct before we decrypt
+                # which strips them
+                self.assertTrue(rx.haslayer(UDP))
+                self.assert_equal(rx[UDP].sport, 4500)
+                self.assert_equal(rx[UDP].dport, 4500)
+
+                pkt = sa.decrypt(rx[IP])
+                if not pkt.haslayer(IP):
+                    pkt = IP(pkt[Raw].load)
+
+                self.assert_packet_checksums_valid(pkt)
+                self.assert_equal(pkt[IP].dst, "1.1.1.1")
+                self.assert_equal(pkt[IP].src, self.pg1.remote_ip4)
+            except (IndexError, AssertionError):
+                self.logger.debug(ppp("Unexpected packet:", rx))
+                try:
+                    self.logger.debug(ppp("Decrypted packet:", pkt))
+                except:
+                    pass
+                raise
+
     def setUp(self):
         super(TemplateIpsec4TunIfEspUdp, self).setUp()
 
-        self.tun_if = self.pg0
-
         p = self.ipv4_params
         p.flags = (VppEnum.vl_api_ipsec_sad_flags_t.
                    IPSEC_API_SAD_FLAG_UDP_ENCAP)
         p.nat_header = UDP(sport=5454, dport=4500)
 
+    def config_network(self):
+
+        self.tun_if = self.pg0
+        p = self.ipv4_params
         p.tun_if = VppIpsecTunInterface(self, self.pg0, p.vpp_tun_spi,
                                         p.scapy_tun_spi, p.crypt_algo_vpp_id,
                                         p.crypt_key, p.crypt_key,
@@ -204,11 +230,34 @@ class TestIpsec4TunIfEspUdp(TemplateIpsec4TunIfEspUdp, IpsecTun4Tests):
 
     tun4_input_node = "ipsec4-tun-input"
 
+    def setUp(self):
+        super(TemplateIpsec4TunIfEspUdp, self).setUp()
+        self.config_network()
+
     def test_keepalive(self):
         """ IPSEC NAT Keepalive """
         self.verify_keepalive(self.ipv4_params)
 
 
+class TestIpsec4TunIfEspUdpGCM(TemplateIpsec4TunIfEspUdp, IpsecTun4Tests):
+    """ Ipsec ESP UDP GCM tests """
+
+    tun4_input_node = "ipsec4-tun-input"
+
+    def setUp(self):
+        super(TemplateIpsec4TunIfEspUdp, self).setUp()
+        p = self.ipv4_params
+        p.auth_algo_vpp_id = (VppEnum.vl_api_ipsec_integ_alg_t.
+                              IPSEC_API_INTEG_ALG_NONE)
+        p.crypt_algo_vpp_id = (VppEnum.vl_api_ipsec_crypto_alg_t.
+                               IPSEC_API_CRYPTO_ALG_AES_GCM_256)
+        p.crypt_algo = "AES-GCM"
+        p.auth_algo = "NULL"
+        p.crypt_key = b"JPjyOWBeVEQiMe7hJPjyOWBeVEQiMe7h"
+        p.salt = 0
+        self.config_network()
+
+
 class TestIpsec4TunIfEsp2(TemplateIpsec4TunIfEsp, IpsecTcpTests):
     """ Ipsec ESP - TCP tests """
     pass