On Mon, Apr 20, 2020 at 8:47 PM Mark Michelson <[email protected]> wrote: > > 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 <[email protected]> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1819069 > Reported-by: Jianlin Shi <[email protected]>
Acked-by: Numan Siddique <[email protected]> > --- > v1 -> v2 > Fixed 0-day Robot's complaint about line length in pinctrl.c > --- > 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(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 8703641c2..8592d4e3f 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. > -- > 2.25.1 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
