Bugzilla Bug 320589: fixed PK11_SignatureLen to return the exact length of

ECDSA signatures.  Backed out a temporary workaround in
ECDSA_SignDigestWithSeed.  Made other changes related to signature lengths.
r=relyea,nelson.bolyard.
Modified Files:
	cryptohi/keyhi.h cryptohi/seckey.c cryptohi/secsign.c
	freebl/ec.c pk11wrap/pk11obj.c pk11wrap/pk11pub.h
	ssl/ssl3con.c

security/nss/lib/cryptohi/keyhi.h

@@ -35,7 +35,7 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-/* $Id: keyhi.h,v 1.14 2006-02-08 06:14:07 rrelyea%redhat.com Exp $ */
+/* $Id: keyhi.h,v 1.15 2006-03-02 00:06:58 wtchang%redhat.com Exp $ */
 
 #ifndef _KEYHI_H_
 #define _KEYHI_H_
@@ -283,8 +283,24 @@ SECKEY_AddPublicKeyToListTail( SECKEYPublicKeyList *list,
 #define PUBKEY_LIST_NEXT(n) ((SECKEYPublicKeyListNode *)n->links.next)
 #define PUBKEY_LIST_END(n,l) (((void *)n) == ((void *)&l->list))
 
+/*
+ * Length in bits of the EC's field size.  This is also the length of
+ * the x and y coordinates of EC points, such as EC public keys and
+ * base points.
+ *
+ * Return 0 on failure (unknown EC domain parameters).
+ */
 extern int SECKEY_ECParamsToKeySize(const SECItem *params);
 
+/*
+ * Length in bits of the EC base point order, usually denoted n.  This
+ * is also the length of EC private keys and ECDSA signature components
+ * r and s.
+ *
+ * Return 0 on failure (unknown EC domain parameters).
+ */
+extern int SECKEY_ECParamsToBasePointOrderLen(const SECItem *params);
+
 SEC_END_PROTOS
 
 #endif /* _KEYHI_H_ */

security/nss/lib/cryptohi/seckey.c

@@ -1294,6 +1294,152 @@ SECKEY_ECParamsToKeySize(const SECItem *encodedParams)
     }
 }
 
+int
+SECKEY_ECParamsToBasePointOrderLen(const SECItem *encodedParams)
+{
+    SECOidTag tag;
+    SECItem oid = { siBuffer, NULL, 0};
+	
+    /* The encodedParams data contains 0x06 (SEC_ASN1_OBJECT_ID),
+     * followed by the length of the curve oid and the curve oid.
+     */
+    oid.len = encodedParams->data[1];
+    oid.data = encodedParams->data + 2;
+    if ((tag = SECOID_FindOIDTag(&oid)) == SEC_OID_UNKNOWN)
+	return 0;
+
+    switch (tag) {
+    case SEC_OID_SECG_EC_SECP112R1:
+        return 112;
+    case SEC_OID_SECG_EC_SECP112R2:
+        return 110;
+
+    case SEC_OID_SECG_EC_SECT113R1:
+    case SEC_OID_SECG_EC_SECT113R2:
+	return 113;
+
+    case SEC_OID_SECG_EC_SECP128R1:
+	return 128;
+    case SEC_OID_SECG_EC_SECP128R2:
+	return 126;
+
+    case SEC_OID_SECG_EC_SECT131R1:
+    case SEC_OID_SECG_EC_SECT131R2:
+	return 131;
+
+    case SEC_OID_SECG_EC_SECP160K1:
+    case SEC_OID_SECG_EC_SECP160R1:
+    case SEC_OID_SECG_EC_SECP160R2:
+	return 161;
+
+    case SEC_OID_SECG_EC_SECT163K1:
+	return 163;
+    case SEC_OID_SECG_EC_SECT163R1:
+	return 162;
+    case SEC_OID_SECG_EC_SECT163R2:
+    case SEC_OID_ANSIX962_EC_C2PNB163V1:
+	return 163;
+    case SEC_OID_ANSIX962_EC_C2PNB163V2:
+    case SEC_OID_ANSIX962_EC_C2PNB163V3:
+	return 162;
+
+    case SEC_OID_ANSIX962_EC_C2PNB176V1:
+	return 161;
+
+    case SEC_OID_ANSIX962_EC_C2TNB191V1:
+	return 191;
+    case SEC_OID_ANSIX962_EC_C2TNB191V2:
+	return 190;
+    case SEC_OID_ANSIX962_EC_C2TNB191V3:
+	return 189;
+    case SEC_OID_ANSIX962_EC_C2ONB191V4:
+	return 191;
+    case SEC_OID_ANSIX962_EC_C2ONB191V5:
+	return 188;
+
+    case SEC_OID_SECG_EC_SECP192K1:
+    case SEC_OID_ANSIX962_EC_PRIME192V1:
+    case SEC_OID_ANSIX962_EC_PRIME192V2:
+    case SEC_OID_ANSIX962_EC_PRIME192V3:
+	return 192;
+
+    case SEC_OID_SECG_EC_SECT193R1:
+    case SEC_OID_SECG_EC_SECT193R2:
+	return 193;
+
+    case SEC_OID_ANSIX962_EC_C2PNB208W1:
+	return 193;
+
+    case SEC_OID_SECG_EC_SECP224K1:
+	return 225;
+    case SEC_OID_SECG_EC_SECP224R1:
+	return 224;
+
+    case SEC_OID_SECG_EC_SECT233K1:
+	return 232;
+    case SEC_OID_SECG_EC_SECT233R1:
+	return 233;
+
+    case SEC_OID_SECG_EC_SECT239K1:
+    case SEC_OID_ANSIX962_EC_C2TNB239V1:
+	return 238;
+    case SEC_OID_ANSIX962_EC_C2TNB239V2:
+	return 237;
+    case SEC_OID_ANSIX962_EC_C2TNB239V3:
+	return 236;
+    case SEC_OID_ANSIX962_EC_C2ONB239V4:
+	return 238;
+    case SEC_OID_ANSIX962_EC_C2ONB239V5:
+	return 237;
+    case SEC_OID_ANSIX962_EC_PRIME239V1:
+    case SEC_OID_ANSIX962_EC_PRIME239V2:
+    case SEC_OID_ANSIX962_EC_PRIME239V3:
+	return 239;
+
+    case SEC_OID_SECG_EC_SECP256K1:
+    case SEC_OID_ANSIX962_EC_PRIME256V1:
+	return 256;
+
+    case SEC_OID_ANSIX962_EC_C2PNB272W1:
+	return 257;
+
+    case SEC_OID_SECG_EC_SECT283K1:
+	return 281;
+    case SEC_OID_SECG_EC_SECT283R1:
+	return 282;
+
+    case SEC_OID_ANSIX962_EC_C2PNB304W1:
+	return 289;
+
+    case SEC_OID_ANSIX962_EC_C2TNB359V1:
+	return 353;
+
+    case SEC_OID_ANSIX962_EC_C2PNB368W1:
+	return 353;
+
+    case SEC_OID_SECG_EC_SECP384R1:
+	return 384;
+
+    case SEC_OID_SECG_EC_SECT409K1:
+	return 407;
+    case SEC_OID_SECG_EC_SECT409R1:
+	return 409;
+
+    case SEC_OID_ANSIX962_EC_C2TNB431R1:
+	return 418;
+
+    case SEC_OID_SECG_EC_SECP521R1:
+	return 521;
+
+    case SEC_OID_SECG_EC_SECT571K1:
+    case SEC_OID_SECG_EC_SECT571R1:
+	return 570;
+
+    default:
+	    return 0;
+    }
+}
+
 /* returns key strength in bytes (not bits) */
 unsigned
 SECKEY_PublicKeyStrength(const SECKEYPublicKey *pubk)

security/nss/lib/cryptohi/secsign.c

@@ -37,7 +37,7 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-/* $Id: secsign.c,v 1.15 2006-02-08 06:14:07 rrelyea%redhat.com Exp $ */
+/* $Id: secsign.c,v 1.16 2006-03-02 00:06:58 wtchang%redhat.com Exp $ */
 
 #include <stdio.h>
 #include "cryptohi.h"
@@ -147,7 +147,8 @@ SECStatus
 SGN_End(SGNContext *cx, SECItem *result)
 {
     unsigned char digest[HASH_LENGTH_MAX];
-    unsigned part1, signatureLen;
+    unsigned part1;
+    int signatureLen;
     SECStatus rv;
     SECItem digder, sigitem;
     PRArenaPool *arena = 0;
@@ -195,6 +196,11 @@ SGN_End(SGNContext *cx, SECItem *result)
     ** block
     */
     signatureLen = PK11_SignatureLen(privKey);
+    if (signatureLen <= 0) {
+	PORT_SetError(SEC_ERROR_INVALID_KEY);
+	rv = SECFailure;
+	goto loser;
+    }
     sigitem.len = signatureLen;
     sigitem.data = (unsigned char*) PORT_Alloc(signatureLen);
 
@@ -213,7 +219,7 @@ SGN_End(SGNContext *cx, SECItem *result)
     if ((cx->signalg == SEC_OID_ANSIX9_DSA_SIGNATURE) ||
         (cx->signalg == SEC_OID_ANSIX962_EC_PUBLIC_KEY)) {
         /* DSAU_EncodeDerSigWithLen works for DSA and ECDSA */
-	rv = DSAU_EncodeDerSigWithLen(result, &sigitem, signatureLen); 
+	rv = DSAU_EncodeDerSigWithLen(result, &sigitem, sigitem.len); 
 	PORT_Free(sigitem.data);
 	if (rv != SECSuccess)
 	    goto loser;
@@ -354,7 +360,7 @@ SECStatus
 SGN_Digest(SECKEYPrivateKey *privKey,
 		SECOidTag algtag, SECItem *result, SECItem *digest)
 {
-    unsigned modulusLen;
+    int modulusLen;
     SECStatus rv;
     SECItem digder;
     PRArenaPool *arena = 0;
@@ -393,6 +399,11 @@ SGN_Digest(SECKEYPrivateKey *privKey,
     ** block
     */
     modulusLen = PK11_SignatureLen(privKey);
+    if (modulusLen <= 0) {
+	PORT_SetError(SEC_ERROR_INVALID_KEY);
+	rv = SECFailure;
+	goto loser;
+    }
     result->len = modulusLen;
     result->data = (unsigned char*) PORT_Alloc(modulusLen);
 

security/nss/lib/freebl/ec.c

@@ -638,15 +638,11 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature,
 
     ecParams = &(key->ecParams);
     flen = (ecParams->fieldID.size + 7) >> 3;
-    /*
-     * FIXME: temporary workaround until we change PK11_SignatureLen
-     * to use the length of the base point order.
-     */
-#if 0
     olen = ecParams->order.len;  
-#else
+    if (signature->data == NULL) {
-    olen = flen;  
+	/* a call to get the signature length only */
-#endif
+	goto finish;
+    }
     if (signature->len < 2*olen) {
 	PORT_SetError(SEC_ERROR_OUTPUT_LEN);
 	goto cleanup;
@@ -769,6 +765,7 @@ ECDSA_SignDigestWithSeed(ECPrivateKey *key, SECItem *signature,
     */
     CHECK_MPI_OK( mp_to_fixlen_octets(&r, signature->data, olen) );
     CHECK_MPI_OK( mp_to_fixlen_octets(&s, signature->data + olen, olen) );
+finish:
     signature->len = 2*olen;
 
     rv = SECSuccess;
@@ -906,14 +903,15 @@ ECDSA_VerifyDigest(ECPublicKey *key, const SECItem *signature,
 	goto cleanup;
     }
 
-    if (signature->len == 0 || signature->len%2 != 0) {
-	PORT_SetError(SEC_ERROR_INVALID_ARGS);
-	goto cleanup;
-    }
-    slen = signature->len/2;
     ecParams = &(key->ecParams);
     flen = (ecParams->fieldID.size + 7) >> 3;  
     olen = ecParams->order.len;  
+    if (signature->len == 0 || signature->len%2 != 0 ||
+	signature->len > 2*olen) {
+	PORT_SetError(SEC_ERROR_INPUT_LEN);
+	goto cleanup;
+    }
+    slen = signature->len/2;
 
     /* truncate digest to the length of the base point order */
     localDigest = *digest;

security/nss/lib/pk11wrap/pk11obj.c

@@ -571,11 +571,14 @@ PK11_SignatureLen(SECKEYPrivateKey *key)
 	    if (theTemplate.pValue != NULL) {
 	        params.len = theTemplate.ulValueLen;
 		params.data = (unsigned char *) theTemplate.pValue;
-	        length = SECKEY_ECParamsToKeySize(&params);
+	        length = SECKEY_ECParamsToBasePointOrderLen(&params);
 	        PORT_Free(theTemplate.pValue);
+		if (length == 0) {
+		    return pk11_backupGetSignLength(key);
+		}
+		length = ((length + 7)/8) * 2;
+		return length;
 	    }
-	    length = ((length + 7)/8) * 2;
-	    return length;
 	}
 	break;
     default:

security/nss/lib/pk11wrap/pk11pub.h

@@ -582,6 +582,14 @@ CERTSignedCrl* PK11_ImportCRL(PK11SlotInfo * slot, SECItem *derCRL, char *url,
 /**********************************************************************
  *                   Sign/Verify 
  **********************************************************************/
+
+/*
+ * Return the length in bytes of a signature generated with the
+ * private key.
+ *
+ * Return 0 or -1 on failure.  (XXX Should we fix it to always return
+ * -1 on failure?)
+ */
 int PK11_SignatureLen(SECKEYPrivateKey *key);
 PK11SlotInfo * PK11_GetSlotFromPrivateKey(SECKEYPrivateKey *key);
 SECStatus PK11_Sign(SECKEYPrivateKey *key, SECItem *sig, SECItem *hash);

security/nss/lib/ssl/ssl3con.c

@@ -39,7 +39,7 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
-/* $Id: ssl3con.c,v 1.80 2006-03-01 05:45:45 nelson%bolyard.com Exp $ */
+/* $Id: ssl3con.c,v 1.81 2006-03-02 00:07:08 wtchang%redhat.com Exp $ */
 
 #include "nssrenam.h"
 #include "cert.h"
@@ -765,7 +765,7 @@ ssl3_SignHashes(SSL3Hashes *hash, SECKEYPrivateKey *key, SECItem *buf,
     }
 
     buf->len  = (unsigned)signatureLen;
-    buf->data = (unsigned char *)PORT_Alloc(signatureLen + 1);
+    buf->data = (unsigned char *)PORT_Alloc(signatureLen);
     if (!buf->data)
         goto done;	/* error code was set. */
 
@@ -799,7 +799,7 @@ ssl3_SignHashes(SSL3Hashes *hash, SECKEYPrivateKey *key, SECItem *buf,
 	SECItem   derSig	= {siBuffer, NULL, 0};
 
 	/* This also works for an ECDSA signature */
-	rv = DSAU_EncodeDerSigWithLen(&derSig, buf, (unsigned) signatureLen);
+	rv = DSAU_EncodeDerSigWithLen(&derSig, buf, buf->len);
 	if (rv == SECSuccess) {
 	    PORT_Free(buf->data);	/* discard unencoded signature. */
 	    *buf = derSig;		/* give caller encoded signature. */