diff mbox series

[ovs-dev,ovn] DNS: Make DNS lookups case insensitive.

Message ID 20200420140227.4171953-1-mmichels@redhat.com
State Superseded
Headers show
Series [ovs-dev,ovn] DNS: Make DNS lookups case insensitive. | expand

Commit Message

Mark Michelson April 20, 2020, 2:02 p.m. UTC 
From RFC 1035 Section 2.3.3:

"For all parts of the DNS that are part of the official protocol, all
comparisons between character strings (e.g., labels, domain names, etc.)
are done in a case-insensitive manner."

OVN was using case-sensitive lookups and therefore was not complying.
This change makes lookups case insensitive by storing lowercase record
names in the southbound database and converting incoming query names to
lowercase.

Signed-off-by: Mark Michelson <mmichels@redhat.com>
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1819069
Reported-by: Jianlin Shi <jishi@redhat.com>
---
 controller/pinctrl.c |  7 ++++-
 lib/ovn-util.c       | 15 +++++++++++
 lib/ovn-util.h       |  5 ++++
 northd/ovn-northd.c  | 15 ++++++++++-
 ovn-sb.xml           |  3 ++-
 tests/ovn.at         | 61 ++++++++++++++++++++++++++++++++------------
 6 files changed, 87 insertions(+), 19 deletions(-)

Comments

0-day Robot April 20, 2020, 3:02 p.m. UTC | #1 
Bleep bloop.  Greetings Mark Michelson, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 84 characters long (recommended limit is 79)
#38 FILE: controller/pinctrl.c:2371:
                /* DNS records in SBDB are stored in lowercase. Convert to lowercase

Lines checked: 268, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 8703641c2..79ae86968 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2368,7 +2368,12 @@  pinctrl_handle_dns_lookup(
         struct dns_data *d = iter->data;
         for (size_t i = 0; i < d->n_dps; i++) {
             if (d->dps[i] == dp_key) {
-                answer_ips = smap_get(&d->records, ds_cstr(&query_name));
+                /* DNS records in SBDB are stored in lowercase. Convert to lowercase
+                 * to perform case insensitive lookup
+                 */
+                char *query_name_lower = str_tolower(ds_cstr(&query_name));
+                answer_ips = smap_get(&d->records, query_name_lower);
+                free(query_name_lower);
                 if (answer_ips) {
                     break;
                 }
diff --git a/lib/ovn-util.c b/lib/ovn-util.c
index 514e2489f..1b30c2e9a 100644
--- a/lib/ovn-util.c
+++ b/lib/ovn-util.c
@@ -21,6 +21,7 @@ 
 #include "openvswitch/ofp-parse.h"
 #include "ovn-nb-idl.h"
 #include "ovn-sb-idl.h"
+#include <ctype.h>
 
 VLOG_DEFINE_THIS_MODULE(ovn_util);
 
@@ -550,3 +551,17 @@  ip46_equals(const struct v46_ip *addr1, const struct v46_ip *addr2)
             (addr1->family == AF_INET ? addr1->ipv4 == addr2->ipv4 :
              IN6_ARE_ADDR_EQUAL(&addr1->ipv6, &addr2->ipv6)));
 }
+
+char *
+str_tolower(const char *orig)
+{
+    char *copy = xmalloc(strlen(orig) + 1);
+    char *p = copy;
+
+    while (*orig) {
+        *p++ = tolower(*orig++);
+    }
+    *p = '\0';
+
+    return copy;
+}
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 11238f61c..4076e8b9a 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -124,4 +124,9 @@  struct v46_ip {
 bool ip46_parse_cidr(const char *str, struct v46_ip *prefix,
                      unsigned int *plen);
 bool ip46_equals(const struct v46_ip *addr1, const struct v46_ip *addr2);
+
+/* Returns a lowercase copy of orig.
+ * Caller must free the returned string.
+ */
+char *str_tolower(const char *orig);
 #endif
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 616e52bb2..0219a4bb0 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -10706,7 +10706,20 @@  sync_dns_entries(struct northd_context *ctx, struct hmap *datapaths)
             dns_info->sb_dns,
             (struct sbrec_datapath_binding **)dns_info->sbs,
             dns_info->n_sbs);
-        sbrec_dns_set_records(dns_info->sb_dns, &dns_info->nb_dns->records);
+
+        /* DNS lookups are case-insensitive. Convert records to lowercase so
+         * we can do consistent lookups when DNS requests arrive
+         */
+        struct smap lower_records = SMAP_INITIALIZER(&lower_records);
+        struct smap_node *node;
+        SMAP_FOR_EACH (node, &dns_info->nb_dns->records) {
+            smap_add_nocopy(&lower_records, xstrdup(node->key),
+                            str_tolower(node->value));
+        }
+
+        sbrec_dns_set_records(dns_info->sb_dns, &lower_records);
+
+        smap_destroy(&lower_records);
         free(dns_info->sbs);
         free(dns_info);
     }
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 72466b97e..5f8da534c 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -3597,7 +3597,8 @@  tcp.flags = RST;
     <column name="records">
       Key-value pair of DNS records with <code>DNS query name</code> as the key
       and a string of IP address(es) separated by comma or space as the
-      value.
+      value. ovn-northd stores the DNS query name in all lowercase in order to
+      facilitate case-insensitive lookups.
 
       <p><b>Example: </b> "vm1.ovn.org" = "10.0.0.4 aef0::4"</p>
     </column>
diff --git a/tests/ovn.at b/tests/ovn.at
index 7858c496e..a52e644f0 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8357,6 +8357,12 @@  set_dns_params() {
         # IPv4 address - 10.0.0.4
         expected_dns_answer=${query_name}00010001${ttl}00040a000004
         ;;
+    VM1)
+        # VM1.OVN.ORG
+        query_name=03564d31034f564e034f524700
+        # IPv4 address - 10.0.0.4
+        expected_dns_answer=${query_name}00010001${ttl}00040a000004
+        ;;
     vm2)
         # vm2.ovn.org
         query_name=03766d32036f766e036f726700
@@ -8519,6 +8525,29 @@  reset_pcap_file hv1-vif2 hv1/vif2
 rm -f 1.expected
 rm -f 2.expected
 
+# Try vm1 again but an all-caps query name
+
+set_dns_params VM1
+src_ip=`ip_to_hex 10 0 0 6`
+dst_ip=`ip_to_hex 10 0 0 1`
+dns_reply=1
+test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data $dns_resp_data
+
+# NXT_RESUMEs should be 3.
+OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
+cat 2.expected | cut -c -48 > expout
+AT_CHECK([cat 2.packets | cut -c -48], [0], [expout])
+# Skipping the IPv4 checksum.
+cat 2.expected | cut -c 53- > expout
+AT_CHECK([cat 2.packets | cut -c 53-], [0], [expout])
+
+reset_pcap_file hv1-vif1 hv1/vif1
+reset_pcap_file hv1-vif2 hv1/vif2
+rm -f 1.expected
+rm -f 2.expected
+
 # Clear the query name options for ls1-lp2
 ovn-nbctl --wait=hv remove DNS $DNS1 records vm2.ovn.org
 
@@ -8528,8 +8557,8 @@  dst_ip=`ip_to_hex 10 0 0 1`
 dns_reply=0
 test_dns 1 f00000000001 f00000000002 $src_ip $dst_ip $dns_reply $dns_req_data
 
-# NXT_RESUMEs should be 3.
-OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 4.
+OVS_WAIT_UNTIL([test 4 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets
 AT_CHECK([cat 1.packets], [0], [])
@@ -8550,8 +8579,8 @@  dst_ip=`ip_to_hex 10 0 0 1`
 dns_reply=0
 test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data
 
-# NXT_RESUMEs should be 3 only.
-OVS_WAIT_UNTIL([test 3 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 4 only.
+OVS_WAIT_UNTIL([test 4 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
 AT_CHECK([cat 2.packets], [0], [])
@@ -8571,8 +8600,8 @@  dst_ip=`ip_to_hex 10 0 0 1`
 dns_reply=1
 test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data $dns_resp_data
 
-# NXT_RESUMEs should be 4.
-OVS_WAIT_UNTIL([test 4 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 5.
+OVS_WAIT_UNTIL([test 5 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
 cat 2.expected | cut -c -48 > expout
@@ -8593,8 +8622,8 @@  dst_ip=`ip_to_hex 10 0 0 1`
 dns_reply=1
 test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data $dns_resp_data
 
-# NXT_RESUMEs should be 5.
-OVS_WAIT_UNTIL([test 5 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 6.
+OVS_WAIT_UNTIL([test 6 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
 cat 2.expected | cut -c -48 > expout
@@ -8615,8 +8644,8 @@  dst_ip=`ip_to_hex 10 0 0 1`
 dns_reply=0
 test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data
 
-# NXT_RESUMEs should be 6.
-OVS_WAIT_UNTIL([test 6 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 7.
+OVS_WAIT_UNTIL([test 7 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
 AT_CHECK([cat 2.packets], [0], [])
@@ -8633,8 +8662,8 @@  dst_ip=`ip_to_hex 10 0 0 1`
 dns_reply=0
 test_dns 2 f00000000002 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data
 
-# NXT_RESUMEs should be 7.
-OVS_WAIT_UNTIL([test 7 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 8.
+OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif2-tx.pcap > 2.packets
 AT_CHECK([cat 2.packets], [0], [])
@@ -8653,8 +8682,8 @@  dst_ip=`ip_to_hex 10 0 0 1`
 dns_reply=1
 test_dns 1 f00000000001 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data $dns_resp_data
 
-# NXT_RESUMEs should be 8.
-OVS_WAIT_UNTIL([test 8 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 9.
+OVS_WAIT_UNTIL([test 9 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets
 cat 1.expected | cut -c -48 > expout
@@ -8675,8 +8704,8 @@  dst_ip=aef00000000000000000000000000001
 dns_reply=1
 test_dns6 1 f00000000001 f000000000f0 $src_ip $dst_ip $dns_reply $dns_req_data $dns_resp_data
 
-# NXT_RESUMEs should be 9.
-OVS_WAIT_UNTIL([test 9 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
+# NXT_RESUMEs should be 10
+OVS_WAIT_UNTIL([test 10 = `cat ofctl_monitor*.log | grep -c NXT_RESUME`])
 
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/vif1-tx.pcap > 1.packets
 # Skipping the UDP checksum.