diff mbox series

[ovs-dev,v4] controller: Ignore DNS queries with RRs

Message ID 20230526144804.2341165-1-haleyb.dev@gmail.com
State Accepted
Headers show
Series [ovs-dev,v4] controller: Ignore DNS queries with RRs | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Brian Haley May 26, 2023, 2:48 p.m. UTC
DNS queries with optional records (RRs), for example, with
cookies for EDNS, are not supported by the OVN resolver.
Trying to reply will result in mangled responses that
clients do not understand - the ANSWER section will
contain an incorrect option.

Instead, just return early when one is present, which
will trigger a negative response and cause clients to
go to the upstream forwarder, hopefully resulting in a
successful query.

In our testing, the resolver only retries if the
response is correctly formatted, which now happens
with this change.

Reported-at: https://github.com/ovn-org/ovn/issues/192
Reported-by: Nicolas Bock <nicolasbock@gmail.com>
Signed-off-by: Brian Haley <haleyb.dev@gmail.com>
---
Changes since v3:
- Updated commit message to include reporter info
---
Changes since v2:
- Updated commit message to be more clear
---
Changes since v1:
- Added issue #192 to commit message
---
 controller/pinctrl.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Dumitru Ceara May 30, 2023, 2:54 p.m. UTC | #1
On 5/26/23 16:48, Brian Haley wrote:
> DNS queries with optional records (RRs), for example, with
> cookies for EDNS, are not supported by the OVN resolver.
> Trying to reply will result in mangled responses that
> clients do not understand - the ANSWER section will
> contain an incorrect option.
> 
> Instead, just return early when one is present, which
> will trigger a negative response and cause clients to
> go to the upstream forwarder, hopefully resulting in a
> successful query.
> 
> In our testing, the resolver only retries if the
> response is correctly formatted, which now happens
> with this change.
> 
> Reported-at: https://github.com/ovn-org/ovn/issues/192
> Reported-by: Nicolas Bock <nicolasbock@gmail.com>
> Signed-off-by: Brian Haley <haleyb.dev@gmail.com>
> ---

Thanks for the patch, Brian!

Applied to main and backported to all branches down to 22.03.

Regards,
Dumitru
diff mbox series

Patch

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index b5df8b1eb..b45b4c747 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2864,6 +2864,13 @@  pinctrl_handle_dns_lookup(
         goto exit;
     }
 
+    /* Check if there is an additional record present, which is unsupported */
+    if (in_dns_header->arcount) {
+        VLOG_DBG_RL(&rl, "Received DNS query with additional records, which"
+                    " is unsupported");
+        goto exit;
+    }
+
     struct udp_header *in_udp = dp_packet_l4(pkt_in);
     size_t udp_len = ntohs(in_udp->udp_len);
     size_t l4_len = dp_packet_l4_size(pkt_in);