]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
4154. [bug] A OPT record should be included with the FORMERR
authorMark Andrews <marka@isc.org>
Mon, 6 Jul 2015 02:52:37 +0000 (12:52 +1000)
committerMark Andrews <marka@isc.org>
Mon, 6 Jul 2015 06:34:48 +0000 (16:34 +1000)
                        response when there is a malformed EDNS option.
                        [RT #39647]

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

CHANGES
bin/named/client.c
bin/tests/system/formerr/badoptoption [new file with mode: 0644]
bin/tests/system/formerr/clean.sh
bin/tests/system/formerr/tests.sh
lib/dns/include/dns/result.h
lib/dns/rdata/generic/opt_41.c
lib/dns/result.c

diff --git a/CHANGES b/CHANGES
index e9dd725cc543bde0cf0044d7202849e7cd2604cd..795a7c1a5d8c43e5e4ff1cbc220296b278cb55fa 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,10 @@
+4154.  [bug]           A OPT record should be included with the FORMERR
+                       response when there is a malformed EDNS option.
+                       [RT #39647]
+
+4153.  [bug]           Check that non significant ECS bits are zero on
+                       receipt. [RT #39647]
+
 4151.  [bug]           'rndc flush' could cause a deadlock. [RT #39835]
 
 4150.  [bug]           win32: listen-on-v6 { any; }; was not working.  Apply
index 26222d0a1df502ea22cddd54799d486e5040fb69..54da0c2024000d49da282ff4278ebbf7c7dbba9b 100644 (file)
@@ -1660,6 +1660,8 @@ client_request(isc_task_t *task, isc_event_t *event) {
                 * Parsing the request failed.  Send a response
                 * (typically FORMERR or SERVFAIL).
                 */
+               if (result == DNS_R_OPTERR)
+                       (void)client_addopt(client);
                ns_client_error(client, result);
                goto cleanup;
        }
diff --git a/bin/tests/system/formerr/badoptoption b/bin/tests/system/formerr/badoptoption
new file mode 100644 (file)
index 0000000..eec4129
--- /dev/null
@@ -0,0 +1,3 @@
+00 00 00 00 00 01 00 00 00 00 00 01
+00 00 01 00 02
+00 00 29 10 00 00 00 00 00 00 04 00 08 00 00
index 4978c71b9fbfe4ecf72d7e972c145025bb6e3587..da5df3ce9583919a4ea2b9d365db3d61b5a45ac5 100644 (file)
@@ -12,6 +12,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
+rm -f badoptoption.out 
 rm -f nametoolong.out 
-rm -f twoquestions.out 
 rm -f noquestions.out 
+rm -f twoquestions.out 
index ea6aca53eda41dc66780cc30ab887628b826d784..4ee4fd5f6b850f2fbbb0c3ab8f5948f98a4ba0e1 100644 (file)
@@ -44,6 +44,14 @@ then
        echo "I:failed"; status=`expr $status + 1`;
 fi
 
+echo "I:bad OPT option"
+$PERL formerr.pl -a 10.53.0.1 -p 5300 badoptoption > badoptoption.out
+ans=`grep got: badoptoption.out`
+if [ "${ans}" != "got: 00008001000100000000000100000100020000291000000000000000" ];
+then
+       echo "I:failed"; status=`expr $status + 1`;
+fi
+
 echo "I:exit status: $status"
 
 exit $status
index 489737cf7cdb708c8324d987632349f8b6bf03a6..7d11c2beb01e58aeeafcb8054190c7a47b0643e3 100644 (file)
 #define DNS_R_NTACOVERED               (ISC_RESULTCLASS_DNS + 110)
 #define DNS_R_BADCDS                   (ISC_RESULTCLASS_DNS + 111)
 #define DNS_R_BADCDNSKEY               (ISC_RESULTCLASS_DNS + 112)
+#define DNS_R_OPTERR                   (ISC_RESULTCLASS_DNS + 113)
 
-#define DNS_R_NRESULTS                 113     /*%< Number of results */
+#define DNS_R_NRESULTS                 114     /*%< Number of results */
 
 /*
  * DNS wire format rcodes.
index ba3fef001a15c308d9d993c3c395c9caf66ac935..cc391c32e2a415d3fb60b8516b5264415890822f 100644 (file)
@@ -128,7 +128,7 @@ fromwire_opt(ARGS_FROMWIRE) {
                        isc_uint8_t addrbytes;
 
                        if (length < 4)
-                               return (DNS_R_FORMERR);
+                               return (DNS_R_OPTERR);
                        family = uint16_fromregion(&sregion);
                        isc_region_consume(&sregion, 2);
                        addrlen = uint8_fromregion(&sregion);
@@ -138,16 +138,23 @@ fromwire_opt(ARGS_FROMWIRE) {
                        switch (family) {
                        case 1:
                                if (addrlen > 32U || scope > 32U)
-                                       return (DNS_R_FORMERR);
+                                       return (DNS_R_OPTERR);
                                break;
                        case 2:
                                if (addrlen > 128U || scope > 128U)
-                                       return (DNS_R_FORMERR);
+                                       return (DNS_R_OPTERR);
                                break;
                        }
                        addrbytes = (addrlen + 7) / 8;
                        if (addrbytes + 4 != length)
-                               return (DNS_R_FORMERR);
+                               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;
                }
@@ -156,7 +163,7 @@ fromwire_opt(ARGS_FROMWIRE) {
                         * Request has zero length.  Response is 32 bits.
                         */
                        if (length != 0 && length != 4)
-                               return (DNS_R_FORMERR);
+                               return (DNS_R_OPTERR);
                        isc_region_consume(&sregion, length);
                        break;
                default:
index 9387f978930ae00879e29a9eaad07d17a62784d4..7be4f577ed866cf96dbca894b6e38fdbf6bb815c 100644 (file)
@@ -164,9 +164,10 @@ static const char *text[DNS_R_NRESULTS] = {
        "not dynamic",                         /*%< 108 DNS_R_NOTDYNAMIC */
        "bad EUI",                             /*%< 109 DNS_R_BADEUI */
 
-       "covered by negative trust anchor",     /*%< 110 DNS_R_NTACOVERED */
+       "covered by negative trust anchor",    /*%< 110 DNS_R_NTACOVERED */
        "bad CDS",                             /*%< 111 DNS_R_BADCSD */
-       "bad CDNSKEY"                          /*%< 112 DNS_R_BADCDNSKEY */
+       "bad CDNSKEY",                         /*%< 112 DNS_R_BADCDNSKEY */
+       "malformed OPT option"                 /*%< 113 DNS_R_OPTERR */
 };
 
 static const char *rcode_text[DNS_R_NRCODERESULTS] = {
@@ -242,6 +243,7 @@ dns_result_torcode(isc_result_t result) {
                 */
                return ((dns_rcode_t)((result) & 0xFFF));
        }
+
        /*
         * Try to supply an appropriate rcode.
         */
@@ -271,6 +273,7 @@ dns_result_torcode(isc_result_t result) {
        case DNS_R_TSIGERRORSET:
        case DNS_R_UNKNOWN:
        case DNS_R_NAMETOOLONG:
+       case DNS_R_OPTERR:
                rcode = dns_rcode_formerr;
                break;
        case DNS_R_DISALLOWED: