4154. [bug] A OPT record should be included with the FORMERR

                        response when there is a malformed EDNS option.
                        [RT #39647]

4153.   [bug]           Dig should zero non significant +subnet bits.  Check
                        that non significant ECS bits are zero on receipt.
                        [RT #39647]

diff --git a/CHANGES b/CHANGES
index 1756f1b3265547b7b04d8028b73a47535066c74c..f1f6995768d263978e5d316b6cd85bde912fcec0 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,11 @@
+4154.  [bug]           A OPT record should be included with the FORMERR
+                       response when there is a malformed EDNS option.
+                       [RT #39647]
+
+4153.  [bug]           Dig should zero non significant +subnet bits.  Check
+                       that non significant ECS bits are zero on receipt.
+                       [RT #39647]
+
 4152.  [func]          Implement DNS COOKIE option.  This replaces the
                        experimental SIT option of BIND 9.10.  The following
                        named.conf directives are avaliable: send-cookie,

diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c
index 0336dcb90d6522d189def670ca50163943b0bdab..36441f0c2d0fa2ec7d65045a26a013d0466e76fd 100644 (file)
--- a/bin/dig/dighost.c
+++ b/bin/dig/dighost.c
@@ -2491,13 +2491,19 @@ setup_lookup(dig_lookup_t *lookup) {
                        struct sockaddr *sa;
                        struct sockaddr_in *sin;
                        struct sockaddr_in6 *sin6;
+                       const isc_uint8_t *addr;
                        size_t addrl;
+                       isc_uint8_t mask;
 
                        sa = &lookup->ecs_addr->type.sa;
                        prefixlen = lookup->ecs_addr->length;
 
                        /* Round up prefix len to a multiple of 8 */
                        addrl = (prefixlen + 7) / 8;
+                       if (prefixlen % 8 == 0)
+                               mask = 0xff;
+                       else
+                               mask = 0xffU << (8 - (prefixlen % 8));
 
                        INSIST(i < DNS_EDNSOPTIONS);
                        opts[i].code = DNS_OPT_CLIENT_SUBNET;
@@ -2506,20 +2512,36 @@ setup_lookup(dig_lookup_t *lookup) {
                        isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf));
                        if (sa->sa_family == AF_INET) {
                                sin = (struct sockaddr_in *) sa;
+                               addr = (isc_uint8_t *) &sin->sin_addr;
+                               /* family */
                                isc_buffer_putuint16(&b, 1);
+                               /* source prefix-length */
                                isc_buffer_putuint8(&b, prefixlen);
+                               /* scope prefix-length */
                                isc_buffer_putuint8(&b, 0);
-                               isc_buffer_putmem(&b,
-                                         (isc_uint8_t *) &sin->sin_addr,
-                                         (unsigned int) addrl);
+                               /* address */
+                               if (addrl > 0) {
+                                       isc_buffer_putmem(&b, addr, addrl - 1);
+                                       isc_buffer_putuint8(&b,
+                                                           (addr[addrl - 1] &
+                                                            mask));
+                               }
                        } else {
                                sin6 = (struct sockaddr_in6 *) sa;
+                               addr = (isc_uint8_t *) &sin6->sin6_addr;
+                               /* family */
                                isc_buffer_putuint16(&b, 2);
+                               /* source prefix-length */
                                isc_buffer_putuint8(&b, prefixlen);
+                               /* scope prefix-length */
                                isc_buffer_putuint8(&b, 0);
-                               isc_buffer_putmem(&b,
-                                         (isc_uint8_t *) &sin6->sin6_addr,
-                                         (unsigned int) addrl);
+                               /* address */
+                               if (addrl > 0) {
+                                       isc_buffer_putmem(&b, addr, addrl - 1);
+                                       isc_buffer_putuint8(&b,
+                                                           (addr[addrl - 1] &
+                                                            mask));
+                               }
                        }
 
                        opts[i].value = (isc_uint8_t *) ecsbuf;

diff --git a/bin/named/client.c b/bin/named/client.c
index fc327ca11e16b055518c2481cec27f7fbf9bd626..5103fb643604f11dcc3b3a673f4fdf5b0e1a2291 100644 (file)
--- a/bin/named/client.c
+++ b/bin/named/client.c
@@ -1514,7 +1514,7 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message,
                        uc = paddr[i];
                        if (i == addrbytes - 1 &&
                            ((client->ecs_addrlen % 8) != 0))
-                               uc &= (1U << (8 - (client->ecs_addrlen % 8)));
+                               uc &= (0xffU << (8 - (client->ecs_addrlen % 8)));
                        isc_buffer_putuint8(&buf, uc);
                }
 
@@ -1818,10 +1818,16 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
        isc_netaddr_t caddr;
        int i;
 
+       /*
+        * XXXMUKS: Is there any need to repeat these checks here
+        * (except query's scope length) when they are done in the OPT
+        * RDATA fromwire code?
+        */
+
        if (optlen < 4U) {
                ns_client_log(client, NS_LOGCATEGORY_CLIENT,
                              NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
-                             "EDNS client subnet option too short");
+                             "EDNS client-subnet option too short");
                return (DNS_R_FORMERR);
        }
 
@@ -1833,8 +1839,8 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
        if (scope != 0U) {
                ns_client_log(client, NS_LOGCATEGORY_CLIENT,
                              NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
-                             "EDNS client subnet option: invalid scope");
-               return (DNS_R_FORMERR);
+                             "EDNS client-subnet option: invalid scope");
+               return (DNS_R_OPTERR);
        }
 
        memset(&caddr, 0, sizeof(caddr));
@@ -1849,26 +1855,26 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
        invalid_length:
                        ns_client_log(client, NS_LOGCATEGORY_CLIENT,
                                      NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
-                                     "EDNS client subnet option: invalid "
+                                     "EDNS client-subnet option: invalid "
                                      "address length (%u) for %s",
                                      addrlen, family == 1 ? "IPv4" : "IPv6");
-                       return (DNS_R_FORMERR);
+                       return (DNS_R_OPTERR);
                }
                caddr.family = AF_INET6;
                break;
        default:
                ns_client_log(client, NS_LOGCATEGORY_CLIENT,
                              NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
-                             "EDNS client subnet option: invalid family");
-               return (DNS_R_FORMERR);
+                             "EDNS client-subnet option: invalid family");
+               return (DNS_R_OPTERR);
        }
 
        addrbytes = (addrlen + 7) / 8;
        if (isc_buffer_remaininglength(buf) < addrbytes) {
                ns_client_log(client, NS_LOGCATEGORY_CLIENT,
                              NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(2),
-                             "EDNS client subnet option: address too short");
-               return (DNS_R_FORMERR);
+                             "EDNS client-subnet option: address too short");
+               return (DNS_R_OPTERR);
        }
 
        paddr = (isc_uint8_t *) &caddr.type;
@@ -1877,6 +1883,13 @@ process_ecs(ns_client_t *client, isc_buffer_t *buf, size_t optlen) {
                optlen--;
        }
 
+       if (addrbytes != 0U && (addrlen % 8) != 0) {
+               isc_uint8_t bits = ~0 << (8 - (addrlen % 8));
+               bits &= paddr[addrbytes - 1];
+               if (bits != paddr[addrbytes - 1])
+                       return (DNS_R_OPTERR);
+       }
+
        memmove(&client->ecs_addr, &caddr, sizeof(caddr));
        client->ecs_addrlen = addrlen;
        client->ecs_scope = 0;

diff --git a/bin/tests/system/resolver/tests.sh b/bin/tests/system/resolver/tests.sh
index a8a770ccd90cd9dad951642cf900d721f6586ef0..f8365386b8d7daa6ba9ff14c7ca9cc3938458da6 100755 (executable)
--- a/bin/tests/system/resolver/tests.sh
+++ b/bin/tests/system/resolver/tests.sh
@@ -544,5 +544,24 @@ grep "status: NOTIMP" dig.out.ns5.test${n} > /dev/null || ret=1
 if [ $ret != 0 ]; then echo "I:failed"; fi
 status=`expr $status + $ret`
 
+n=`expr $n + 1`
+echo "I:check that EDNS client subnet with non-zeroed bits is handled correctly (${n})"
+ret=0
+# 0001 (IPv4) 1f (31 significant bits) 00 (0) ffffffff (255.255.255.255)
+$DIG soa . @10.53.0.5 -p 5300 +ednsopt=8:00011f00ffffffff > dig.out.ns5.test${n} || ret=1
+grep "status: FORMERR" dig.out.ns5.test${n} > /dev/null || ret=1
+grep "; EDNS: version:" dig.out.ns5.test${n} > /dev/null || ret=1
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
+n=`expr $n + 1`
+echo "I:check that dig +subnet zeros address bits correctly (${n})"
+ret=0
+$DIG soa . @10.53.0.5 -p 5300 +subnet=255.255.255.255/23 > dig.out.ns5.test${n} || ret=1
+grep "status: NOERROR" dig.out.ns5.test${n} > /dev/null || ret=1
+grep "CLIENT-SUBNET: 255.255.254.0/23/0" dig.out.ns5.test${n} > /dev/null || ret=1
+if [ $ret != 0 ]; then echo "I:failed"; fi
+status=`expr $status + $ret`
+
 echo "I:exit status: $status"
 exit $status

diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c
index b3f49949da0b5ffcddda1ff4e350a0336695f844..c1fe59ec2116a143f698b8a5272ccd3d59d29105 100644 (file)
--- a/bin/tools/mdig.c
+++ b/bin/tools/mdig.c
@@ -588,7 +588,9 @@ sendquery(struct query *query, isc_task_t *task)
                        struct sockaddr *sa;
                        struct sockaddr_in *sin;
                        struct sockaddr_in6 *sin6;
+                       const isc_uint8_t *addr;
                        size_t addrl;
+                       isc_uint8_t mask;
                        isc_buffer_t b;
 
                        sa = &query->ecs_addr->type.sa;
@@ -596,6 +598,10 @@ sendquery(struct query *query, isc_task_t *task)
 
                        /* Round up prefix len to a multiple of 8 */
                        addrl = (prefixlen + 7) / 8;
+                       if (prefixlen % 8 == 0)
+                               mask = 0xff;
+                       else
+                               mask = 0xffU << (8 - (prefixlen % 8));
 
                        INSIST(i < DNS_EDNSOPTIONS);
                        opts[i].code = DNS_OPT_CLIENT_SUBNET;
@@ -604,20 +610,36 @@ sendquery(struct query *query, isc_task_t *task)
                        isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf));
                        if (sa->sa_family == AF_INET) {
                                sin = (struct sockaddr_in *) sa;
+                               addr = (isc_uint8_t *) &sin->sin_addr;
+                               /* family */
                                isc_buffer_putuint16(&b, 1);
+                               /* source prefix-length */
                                isc_buffer_putuint8(&b, prefixlen);
+                               /* scope prefix-length */
                                isc_buffer_putuint8(&b, 0);
-                               isc_buffer_putmem(&b,
-                                         (isc_uint8_t *) &sin->sin_addr,
-                                         (unsigned int) addrl);
+                               /* address */
+                               if (addrl > 0) {
+                                       isc_buffer_putmem(&b, addr, addrl - 1);
+                                       isc_buffer_putuint8(&b,
+                                                           (addr[addrl - 1] &
+                                                            mask));
+                               }
                        } else {
                                sin6 = (struct sockaddr_in6 *) sa;
+                               addr = (isc_uint8_t *) &sin6->sin6_addr;
+                               /* family */
                                isc_buffer_putuint16(&b, 2);
+                               /* source prefix-length */
                                isc_buffer_putuint8(&b, prefixlen);
+                               /* scope prefix-length */
                                isc_buffer_putuint8(&b, 0);
-                               isc_buffer_putmem(&b,
-                                         (isc_uint8_t *) &sin6->sin6_addr,
-                                         (unsigned int) addrl);
+                               /* address */
+                               if (addrl > 0) {
+                                       isc_buffer_putmem(&b, addr, addrl - 1);
+                                       isc_buffer_putuint8(&b,
+                                                           (addr[addrl - 1] &
+                                                            mask));
+                               }
                        }
 
                        opts[i].value = (isc_uint8_t *) ecsbuf;

diff --git a/lib/dns/rdata/generic/opt_41.c b/lib/dns/rdata/generic/opt_41.c
index 7ae5e865bb6b7521c570bd6f062ac24589666c8f..853bfbb39aa13f22144c170c4eca8117ad21aaca 100644 (file)
--- a/lib/dns/rdata/generic/opt_41.c
+++ b/lib/dns/rdata/generic/opt_41.c
@@ -150,6 +150,13 @@ fromwire_opt(ARGS_FROMWIRE) {
                        addrbytes = (addrlen + 7) / 8;
                        if (addrbytes + 4 != length)
                                return (DNS_R_OPTERR);
+
+                       if (addrbytes != 0U && (addrlen % 8) != 0) {
+                               isc_uint8_t bits = ~0 << (8 - (addrlen % 8));
+                               bits &= sregion.base[addrbytes - 1];
+                               if (bits != sregion.base[addrbytes - 1])
+                                       return (DNS_R_OPTERR);
+                       }
                        isc_region_consume(&sregion, addrbytes);
                        break;
                }
@@ -163,7 +170,7 @@ fromwire_opt(ARGS_FROMWIRE) {
                        break;
                case DNS_OPT_COOKIE:
                        if (length != 8 && (length < 16 || length > 40))
-                               return (DNS_R_FORMERR);
+                               return (DNS_R_OPTERR);
                        isc_region_consume(&sregion, length);
                        break;
                default: