Fix OID check for PRIVATEOID keys and signatures

We were failing to account for the length byte before the OID.
See RFC 4034.

   Algorithm number 254 is reserved for private use and will never be
   assigned to a specific algorithm.  The public key area in the DNSKEY
   RR and the signature area in the RRSIG RR begin with an unsigned
   length byte followed by a BER encoded Object Identifier (ISO OID) of
   that length.  The OID indicates the private algorithm in use, and the
   remainder of the area is whatever is required by that algorithm.
   Entities should only use OIDs they control to designate their private
   algorithms.

diff --git a/lib/dns/rdata.c b/lib/dns/rdata.c
index 2b393e72bcdc71de2b4c36c4ab831e19d4b7a8bc..ea7252e264f4b613ae04807c359ad44bc282e442 100644 (file)
--- a/lib/dns/rdata.c
+++ b/lib/dns/rdata.c
@@ -607,19 +607,26 @@ check_private(isc_buffer_t *source, dns_secalg_t alg) {
        } else if (alg == DNS_KEYALG_PRIVATEOID) {
                /*
                 * Check that we can extract the OID from the start of the
-                * key data.
+                * key data. We have a length byte followed by the OID BER
+                * encoded.
                 */
                const unsigned char *in = NULL;
                ASN1_OBJECT *obj = NULL;
 
                isc_buffer_activeregion(source, &sr);
-               in = sr.base;
-               obj = d2i_ASN1_OBJECT(NULL, &in, sr.length);
+               if (sr.length < 1 || (unsigned int)*sr.base + 1 > sr.length) {
+                       RETERR(DNS_R_FORMERR);
+               }
+               in = sr.base + 1;
+               obj = d2i_ASN1_OBJECT(NULL, &in, *sr.base);
                if (obj == NULL) {
                        ERR_clear_error();
                        RETERR(DNS_R_FORMERR);
                }
                ASN1_OBJECT_free(obj);
+               if ((in - sr.base) != (*sr.base + 1)) {
+                       RETERR(DNS_R_FORMERR);
+               }
        }
        return ISC_R_SUCCESS;
 }

diff --git a/lib/dns/rdata/generic/key_25.c b/lib/dns/rdata/generic/key_25.c
index d44fec68acad1c45a0032dd647723850ad3c7865..70a14269c873e48dbefefbfd9562146ec45fe7b7 100644 (file)
--- a/lib/dns/rdata/generic/key_25.c
+++ b/lib/dns/rdata/generic/key_25.c
@@ -164,11 +164,10 @@ generic_totext_key(ARGS_TOTEXT) {
        } else if ((tctx->flags & DNS_STYLEFLAG_RRCOMMENT) != 0 &&
                   algorithm == DNS_KEYALG_PRIVATEOID)
        {
-               const unsigned char *in = sr.base;
-               ASN1_OBJECT *obj = d2i_ASN1_OBJECT(NULL, &in, sr.length);
-               int n;
+               const unsigned char *in = sr.base + 1;
+               ASN1_OBJECT *obj = d2i_ASN1_OBJECT(NULL, &in, *sr.base);
                INSIST(obj != NULL);
-               n = i2t_ASN1_OBJECT(algbuf, sizeof(buf), obj);
+               int n = i2t_ASN1_OBJECT(algbuf, sizeof(buf), obj);
                ASN1_OBJECT_free(obj);
                if (n == -1 || (size_t)n >= sizeof(algbuf)) {
                        dns_secalg_format((dns_secalg_t)algorithm, algbuf,

diff --git a/tests/dns/rdata_test.c b/tests/dns/rdata_test.c
index 9dacc0d61328b796219f7289b42c6baf4ff86e19..6366299a412f64e4b5f186db2d2913e7b71f85ef 100644 (file)
--- a/tests/dns/rdata_test.c
+++ b/tests/dns/rdata_test.c
@@ -2098,17 +2098,17 @@ ISC_RUN_TEST_IMPL(key) {
                /* PRIVATEOID */
                WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x00),
                /* PRIVATEOID 1.3.6.1.4.1.2495 without key data */
-               WIRE_VALID(0x00, 0x00, 0x00, 254, 0x06, 0x07, 0x2b, 0x06, 0x01,
-                          0x04, 0x01, 0x93, 0x3f),
+               WIRE_VALID(0x00, 0x00, 0x00, 254, 0x09, 0x06, 0x07, 0x2b, 0x06,
+                          0x01, 0x04, 0x01, 0x93, 0x3f),
                /* PRIVATEOID 1.3.6.1.4.1.2495 + keydata */
-               WIRE_VALID(0x00, 0x00, 0x00, 254, 0x06, 0x07, 0x2b, 0x06, 0x01,
-                          0x04, 0x01, 0x93, 0x3f, 0x00),
+               WIRE_VALID(0x00, 0x00, 0x00, 254, 0x09, 0x06, 0x07, 0x2b, 0x06,
+                          0x01, 0x04, 0x01, 0x93, 0x3f, 0x00),
                /* PRIVATEOID malformed OID - high-bit set on last octet */
                WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x06, 0x07, 0x2b, 0x06,
                             0x01, 0x04, 0x01, 0x93, 0xbf, 0x00),
                /* PRIVATEOID malformed OID - wrong tag */
-               WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x07, 0x07, 0x2b, 0x06,
-                            0x01, 0x04, 0x01, 0x93, 0x3f, 0x00),
+               WIRE_INVALID(0x00, 0x00, 0x00, 254, 0x09, 0x07, 0x07, 0x2b,
+                            0x06, 0x01, 0x04, 0x01, 0x93, 0x3f, 0x00),
                WIRE_SENTINEL()
        };
        text_ok_t text_ok[] = { /* PRIVATEDNS example. */
@@ -2120,14 +2120,14 @@ ISC_RUN_TEST_IMPL(key) {
                                /* PRIVATEOID */
                                TEXT_INVALID("0 0 254 AA=="),
                                /* PRIVATEOID 1.3.6.1.4.1.2495 */
-                               TEXT_VALID("0 0 254 BgcrBgEEAZM/"),
+                               TEXT_VALID("0 0 254 CQYHKwYBBAGTPw=="),
                                /* PRIVATEOID 1.3.6.1.4.1.2495 + keydata */
-                               TEXT_VALID("0 0 254 BgcrBgEEAZM/AA=="),
+                               TEXT_VALID("0 0 254 CQYHKwYBBAGTPwA="),
                                /* PRIVATEOID malformed OID - high-bit set on
                                   last octet */
-                               TEXT_INVALID("0 0 254 BgcrBgEEAZO/AA=="),
+                               TEXT_INVALID("0 0 254 CQYHKwYBBAGTvwA="),
                                /* PRIVATEOID malformed OID - wrong tag */
-                               TEXT_INVALID("0 0 254 BwcrBgEEAZM/AA=="),
+                               TEXT_INVALID("0 0 254 CQcHKwYBBAGTPwA="),
                                /*
                                 * Sentinel.
                                 */