Fix crash in pk11_numbits() when native-pkcs11 is used

When pk11_numbits() is passed a user provided input that contains all
zeroes (via crafted DNS message), it would crash with assertion
failure.  Fix that by properly handling such input.

diff --git a/lib/dns/pkcs11dh_link.c b/lib/dns/pkcs11dh_link.c
index e2b60ea7c5441b7ca3f51b52ba029208ea97dd18..4cd8e32d60d5fc3c1c095bb5443769a702dcaf95 100644 (file)
--- a/lib/dns/pkcs11dh_link.c
+++ b/lib/dns/pkcs11dh_link.c
@@ -748,6 +748,7 @@ pkcs11dh_fromdns(dst_key_t *key, isc_buffer_t *data) {
        CK_BYTE *prime = NULL, *base = NULL, *pub = NULL;
        CK_ATTRIBUTE *attr;
        int special = 0;
+       unsigned int bits;
        isc_result_t result;
 
        isc_buffer_remainingregion(data, &r);
@@ -852,7 +853,11 @@ pkcs11dh_fromdns(dst_key_t *key, isc_buffer_t *data) {
        pub = r.base;
        isc_region_consume(&r, publen);
 
-       key->key_size = pk11_numbits(prime, plen_);
+       result = pk11_numbits(prime, plen_, &bits);
+       if (result != ISC_R_SUCCESS) {
+               goto cleanup;
+       }
+       key->key_size = bits;
 
        dh->repr = (CK_ATTRIBUTE *) isc_mem_get(key->mctx, sizeof(*attr) * 3);
        if (dh->repr == NULL)
@@ -1012,6 +1017,7 @@ pkcs11dh_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
        dst_private_t priv;
        isc_result_t ret;
        int i;
+       unsigned int bits;
        pk11_object_t *dh = NULL;
        CK_ATTRIBUTE *attr;
        isc_mem_t *mctx;
@@ -1082,7 +1088,12 @@ pkcs11dh_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
 
        attr = pk11_attribute_bytype(dh, CKA_PRIME);
        INSIST(attr != NULL);
-       key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen);
+
+       ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       key->key_size = bits;
 
        return (ISC_R_SUCCESS);
 

diff --git a/lib/dns/pkcs11dsa_link.c b/lib/dns/pkcs11dsa_link.c
index 12d707a1125217dce3db4bdbf08d0d8b55e52496..24d4c149ff0f80e561eabb3e0ae33cbb9b92ddb1 100644 (file)
--- a/lib/dns/pkcs11dsa_link.c
+++ b/lib/dns/pkcs11dsa_link.c
@@ -983,6 +983,7 @@ pkcs11dsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
        dst_private_t priv;
        isc_result_t ret;
        int i;
+       unsigned int bits;
        pk11_object_t *dsa = NULL;
        CK_ATTRIBUTE *attr;
        isc_mem_t *mctx = key->mctx;
@@ -1072,7 +1073,12 @@ pkcs11dsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
 
        attr = pk11_attribute_bytype(dsa, CKA_PRIME);
        INSIST(attr != NULL);
-       key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen);
+
+       ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       key->key_size = bits;
 
        return (ISC_R_SUCCESS);
 

diff --git a/lib/dns/pkcs11rsa_link.c b/lib/dns/pkcs11rsa_link.c
index 096c1a8e915ebff801c5b134a5f1503cbc48d24f..1d10d26564b4366da1c23821d78c3ba1bdfea3f2 100644 (file)
--- a/lib/dns/pkcs11rsa_link.c
+++ b/lib/dns/pkcs11rsa_link.c
@@ -332,6 +332,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits,
                key->key_alg == DST_ALG_RSASHA256 ||
                key->key_alg == DST_ALG_RSASHA512);
 #endif
+       REQUIRE(maxbits <= RSA_MAX_PUBEXP_BITS);
 
        /*
         * Reject incorrect RSA key lengths.
@@ -376,6 +377,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits,
        for (attr = pk11_attribute_first(rsa);
             attr != NULL;
             attr = pk11_attribute_next(rsa, attr))
+       {
                switch (attr->type) {
                case CKA_MODULUS:
                        INSIST(keyTemplate[5].type == attr->type);
@@ -396,12 +398,16 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits,
                        memmove(keyTemplate[6].pValue, attr->pValue,
                                attr->ulValueLen);
                        keyTemplate[6].ulValueLen = attr->ulValueLen;
-                       if (pk11_numbits(attr->pValue,
-                                        attr->ulValueLen) > maxbits &&
-                           maxbits != 0)
+                       unsigned int bits;
+                       ret = pk11_numbits(attr->pValue, attr->ulValueLen,
+                                          &bits);
+                       if (ret != ISC_R_SUCCESS ||
+                           (bits > maxbits && maxbits != 0)) {
                                DST_RET(DST_R_VERIFYFAILURE);
+                       }
                        break;
                }
+       }
        pk11_ctx->object = CK_INVALID_HANDLE;
        pk11_ctx->ontoken = false;
        PK11_RET(pkcs_C_CreateObject,
@@ -1072,6 +1078,7 @@ pkcs11rsa_verify(dst_context_t *dctx, const isc_region_t *sig) {
                        keyTemplate[5].ulValueLen = attr->ulValueLen;
                        break;
                case CKA_PUBLIC_EXPONENT:
+                       unsigned int bits;
                        INSIST(keyTemplate[6].type == attr->type);
                        keyTemplate[6].pValue = isc_mem_get(dctx->mctx,
                                                            attr->ulValueLen);
@@ -1080,10 +1087,12 @@ pkcs11rsa_verify(dst_context_t *dctx, const isc_region_t *sig) {
                        memmove(keyTemplate[6].pValue, attr->pValue,
                                attr->ulValueLen);
                        keyTemplate[6].ulValueLen = attr->ulValueLen;
-                       if (pk11_numbits(attr->pValue,
-                                        attr->ulValueLen)
-                               > RSA_MAX_PUBEXP_BITS)
+                       ret = pk11_numbits(attr->pValue, attr->ulValueLen,
+                                          &bits);
+                       if (ret != ISC_R_SUCCESS || bits > RSA_MAX_PUBEXP_BITS)
+                       {
                                DST_RET(DST_R_VERIFYFAILURE);
+                       }
                        break;
                }
        pk11_ctx->object = CK_INVALID_HANDLE;
@@ -1461,6 +1470,8 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
        CK_BYTE *exponent = NULL, *modulus = NULL;
        CK_ATTRIBUTE *attr;
        unsigned int length;
+       unsigned int bits;
+       isc_result_t ret = ISC_R_SUCCESS;
 
        isc_buffer_remainingregion(data, &r);
        if (r.length == 0)
@@ -1478,9 +1489,7 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
 
        if (e_bytes == 0) {
                if (r.length < 2) {
-                       isc_safe_memwipe(rsa, sizeof(*rsa));
-                       isc_mem_put(key->mctx, rsa, sizeof(*rsa));
-                       return (DST_R_INVALIDPUBLICKEY);
+                       DST_RET(DST_R_INVALIDPUBLICKEY);
                }
                e_bytes = (*r.base) << 8;
                isc_region_consume(&r, 1);
@@ -1489,16 +1498,18 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
        }
 
        if (r.length < e_bytes) {
-               isc_safe_memwipe(rsa, sizeof(*rsa));
-               isc_mem_put(key->mctx, rsa, sizeof(*rsa));
-               return (DST_R_INVALIDPUBLICKEY);
+               DST_RET(DST_R_INVALIDPUBLICKEY);
        }
        exponent = r.base;
        isc_region_consume(&r, e_bytes);
        modulus = r.base;
        mod_bytes = r.length;
 
-       key->key_size = pk11_numbits(modulus, mod_bytes);
+       ret = pk11_numbits(modulus, mod_bytes, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       key->key_size = bits;
 
        isc_buffer_forward(data, length);
 
@@ -1548,9 +1559,12 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
                            rsa->repr,
                            rsa->attrcnt * sizeof(*attr));
        }
+       ret = ISC_R_NOMEMORY;
+
+    err:
        isc_safe_memwipe(rsa, sizeof(*rsa));
        isc_mem_put(key->mctx, rsa, sizeof(*rsa));
-       return (ISC_R_NOMEMORY);
+       return (ret);
 }
 
 static isc_result_t
@@ -1729,6 +1743,7 @@ pkcs11rsa_fetch(dst_key_t *key, const char *engine, const char *label,
        pk11_object_t *pubrsa;
        pk11_context_t *pk11_ctx = NULL;
        isc_result_t ret;
+       unsigned int bits;
 
        if (label == NULL)
                return (DST_R_NOENGINE);
@@ -1815,7 +1830,11 @@ pkcs11rsa_fetch(dst_key_t *key, const char *engine, const char *label,
 
        attr = pk11_attribute_bytype(rsa, CKA_MODULUS);
        INSIST(attr != NULL);
-       key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen);
+       ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       key->key_size = bits;
 
        return (ISC_R_SUCCESS);
 
@@ -1901,6 +1920,7 @@ pkcs11rsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
        CK_ATTRIBUTE *attr;
        isc_mem_t *mctx = key->mctx;
        const char *engine = NULL, *label = NULL;
+       unsigned int bits;
 
        /* read private key file */
        ret = dst__privstruct_parse(key, DST_ALG_RSA, lexer, mctx, &priv);
@@ -2044,12 +2064,22 @@ pkcs11rsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
 
        attr = pk11_attribute_bytype(rsa, CKA_MODULUS);
        INSIST(attr != NULL);
-       key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen);
+       ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       key->key_size = bits;
 
        attr = pk11_attribute_bytype(rsa, CKA_PUBLIC_EXPONENT);
        INSIST(attr != NULL);
-       if (pk11_numbits(attr->pValue, attr->ulValueLen) > RSA_MAX_PUBEXP_BITS)
+
+       ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       if (bits > RSA_MAX_PUBEXP_BITS) {
                DST_RET(ISC_R_RANGE);
+       }
 
        dst__privstruct_free(&priv, mctx);
        isc_safe_memwipe(&priv, sizeof(priv));
@@ -2084,6 +2114,7 @@ pkcs11rsa_fromlabel(dst_key_t *key, const char *engine, const char *label,
        pk11_context_t *pk11_ctx = NULL;
        isc_result_t ret;
        unsigned int i;
+       unsigned int bits;
 
        UNUSED(pin);
 
@@ -2178,12 +2209,22 @@ pkcs11rsa_fromlabel(dst_key_t *key, const char *engine, const char *label,
 
        attr = pk11_attribute_bytype(rsa, CKA_PUBLIC_EXPONENT);
        INSIST(attr != NULL);
-       if (pk11_numbits(attr->pValue, attr->ulValueLen) > RSA_MAX_PUBEXP_BITS)
+
+       ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       if (bits > RSA_MAX_PUBEXP_BITS) {
                DST_RET(ISC_R_RANGE);
+       }
 
        attr = pk11_attribute_bytype(rsa, CKA_MODULUS);
        INSIST(attr != NULL);
-       key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen);
+       ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits);
+       if (ret != ISC_R_SUCCESS) {
+               goto err;
+       }
+       key->key_size = bits;
 
        pk11_return_session(pk11_ctx);
        isc_safe_memwipe(pk11_ctx, sizeof(*pk11_ctx));

diff --git a/lib/isc/include/pk11/internal.h b/lib/isc/include/pk11/internal.h
index aa8907ab0805558a8a8f38b8372a216e569b8d12..7cc8ec812b06b2c5578244a92f69b75856be5ead 100644 (file)
--- a/lib/isc/include/pk11/internal.h
+++ b/lib/isc/include/pk11/internal.h
@@ -25,7 +25,8 @@ void pk11_mem_put(void *ptr, size_t size);
 
 CK_SLOT_ID pk11_get_best_token(pk11_optype_t optype);
 
-unsigned int pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt);
+isc_result_t
+pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt, unsigned int *bits);
 
 CK_ATTRIBUTE *pk11_attribute_first(const pk11_object_t *obj);
 

diff --git a/lib/isc/pk11.c b/lib/isc/pk11.c
index 012afd968af1911cf36e908bb731c97a97652fd6..4e4052044b2f581e15a253595242af7ff0251265 100644 (file)
--- a/lib/isc/pk11.c
+++ b/lib/isc/pk11.c
@@ -962,13 +962,15 @@ pk11_get_best_token(pk11_optype_t optype) {
        return (token->slotid);
 }
 
-unsigned int
-pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) {
+isc_result_t
+pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt, unsigned int *bits) {
        unsigned int bitcnt, i;
        CK_BYTE top;
 
-       if (bytecnt == 0)
-               return (0);
+       if (bytecnt == 0) {
+               *bits = 0;
+               return (ISC_R_SUCCESS);
+       }
        bitcnt = bytecnt * 8;
        for (i = 0; i < bytecnt; i++) {
                top = data[i];
@@ -976,26 +978,41 @@ pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) {
                        bitcnt -= 8;
                        continue;
                }
-               if (top & 0x80)
-                       return (bitcnt);
-               if (top & 0x40)
-                       return (bitcnt - 1);
-               if (top & 0x20)
-                       return (bitcnt - 2);
-               if (top & 0x10)
-                       return (bitcnt - 3);
-               if (top & 0x08)
-                       return (bitcnt - 4);
-               if (top & 0x04)
-                       return (bitcnt - 5);
-               if (top & 0x02)
-                       return (bitcnt - 6);
-               if (top & 0x01)
-                       return (bitcnt - 7);
+               if (top & 0x80) {
+                       *bits = bitcnt;
+                       return (ISC_R_SUCCESS);
+               }
+               if (top & 0x40) {
+                       *bits = bitcnt - 1;
+                       return (ISC_R_SUCCESS);
+               }
+               if (top & 0x20) {
+                       *bits = bitcnt - 2;
+                       return (ISC_R_SUCCESS);
+               }
+               if (top & 0x10) {
+                       *bits = bitcnt - 3;
+                       return (ISC_R_SUCCESS);
+               }
+               if (top & 0x08) {
+                       *bits = bitcnt - 4;
+                       return (ISC_R_SUCCESS);
+               }
+               if (top & 0x04) {
+                       *bits = bitcnt - 5;
+                       return (ISC_R_SUCCESS);
+               }
+               if (top & 0x02) {
+                       *bits = bitcnt - 6;
+                       return (ISC_R_SUCCESS);
+               }
+               if (top & 0x01) {
+                       *bits = bitcnt - 7;
+                       return (ISC_R_SUCCESS);
+               }
                break;
        }
-       INSIST(0);
-       ISC_UNREACHABLE();
+       return (ISC_R_RANGE);
 }
 
 CK_ATTRIBUTE *