Bug 1820175 - Fix various compiler warnings in NSS. r=jschanck.

Differential Revision: https://phabricator.services.mozilla.com/D171581

8 files changed, 54 insertions, 122 deletions

diff --git a/cmd/addbuiltin/addbuiltin.c b/cmd/addbuiltin/addbuiltin.c
index 23fd1fec8..da2533144 100644
--- a/cmd/addbuiltin/addbuiltin.c
+++ b/cmd/addbuiltin/addbuiltin.c
@@ -74,7 +74,6 @@ getTrustString(unsigned int trust)
             return "CKT_NSS_MUST_VERIFY_TRUST";
         }
     }
-    return "CKT_NSS_TRUST_UNKNOWN"; /* not reached */
 }
 
 static const SEC_ASN1Template serialTemplate[] = {
diff --git a/cmd/bltest/blapitest.c b/cmd/bltest/blapitest.c
index de7f0d9c7..f9a2f0221 100644
--- a/cmd/bltest/blapitest.c
+++ b/cmd/bltest/blapitest.c
@@ -2319,9 +2319,9 @@ cipherInit(bltestCipherInfo *cipherInfo, PRBool encrypt)
         case bltestRC5_CBC:
             SECITEM_AllocItem(cipherInfo->arena, &cipherInfo->output.buf,
                               cipherInfo->input.pBuf.len);
-#endif
             return bltest_rc5_init(cipherInfo, encrypt);
             break;
+#endif
         case bltestAES_ECB:
         case bltestAES_CBC:
         case bltestAES_CTS:
diff --git a/cmd/signtool/util.c b/cmd/signtool/util.c
index ecd22e39c..a2637a74e 100644
--- a/cmd/signtool/util.c
+++ b/cmd/signtool/util.c
@@ -430,51 +430,9 @@ out_of_memory(void)
 void
 VerifyCertDir(char *dir, char *keyName)
 {
-    char fn[FNSIZE];
-
-    /* don't try verifying if we don't have a local directory */
-    if (strncmp(dir, "multiaccess:", sizeof("multiaccess:") - 1) == 0) {
-        return;
-    }
     /* this function is truly evil. Tools and applications should not have
      * any knowledge of actual cert databases! */
     return;
-
-    /* This code is really broken because it makes underlying assumptions about
-     * how the NSS profile directory is laid out, but these names can change
-     * from release to release. */
-    sprintf(fn, "%s/cert8.db", dir);
-
-    if (PR_Access(fn, PR_ACCESS_EXISTS)) {
-        PR_fprintf(errorFD, "%s: No certificate database in \"%s\"\n",
-                   PROGRAM_NAME, dir);
-        PR_fprintf(errorFD, "%s: Check the -d arguments that you gave\n",
-                   PROGRAM_NAME);
-        errorCount++;
-        exit(ERRX);
-    }
-
-    if (verbosity >= 0) {
-        PR_fprintf(outputFD, "using certificate directory: %s\n", dir);
-    }
-
-    if (keyName == NULL)
-        return;
-
-    /* if the user gave the -k key argument, verify that
-     a key database already exists */
-
-    sprintf(fn, "%s/key3.db", dir);
-
-    if (PR_Access(fn, PR_ACCESS_EXISTS)) {
-        PR_fprintf(errorFD, "%s: No private key database in \"%s\"\n",
-                   PROGRAM_NAME,
-                   dir);
-        PR_fprintf(errorFD, "%s: Check the -d arguments that you gave\n",
-                   PROGRAM_NAME);
-        errorCount++;
-        exit(ERRX);
-    }
 }
 
 /*
diff --git a/gtests/freebl_gtest/ecl_unittest.cc b/gtests/freebl_gtest/ecl_unittest.cc
index ae53284eb..f3f8ddc74 100644
--- a/gtests/freebl_gtest/ecl_unittest.cc
+++ b/gtests/freebl_gtest/ecl_unittest.cc
@@ -14,7 +14,7 @@ namespace nss_test {
 
 class ECLTest : public ::testing::Test {
  protected:
-  const ECCurveName GetCurveName(std::string name) {
+  ECCurveName GetCurveName(std::string name) {
     if (name == "P256") return ECCurve_NIST_P256;
     if (name == "P384") return ECCurve_NIST_P384;
     if (name == "P521") return ECCurve_NIST_P521;
diff --git a/gtests/nss_bogo_shim/nss_bogo_shim.cc b/gtests/nss_bogo_shim/nss_bogo_shim.cc
index 52a3e9a94..133af518b 100644
--- a/gtests/nss_bogo_shim/nss_bogo_shim.cc
+++ b/gtests/nss_bogo_shim/nss_bogo_shim.cc
@@ -502,7 +502,6 @@ class TestAgent {
         return SECFailure;
       }
     }
-    return SECSuccess;
   }
 
   // Write bytes to the other side then read them back and check
diff --git a/gtests/ssl_gtest/tls_subcerts_unittest.cc b/gtests/ssl_gtest/tls_subcerts_unittest.cc
index 77bb41a0b..3bb297ffc 100644
--- a/gtests/ssl_gtest/tls_subcerts_unittest.cc
+++ b/gtests/ssl_gtest/tls_subcerts_unittest.cc
@@ -353,33 +353,12 @@ static void GenerateWeakRsaKey(ScopedSECKEYPrivateKey& priv,
   rsaparams.keySizeInBits = 528;
   rsaparams.pe = 65537;
 
-  // Bug 1012786: PK11_GenerateKeyPair can fail if there is insufficient
-  // entropy to generate a random key. We can fake some.
-  for (int retry = 0; retry < 10; ++retry) {
-    SECKEYPublicKey* p_pub = nullptr;
-    priv.reset(PK11_GenerateKeyPair(slot.get(), CKM_RSA_PKCS_KEY_PAIR_GEN,
-                                    &rsaparams, &p_pub, false, false, nullptr));
-    pub.reset(p_pub);
-    if (priv) {
-      return;
-    }
-
-    ASSERT_FALSE(pub);
-    if (PORT_GetError() != SEC_ERROR_PKCS11_FUNCTION_FAILED) {
-      break;
-    }
-
-    // https://xkcd.com/221/
-    static const uint8_t FRESH_ENTROPY[16] = {4};
-    ASSERT_EQ(
-        SECSuccess,
-        PK11_RandomUpdate(
-            const_cast<void*>(reinterpret_cast<const void*>(FRESH_ENTROPY)),
-            sizeof(FRESH_ENTROPY)));
-    break;
-  }
-  ADD_FAILURE() << "Unable to generate an RSA key: "
-                << PORT_ErrorToName(PORT_GetError());
+  SECKEYPublicKey* p_pub = nullptr;
+  priv.reset(PK11_GenerateKeyPair(slot.get(), CKM_RSA_PKCS_KEY_PAIR_GEN,
+                                  &rsaparams, &p_pub, false, false, nullptr));
+  pub.reset(p_pub);
+  PR_ASSERT(priv);
+  return;
 }
 
 // Fail to connect with a weak RSA key.
diff --git a/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c b/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c
index 1b5d7577e..dd32334ab 100644
--- a/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c
+++ b/lib/libpkix/pkix_pl_nss/module/pkix_pl_ldapcertstore.c
@@ -309,7 +309,7 @@ pkix_pl_LdapCertStore_BuildCrlList(
                             }
                             /* pkix crl own the der. */
                             PKIX_CHECK(
-                                pkix_pl_CRL_CreateWithSignedCRL(nssCrl, 
+                                pkix_pl_CRL_CreateWithSignedCRL(nssCrl,
                                        derCrlCopy, NULL, &crl, plContext),
                                 PKIX_CRLCREATEWITHSIGNEDCRLFAILED);
                             /* Left control over memory pointed by derCrlCopy and
@@ -427,7 +427,7 @@ pkix_pl_LdapCertStore_DestroyAVAList(
 static PKIX_Error *
 pkix_pl_LdapCertStore_MakeNameAVAList(
         PLArenaPool *arena,
-        PKIX_PL_X500Name *subjectName, 
+        PKIX_PL_X500Name *subjectName,
         LDAPNameComponent ***pList,
         void *plContext)
 {
@@ -874,55 +874,52 @@ pkix_pl_LdapCertStore_GetCRL(
                         PKIX_LISTGETLENGTHFAILED);
 
                 if (numNames > 0) {
-                        for (thisName = 0; thisName < numNames; thisName++) {
-                                PKIX_CHECK(PKIX_List_GetItem
-                                (issuerNames,
-                                thisName,
-                                (PKIX_PL_Object **)&issuer,
+                        /*
+                                * LDAP Servers don't seem to be able to handle
+                                * requests with more than more than one name.
+                                * So only use first name.
+                                */
+                        PKIX_CHECK(PKIX_List_GetItem
+                        (issuerNames,
+                        thisName,
+                        (PKIX_PL_Object **)&issuer,
+                        plContext),
+                        PKIX_LISTGETITEMFAILED);
+
+                        PKIX_CHECK
+                                (pkix_pl_LdapCertStore_MakeNameAVAList
+                                (requestArena,
+                                issuer,
+                                &(requestParams.nc),
                                 plContext),
-                                PKIX_LISTGETITEMFAILED);
-
-                                PKIX_CHECK
-                                        (pkix_pl_LdapCertStore_MakeNameAVAList
-                                        (requestArena,
-                                        issuer,
-                                        &(requestParams.nc),
-                                        plContext),
-                                        PKIX_LDAPCERTSTOREMAKENAMEAVALISTFAILED);
-
-                                PKIX_DECREF(issuer);
-
-                                if (*requestParams.nc == NULL) {
-                                        /*
-                                         * The issuer may not include any
-                                         * components that we know how to
-                                         * encode. We do not return an error,
-                                         * because the caller did not
-                                         * necessarily do anything wrong, but
-                                         * we return an empty List.
-                                         */
-                                        PKIX_PL_NSSCALL
-                                                (CERTSTORE, PORT_FreeArena,
-                                                (requestArena, PR_FALSE));
-
-                                        PKIX_CHECK(PKIX_List_Create
-                                                (&filteredCRLs, plContext),
-                                                PKIX_LISTCREATEFAILED);
-
-                                        PKIX_CHECK(PKIX_List_SetImmutable
-                                                (filteredCRLs, plContext),
-                                               PKIX_LISTSETIMMUTABLEFAILED);
-
-                                        *pNBIOContext = NULL;
-                                        *pCrlList = filteredCRLs;
-                                        goto cleanup;
-                                }
+                                PKIX_LDAPCERTSTOREMAKENAMEAVALISTFAILED);
+
+                        PKIX_DECREF(issuer);
 
+                        if (*requestParams.nc == NULL) {
                                 /*
-                                 * LDAP Servers don't seem to be able to handle
-                                 * requests with more than more than one name.
-                                 */
-                                break;
+                                        * The issuer may not include any
+                                        * components that we know how to
+                                        * encode. We do not return an error,
+                                        * because the caller did not
+                                        * necessarily do anything wrong, but
+                                        * we return an empty List.
+                                        */
+                                PKIX_PL_NSSCALL
+                                        (CERTSTORE, PORT_FreeArena,
+                                        (requestArena, PR_FALSE));
+
+                                PKIX_CHECK(PKIX_List_Create
+                                        (&filteredCRLs, plContext),
+                                        PKIX_LISTCREATEFAILED);
+
+                                PKIX_CHECK(PKIX_List_SetImmutable
+                                        (filteredCRLs, plContext),
+                                        PKIX_LISTSETIMMUTABLEFAILED);
+
+                                *pNBIOContext = NULL;
+                                *pCrlList = filteredCRLs;
+                                goto cleanup;
                         }
                 } else {
                         PKIX_ERROR(PKIX_IMPOSSIBLECRITERIONFORCRLQUERY);
diff --git a/lib/ssl/ssl3ext.c b/lib/ssl/ssl3ext.c
index 0bc7b955b..18ff39ac9 100644
--- a/lib/ssl/ssl3ext.c
+++ b/lib/ssl/ssl3ext.c
@@ -1138,7 +1138,7 @@ tls_ClientHelloExtensionPermutationSetup(sslSocket *ss)
     PR_ASSERT(!ss->ssl3.hs.chExtensionPermutation);
 
     /* This shuffle handles up to 256 extensions. */
-    PR_ASSERT(buildersLen < 256);
+    PR_STATIC_ASSERT(buildersLen < 256);
     uint8_t permutation[256] = { 0 };
 
     sslExtensionBuilder *builders = PORT_ZAlloc(buildersSize);