diff mbox

[ovs-dev] tunnel: Validate IP header for userspace tunneling.

Message ID 1442023157-53050-1-git-send-email-jesse@nicira.com
State Accepted
Headers show

Commit Message

Jesse Gross Sept. 12, 2015, 1:59 a.m. UTC 
Currently, when doing userspace tunneling we don't perform much in
the way of integrity checks on the incoming IP header. The case of
tunneling is different from the usual case of switching since we are
acting as the endpoint here and should not allow invalid packets to
pass.

This adds checks for IP checksum, version, total length, and options and
drops packets that don't pass.

Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 lib/netdev-vport.c       | 27 +++++++++++++++++++++++++++
 tests/tunnel-push-pop.at |  6 +++---
 2 files changed, 30 insertions(+), 3 deletions(-)

Comments

Pravin B Shelar Sept. 13, 2015, 12:13 a.m. UTC | #1 
On Fri, Sep 11, 2015 at 6:59 PM, Jesse Gross <jesse@nicira.com> wrote:
> Currently, when doing userspace tunneling we don't perform much in
> the way of integrity checks on the incoming IP header. The case of
> tunneling is different from the usual case of switching since we are
> acting as the endpoint here and should not allow invalid packets to
> pass.
>
> This adds checks for IP checksum, version, total length, and options and
> drops packets that don't pass.
>
> Signed-off-by: Jesse Gross <jesse@nicira.com>

Looks good.

Acked-by: Pravin B Shelar <pshelar@nicira.com>
Jesse Gross Sept. 13, 2015, 3:21 p.m. UTC | #2 
On Sat, Sep 12, 2015 at 5:13 PM, Pravin Shelar <pshelar@nicira.com> wrote:
> On Fri, Sep 11, 2015 at 6:59 PM, Jesse Gross <jesse@nicira.com> wrote:
>> Currently, when doing userspace tunneling we don't perform much in
>> the way of integrity checks on the incoming IP header. The case of
>> tunneling is different from the usual case of switching since we are
>> acting as the endpoint here and should not allow invalid packets to
>> pass.
>>
>> This adds checks for IP checksum, version, total length, and options and
>> drops packets that don't pass.
>>
>> Signed-off-by: Jesse Gross <jesse@nicira.com>
>
> Looks good.
>
> Acked-by: Pravin B Shelar <pshelar@nicira.com>

Thanks, applied to master.
diff mbox

Patch

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index eceaa81..ff50563 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -844,6 +844,7 @@  ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl)
 {
     struct ip_header *nh;
     void *l4;
+    int l3_size;
 
     nh = dp_packet_l3(packet);
     l4 = dp_packet_l4(packet);
@@ -852,6 +853,32 @@  ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl)
         return NULL;
     }
 
+    if (csum(nh, IP_IHL(nh->ip_ihl_ver) * 4)) {
+        VLOG_WARN_RL(&err_rl, "ip packet has invalid checksum");
+        return NULL;
+    }
+
+    if (IP_VER(nh->ip_ihl_ver) != 4) {
+        VLOG_WARN_RL(&err_rl, "ipv4 packet has invalid version (%d)",
+                     IP_VER(nh->ip_ihl_ver));
+        return NULL;
+    }
+
+    l3_size = dp_packet_size(packet) -
+              ((char *)nh - (char *)dp_packet_data(packet));
+
+    if (ntohs(nh->ip_tot_len) > l3_size) {
+        VLOG_WARN_RL(&err_rl, "ip packet is truncated (IP length %d, actual %d)",
+                     ntohs(nh->ip_tot_len), l3_size);
+        return NULL;
+    }
+
+    if (IP_IHL(nh->ip_ihl_ver) * 4 > sizeof(struct ip_header)) {
+        VLOG_WARN_RL(&err_rl, "ip options not supported on tunnel packets "
+                     "(%d bytes)", IP_IHL(nh->ip_ihl_ver) * 4);
+        return NULL;
+    }
+
     tnl->ip_src = get_16aligned_be32(&nh->ip_src);
     tnl->ip_dst = get_16aligned_be32(&nh->ip_dst);
     tnl->ip_tos = nh->ip_tos;
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index 502e41f..98f22ea 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -109,7 +109,7 @@  AT_CHECK([tail -1 stdout], [0],
 ])
 
 dnl Check decapsulation of GRE packet
-AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab6408004500007e79464000402f99080101025c0101025820006558000001c8fe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab6408004500007e79464000402fba550101025c0101025820006558000001c8fe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637'])
 ovs-appctl time/warp 1000
 
 AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port  3'], [0], [dnl
@@ -117,7 +117,7 @@  AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port  3'], [0], [dnl
 ])
 
 dnl Check GRE only accepts encapsulated Ethernet frames
-AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab6408004500007e79464000402f99080101025c0101025820000800000001c8fe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab6408004500007e79464000402fba550101025c0101025820000800000001c8fe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637'])
 ovs-appctl time/warp 1000
 
 AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port  3'], [0], [dnl
@@ -130,7 +130,7 @@  AT_CHECK([ovs-ofctl monitor int-br 65534 --detach --no-chdir --pidfile 2> ofctl_
 
 AT_CHECK([ovs-ofctl del-flows int-br])
 AT_CHECK([ovs-ofctl add-flow int-br "tun_metadata0=0xa/0xf,actions=5,controller"])
-AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab64080045000096794640004011ba630101025c01010258308817c1008200000400655800007b00ffff80010000000affff00010000000bfe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637'])
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 'aa55aa550000001b213cab64080045000096794640004011ba5b0101025c01010258308817c1008200000400655800007b00ffff80010000000affff00010000000bfe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637'])
 
 OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 2])
 OVS_APP_EXIT_AND_WAIT(ovs-ofctl)