From f83d9884d11c7f1d11cd9fddd8c7af216e77f70a Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Wed, 11 Mar 2015 18:03:30 +0000 Subject: [PATCH 1/7] check length of received ethernet frame (to avoid cstruct exceptions) provide ethertype_to_protocol / protocol_to_ethertype conversion cleanup code --- lib/ethif.ml | 40 ++++++++++++++++++++-------------------- lib/wire_structs.ml | 11 +++++++++++ 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/lib/ethif.ml b/lib/ethif.ml index 8bfd43bbd..02d6791e0 100644 --- a/lib/ethif.ml +++ b/lib/ethif.ml @@ -21,7 +21,6 @@ module Make(Netif : V1_LWT.NETWORK) = struct type 'a io = 'a Lwt.t type buffer = Cstruct.t - type ipv4addr = Ipaddr.V4.t type macaddr = Macaddr.t type netif = Netif.t @@ -40,25 +39,26 @@ module Make(Netif : V1_LWT.NETWORK) = struct let input ~arpv4 ~ipv4 ~ipv6 t frame = MProf.Trace.label "ethif.input"; - let frame_mac = Macaddr.of_bytes (Wire_structs.copy_ethernet_dst frame) in - match frame_mac with - | None -> Lwt.return_unit - | Some frame_mac -> - if Macaddr.compare frame_mac (mac t) = 0 - || not (Macaddr.is_unicast frame_mac) - then match Wire_structs.get_ethernet_ethertype frame with - | 0x0806 -> arpv4 frame (* ARP *) - | 0x0800 -> (* IPv4 *) - let payload = Cstruct.shift frame Wire_structs.sizeof_ethernet in - ipv4 payload - | 0x86dd -> - let payload = Cstruct.shift frame Wire_structs.sizeof_ethernet in - ipv6 payload - | _etype -> - let _payload = Cstruct.shift frame Wire_structs.sizeof_ethernet in - (* TODO default etype payload *) - Lwt.return_unit - else Lwt.return_unit + let of_interest dest = + Macaddr.compare dest (mac t) = 0 || not (Macaddr.is_unicast dest) + in + if Cstruct.len frame >= 60 then + (* minimum payload is 46 + source + destination + type *) + match Macaddr.of_bytes (Wire_structs.copy_ethernet_dst frame) with + | Some frame_mac when of_interest frame_mac -> + begin + let payload = Cstruct.shift frame Wire_structs.sizeof_ethernet + and ethertype = Wire_structs.get_ethernet_ethertype frame + in + match Wire_structs.ethertype_to_protocol ethertype with + | Some `ARP -> arpv4 frame + | Some `IPv4 -> ipv4 payload + | Some `IPv6 -> ipv6 payload + | None -> return_unit (* TODO default etype payload *) + end + | _ -> return_unit + else + return_unit let write t frame = MProf.Trace.label "ethif.write"; diff --git a/lib/wire_structs.ml b/lib/wire_structs.ml index f594e2164..063567e80 100644 --- a/lib/wire_structs.ml +++ b/lib/wire_structs.ml @@ -4,6 +4,17 @@ cstruct ethernet { uint16_t ethertype } as big_endian +let ethertype_to_protocol = function + | 0x0806 -> Some `ARP + | 0x0800 -> Some `IPv4 + | 0x86dd -> Some `IPv6 + | _ -> None + +let protocol_to_ethertype = function + | `ARP -> 0x0806 + | `IPv4 -> 0x0800 + | `IPv6 -> 0x86dd + cstruct udp { uint16_t source_port; uint16_t dest_port; From 71947c2fa3fafee85c0965b004a41aebeda77c5b Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Wed, 11 Mar 2015 18:21:55 +0000 Subject: [PATCH 2/7] use cenum and variant instead of polyvar --- lib/ethif.ml | 18 ++++++++---------- lib/wire_structs.ml | 15 +++++---------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/lib/ethif.ml b/lib/ethif.ml index 02d6791e0..5611c4650 100644 --- a/lib/ethif.ml +++ b/lib/ethif.ml @@ -46,16 +46,14 @@ module Make(Netif : V1_LWT.NETWORK) = struct (* minimum payload is 46 + source + destination + type *) match Macaddr.of_bytes (Wire_structs.copy_ethernet_dst frame) with | Some frame_mac when of_interest frame_mac -> - begin - let payload = Cstruct.shift frame Wire_structs.sizeof_ethernet - and ethertype = Wire_structs.get_ethernet_ethertype frame - in - match Wire_structs.ethertype_to_protocol ethertype with - | Some `ARP -> arpv4 frame - | Some `IPv4 -> ipv4 payload - | Some `IPv6 -> ipv6 payload - | None -> return_unit (* TODO default etype payload *) - end + let payload = Cstruct.shift frame Wire_structs.sizeof_ethernet + and ethertype = Wire_structs.get_ethernet_ethertype frame + in + Wire_structs.(match int_to_ethertype ethertype with + | Some ARP -> arpv4 frame + | Some IPv4 -> ipv4 payload + | Some IPv6 -> ipv6 payload + | None -> (* TODO default etype payload *) return_unit) | _ -> return_unit else return_unit diff --git a/lib/wire_structs.ml b/lib/wire_structs.ml index 063567e80..aee3a17ee 100644 --- a/lib/wire_structs.ml +++ b/lib/wire_structs.ml @@ -4,16 +4,11 @@ cstruct ethernet { uint16_t ethertype } as big_endian -let ethertype_to_protocol = function - | 0x0806 -> Some `ARP - | 0x0800 -> Some `IPv4 - | 0x86dd -> Some `IPv6 - | _ -> None - -let protocol_to_ethertype = function - | `ARP -> 0x0806 - | `IPv4 -> 0x0800 - | `IPv6 -> 0x86dd +cenum ethertype { + ARP = 0x0806; + IPv4 = 0x0800; + IPv6 = 0x86dd; + } as uint16_t cstruct udp { uint16_t source_port; From c9fb42ecf952b2abcf312cef63f4194bc1d4acb1 Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Wed, 11 Mar 2015 18:23:37 +0000 Subject: [PATCH 3/7] style --- lib/ethif.ml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/ethif.ml b/lib/ethif.ml index 5611c4650..3fab70018 100644 --- a/lib/ethif.ml +++ b/lib/ethif.ml @@ -49,11 +49,13 @@ module Make(Netif : V1_LWT.NETWORK) = struct let payload = Cstruct.shift frame Wire_structs.sizeof_ethernet and ethertype = Wire_structs.get_ethernet_ethertype frame in - Wire_structs.(match int_to_ethertype ethertype with + Wire_structs.( + match int_to_ethertype ethertype with | Some ARP -> arpv4 frame | Some IPv4 -> ipv4 payload | Some IPv6 -> ipv6 payload - | None -> (* TODO default etype payload *) return_unit) + | None -> return_unit (* TODO default etype payload *) + ) | _ -> return_unit else return_unit From b349a9202cfd70529a493b217f1ef6bad6640949 Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Wed, 11 Mar 2015 18:37:11 +0000 Subject: [PATCH 4/7] separate pure from effectful bits --- lib/ethif.ml | 27 ++++++++++----------------- lib/wire_structs.ml | 13 +++++++++++++ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/lib/ethif.ml b/lib/ethif.ml index 3fab70018..a3ebcd963 100644 --- a/lib/ethif.ml +++ b/lib/ethif.ml @@ -42,23 +42,16 @@ module Make(Netif : V1_LWT.NETWORK) = struct let of_interest dest = Macaddr.compare dest (mac t) = 0 || not (Macaddr.is_unicast dest) in - if Cstruct.len frame >= 60 then - (* minimum payload is 46 + source + destination + type *) - match Macaddr.of_bytes (Wire_structs.copy_ethernet_dst frame) with - | Some frame_mac when of_interest frame_mac -> - let payload = Cstruct.shift frame Wire_structs.sizeof_ethernet - and ethertype = Wire_structs.get_ethernet_ethertype frame - in - Wire_structs.( - match int_to_ethertype ethertype with - | Some ARP -> arpv4 frame - | Some IPv4 -> ipv4 payload - | Some IPv6 -> ipv6 payload - | None -> return_unit (* TODO default etype payload *) - ) - | _ -> return_unit - else - return_unit + match Wire_structs.parse_ethernet_frame frame with + | Some (typ, destination, payload) when of_interest destination -> + begin + match typ with + | Some Wire_structs.ARP -> arpv4 frame + | Some Wire_structs.IPv4 -> ipv4 payload + | Some Wire_structs.IPv6 -> ipv6 payload + | None -> return_unit (* TODO: default ethertype payload handler *) + end + | _ -> return_unit let write t frame = MProf.Trace.label "ethif.write"; diff --git a/lib/wire_structs.ml b/lib/wire_structs.ml index aee3a17ee..583ef49ed 100644 --- a/lib/wire_structs.ml +++ b/lib/wire_structs.ml @@ -10,6 +10,19 @@ cenum ethertype { IPv6 = 0x86dd; } as uint16_t +let parse_ethernet_frame frame = + if Cstruct.len frame >= 60 then + (* minimum payload is 46 + source + destination + type *) + let payload = Cstruct.shift frame sizeof_ethernet + and typ = get_ethernet_ethertype frame + in + match Macaddr.of_bytes (copy_ethernet_dst frame) with + | Some destination_mac -> Some (int_to_ethertype typ, destination_mac, payload) + | None -> None + else + None + + cstruct udp { uint16_t source_port; uint16_t dest_port; From 1e09eb1fc8d2033105e1044fb23fd6412581312c Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Sat, 14 Mar 2015 10:37:07 +0000 Subject: [PATCH 5/7] the buffer length is already checked, Macaddr.of_bytes_exn will never raise --- lib/wire_structs.ml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/wire_structs.ml b/lib/wire_structs.ml index 583ef49ed..15b6bd59a 100644 --- a/lib/wire_structs.ml +++ b/lib/wire_structs.ml @@ -15,10 +15,9 @@ let parse_ethernet_frame frame = (* minimum payload is 46 + source + destination + type *) let payload = Cstruct.shift frame sizeof_ethernet and typ = get_ethernet_ethertype frame + and dst = Macaddr.of_bytes_exn (copy_ethernet_dst frame) in - match Macaddr.of_bytes (copy_ethernet_dst frame) with - | Some destination_mac -> Some (int_to_ethertype typ, destination_mac, payload) - | None -> None + Some (int_to_ethertype typ, dst, payload) else None From c5897703327992265cbf620b1c0877bc4c09dd86 Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Wed, 29 Jul 2015 10:07:49 +0100 Subject: [PATCH 6/7] fix compilation --- lib/ethif.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ethif.ml b/lib/ethif.ml index a3ebcd963..59c822356 100644 --- a/lib/ethif.ml +++ b/lib/ethif.ml @@ -49,9 +49,9 @@ module Make(Netif : V1_LWT.NETWORK) = struct | Some Wire_structs.ARP -> arpv4 frame | Some Wire_structs.IPv4 -> ipv4 payload | Some Wire_structs.IPv6 -> ipv6 payload - | None -> return_unit (* TODO: default ethertype payload handler *) + | None -> Lwt.return_unit (* TODO: default ethertype payload handler *) end - | _ -> return_unit + | _ -> Lwt.return_unit let write t frame = MProf.Trace.label "ethif.write"; From b9ee12c5bdab2adb691f788d71224263b86de3f8 Mon Sep 17 00:00:00 2001 From: Hannes Mehnert Date: Wed, 29 Jul 2015 10:30:25 +0100 Subject: [PATCH 7/7] accept all the ethernet frames of size 14 or more --- lib/wire_structs.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/wire_structs.ml b/lib/wire_structs.ml index 15b6bd59a..0223e8911 100644 --- a/lib/wire_structs.ml +++ b/lib/wire_structs.ml @@ -11,8 +11,8 @@ cenum ethertype { } as uint16_t let parse_ethernet_frame frame = - if Cstruct.len frame >= 60 then - (* minimum payload is 46 + source + destination + type *) + if Cstruct.len frame >= 14 then + (* source + destination + type = 14 *) let payload = Cstruct.shift frame sizeof_ethernet and typ = get_ethernet_ethertype frame and dst = Macaddr.of_bytes_exn (copy_ethernet_dst frame)