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/pkcs11rsa_link.c b/lib/dns/pkcs11rsa_link.c
index 5487f2986947eb04faea72438f31a1e07ba64038..dca3f1b8a3dbe4d698c704f9a2095c544496909b 100644 (file)
--- a/lib/dns/pkcs11rsa_link.c
+++ b/lib/dns/pkcs11rsa_link.c
@@ -294,6 +294,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits,
                key->key_alg == DST_ALG_NSEC3RSASHA1 ||
                key->key_alg == DST_ALG_RSASHA256 ||
                key->key_alg == DST_ALG_RSASHA512);
+       REQUIRE(maxbits <= RSA_MAX_PUBEXP_BITS);
 
        /*
         * Reject incorrect RSA key lengths.
@@ -336,6 +337,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);
@@ -352,13 +354,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,
@@ -959,14 +964,17 @@ 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);
                        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;
@@ -1337,6 +1345,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) {
@@ -1353,9 +1363,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);
@@ -1364,16 +1372,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);
 
@@ -1393,6 +1403,10 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
        key->keydata.pkey = rsa;
 
        return (ISC_R_SUCCESS);
+err:
+       isc_safe_memwipe(rsa, sizeof(*rsa));
+       isc_mem_put(key->mctx, rsa, sizeof(*rsa));
+       return (ret);
 }
 
 static isc_result_t
@@ -1566,6 +1580,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);
@@ -1644,7 +1659,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);
 
@@ -1736,6 +1755,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);
@@ -1873,12 +1893,20 @@ 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);
        }
 
@@ -1913,6 +1941,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);
 
@@ -1998,14 +2027,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 dfdf526eebd8b6f050d1c57adf4c967f3db47e3d..05be8997d370dc04fa2c92d05aa890c7728bfcbb 100644 (file)
--- a/lib/isc/include/pk11/internal.h
+++ b/lib/isc/include/pk11/internal.h
@@ -30,8 +30,8 @@ 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 7841c4957fa23e59d67b2af27a09fc05ec864d64..5abf53d0e346196fc3da6b03726258ba6702680e 100644 (file)
--- a/lib/isc/pk11.c
+++ b/lib/isc/pk11.c
@@ -654,13 +654,14 @@ 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);
+               *bits = 0;
+               return (ISC_R_SUCCESS);
        }
        bitcnt = bytecnt * 8;
        for (i = 0; i < bytecnt; i++) {
@@ -670,33 +671,40 @@ pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) {
                        continue;
                }
                if (top & 0x80) {
-                       return (bitcnt);
+                       *bits = bitcnt;
+                       return (ISC_R_SUCCESS);
                }
                if (top & 0x40) {
-                       return (bitcnt - 1);
+                       *bits = bitcnt - 1;
+                       return (ISC_R_SUCCESS);
                }
                if (top & 0x20) {
-                       return (bitcnt - 2);
+                       *bits = bitcnt - 2;
+                       return (ISC_R_SUCCESS);
                }
                if (top & 0x10) {
-                       return (bitcnt - 3);
+                       *bits = bitcnt - 3;
+                       return (ISC_R_SUCCESS);
                }
                if (top & 0x08) {
-                       return (bitcnt - 4);
+                       *bits = bitcnt - 4;
+                       return (ISC_R_SUCCESS);
                }
                if (top & 0x04) {
-                       return (bitcnt - 5);
+                       *bits = bitcnt - 5;
+                       return (ISC_R_SUCCESS);
                }
                if (top & 0x02) {
-                       return (bitcnt - 6);
+                       *bits = bitcnt - 6;
+                       return (ISC_R_SUCCESS);
                }
                if (top & 0x01) {
-                       return (bitcnt - 7);
+                       *bits = bitcnt - 7;
+                       return (ISC_R_SUCCESS);
                }
                break;
        }
-       INSIST(0);
-       ISC_UNREACHABLE();
+       return (ISC_R_RANGE);
 }
 
 CK_ATTRIBUTE *