1. 12 May, 2021 1 commit
    • Robert Relyea's avatar
      Bug 1710773 NSS needs FIPS 180-3 FIPS indicators. r=mt · 595deb8f
      Robert Relyea authored
      Changes from the review:
      The while loop was taken out of it's subshell pipe, which prevented the selfserv PID from being passed on to the final selfserv-kill. This eventally lead to a freeze on windows.
      
      The last paragraph of ISO 19790:2012 section 7.2.4.2 states:
      
      All services shall [02.24] provide an indicator when the service utilises an approved cryptographic algorithm, security function or process in an approved manner and those services or processes specified in 7.4.3
      
      This means our libraries need to grow an API or provide some additional information via contexts or similar in order for an application to be able to query this indicator. This can't be just a Security Policy description because ISO 24759:2017 section 6.2.4.2 states:
      
      TE02.24.02: The tester shall execute all services and verify that the indicator provides an unambiguous indication of whether the service utilizes an approved cryptographic algorithm, security function or process in an approved manner or not.
      
      The indicator can't be just a marker over an algorithm either, because it needs to show different values based on whether the algorithm parameters causes the algorithm to run in approved or non-approved mode (ie keys outside of valid range for RSA means RSA is being used in non-approved mode ...)
      
      For NSS, there is a PKCS #11 design:
      https://docs.google.com/document/d/1Me9YksPE7K1Suvk9Ls5PqJXPpDmpAboLsrq0z54m_tA/edit?usp=sharing
      
      This patch implments the above design as well as:
         1) NSS proper functions to access these indicators from either the pk11wrap layer or the ssl layer.
         2) Updates to the ssl tests which will output the value of the
      
      Changes decription by file:
      cmd/selfserv/selfserv.c
         Add a FIPS indicator if the connection was excuted in FIPS mode on a FIPS token.
      cmd/strsclnt/strsclnt.c
         Add a FIPS indicator if the connection was excuted in FIPS mode on a FIPS token.
      cmd/tstclnt/tstclnt.c
         Add a FIPS indicator if the connection was excuted in FIPS mode on a FIPS token.
      lib/nss/nss.def
         Add the new pk11 functions to access the fips indicator.
      lib/pk11wrap/pk11cxt.c
         Implement a function to get the FIPS indicator for the current PK11Context.
      lib/pk11wrap/pk11load.c
         Get the fips indicator function from the PKCS #11 module using the vendor function interface from PKCS #11 v3.0
      lib/pk11wrap/pk11obj.c
         Implement a function to get the FIPS indicator for a specific PKCS #11 object.
      lib/pk11wrap/pk11priv.h
         Add a generalized helper function to get the FIPS indicator used by all the other exported functions to get FIPS indicator.
      lib/pk11wrap/pk11pub.h
         Add function to get the FIPS indicator for the current PK11Context.
      lib/pk11wrap/pk11slot.c
         Implement a generalized helper function to get the FIPS indicator.
         Implement a function to get the FIPS indicator for the latest single shot operation on the slot.
      lib/pk11wrap/secmodt.h
         Add a new field to hold the fipsIndicator function.
      lib/softoken/fips_algorithms.h
         New sample header which vendors can replace with their own table. In the default NSS case, the table in this header will be empty.
      lib/softoken/fipstokn.c
         Add Vendor specific interface for the FIPS indicator to the FIPS token.
      lib/softoken/pkcs11.c
         Add Vendor specific interface for the FIPS indicator to the non-FIPS token.
         Factor out the code tha maps an attribute value to a mechanism flag to it's own file so it can
      be used by other parts of softoken. (new function is in pkcs11u.c
         Implement the function that returns the FIPS indicator. This function fetches the indicator from either the session or the object or both. The session indicator is in the crypto context (except the last operation indicator, which is in the session itself. The object indicator is in the base object.
      lib/softoken/pkcs11c.c
         Record the FIPS indicator in the various helper function.
          - sftk_TerminateOp is called when a crypto operation had been finalized, so we can store that fips indicator in the lastOpWasFIPS field.
          - sftk_InitGeneric is called when a crypto operation has been initialized, so we can make a preliminary determination if the operation is within the FIPS policy (could later change bases on other operations. For this to work, we need the actual mechanism, so pMechanism is now a parameter to sftk_InitGeneric.
          - sftk_HKDF - HKDF when used in TLS has the unusual characteristic that the salt could actually be a key. In this case, usually the base key is some known public value which would not be FIPS generated, but the security is based on whether the salt is really a FIPS generated key. In this case we redo the calculation based on the salt key.
      lib/softoken/pkcs11i.h
          - add the FIPS indicators to the various structures (crypto contexts, sessions, objects).
          - add the FIPS indicators function list
          - add pMechanism the the sftkInitGeneric function.
          - add the helper function to map Attribute Types to Mechanism Flags.
          - add the function that will look up the current operation in the FIPS table to determine that it is allowed by policy.
      lib/softoken/pkcs11u.c
          - include the new fips_algorithms.h (if NSS_FIPS_DISABLED is not on)
          - handle the FIPS status for objects and session on creation an copy.
          - implement the helper function to map Attribute Types to Mechanism Flags.
          - get the key length of a key. This involves getting the key type and then using the key type to determin the appropriate attribute to fetch. Most keys it's simply the CKA_VALUE. ECC is special, we get the key length from the curve. Since only a subset of curves can be FIPS Curves, we use key length to return false for other curves.
          - the handle special function handles any unusal semantics for various mechanism types. This function precodes possible mechanism semantics we may need to check. The special handling can be selected by the mechanism table in fips_algorithms.h
          - sftk_operationIsFIPS - the actual function to determine if the givelib/n operation is in the FIPS table.
      lib/softoken/sftkmessage.c
          - just need to update the sftk_InitGeneric function to pass the mechanism.
      lib/ssl/ssl3con.c
          - and functions to query the underlying crypto contexts to see if the current ssl session is running in FIPS approved mode based on the security policy. It does so by checking the CipherSpecIsFIPS function to verify that both the mac and the encryption algorithm FIPS conforms to the ciphers in the security profile (using PK11_GetFIPSStatus). We check both the cipher specs for read and write. These underlying specs depends on the keys used in these specs being generated with FIPS approved algorithms as well, so this verifies the kea and kdf functions as well.
      lib/ssl/sslimpl.h
         - ass ssl_isFIPS() so it can be used by other files here in the ssl directory.
      lib/ssl/sslinfo.c
         - set the new isFIPS field in the existing sslinfo structure. SSL_GetChannelInfo knows how to handle sslinfo structures that are smaller then expected and larger than expected. unknown fields will be set to '0' (so new applications running against old versions will always get zero for new fields). sslinfo that are smaller will only return a the subset the calling application expects (so old applications will not get the new fields).
      lib/ssl/sslt.h
          - Add the new isFIPS field (must be at the end of the ChannelInfo structure).
      lib/util/pkcs11n.h
          - add the new FIPS indicator defines.
      tests/ssl/ssl.h
          - The main changes was to turn on verbose for the coverage tests so we can test the FIPS indicators on various cipher suites. NOTE: this only works with either NSS_TEST_FIPS_ALGORIHTMS set, or a vendor fips_algorthims.h, so vendors will need to do their own test interpretation. While working in ssl.sh I fixed an number of other issues:
          - many tests that were skipped in FIPS mode were skipped not because they didn't work in FIPS mode, but because tstclnt requires a password when running in FIPS mode. I've now added the password if the function is running in fips mode and removed the fips restrictions.
          - dtls had a race condition. the server side needed to come up before the client, but couldn't end before the client ran. We already had a sleep to guarrentee the former, I added a sleep before sending the server it's data to handle the latter.
          - CURVE25519 is the default ECC curve, but it's not a fiPS curve, so I disable it in FIPS mode so we will actually get FIPS indicators when using ECDHE.
          - I added TLS 1.3 to the coverage tests.
      
      Differential Revision: https://phabricator.services.mozilla.com/D115034
      
      --HG--
      extra : rebase_source : a1362bcf8107d26492578475d95522001ed0c3bd
      595deb8f
  2. 17 Mar, 2021 1 commit
    • Martin Thomson's avatar
      Bug 1698419 - ECH -10 updates, r=bbeurdouche · b1b15770
      Martin Thomson authored
      The main changes here are:
      
      * an update to HPKE -08
      * a move to the single-byte configuration ID
      * reordering of ECHConfig
      
      The addition of the explicit configuration ID means that the API for
      constructing ECHConfig(List) needs to change.  That means a name change,
      unfortunately.  I took the opportunity to make further changes to the arguments.
      
      Differential Revision: https://phabricator.services.mozilla.com/D108392
      
      --HG--
      extra : rebase_source : e66ae86be746afbadc6c444d0debcc5aaabd2eb5
      b1b15770
  3. 04 May, 2021 1 commit
    • Robert Relyea's avatar
      Bug 1707130 NSS should use modern algorithms in PKCS#12 files by default r=mt · 71f201b8
      Robert Relyea authored
      Also fixes:
      Bug 452464 pk12util -o fails when -C option specifies AES or Camellia ciphers
      
      Related:
      Bug 1694689 Firefox should use modern algorithms in PKCS#12 files by default
      Bug 452471 pk12util -o fails when -c option specifies pkcs12v2 PBE ciphers
      
      
      The base of this fix is was a simple 3 line fix in pkcs12.c, changing the initial setting of cipher and cert cipher.
      
      Overview for why this patch is larger than just 3 lines:
      1. First issue was found in trying to change the mac hashing value.
         a. While the decrypt side knew how to handle SHA2 hashes, the equivalent code was not updated on the encrypt side. I refactored that code and placed the common function in p12local.c. Now p12e.c and p12d.c share common code to find the required function to produce the mac key.
         b. The prf hmac was hard coded to SHA1. I changed the code to pass the hmac matching the hashing algorithm for the mac. This required changes to p12e.c to calculate and pass the new hmac as well and adding new PK11_ExportEncryptedPrivateKey and PK11_ExportEncryptedPrivKey to take the PKCS #5 v2 parameters. I also corrected an error which prevented pkcs12 encoding of ciphers other than AES.
      2. Once I've made my changes, I realized we didn't have a way of testing them. While we had code that verified that particular sets of parameters for pkcs12 worked together and could be listed and imported, we didn't have a way to verify what algorithms were actually generated by our tools.
        a. pk12util -l doesn't list the encryption used for the certs, so I updated pp to take a pkcs12 option. In doing so I had to update pp to handle indefinite encoding when decoding blocks. I also factored that decoding out in it's own function so the change only needed to be placed once. Finally I renabled a function which prints the output of an EncryptedPrivate key. This function was disabled long ago when the Encrypted Private key info was made private for NSS. It has since been exported, so these functions could easily be enabled (archeological note: I verified that this disabling was not a recent think I found I had done it back when I still have a netscape email address;).
       b. I updated tools.sh to us the new pp -t pkcs12 feature to verify that the key encryption, cert encryption, and hash functions matched what we expected when we exported a new key. I also updated tools.sh to handle the new hash variable option to pk12util.
       c. I discovered several tests commented out with comments that the don't work. I enabled those tests and discovered that they can now encrypt, but the can't decrypt because of pkcs12 policy. I updated the policy code, but I updated it to use the new NSS system wide policy mechanism. This enabled all the ciphers to work. There is still policy work to do. The pk12 policy currently only prevents ciphers from use in decrypting the certificates, not decrypting the keys and not encrypting. I left that for future work.
      3. New options for pp and pk12util were added to the man pages for these tools.
      
      ---------------------------------------------------------------------------
      With that in mind, here's a file by file description of the patch:
      
      automation/abi-check/expected-report-libnss3.so.txt
      -Add new exported functions. (see lib/nss/nss.def)
      
      cmd/lib/basicutil.h:
      -Removed the HAVE_EPV_TEMPLATE ifdefs (NSS has exported the Encrypted Private Key data structure for a while now.
      
      cmd/lib/secutil.c:
      global: Updated several functions to take a const char * m (message) rather than a char * m
      global: Made the various PrintPKCS7 return an error code.
      global: Added a state variable to be passed around the various PKCS7 Print functions. It gives the proper context to interpret PKCS7 Data Content. PKCS 12 used PKCS7 to package the various PKCS12 Safes and Bags.
      -Updated SECU_StripTagAndLength to handle indefinite encoding, and to set the Error code.
      -Added SECU_ExtractDERAndStep to grab the next DER Tag, Length, and Data.
      -Updated secu_PrintRawStringQuotesOptional to remove the inline DER parsing and use SECU_ExtractDERAndStep().
      -Updated SECU_PrintEncodedObjectID to return the SECOidTag just like SECU_PrintObjectID.
      -Renable SECU_PrintPrivateKey
      -Added secu_PrintPKCS12Attributes to print out the Attributes tied to a PKCS #12 Bag
      -Added secu_PrintPKCS12Bag to print out a PKCS #12 Bag
      -Added secu_PrintPKCS7Data, which uses the state to determine what it was printing out.
      -Added secu_PrintDERPKCS7ContentInfo which is identical to the global function SECU_PrintPKCS7ContentInfo except it takes a state variable. The latter function now calls the former.
      -Added secu_PrintPKCS12DigestInfo to print the Hash information of the Mac. DigestInfo is the name in the PKCS 12 spec.
      -Added secu_PrintPKCS12MacData to print the Mac portion of the PKCS 12 file.
      -Added SECU_PrintPKCS12 to print otu the pkcs12 file.
      
      cmd/lib/secutil.h
      -Added string for pkc12 for the command line of pp
      reenabled SECU_PrintPrivateKey
      -Added SECU_PrintPKCS12 for export.
      
      cmd/pk12util/pk12util.c
      -Added the -M option to specify a hash algorithm for the mac.
      updated P12U_ExportPKCS12Object: pass the hash algorithm to the PasswordIntegrity handler.
      -Added PKCS12U_FindTagFromString: generalized string to SECOidTag which only filters based on the oid having a matching PKCS #11 mechanism.
      updated PKCS12U_MapCipherFromString to call use PKCS12U_FindTagFromString to get the candidate tag before doing it's post processing to decide if the tag is really an encryption algorithm.
      -Added PKCS12U_MapHashFromString with is like MapCipherFromString except it verifies the resulting tag is a hash object.
      -Updated main to 1) change the default cipher, change the default certCipher, and process the new hash argument. NOTE: in the old code we did not encrypt the certs in FIPS mode. That's because the certs were encrypted with RC4 in the default pkcs12 file, which wasn't a FIPS algorithm. Since AES is, we can use it independent on whether or not we are in FIPS mode.
      
      cmd/pp/pp.c
      -Added the pkcs12 option which calls SECU_PrintPKCS12 from secutil.c
      
      lib/nss/nss.def
      -Add exports to the new PK11_ExportEncryptedPrivKeyInfoV2 and PK11_ExportEncryptedPrivateKeyInfoV2 (V2 means PKCS 5 v2, not Version 2 of ExportEncrypted*Info).
      -Add export for the old HASH_GetHMACOidTagByHashOidTag which should have been exported long ago to avoid the proliferation of copies of this function in places like ssl.
      
      lib/pk11wrap/pk11akey.c
      -Add PK11_ExportEncryptedPrivKeyInfoV2 (which the old function now calls), which takes the 3 PKCS 5 v2 parameters. The underlying pkcs5 code can fill in missing tags if necessary, but supplying all three gives the caller full control of the underlying pkcs5 PBE used.
      -Add PK11_ExportEncryptedPrivateKeyInfoV2, same as the above function except it takes a cert which is used to look up the private key. It's the function that pkcs12 actually uses, but the former was exported for completeness.
      
      lib/pk11wrap/pk11pub.h
      -Added the new PK11_ExportEncryptedPriv*KeyInfoV2 functions.
      
      lib/pkcs12/p12d.c
      -Remove the switch statement and place it in p12local.c so that p12e.c can use the same function.
      
      lib/pkc12/p12e.c
      -Remove the unnecessary privAlg check so we can encode any mechanism we support. This only prevented encoding certificates in the pk12 file, not the keys.
      -add code to get the hmac used in the pbe prf from the integrity hash, which is under application control.
      -Do the same for key encryption, then use the new PK11_ExportEncryptedPrivateKeyInfo to pass that hash value.
      -Use the new sec_pkcs12_algtag_to_keygen_mech so there is only one switch statement to update rather than 2.
      -Update the hash data to old the length of the largest hash rather than the length of a SHA1 hash.
      
      lib/pkcs12/p12local.c
      - Add new function new sec_pkcs12_algtag_to_keygen_mech to factor out the common switch statement between p12e and p12d.
      
      lib/pkcs12/p12local.h
      -Export the new sec_pkcs12_algtag_to_keygen_mech
      
      lib/pkcs12/p12plcy.c
      -Map the old p12 policy functions to use the new NSS_GetAlgorithmPolicy. We keep the old table so that applications can change the policy with the old PKCS12 specific defines (so the old code keeps working). NOTE: policies now default to true rather than false.
      
      lib/util/secoidt.h
      -Add new NSS_USE_ALG_IN_PKCS12 used by pk11plcy.c
      NOTE: I have not updated the policy table in pk11wrap/pk11pars.c, so we can't yet control pkcs12 policy with the nss system policy table. That's a patch for another time.
      
      test/tools/tool.sh
      -global: Remove trailing spaces
      -global: DEFAULT is changed to 'default'
      -Update the PBE mechanism to exactly match the string in secoid.c. PKCS #12 does case independent compares, so case doesn't matter there, but now I'm comparing to the output of pp, and I didn't want to spend the time to figure out case independent compares in bash.
      -Add our defauts and shell variables at the top so there are easy to change in the future.
      export_with_*** have all been colapsed into a single export_p12_file which handles taking 'default' and turning off that argument.
      -Add for loops for the hash functions.
      -Restore the camellia ciphers back now that they work.
      -Restore the pkcs12V2pbe back now that they work.
      -Collect various pbe types into single variables and use those variables in loops
      -Reduce the number of tests ran in optimized mode (which takes 60x the time to do a pbe then than debug mode based on a larger iterator).
      -Add verify_p12 which dumps out the p12 file and makes sure the expected CERT_ENCRYPTION, KEY_ENCRYPTION, and HASH are used.
      
      doc/pp.xml
      -Add pkcs12 option
      
      doc/pk12util.xml
      -Add -M option
      -Update synopsis with options in the description but not in the synopsis
      
      Differential Revision: https://phabricator.services.mozilla.com/D113699
      71f201b8
  4. 08 Apr, 2021 1 commit
    • Robert Relyea's avatar
      Bug 1703936 New coverity/cpp scanner errors. · 350807b3
      Robert Relyea authored
      Redhat has run our scanners on the full NSS tree and identifed 123
      errors our security team determined where 'critical'. I've reviewed
      the errors and identified a much smaller subset of errors that are
      either real, or confusing enough to warrent suppression comments.
      
      Many errors are in cmd and gtest. I've skip those commands red hat does
      not ship in this report, and I've skipped the issues in gtest.
      
      Also, There's a large number of leaked_storage errors because evidently
      coverity gets confused when you have a pointer in a local variable and
      you pass that pointer off to a global or a function variable. I've
      skipped most of those as well.
      
      changes:
      crlutil.c: add missing arena free in error path. #def4
      secutil.c: (Not coverity found) make sure we don't overflow our buffer
                 in badly encoded ECC oids.
      modultil.c: Fix incorrect free operation in pk11install case. #def6
      signtool/javascript.c: free old archiveDir after use in PR_smprintf #def8
                             don't double free curitem (curitem is almost
                             certainly NULL at this point, so the current code
                             is a noop). #def9, #def10
      signtool/list.c: remove unused ugly_list variable (which is leaked) #def12
      signtool/util.c: don't leak 'dir' in error path #def13 #def14
      sigver/pk7print.c: coverity: use static pk rather than an allocated and
                         leaked pointer. #def15
                         add code for EC
                         disbled DSA code that isn't actually working
                         (PQG params).
      symkeyutil.c: free name (depends on PORT_Free null check). #def16
      pkix_pl_nameconstraints.c: Fix coverity double free warning.
                                 PKIX_ERROR_RECEIVED is almost
                                 certainly false in this case, but by
                                 setting arena to NULL we make
                                 sure it's not used or freed again. #def99
      pkix_pl_string.c: Fix varargs leak in the error path. #def109
      pk11parse.c: secmod_doDescCopy can reallocate our newSpec, but the
                   pointer we are passed is an offset from newSpec. Pass in
                   both pointers and return our newly allocated spec and
                   length in that case.#def113
      cmsutil.c: suppress cppcheck warnings do to cmsutil use of unions to
                 cast pointers. #def113-117
      pkcs11.c: support coverity incorrect use_after_free warning. #def118
      
      scanner errors:
      Error: USE_AFTER_FREE (CWE-416): [#def4]
      nss-3.60.1/nss/cmd/crlutil/crlutil.c:389: freed_arg: "PORT_FreeArena_Util" frees "modArena".
      nss-3.60.1/nss/cmd/crlutil/crlutil.c:455: double_free: Calling "PORT_FreeArena_Util" frees pointer "modArena" which has already been freed.
      453| }
      454| if (modArena && (!modCrl || modCrl->arena != modArena)) {
      455|-> PORT_FreeArena(modArena, PR_FALSE);
      456| }
      457| if (modCrl)
      
      Error: BAD_FREE (CWE-590): [#def6]
      nss-3.60.1/nss/cmd/modutil/install-ds.c:1046: address_free: "PR_Free" frees address of "_this->forwardCompatible".
      1044| Pk11Install_PlatformName_delete(&_this->forwardCompatible[i]);
      1045| }
      1046|-> PR_Free(&_this->forwardCompatible);
      1047| _this->numForwardCompatible = 0;
      1048| }
      
      Error: USE_AFTER_FREE (CWE-416): [#def8]
      nss-3.60.1/nss/cmd/signtool/javascript.c:1346: freed_arg: "PR_Free" frees "archiveDir".
      nss-3.60.1/nss/cmd/signtool/javascript.c:1347: pass_freed_arg: Passing freed pointer "archiveDir" as an argument to "PR_smprintf".
      1345| warningCount++;
      1346| PR_Free(archiveDir);
      1347|-> archiveDir = PR_smprintf("%s.arc", archiveDir);
      1348| } else {
      1349| PL_strcpy(archiveDir + strlen(archiveDir) - 4, ".arc");
      
      Error: USE_AFTER_FREE (CWE-416): [#def9]
      nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityListTail" = "entityItem". Now both point to the same storage.
      nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityList" = "entityListTail". Now both point to the same storage.
      nss-3.60.1/nss/cmd/signtool/javascript.c:1623: alias: Assigning: "curitem" = "entityList". Now both point to the same storage.
      nss-3.60.1/nss/cmd/signtool/javascript.c:1651: freed_arg: "PR_Free" frees "entityListTail".
      nss-3.60.1/nss/cmd/signtool/javascript.c:1654: double_free: Calling "PR_Free" frees pointer "curitem" which has already been freed.
      1652| }
      1653| if (curitem) {
      1654|-> PR_Free(curitem);
      1655| }
      1656| if (basedir) {
      
      Error: USE_AFTER_FREE (CWE-416): [#def10]
      nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityListTail" = "entityItem". Now both point to the same storage.
      nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityList" = "entityListTail". Now both point to the same storage.
      nss-3.60.1/nss/cmd/signtool/javascript.c:1623: alias: Assigning: "curitem" = "entityList". Now both point to the same storage.
      nss-3.60.1/nss/cmd/signtool/javascript.c:1651: freed_arg: "PR_Free" frees "entityListTail".
      nss-3.60.1/nss/cmd/signtool/javascript.c:1654: pass_freed_arg: Passing freed pointer "curitem" as an argument to "PR_Free".
      1652| }
      1653| if (curitem) {
      1654|-> PR_Free(curitem);
      1655| }
      1656| if (basedir) {
      
      Error: RESOURCE_LEAK (CWE-772): [#def12]
      nss-3.60.1/nss/cmd/signtool/list.c:36: alloc_fn: Storage is returned from allocation function "PORT_ZAlloc_Util".
      nss-3.60.1/nss/cmd/signtool/list.c:36: var_assign: Assigning: "ugly_list" = storage returned from "PORT_ZAlloc_Util(16UL)".
      nss-3.60.1/nss/cmd/signtool/list.c:137: leaked_storage: Variable "ugly_list" going out of scope leaks the storage it points to.
      135|
      136| if (failed) {
      137|-> return -1;
      138| }
      139| return 0;
      
      Error: RESOURCE_LEAK (CWE-772): [#def13]
      nss-3.60.1/nss/cmd/signtool/util.c:131: alloc_fn: Storage is returned from allocation function "PR_OpenDir".
      nss-3.60.1/nss/cmd/signtool/util.c:131: var_assign: Assigning: "dir" = storage returned from "PR_OpenDir(path)".
      nss-3.60.1/nss/cmd/signtool/util.c:139: identity_transfer: Passing "dir" as argument 1 to function "PR_ReadDir", which returns an offset off that argument.
      nss-3.60.1/nss/cmd/signtool/util.c:139: noescape: Resource "dir" is not freed or pointed-to in "PR_ReadDir".
      nss-3.60.1/nss/cmd/signtool/util.c:139: var_assign: Assigning: "entry" = storage returned from "PR_ReadDir(dir, PR_SKIP_BOTH)".
      nss-3.60.1/nss/cmd/signtool/util.c:142: leaked_storage: Variable "entry" going out of scope leaks the storage it points to.
      nss-3.60.1/nss/cmd/signtool/util.c:142: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
      140| if (snprintf(filename, sizeof(filename), "%s/%s", path, entry->name) >= sizeof(filename)) {
      141| errorCount++;
      142|-> return -1;
      143| }
      144| if (rm_dash_r(filename))
      
      Error: RESOURCE_LEAK (CWE-772): [#def14]
      nss-3.60.1/nss/cmd/signtool/util.c:131: alloc_fn: Storage is returned from allocation function "PR_OpenDir".
      nss-3.60.1/nss/cmd/signtool/util.c:131: var_assign: Assigning: "dir" = storage returned from "PR_OpenDir(path)".
      nss-3.60.1/nss/cmd/signtool/util.c:139: identity_transfer: Passing "dir" as argument 1 to function "PR_ReadDir", which returns an offset off that argument.
      nss-3.60.1/nss/cmd/signtool/util.c:139: noescape: Resource "dir" is not freed or pointed-to in "PR_ReadDir".
      nss-3.60.1/nss/cmd/signtool/util.c:139: var_assign: Assigning: "entry" = storage returned from "PR_ReadDir(dir, PR_SKIP_BOTH)".
      nss-3.60.1/nss/cmd/signtool/util.c:145: leaked_storage: Variable "entry" going out of scope leaks the storage it points to.
      nss-3.60.1/nss/cmd/signtool/util.c:145: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
      143| }
      144| if (rm_dash_r(filename))
      145|-> return -1;
      146| }
      147|
      
      Error: RESOURCE_LEAK (CWE-772): [#def15]
      nss-3.60.1/nss/cmd/signver/pk7print.c:325: alloc_fn: Storage is returned from allocation function "PORT_ZAlloc_Util".
      nss-3.60.1/nss/cmd/signver/pk7print.c:325: var_assign: Assigning: "pk" = storage returned from "PORT_ZAlloc_Util(328UL)".
      nss-3.60.1/nss/cmd/signver/pk7print.c:351: leaked_storage: Variable "pk" going out of scope leaks the storage it points to.
      349| default:
      350| fprintf(out, "%s=bad SPKI algorithm type\n", msg);
      351|-> return 0;
      352| }
      353|
      
      Error: RESOURCE_LEAK (CWE-772): [#def16]
      nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:289: alloc_fn: Storage is returned from allocation function "PK11_GetSymKeyNickname".
      nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:289: var_assign: Assigning: "name" = storage returned from "PK11_GetSymKeyNickname(symKey)".
      nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:298: noescape: Resource "name ? name : " "" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
      nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:306: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
      304| }
      305| printf("\n");
      306|-> }
      307|
      308| SECStatus
      
      Error: USE_AFTER_FREE (CWE-416): [#def99]
      nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c:835: freed_arg: "PORT_FreeArena_Util" frees "arena".
      nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c:854: double_free: Calling "PORT_FreeArena_Util" frees pointer "arena" which has already been freed.
      852| PKIX_CERTNAMECONSTRAINTS_DEBUG
      853| ("\t\tCalling PORT_FreeArena).\n");
      854|-> PORT_FreeArena(arena, PR_FALSE);
      855| }
      856| }
      
      Error: VARARGS (CWE-237): [#def109]
      nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c:428: va_init: Initializing va_list "args".
      nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c:534: missing_va_end: "va_end" was not called for "args".
      532| }
      533|
      534|-> PKIX_RETURN(STRING);
      535| }
      536|
      
      Error: USE_AFTER_FREE (CWE-416): [#def112]
      nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1099: alias: Assigning: "newSpecPtr" = "newSpec". Now both point to the same storage.
      nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1156: freed_arg: "secmod_doDescCopy" frees "newSpecPtr".
      nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1211: use_after_free: Using freed pointer "newSpec".
      1209| /* no target found, return the newSpec */
      1210| if (target == NULL) {
      1211|-> return newSpec;
      1212| }
      1213|
      
      Error: CPPCHECK_WARNING (CWE-562): [#def113]
      nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'digestedData' that will be invalid when returning.
      307| }
      308| }
      309|-> return cinfo;
      310| }
      311|
      
      Error: CPPCHECK_WARNING (CWE-562): [#def114]
      nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'encryptedData' that will be invalid when returning.
      307| }
      308| }
      309|-> return cinfo;
      310| }
      311|
      
      Error: CPPCHECK_WARNING (CWE-562): [#def115]
      nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'envelopedData' that will be invalid when returning.
      307| }
      308| }
      309|-> return cinfo;
      310| }
      311|
      
      Error: CPPCHECK_WARNING (CWE-562): [#def116]
      nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'genericData' that will be invalid when returning.
      307| }
      308| }
      309|-> return cinfo;
      310| }
      311|
      
      Error: CPPCHECK_WARNING (CWE-562): [#def117]
      nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'signedData' that will be invalid when returning.
      307| }
      308| }
      309|-> return cinfo;
      310| }
      311|
      
      Error: USE_AFTER_FREE (CWE-416): [#def118]
      nss-3.60.1/nss/lib/softoken/pkcs11.c:2671: freed_arg: "PORT_Realloc_Util" frees "oldNscSlotList".
      nss-3.60.1/nss/lib/softoken/pkcs11.c:2674: use_after_free: Using freed pointer "oldNscSlotList".
      2672| nscSlotListSize[index] * sizeof(CK_SLOT_ID));
      2673| if (nscSlotList[index] == NULL) {
      2674|-> nscSlotList[index] = oldNscSlotList;
      2675| nscSlotListSize[index] = oldNscSlotListSize;
      2676| return CKR_HOST_MEMORY;
      
      Some of these are false positives, but they are reasonable issues for the scanners to flag, so scanner suppression comments, along with human reviewer comments will be added.
      
      Differential Revision: https://phabricator.services.mozilla.com/D111339
      
      --HG--
      extra : rebase_source : 45ad7590affb7fdb8a0e182e29c6c88f909ff260
      350807b3
  5. 10 Mar, 2021 2 commits
  6. 27 Feb, 2021 1 commit
  7. 31 Jan, 2021 1 commit
    • Kevin Jacobs's avatar
      Bug 1689228 - Minor ECH -09 fixes for interop testing, fuzzing. r=mt · 4df976da
      Kevin Jacobs authored
      A few minor ECH -09 fixes for interop testing and fuzzing:
      - selfserv now takes a PKCS8 keypair for ECH. This is more maintainable and significantly
        less terrible than parsing the ECHConfigs and cobbling one together within selfserv
        (e.g. we can support other KEMs without modifying the server).
      - Get rid of the newline character in tstclnt retry_configs output.
      - Fuzzer fixes in tls13_HandleHrrCookie:
       - We shouldn't use internal_error when PK11_HPKE_ImportContext fails. Cookies are
         unprotected in fuzzer mode, so this can be expected to occur.
       - Only restore the application token when recovering hash state, otherwise the
         copy could happen twice, leaking one of the allocations.
      
      Differential Revision: https://phabricator.services.mozilla.com/D103247
      
      --HG--
      extra : moz-landing-system : lando
      4df976da
  8. 24 Jan, 2021 1 commit
    • Kevin Jacobs's avatar
      Bug 1681585 - Add ECH support to selfserv. r=mt · bda8540c
      Kevin Jacobs authored
      Usage example:
      mkdir dbdir && cd dbdir
      certutil -N -d .
      certutil -S -s "CN=ech-public.com" -n ech-public.com -x -t "C,C,C" -m 1234 -d .
      certutil -S -s "CN=ech-private-backend.com" -n ech-private-backend.com -x -t "C,C,C" -m 2345 -d .
      ../dist/Debug/bin/selfserv -a ech-public.com -a ech-private-backend.com -n ech-public.com -n ech-private-backend.com -p 8443 -d dbdir/ -X publicname:ech-public.com
      (Copy echconfig from selfserv output and paste into the below command)
      ../dist/Debug/bin/tstclnt -D -p 8443 -v -A tests/ssl/sslreq.dat -h ech-private-backend.com -o -N <echconfig> -v
      
      Differential Revision: https://phabricator.services.mozilla.com/D101050
      
      --HG--
      extra : moz-landing-system : lando
      bda8540c
  9. 03 Dec, 2020 1 commit
  10. 17 Nov, 2020 1 commit
    • Kevin Jacobs's avatar
      Bug 1654332 - Update ESNI to draft-08 (ECH). r=mt · 4516d102
      Kevin Jacobs authored
      This patch adds support for Encrypted Client Hello (draft-ietf-tls-esni-08), replacing the existing ESNI (draft -02) support.
      
      There are five new experimental functions to enable this:
      
        - SSL_EncodeEchConfig: Generates an encoded (not BASE64) ECHConfig given a set of parameters.
        - SSL_SetClientEchConfigs: Configures the provided ECHConfig to the given socket. When configured, an ephemeral HPKE keypair will be generated for the CH encryption.
        - SSL_SetServerEchConfigs: Configures the provided ECHConfig and keypair to the socket. The keypair specified will be used for HPKE operations in order to decrypt encrypted Client Hellos as they are received.
        - SSL_GetEchRetryConfigs: If ECH is rejected by the server and compatible retry_configs are provided, this API allows the application to extract those retry_configs for use in a new connection.
        - SSL_EnableTls13GreaseEch: When enabled, non-ECH Client Hellos will have a "GREASE ECH" (i.e. fake) extension appended. GREASE ECH is disabled by default, as there are known compatibility issues that will be addressed in a subsequent draft.
      
      The following ESNI experimental functions are deprecated by this update:
      
        - SSL_EncodeESNIKeys
        - SSL_EnableESNI
        - SSL_SetESNIKeyPair
      
      
      In order to be used, NSS must be compiled with `NSS_ENABLE_DRAFT_HPKE` defined.
      
      Differential Revision: https://phabricator.services.mozilla.com/D86106
      
      --HG--
      rename : gtests/ssl_gtest/tls_esni_unittest.cc => gtests/ssl_gtest/tls_ech_unittest.cc
      rename : lib/ssl/tls13esni.c => lib/ssl/tls13ech.c
      rename : lib/ssl/tls13esni.h => lib/ssl/tls13ech.h
      extra : moz-landing-system : lando
      4516d102
  11. 24 Jul, 2020 1 commit
  12. 21 Jul, 2020 1 commit
  13. 12 Jun, 2020 1 commit
  14. 10 Jun, 2020 1 commit
    • Kevin Jacobs's avatar
      Bug 1603042 - Support external PSKs in tstclnt/selfserv. r=jcj · 5b672708
      Kevin Jacobs authored
      This patch adds support for TLS 1.3 external PSKs in tstclnt and selfserv with the `-z` option.
      
      Command examples:
      - `selfserv -D -p 4443 -d . -n localhost.localdomain -w nss -V tls1.3: -H 1 -z 0xAAAAAAAABBBBBBBBCCCCCCCCDDDDDDDD[:label] -m`
      - `tstclnt -h 127.0.0.1 -p 4443  -z 0xAAAAAAAABBBBBBBBCCCCCCCCDDDDDDDD[:label] -d . -w nss`
      
      For OpenSSL interop:
      - `openssl s_server -nocert -port 4433 -psk AAAAAAAABBBBBBBBCCCCCCCCDDDDDDDD [-psk_identity label]`
      
      Note: If the optional label is omitted, both NSS tools and OpenSSL default to "Client_identity".
      
      Differential Revision: https://phabricator.services.mozilla.com/D75836
      
      --HG--
      extra : moz-landing-system : lando
      5b672708
  15. 03 Jun, 2020 1 commit
  16. 19 May, 2020 1 commit
  17. 05 May, 2020 6 commits
  18. 14 Apr, 2020 1 commit
    • Robert Relyea's avatar
      Bug 1629661 MPConfig calls in SSL initializes policy before NSS is initialized. r=mt · 108aa431
      Robert Relyea authored
      NSS has several config functions that multiprocess servers must call before NSS is initialized to set up shared memory caches between the processes. These functions call ssl_init(), which initializes the ssl policy. The ssl policy initialization, however needs to happen after NSS itself is initialized. Doing so before hand causes (in the best case) policy to be ignored by these servers, and crashes (in the worst case).
      
      Instead, these cache functions should just initialize those things it needs (that is the NSPR ssl error codes).
      
      This patch does:
      1) fixes the cache init code to only initialize error codes.
      2) fixes the selfserv MP code to 1) be compatible with ssl.sh's selfserv management (at least on Unix), and 2) mimic the way real servers handle the MP_Cache init code (calling NSS_Init after the cache set up).
      3) update ssl.sh server policy test to test policy usage on an MP server. This
      is only done for non-windows like OS's because they can't catch the kill signal
      to force their children to shutdown.
      
      I've verified that the test fails if 2 and 3 are included but 1 is not
      (and succeeds if all three are included).
      
      Differential Revision: https://phabricator.services.mozilla.com/D70948
      108aa431
  19. 26 Mar, 2020 1 commit
  20. 18 Mar, 2020 1 commit
    • Robert Relyea's avatar
      Bug 1603628 Update NSS to handle PKCS #11 v3.0 r=ueno r=mt · a95d4e2c
      Robert Relyea authored
      Update to PKCS #11 v3.0 part 2.
      
      Create the functions and switch to the C_Interface() function to fetch the PKCS #11 function table. Also PKCS #11 v3.0 uses a new fork safe interface. NSS can already handle the case if the PKCS #11 module happens to be fork safe (when asked by the application to refresh the tokens in the child process, NSS can detect that such a refresh is not necessary and continue. Softoken could also be put in fork_safe mode with an environment variable. With this patch it's the default, and NSS asks for the fork safe API by default. Technically softoken should implement the old non-fork safe interface when PKCS #11 v2.0 is called, but NSS no longer needs it, and doing so would double the number of PKCS #11 interfaces are needed. You can still compile with fork unsafe semantics, and the PKCS #11 V3.0 module will do the right thing and not include the fork safe flag. Firefox does not fork(), so for firefox this is simply code that is no longer compilied.
      
      We now use C_GetInterface, which allows us to specify what kind of interface we want (PKCS #11 v3.0, PKCS #11 v2.0, fork safe, etc.). Vendor specific functions can now be accessed through the C_GetInterface. If the C_GetInterface function does not exists, we fall bak to the old C_GetFunctionList.
      
      There are 24 new functions in PKCS #11 v3.0:
      C_GetInterfaceList - return a table of all the supported interfaces
      C_GetInterface - return a specific interface. You can specify interface name, version and flags separately. You can leave off any of these and you will get what the token thinks is the best match of the interfaces that meet the criteria. We do this in softoken by the order of the interface list.
      C_SessionCancel - Cancel one or more multipart operation
      C_LoginUser - Supply a user name to C_Login(). This function has no meaning for softoken, so it just returns CKR_OPERATION_NOT_INITIALIZED under the theory that if we in the future want to support usernames, the NSS db would need special initialization to make that happen.
      C_Message* and C_*Message*  (20 functions in all) are the new AEAD interface (they are written generally so that it can be used for things other than AEAD). In this patch they are unimplemented (see the next patch).
      
      This patch adds regular (NSC_) and FIPS (FC_) versions of these functions.
      Also when creating the PKCS #11 v2.0 interface, we had to create a 2.0 specific version of C_GetInfo so that it can return a 2.40 in the CK_VERSION field rather than 3.00. We do this with #defines since all the function tables are generated automagically with pkcs11f.h.
      
      Differential Revision: https://phabricator.services.mozilla.com/D67240
      a95d4e2c
  21. 18 Feb, 2020 1 commit
    • Robert Relyea's avatar
      Bug 1603628 Update NSS to handle PKCS #11 v3.0 r=daiki r=mhoye · ba931199
      Robert Relyea authored
      https://phabricator.services.mozilla.com/D63241
      
      This patch implements the first phase: updating the headers.
      
      lib/util/pkcs11.h
      lib/util/pkcs11f.h
      lib/util/pkcs11t.h
      
      Were updated using the released OASIS PKCS #11 v3.0 header files.
      lib/util/pkcs11n.h was updated to finally deprecate all uses of CK?_NETSCAPE_?.
      
      A new define as added: NSS_PKCS11_2_0_COMPAT. If it's defined, the small
      semantic changes (including the removal of deprecated defines) between the
      NSS PKCS #11 v2 header file and the new PKCS #11 v3 are reverted in favor of
      the PKCS #11 v2 definitions. This include the removal of CK?_NETSCAPE_? in
      favor of CK?_NSS_?.
      
      One notable change was caused by an inconsistancy between the spec and the
      released headers in PKCS #11 v2.40. CK_GCM_PARAMS had an extra field in
      the header that was not in the spec. OASIS considers the header file to be
      normative, so PKCS #11 v3.0 resolved the issue in favor of the header file
      definition. NSS had the spec definition, so now there are 2 defines for this
      structure:
      
      CK_NSS_GCM_PARAMS - the old nss define. Still used internally in freebl.
      CK_GCM_PARAMS_V3 - the new define.
      CK_GCM_PARAMS - no longer referenced in NSS itself. It's defined as
      CK_GCM_PARAMS_V3 if NSS_PKCS11_2_0_COMPAT is *not* defined, and it's defined as
      CKM_NSS_GCM_PARAMS if NSS_PKCS11_2_0_COMPAT is defined.
      
      Softoken has been updated to accept either CK_NSS_GCM_PARAMS or
      CK_GCM_PARAMS_V3. In a future patch NSS will be updated to use
      CK_GCM_PARAMS_V3 and fall back to CK_NSS_GMC_PARAMS.
      
      One other semantic difference between the 3.0 version of pkcs11f.h and the
      version here: In the oasis version of the header, you must define
      CK_PKCS11_2_0_ONLY to get just the PKCS #11 v2 defines. In our version you
      must define CK_PKCS11_3 to get the PCKS #11 v3 defines.
      
      Most of this patch is to handle changing the deprecated defines that have been
      removed in PCKS #11 v3 from NSS.
      
      Differential Revision: https://phabricator.services.mozilla.com/D63241
      ba931199
  22. 13 Mar, 2020 1 commit
  23. 11 Feb, 2020 1 commit
  24. 07 Jan, 2020 1 commit
  25. 06 Jan, 2020 1 commit
  26. 01 Jan, 2020 1 commit
  27. 22 Oct, 2019 1 commit
  28. 15 Oct, 2019 1 commit
  29. 11 Oct, 2019 1 commit
  30. 27 Sep, 2019 2 commits
  31. 24 Sep, 2019 1 commit
  32. 18 Sep, 2019 2 commits