Bug 1215779 - Remove broken (non-EC) DSA keygen code. r=keeler
authorCykesiopka <cykesiopka.bmo@gmail.com>
Mon, 19 Oct 2015 22:54:00 +0200
changeset 303678 fb1068402c0151176ad4000d8044cd7f8eb617df
parent 303677 780061f96448c9ec4a716532f552c323bd6ef8da
child 303679 8dc73ca2ec858a58237c5a4ec66244fa822d9cfc
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1215779
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1215779 - Remove broken (non-EC) DSA keygen code. r=keeler
config/external/nss/nss.def
security/manager/ssl/nsKeygenHandler.cpp
--- a/config/external/nss/nss.def
+++ b/config/external/nss/nss.def
@@ -398,19 +398,16 @@ PK11_Logout
 PK11_LogoutAll
 PK11_MakeIDFromPubKey
 PK11_MechanismToAlgtag
 PK11_MergeTokens
 PK11_NeedLogin
 PK11_NeedUserInit
 PK11_ParamFromIV
 PK11_PBEKeyGen
-PK11_PQG_DestroyParams
-PK11_PQG_DestroyVerify
-PK11_PQG_ParamGen
 PK11_PrivDecrypt
 PK11_PrivDecryptPKCS1
 PK11_ProtectedAuthenticationPath
 PK11_PubDeriveWithKDF
 PK11_PubEncrypt
 PK11_PubEncryptPKCS1
 PK11_PubUnwrapSymKey
 PK11_PubWrapSymKey
@@ -680,9 +677,8 @@ VFY_CreateContext
 VFY_DestroyContext
 VFY_End
 VFY_EndWithSignature
 VFY_Update
 VFY_VerifyData
 VFY_VerifyDataWithAlgorithmID
 VFY_VerifyDigestDirect
 _SGN_VerifyPKCS1DigestInfo
-PK11_PQG_ParamGenV2
--- a/security/manager/ssl/nsKeygenHandler.cpp
+++ b/security/manager/ssl/nsKeygenHandler.cpp
@@ -7,17 +7,16 @@
 #include "secdert.h"
 #include "nspr.h"
 #include "nsNSSComponent.h" // for PIPNSS string bundle calls.
 #include "keyhi.h"
 #include "secder.h"
 #include "cryptohi.h"
 #include "base64.h"
 #include "secasn1.h"
-#include "pk11pqg.h"
 #include "nsKeygenHandler.h"
 #include "nsKeygenHandlerContent.h"
 #include "nsIServiceManager.h"
 #include "nsIDOMHTMLSelectElement.h"
 #include "nsIContent.h"
 #include "nsKeygenThread.h"
 #include "nsReadableUtils.h"
 #include "nsUnicharUtils.h"
@@ -27,17 +26,16 @@
 #include "nsNSSShutDown.h"
 #include "nsXULAppAPI.h"
 
 #include "mozilla/Telemetry.h"
 
 //These defines are taken from the PKCS#11 spec
 #define CKM_RSA_PKCS_KEY_PAIR_GEN     0x00000000
 #define CKM_DH_PKCS_KEY_PAIR_GEN      0x00000020
-#define CKM_DSA_KEY_PAIR_GEN          0x00000010
 
 DERTemplate SECAlgorithmIDTemplate[] = {
     { DER_SEQUENCE,
           0, nullptr, sizeof(SECAlgorithmID) },
     { DER_OBJECT_ID,
           offsetof(SECAlgorithmID,algorithm), },
     { DER_OPTIONAL | DER_ANY,
           offsetof(SECAlgorithmID,parameters), },
@@ -58,82 +56,16 @@ DERTemplate CERTSubjectPublicKeyInfoTemp
 DERTemplate CERTPublicKeyAndChallengeTemplate[] =
 {
     { DER_SEQUENCE, 0, nullptr, sizeof(CERTPublicKeyAndChallenge) },
     { DER_ANY, offsetof(CERTPublicKeyAndChallenge,spki), },
     { DER_IA5_STRING, offsetof(CERTPublicKeyAndChallenge,challenge), },
     { 0, }
 };
 
-const SEC_ASN1Template SECKEY_PQGParamsTemplate[] = {
-    { SEC_ASN1_SEQUENCE, 0, nullptr, sizeof(PQGParams) },
-    { SEC_ASN1_INTEGER, offsetof(PQGParams,prime) },
-    { SEC_ASN1_INTEGER, offsetof(PQGParams,subPrime) },
-    { SEC_ASN1_INTEGER, offsetof(PQGParams,base) },
-    { 0, }
-};
-
-static PQGParams *
-decode_pqg_params(char *aStr)
-{
-    unsigned char *buf = nullptr;
-    unsigned int len;
-    PLArenaPool *arena = nullptr;
-    PQGParams *params = nullptr;
-    SECStatus status;
-
-    arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
-    if (!arena)
-        return nullptr;
-
-    params = static_cast<PQGParams*>(PORT_ArenaZAlloc(arena, sizeof(PQGParams)));
-    if (!params)
-        goto loser;
-    params->arena = arena;
-
-    buf = ATOB_AsciiToData(aStr, &len);
-    if ((!buf) || (len == 0))
-        goto loser;
-
-    status = SEC_ASN1Decode(arena, params, SECKEY_PQGParamsTemplate, (const char*)buf, len);
-    if (status != SECSuccess)
-        goto loser;
-
-    return params;
-
-loser:
-    if (arena) {
-      PORT_FreeArena(arena, false);
-    }
-    if (buf) {
-      PR_Free(buf);
-    }
-    return nullptr;
-}
-
-static int
-pqg_prime_bits(char *str)
-{
-    PQGParams *params = nullptr;
-    int primeBits = 0, i;
-
-    params = decode_pqg_params(str);
-    if (!params)
-        goto done; /* lose */
-
-    for (i = 0; params->prime.data[i] == 0; i++)
-        /* empty */;
-    primeBits = (params->prime.len - i) * 8;
-
-done:
-    if (params)
-        PK11_PQG_DestroyParams(params);
-    return primeBits;
-}
-
 typedef struct curveNameTagPairStr {
     const char *curveName;
     SECOidTag curveOidTag;
 } CurveNameTagPair;
 
 static CurveNameTagPair nameTagPair[] =
 { 
   { "prime192v1", SEC_OID_ANSIX962_EC_PRIME192V1 },
@@ -320,25 +252,22 @@ nsKeygenFormProcessor::GetSlot(uint32_t 
 
 uint32_t MapGenMechToAlgoMech(uint32_t mechanism)
 {
     uint32_t searchMech;
 
     /* We are interested in slots based on the ability to perform
        a given algorithm, not on their ability to generate keys usable
        by that algorithm. Therefore, map keygen-specific mechanism tags
-       to tags for the corresponding crypto algorthm. */
+       to tags for the corresponding crypto algorithm. */
     switch(mechanism)
     {
     case CKM_RSA_PKCS_KEY_PAIR_GEN:
         searchMech = CKM_RSA_PKCS;
         break;
-    case CKM_DSA_KEY_PAIR_GEN:
-        searchMech = CKM_DSA;
-        break;
     case CKM_RC4_KEY_GEN:
         searchMech = CKM_RC4;
         break;
     case CKM_DH_PKCS_KEY_PAIR_GEN:
         searchMech = CKM_DH_PKCS_DERIVE; /* ### mwelch  is this right? */
         break;
     case CKM_DES_KEY_GEN:
         /* What do we do about DES keygen? Right now, we're just using
@@ -504,35 +433,34 @@ GatherKeygenTelemetry(uint32_t keyGenMec
       } else if (secp256r1.EqualsIgnoreCase(curve, secp256r1.Length())) {
           mozilla::Telemetry::Accumulate(
               mozilla::Telemetry::KEYGEN_GENERATED_KEY_TYPE, secp256r1);
       } else {
         mozilla::Telemetry::Accumulate(
             mozilla::Telemetry::KEYGEN_GENERATED_KEY_TYPE, NS_LITERAL_CSTRING("other_ec"));
       }
     }
-  } else if (keyGenMechanism == CKM_DSA_KEY_PAIR_GEN) {
-    MOZ_CRASH("DSA key generation is currently unimplemented");
+  } else {
+    MOZ_CRASH("Unknown keygen algorithm");
     return;
   }
 }
 
 nsresult
 nsKeygenFormProcessor::GetPublicKey(const nsAString& aValue,
                                     const nsAString& aChallenge,
                                     const nsAFlatString& aKeyType,
                                     nsAString& aOutPublicKey,
                                     const nsAString& aKeyParams)
 {
     nsNSSShutDownPreventionLock locker;
     nsresult rv = NS_ERROR_FAILURE;
     char *keystring = nullptr;
-    char *keyparamsString = nullptr, *str = nullptr;
+    char *keyparamsString = nullptr;
     uint32_t keyGenMechanism;
-    int32_t primeBits;
     PK11SlotInfo *slot = nullptr;
     PK11RSAGenParams rsaParams;
     SECOidTag algTag;
     int keysize = 0;
     void *params = nullptr;
     SECKEYPrivateKey *privateKey = nullptr;
     SECKEYPublicKey *publicKey = nullptr;
     CERTSubjectPublicKeyInfo *spkInfo = nullptr;
@@ -541,17 +469,17 @@ nsKeygenFormProcessor::GetPublicKey(cons
     SECItem spkiItem;
     SECItem pkacItem;
     SECItem signedItem;
     CERTPublicKeyAndChallenge pkac;
     pkac.challenge.data = nullptr;
     nsIGeneratingKeypairInfoDialogs * dialogs;
     nsKeygenThread *KeygenRunnable = 0;
     nsCOMPtr<nsIKeygenThread> runnable;
-    
+
     // permanent and sensitive flags for keygen
     PK11AttrFlags attrFlags = PK11_ATTR_TOKEN | PK11_ATTR_SENSITIVE | PK11_ATTR_PRIVATE;
 
     // Get the key size //
     for (size_t i = 0; i < number_of_key_size_choices; ++i) {
         if (aValue.Equals(mSECKeySizeChoiceList[i].name)) {
             keysize = mSECKeySizeChoiceList[i].size;
             break;
@@ -564,43 +492,16 @@ nsKeygenFormProcessor::GetPublicKey(cons
     arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
     if (!arena) {
         goto loser;
     }
 
     // Set the keygen mechanism
     if (aKeyType.IsEmpty() || aKeyType.LowerCaseEqualsLiteral("rsa")) {
         keyGenMechanism = CKM_RSA_PKCS_KEY_PAIR_GEN;
-    } else if (aKeyType.LowerCaseEqualsLiteral("dsa")) {
-        char * end;
-        keyparamsString = ToNewCString(aKeyParams);
-        if (!keyparamsString) {
-            rv = NS_ERROR_OUT_OF_MEMORY;
-            goto loser;
-        }
-
-        keyGenMechanism = CKM_DSA_KEY_PAIR_GEN;
-        if (strcmp(keyparamsString, "null") == 0)
-            goto loser;
-        str = keyparamsString;
-        bool found_match = false;
-        do {
-            end = strchr(str, ',');
-            if (end)
-                *end = '\0';
-            primeBits = pqg_prime_bits(str);
-            if (keysize == primeBits) {
-                found_match = true;
-                break;
-            }
-            str = end + 1;
-        } while (end);
-        if (!found_match) {
-            goto loser;
-        }
     } else if (aKeyType.LowerCaseEqualsLiteral("ec")) {
         keyparamsString = ToNewCString(aKeyParams);
         if (!keyparamsString) {
             rv = NS_ERROR_OUT_OF_MEMORY;
             goto loser;
         }
 
         keyGenMechanism = CKM_EC_KEY_PAIR_GEN;
@@ -616,19 +517,16 @@ nsKeygenFormProcessor::GetPublicKey(cons
     }
     switch (keyGenMechanism) {
         case CKM_RSA_PKCS_KEY_PAIR_GEN:
             rsaParams.keySizeInBits = keysize;
             rsaParams.pe = DEFAULT_RSA_KEYGEN_PE;
             algTag = DEFAULT_RSA_KEYGEN_ALG;
             params = &rsaParams;
             break;
-        case CKM_DSA_KEY_PAIR_GEN:
-            // XXX Fix this! XXX //
-            goto loser;
         case CKM_EC_KEY_PAIR_GEN:
             /* XXX We ought to rethink how the KEYGEN tag is 
              * displayed. The pulldown selections presented
              * to the user must depend on the keytype.
              * The displayed selection could be picked
              * from the keyparams attribute (this is currently called
              * the pqg attribute).
              * For now, we pick ecparams from the keyparams field