Page MenuHomePhabricator

Bug 1705477 - land NSS 1d066793c349 UPGRADE_NSS_RELEASE, r=beurdouche
ClosedPublic

Authored by bbeurdouche on May 7 2021, 10:38 AM.
Referenced Files
Unknown Object (File)
Wed, Apr 8, 10:32 PM
Unknown Object (File)
Feb 28 2026, 11:18 AM
Unknown Object (File)
Jan 29 2026, 9:06 AM
Unknown Object (File)
Oct 15 2025, 2:36 PM
Unknown Object (File)
Sep 20 2025, 2:20 PM
Unknown Object (File)
May 28 2025, 8:09 AM
Unknown Object (File)
May 19 2025, 1:34 AM
Unknown Object (File)
May 14 2025, 7:04 PM
Subscribers

Details

Summary

2021-05-06 Martin Thomson <[email protected]>

  • gtests/pk11_gtest/pk11_hpke_unittest.cc:

Bug 1709750 - Disable HPKE test when fuzzing, r=bbeurdouche

[1d066793c349] [tip]

2021-05-05 Benjamin Beurdouche <[email protected]>

  • lib/freebl/ppc-gcm-wrap.c, lib/freebl/ppc-gcm.h:

Bug 1566124 - Clang format run. r=beurdouche
[cb714d62058c]

2021-05-05 mamonet <[email protected]>

  • lib/freebl/Makefile, lib/freebl/freebl.gyp, lib/freebl/ppc-gcm-

wrap.c, lib/freebl/ppc-gcm.h, lib/freebl/ppc-gcm.s,
lib/freebl/rijndael.c:

[1133fef2f7ce]

2021-03-17 Martin Thomson <[email protected]>

  • gtests/common/testvectors/hpke-convert.py,

gtests/common/testvectors/hpke-vectors.h, lib/pk11wrap/pk11hpke.c,
lib/pk11wrap/pk11hpke.h:
Bug 1699021 - Add AES-256-GCM to HPKE, r=bbeurdouche

[9fa53d717386]

  • automation/abi-check/expected-report-libssl3.so.txt,

cmd/selfserv/selfserv.c, gtests/ssl_gtest/libssl_internals.c,
gtests/ssl_gtest/libssl_internals.h,
gtests/ssl_gtest/tls_connect.cc, gtests/ssl_gtest/tls_connect.h,
gtests/ssl_gtest/tls_ech_unittest.cc, lib/ssl/sslexp.h,
lib/ssl/sslsock.c, lib/ssl/sslt.h, lib/ssl/tls13ech.c,
lib/ssl/tls13ech.h, lib/ssl/tls13exthandle.c,
lib/ssl/tls13hashstate.c, lib/ssl/tls13hashstate.h:
Bug 1698419 - ECH -10 updates, r=bbeurdouche

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.

[fa93bd88b690]

2021-03-16 Martin Thomson <[email protected]>

  • coreconf/config.gypi, coreconf/config.mk,

gtests/common/testvectors/hpke-convert.py,
gtests/common/testvectors/hpke-vectors.h,
gtests/pk11_gtest/pk11_hpke_unittest.cc,
gtests/ssl_gtest/ssl_auth_unittest.cc,
gtests/ssl_gtest/ssl_tls13compat_unittest.cc,
gtests/ssl_gtest/tls_ech_unittest.cc, lib/pk11wrap/pk11hpke.c,
lib/pk11wrap/pk11hpke.h, lib/pk11wrap/pk11pub.h, lib/ssl/tls13ech.c:
Bug 1692930 - Update HPKE to final version, r=bbeurdouche

This adds the final HPKE version string.

This removes the draft version markers from the implementation and
stops tracking the draft version with the exported syntax.

I've added the script that I used to convert the JSON test vectors
from the specification; that should allow us to pick up new tests
relatively easily, especially if we need to add new algorithms.

This change breaks several ECH test cases. As fixing those tests is
extraordinarily fiddly, I'm going to defer making those changes
until we need to update ECH. As we can't land this code until ECH is
updated to depend on the final HPKE and until we have coordinated
with servers on when the ECH update can be deployed, it should be OK
to defer.

In short, don't land this without the matching ECH changes.

[e78141a928f4]

2021-05-04 Robert Relyea <[email protected]>

  • automation/abi-check/expected-report-libnss3.so.txt,

cmd/lib/basicutil.h, cmd/lib/secutil.c, cmd/lib/secutil.h,
cmd/pk12util/pk12util.c, cmd/pp/pp.c, doc/pk12util.xml, doc/pp.xml,
lib/nss/nss.def, lib/pk11wrap/pk11akey.c, lib/pk11wrap/pk11pub.h,
lib/pkcs12/p12d.c, lib/pkcs12/p12e.c, lib/pkcs12/p12local.c,
lib/pkcs12/p12local.h, lib/pkcs12/p12plcy.c, lib/util/secoidt.h,
tests/tools/tools.sh:
Bug 1707130 NSS should use modern algorithms in PKCS#12 files by
default r=mt

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

[0a1687e1b39e]

Event Timeline

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: secure-revision.
This revision is now accepted and ready to land.May 7 2021, 10:40 AM

This revision requires a Testing Policy Project Tag to be set before landing. Please apply one of testing-approved, testing-exception-unchanged, testing-exception-ui, testing-exception-elsewhere, testing-exception-other. Tip: this Firefox add-on makes it easy!

Code analysis found 1 defect in the diff 436700:

  • 1 defect found by code coverage analysis

In our previous code coverage analysis run, we found some files which had no coverage and are being modified in this patch:
Should they have tests, or are they dead code?

  • You can file a bug blocking Bug 1415824 for untested files that should be tested.
  • You can file a bug blocking Bug 1415819 for untested files that should be removed.

If you see a problem in this automated review, please report it here.

You can view these defects on the code-review frontend and on Treeherder.