Bug 1138554 - Raising minimum key size on DH and RSA to 1023, r=wtc

2 files changed, 60 insertions, 40 deletions

diff --git a/lib/cryptohi/keyhi.h b/lib/cryptohi/keyhi.h
index 88a77f15c..411ea00e3 100644
--- a/lib/cryptohi/keyhi.h
+++ b/lib/cryptohi/keyhi.h
@@ -37,6 +37,11 @@ extern SECStatus SECKEY_CopySubjectPublicKeyInfo(PLArenaPool *arena,
 extern SECStatus
 SECKEY_UpdateCertPQG(CERTCertificate * subjectCert);
 
+/*
+** Return the number of bits in the provided big integer.  This assumes that the
+** SECItem contains a big-endian number and counts from the first non-zero bit.
+*/
+extern unsigned SECKEY_BigIntegerBitLength(const SECItem *number);
 
 /*
 ** Return the strength of the public key in bytes
diff --git a/lib/cryptohi/seckey.c b/lib/cryptohi/seckey.c
index 16d2a499b..1eb0a7c4d 100644
--- a/lib/cryptohi/seckey.c
+++ b/lib/cryptohi/seckey.c
@@ -178,8 +178,8 @@ SECKEY_CreateDHPrivateKey(SECKEYDHParams *param, SECKEYPublicKey **pubk, void *c
     PK11SlotInfo *slot;
 
     if (!param || !param->base.data || !param->prime.data ||
-        param->prime.len < 512/8 || param->base.len == 0 || 
-        param->base.len > param->prime.len + 1 || 
+        SECKEY_BigIntegerBitLength(&param->prime) < DH_MIN_P_BITS ||
+        param->base.len == 0 || param->base.len > param->prime.len + 1 ||
 	(param->base.len == 1 && param->base.data[0] == 0)) {
 	PORT_SetError(SEC_ERROR_INVALID_ARGS);
 	return NULL;
@@ -941,61 +941,76 @@ SECKEY_ECParamsToBasePointOrderLen(const SECItem *encodedParams)
     }
 }
 
-/* returns key strength in bytes (not bits) */
+/* The number of bits in the number from the first non-zero bit onward. */
 unsigned
-SECKEY_PublicKeyStrength(const SECKEYPublicKey *pubk)
+SECKEY_BigIntegerBitLength(const SECItem *number)
 {
-    unsigned char b0;
-    unsigned size;
+    const unsigned char *p;
+    unsigned octets;
+    unsigned bits;
 
-    /* interpret modulus length as key strength */
-    if (!pubk)
-    	goto loser;
-    switch (pubk->keyType) {
-    case rsaKey:
-	if (!pubk->u.rsa.modulus.data) break;
-    	b0 = pubk->u.rsa.modulus.data[0];
-    	return b0 ? pubk->u.rsa.modulus.len : pubk->u.rsa.modulus.len - 1;
-    case dsaKey:
-	if (!pubk->u.dsa.publicValue.data) break;
-    	b0 = pubk->u.dsa.publicValue.data[0];
-    	return b0 ? pubk->u.dsa.publicValue.len :
-	    pubk->u.dsa.publicValue.len - 1;
-    case dhKey:
-	if (!pubk->u.dh.publicValue.data) break;
-    	b0 = pubk->u.dh.publicValue.data[0];
-    	return b0 ? pubk->u.dh.publicValue.len :
-	    pubk->u.dh.publicValue.len - 1;
-    case ecKey:
-	/* Get the key size in bits and adjust */
-	size =	SECKEY_ECParamsToKeySize(&pubk->u.ec.DEREncodedParams);
-	return (size + 7)/8;
-    default:
-	break;
+    if (!number || !number->data) {
+        PORT_SetError(SEC_ERROR_INVALID_KEY);
+        return 0;
     }
-loser:
-    PORT_SetError(SEC_ERROR_INVALID_KEY);
-    return 0;
+
+    p = number->data;
+    octets = number->len;
+    while (octets > 0 && !*p) {
+        ++p;
+        --octets;
+    }
+    if (octets == 0) {
+        return 0;
+    }
+    /* bits = 7..1 because we know at least one bit is set already */
+    /* Note: This could do a binary search, but this is faster for keys if we
+     * assume that good keys will have the MSB set. */
+    for (bits = 7; bits > 0; --bits) {
+        if (*p & (1 << bits)) {
+            break;
+        }
+    }
+    return octets * 8 + bits - 7;
+}
+
+/* returns key strength in bytes (not bits) */
+unsigned
+SECKEY_PublicKeyStrength(const SECKEYPublicKey *pubk)
+{
+    return (SECKEY_PublicKeyStrengthInBits(pubk) + 7) / 8;
 }
 
 /* returns key strength in bits */
 unsigned
 SECKEY_PublicKeyStrengthInBits(const SECKEYPublicKey *pubk)
 {
-    unsigned size;
+    unsigned bitSize = 0;
+
+    if (!pubk) {
+        PORT_SetError(SEC_ERROR_INVALID_KEY);
+        return 0;
+    }
+
+    /* interpret modulus length as key strength */
     switch (pubk->keyType) {
     case rsaKey:
+        bitSize = SECKEY_BigIntegerBitLength(&pubk->u.rsa.modulus);
+        break;
     case dsaKey:
+        bitSize = SECKEY_BigIntegerBitLength(&pubk->u.dsa.publicValue);
+        break;
     case dhKey:
-	return SECKEY_PublicKeyStrength(pubk) * 8; /* 1 byte = 8 bits */
+        bitSize = SECKEY_BigIntegerBitLength(&pubk->u.dh.publicValue);
+        break;
     case ecKey:
-	size = SECKEY_ECParamsToKeySize(&pubk->u.ec.DEREncodedParams);
-	return size;
+        bitSize = SECKEY_ECParamsToKeySize(&pubk->u.ec.DEREncodedParams);
+        break;
     default:
-	break;
+        PORT_SetError(SEC_ERROR_INVALID_KEY);
+        break;
     }
-    PORT_SetError(SEC_ERROR_INVALID_KEY);
-    return 0;
+    return bitSize;
 }
 
 /* returns signature length in bytes (not bits) */