Check that SIG and RRSIG records for private algorithms are valid

SIG and RRSIG records for private algorithms are supposed to contain
the name / OID of the algorithm used to generate them at the start
of the signature field.

diff --git a/lib/dns/rdata.c b/lib/dns/rdata.c
index 8dc7e6e4dae947411dfb1268ddad9c95be5b0e6e..1665f87d9388302d10799f013fc962d82c8fe6e1 100644 (file)
--- a/lib/dns/rdata.c
+++ b/lib/dns/rdata.c
@@ -17,6 +17,8 @@
 #include <inttypes.h>
 #include <stdbool.h>
 
+#include <openssl/objects.h>
+
 #include <isc/base64.h>
 #include <isc/hex.h>
 #include <isc/lex.h>
@@ -604,6 +606,46 @@ typemap_test(isc_region_t *sr, bool allow_empty) {
 static const char hexdigits[] = "0123456789abcdef";
 static const char decdigits[] = "0123456789";
 
+static isc_result_t
+check_private(isc_buffer_t *source, dns_secalg_t alg) {
+       isc_region_t sr;
+       if (alg == DNS_KEYALG_PRIVATEDNS) {
+               dns_fixedname_t fixed;
+               dns_decompress_t dctx;
+
+               dns_decompress_init(&dctx, -1, DNS_DECOMPRESS_STRICT);
+               RETERR(dns_name_fromwire(dns_fixedname_initname(&fixed), source,
+                                        &dctx, 0, NULL));
+               /*
+                * There should be a public key or signature after the key name.
+                */
+               isc_buffer_activeregion(source, &sr);
+               if (sr.length == 0) {
+                       return (ISC_R_UNEXPECTEDEND);
+               }
+       } else if (alg == DNS_KEYALG_PRIVATEOID) {
+               /*
+                * Check that we can extract the OID from the start of the
+                * key data.
+                */
+               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 (obj == NULL) {
+                       RETERR(DNS_R_FORMERR);
+               }
+               ASN1_OBJECT_free(obj);
+               /* There should be a public key or signature after the OID. */
+               if (in >= sr.base + sr.length) {
+                       return (ISC_R_UNEXPECTEDEND);
+               }
+       }
+       return (ISC_R_SUCCESS);
+}
+
 #include "code.h"
 
 #define META    0x0001

diff --git a/lib/dns/rdata/generic/key_25.c b/lib/dns/rdata/generic/key_25.c
index efee3a32dcc80c42bf400bcf9383e6247db53f0b..6b8a14d9ca3e81ec8738380eaea4cc8737afc56b 100644 (file)
--- a/lib/dns/rdata/generic/key_25.c
+++ b/lib/dns/rdata/generic/key_25.c
@@ -16,8 +16,6 @@
 #ifndef RDATA_GENERIC_KEY_25_C
 #define RDATA_GENERIC_KEY_25_C
 
-#include <openssl/objects.h>
-
 #include <dst/dst.h>
 
 #define RRTYPE_KEY_ATTRIBUTES \
@@ -46,44 +44,6 @@ generic_key_nokey(dns_rdatatype_t type, unsigned int flags) {
        }
 }
 
-static isc_result_t
-check_private(isc_buffer_t *source, dns_secalg_t alg) {
-       isc_region_t sr;
-       if (alg == DNS_KEYALG_PRIVATEDNS) {
-               dns_fixedname_t fixed;
-               dns_decompress_t dctx;
-
-               dns_decompress_init(&dctx, -1, DNS_DECOMPRESS_STRICT);
-               RETERR(dns_name_fromwire(dns_fixedname_initname(&fixed), source,
-                                        &dctx, 0, NULL));
-               /* There should be a public key after the key name. */
-               isc_buffer_activeregion(source, &sr);
-               if (sr.length == 0) {
-                       return (ISC_R_UNEXPECTEDEND);
-               }
-       } else if (alg == DNS_KEYALG_PRIVATEOID) {
-               /*
-                * Check that we can extract the OID from the start of the
-                * key data.
-                */
-               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 (obj == NULL) {
-                       RETERR(DNS_R_FORMERR);
-               }
-               ASN1_OBJECT_free(obj);
-               /* There should be a public key after the OID. */
-               if (in >= sr.base + sr.length) {
-                       return (ISC_R_UNEXPECTEDEND);
-               }
-       }
-       return (ISC_R_SUCCESS);
-}
-
 static isc_result_t
 generic_fromtext_key(ARGS_FROMTEXT) {
        isc_token_t token;

diff --git a/lib/dns/rdata/generic/rrsig_46.c b/lib/dns/rdata/generic/rrsig_46.c
index 7c4159a752beec726deefdb2f3d1d01af7af1dda..bb9190431d2edc330bbe939d652e0a61427dd38d 100644 (file)
--- a/lib/dns/rdata/generic/rrsig_46.c
+++ b/lib/dns/rdata/generic/rrsig_46.c
@@ -23,7 +23,7 @@
 static isc_result_t
 fromtext_rrsig(ARGS_FROMTEXT) {
        isc_token_t token;
-       unsigned char c;
+       unsigned char alg, c;
        long i;
        dns_rdatatype_t covered;
        char *e;
@@ -31,6 +31,7 @@ fromtext_rrsig(ARGS_FROMTEXT) {
        dns_name_t name;
        isc_buffer_t buffer;
        uint32_t time_signed, time_expire;
+       unsigned int used;
 
        REQUIRE(type == dns_rdatatype_rrsig);
 
@@ -61,8 +62,8 @@ fromtext_rrsig(ARGS_FROMTEXT) {
         */
        RETERR(isc_lex_getmastertoken(lexer, &token, isc_tokentype_string,
                                      false));
-       RETTOK(dns_secalg_fromtext(&c, &token.value.as_textregion));
-       RETERR(mem_tobuffer(target, &c, 1));
+       RETTOK(dns_secalg_fromtext(&alg, &token.value.as_textregion));
+       RETERR(mem_tobuffer(target, &alg, 1));
 
        /*
         * Labels.
@@ -154,7 +155,24 @@ fromtext_rrsig(ARGS_FROMTEXT) {
        /*
         * Sig.
         */
-       return (isc_base64_tobuffer(lexer, target, -2));
+       used = isc_buffer_usedlength(target);
+
+       RETERR(isc_base64_tobuffer(lexer, target, -2));
+
+       if (alg == DNS_KEYALG_PRIVATEDNS || alg == DNS_KEYALG_PRIVATEOID) {
+               isc_buffer_t b;
+
+               /*
+                * Set up 'b' so that the signature data can be parsed.
+                */
+               b = *target;
+               b.active = b.used;
+               b.current = used;
+
+               RETERR(check_private(&b, alg));
+       }
+
+       return (ISC_R_SUCCESS);
 }
 
 static isc_result_t
@@ -278,6 +296,7 @@ static isc_result_t
 fromwire_rrsig(ARGS_FROMWIRE) {
        isc_region_t sr;
        dns_name_t name;
+       unsigned char algorithm;
 
        REQUIRE(type == dns_rdatatype_rrsig);
 
@@ -300,6 +319,8 @@ fromwire_rrsig(ARGS_FROMWIRE) {
                return (ISC_R_UNEXPECTEDEND);
        }
 
+       algorithm = sr.base[2];
+
        isc_buffer_forward(source, 18);
        RETERR(mem_tobuffer(target, sr.base, 18));
 
@@ -316,6 +337,13 @@ fromwire_rrsig(ARGS_FROMWIRE) {
        if (sr.length < 1) {
                return (DNS_R_FORMERR);
        }
+
+       if (algorithm == DNS_KEYALG_PRIVATEDNS ||
+           algorithm == DNS_KEYALG_PRIVATEOID) {
+               isc_buffer_t b = *source;
+               RETERR(check_private(&b, algorithm));
+       }
+
        isc_buffer_forward(source, sr.length);
        return (mem_tobuffer(target, sr.base, sr.length));
 }

diff --git a/lib/dns/rdata/generic/sig_24.c b/lib/dns/rdata/generic/sig_24.c
index d80beb63c51f41794dafe44d625eec661d639808..7cfec7bfed978d0f56695b5a05ab256dd30a1141 100644 (file)
--- a/lib/dns/rdata/generic/sig_24.c
+++ b/lib/dns/rdata/generic/sig_24.c
@@ -21,7 +21,7 @@
 static isc_result_t
 fromtext_sig(ARGS_FROMTEXT) {
        isc_token_t token;
-       unsigned char c;
+       unsigned char alg, c;
        long i;
        dns_rdatatype_t covered;
        char *e;
@@ -29,6 +29,7 @@ fromtext_sig(ARGS_FROMTEXT) {
        dns_name_t name;
        isc_buffer_t buffer;
        uint32_t time_signed, time_expire;
+       unsigned int used;
 
        REQUIRE(type == dns_rdatatype_sig);
 
@@ -59,8 +60,8 @@ fromtext_sig(ARGS_FROMTEXT) {
         */
        RETERR(isc_lex_getmastertoken(lexer, &token, isc_tokentype_string,
                                      false));
-       RETTOK(dns_secalg_fromtext(&c, &token.value.as_textregion));
-       RETERR(mem_tobuffer(target, &c, 1));
+       RETTOK(dns_secalg_fromtext(&alg, &token.value.as_textregion));
+       RETERR(mem_tobuffer(target, &alg, 1));
 
        /*
         * Labels.
@@ -118,7 +119,24 @@ fromtext_sig(ARGS_FROMTEXT) {
        /*
         * Sig.
         */
-       return (isc_base64_tobuffer(lexer, target, -2));
+       used = isc_buffer_usedlength(target);
+
+       RETERR(isc_base64_tobuffer(lexer, target, -2));
+
+       if (alg == DNS_KEYALG_PRIVATEDNS || alg == DNS_KEYALG_PRIVATEOID) {
+               isc_buffer_t b;
+
+               /*
+                * Set up 'b' so that the signature data can be parsed.
+                */
+               b = *target;
+               b.active = b.used;
+               b.current = used;
+
+               RETERR(check_private(&b, alg));
+       }
+
+       return (ISC_R_SUCCESS);
 }
 
 static isc_result_t
@@ -241,6 +259,7 @@ static isc_result_t
 fromwire_sig(ARGS_FROMWIRE) {
        isc_region_t sr;
        dns_name_t name;
+       unsigned char algorithm;
 
        REQUIRE(type == dns_rdatatype_sig);
 
@@ -263,6 +282,8 @@ fromwire_sig(ARGS_FROMWIRE) {
                return (ISC_R_UNEXPECTEDEND);
        }
 
+       algorithm = sr.base[2];
+
        isc_buffer_forward(source, 18);
        RETERR(mem_tobuffer(target, sr.base, 18));
 
@@ -279,6 +300,13 @@ fromwire_sig(ARGS_FROMWIRE) {
        if (sr.length == 0) {
                return (ISC_R_UNEXPECTEDEND);
        }
+
+       if (algorithm == DNS_KEYALG_PRIVATEDNS ||
+           algorithm == DNS_KEYALG_PRIVATEOID) {
+               isc_buffer_t b = *source;
+               RETERR(check_private(&b, algorithm));
+       }
+
        isc_buffer_forward(source, sr.length);
        return (mem_tobuffer(target, sr.base, sr.length));
 }

diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c
index 8066ed1010775595915e037bc0dbbfe4dced0a52..33b118d87d79bf48d365a8ab6408b0a683c6e660 100644 (file)
--- a/lib/dns/tests/rdata_test.c
+++ b/lib/dns/tests/rdata_test.c
@@ -2420,6 +2420,105 @@ rkey(void **state) {
                    dns_rdatatype_rkey, sizeof(dns_rdata_rkey_t));
 }
 
+/*
+ * rrsig (sig) tests.
+ */
+static void
+sig_rrsig(void **state) {
+       wire_ok_t wire_ok[] = {
+               /*
+                * RDATA is comprised of:
+                *
+                * type covered: 2
+                * algorithm: 1
+                * labels: 1
+                * original ttl: 4
+                * signature expiration: 4
+                * time signed: 4
+                * key footprint: 2
+                * signer: variable
+                * signature: variable
+                * - if algorithm is PRIVATEDNS the algorithm name is embedded
+                *   at the start of the signature
+                * - if algorithm is PRIVATEOID the algorithm OID is embedded
+                *   at the start of the signature
+                */
+               /* PRIVATEDNS example. */
+               WIRE_INVALID(0x00, 0x01, 253, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x06, 's', 'i', 'g', 'n', 'e', 'r',
+                            0x00, 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e',
+                            0x00),
+               /* PRIVATEDNS example. + sigdata */
+               WIRE_VALID(0x00, 0x01, 253, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
+                          0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                          0x06, 's', 'i', 'g', 'n', 'e', 'r', 0x00, 0x07, 'e',
+                          'x', 'a', 'm', 'p', 'l', 'e', 0x00, 0x00),
+               /* PRIVATEDNS compression pointer. */
+               WIRE_INVALID(0x00, 0x00, 0x00, 253, 0xc0, 0x00, 0x00),
+               /* PRIVATEOID */
+               WIRE_INVALID(0x00, 0x01, 254, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x06, 's', 'i', 'g', 'n', 'e', 'r',
+                            0x00, 0x00),
+               /* PRIVATEOID 1.3.6.1.4.1.2495 */
+               WIRE_INVALID(0x00, 0x01, 254, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x06, 's', 'i', 'g', 'n', 'e', 'r',
+                            0x00, 0x06, 0x07, 0x2b, 0x06, 0x01, 0x04, 0x01,
+                            0x93, 0x3f),
+               /* PRIVATEOID 1.3.6.1.4.1.2495 + sigdata */
+               WIRE_VALID(0x00, 0x01, 254, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
+                          0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+                          0x06, 's', 'i', 'g', 'n', 'e', 'r', 0x00, 0x06, 0x07,
+                          0x2b, 0x06, 0x01, 0x04, 0x01, 0x93, 0x3f, 0x00),
+               /* PRIVATEOID malformed OID - high-bit set on last octet */
+               WIRE_INVALID(0x00, 0x01, 254, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00,
+                            0x00, 0x00, 0x06, 's', 'i', 'g', 'n', 'e', 'r',
+                            0x00, 0x06, 0x07, 0x2b, 0x06, 0x01, 0x04, 0x01,
+                            0x93, 0xbf, 0x00),
+               WIRE_SENTINEL()
+       };
+       text_ok_t text_ok[] = {
+               /* PRIVATEDNS example. */
+               TEXT_INVALID("A 253 1 0 19700101000001 19700101000000 0 "
+                            "signer. B2V4YW1wbGUA"),
+               /* PRIVATEDNS example. + sigdata */
+               TEXT_VALID("A 253 1 0 19700101000001 19700101000000 0 signer. "
+                          "B2V4YW1wbGUAAA=="),
+               /* PRIVATEDNS compression pointer. */
+               TEXT_INVALID("A 253 1 0 19700101000001 19700101000000 0 "
+                            "signer. wAAA"),
+               /* PRIVATEOID */
+               TEXT_INVALID("A 254 1 0 19700101000001 19700101000000 0 "
+                            "signer. AA=="),
+               /* PRIVATEOID 1.3.6.1.4.1.2495 */
+               TEXT_INVALID("A 254 1 0 19700101000001 19700101000000 0 "
+                            "signer. BgcrBgEEAZM/"),
+               /* PRIVATEOID 1.3.6.1.4.1.2495 + sigdata */
+               TEXT_VALID("A 254 1 0 19700101000001 19700101000000 0 signer. "
+                          "BgcrBgEEAZM/AA=="),
+               /* PRIVATEOID malformed OID - high-bit set on last octet */
+               TEXT_INVALID("A 254 1 0 19700101000001 19700101000000 0 "
+                            "signer.  BgcrBgEEAZO/AA=="),
+               /* PRIVATEOID malformed OID - wrong tag */
+               TEXT_INVALID("A 254 1 0 19700101000001 19700101000000 0 "
+                            "signer. BwcrBgEEAZM/AA=="),
+               /*
+                * Sentinel.
+                */
+               TEXT_SENTINEL()
+       };
+
+       UNUSED(state);
+
+       check_rdata(text_ok, wire_ok, NULL, false, dns_rdataclass_in,
+                   dns_rdatatype_sig, sizeof(dns_rdata_sig_t));
+       check_rdata(text_ok, wire_ok, NULL, false, dns_rdataclass_in,
+                   dns_rdatatype_rrsig, sizeof(dns_rdata_rrsig_t));
+}
+
 /* SSHFP RDATA manipulations */
 static void
 sshfp(void **state) {
@@ -3209,6 +3308,7 @@ main(int argc, char **argv) {
                cmocka_unit_test_setup_teardown(nsec3, _setup, _teardown),
                cmocka_unit_test_setup_teardown(nxt, _setup, _teardown),
                cmocka_unit_test_setup_teardown(rkey, _setup, _teardown),
+               cmocka_unit_test_setup_teardown(sig_rrsig, _setup, _teardown),
                cmocka_unit_test_setup_teardown(sshfp, _setup, _teardown),
                cmocka_unit_test_setup_teardown(wks, _setup, _teardown),
                cmocka_unit_test_setup_teardown(zonemd, _setup, _teardown),