From 86add9f57ef10773f129cef6a5802967304f39a0 Mon Sep 17 00:00:00 2001 From: "wtchang%redhat.com" Date: Thu, 2 Mar 2006 00:07:08 +0000 Subject: [PATCH] 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 | 18 +++- security/nss/lib/cryptohi/seckey.c | 146 ++++++++++++++++++++++++++++ security/nss/lib/cryptohi/secsign.c | 19 +++- security/nss/lib/freebl/ec.c | 24 +++-- security/nss/lib/pk11wrap/pk11obj.c | 9 +- security/nss/lib/pk11wrap/pk11pub.h | 8 ++ security/nss/lib/ssl/ssl3con.c | 6 +- 7 files changed, 206 insertions(+), 24 deletions(-) diff --git a/security/nss/lib/cryptohi/keyhi.h b/security/nss/lib/cryptohi/keyhi.h index 0422ad2bb59..225eb84858d 100644 --- a/security/nss/lib/cryptohi/keyhi.h +++ b/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_ */ diff --git a/security/nss/lib/cryptohi/seckey.c b/security/nss/lib/cryptohi/seckey.c index 99fc851cf24..16bf162f29b 100644 --- a/security/nss/lib/cryptohi/seckey.c +++ b/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) diff --git a/security/nss/lib/cryptohi/secsign.c b/security/nss/lib/cryptohi/secsign.c index b77c1e667d0..358e9025836 100644 --- a/security/nss/lib/cryptohi/secsign.c +++ b/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 #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); diff --git a/security/nss/lib/freebl/ec.c b/security/nss/lib/freebl/ec.c index 469e1fc5467..e083ffe1851 100644 --- a/security/nss/lib/freebl/ec.c +++ b/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 - olen = flen; -#endif + if (signature->data == NULL) { + /* a call to get the signature length only */ + 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; diff --git a/security/nss/lib/pk11wrap/pk11obj.c b/security/nss/lib/pk11wrap/pk11obj.c index 65b47ef96f6..07536003cb1 100644 --- a/security/nss/lib/pk11wrap/pk11obj.c +++ b/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(¶ms); + length = SECKEY_ECParamsToBasePointOrderLen(¶ms); 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: diff --git a/security/nss/lib/pk11wrap/pk11pub.h b/security/nss/lib/pk11wrap/pk11pub.h index 8a933cc9308..4c674d48ed6 100644 --- a/security/nss/lib/pk11wrap/pk11pub.h +++ b/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); diff --git a/security/nss/lib/ssl/ssl3con.c b/security/nss/lib/ssl/ssl3con.c index 50f1f07e69c..107535b8ebd 100644 --- a/security/nss/lib/ssl/ssl3con.c +++ b/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. */