[ovs-dev] [PATCH branch-2.12] pinctrl: Fix DNS packet parsing

Dumitru Ceara dceara at redhat.com
Fri Aug 23 07:12:31 UTC 2019


On Fri, Aug 16, 2019 at 4:35 PM Dumitru Ceara <dceara at redhat.com> wrote:
>
> On Fri, Aug 16, 2019 at 4:32 PM Dumitru Ceara <dceara at redhat.com> wrote:
> >
> > From: Dumitru Ceara <dceara at redhat.com>
> >
> > Due to the use of a uint8_t to index inside the DNS payload we could end
> > up in an infinite loop when specific (invalid) DNS packets were
> > processed by ovn-controller. In the infinite loop we keep increasing the
> > query_name dynamic string until running out of memory.
> >
> > One way to replicate the issue is to configure DNS on the logical switch
> > and then inject a manually crafted DNS-like packet. For example, with
> > Scapy:
> >
> > >>> p = IP(dst='10.0.0.2',src='10.0.0.3')/UDP(dport=53)/('a'*364)
> > >>> send(p)
> >
> > Also add a sanity check on minimum L4 size of packets.
> >
> > CC: Numan Siddique <nusiddiq at redhat.com>
> > Fixes: 16cb4fb8ca49 ("ovn-controller: Add 'dns_lookup' action")
> > Reported-at: https://bugzilla.redhat.com/1740335
> > Reported-by: Priscila <pveiga at redhat.com>
> > Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> > Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> >
> > (cherry-picked from ovn commit - 7fbdeaade826da299c20c05050627ebea65fe8c2)
> > ---
> >  ovn/controller/pinctrl.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> > index e443449..e388bbc 100644
> > --- a/ovn/controller/pinctrl.c
> > +++ b/ovn/controller/pinctrl.c
> > @@ -1628,6 +1628,12 @@ pinctrl_handle_dns_lookup(
> >          goto exit;
> >      }
> >
> > +    /* Check that the packet stores at least the minimal headers. */
> > +    if (dp_packet_l4_size(pkt_in) < (UDP_HEADER_LEN + DNS_HEADER_LEN)) {
> > +        VLOG_WARN_RL(&rl, "truncated dns packet");
> > +        goto exit;
> > +    }
> > +
> >      /* Extract the DNS header */
> >      struct dns_header const *in_dns_header = dp_packet_get_udp_payload(pkt_in);
> >      if (!in_dns_header) {
> > @@ -1652,7 +1658,7 @@ pinctrl_handle_dns_lookup(
> >      uint8_t *end = (uint8_t *)in_udp + MIN(udp_len, l4_len);
> >      uint8_t *in_dns_data = (uint8_t *)(in_dns_header + 1);
> >      uint8_t *in_queryname = in_dns_data;
> > -    uint8_t idx = 0;
> > +    uint16_t idx = 0;
> >      struct ds query_name;
> >      ds_init(&query_name);
> >      /* Extract the query_name. If the query name is - 'www.ovn.org' it would be
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
> Hi,
>
> Please backport this to 2.11, 2.10, 2.9 at least.
>
> Thanks,
> Dumitru

Hi Ben,

It would be great if we could have this bug fix in the 2.12 branch
before the release. The fix is already in OVN master.

Thanks,
Dumitru


More information about the dev mailing list