| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D121161
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
with AES_CBC
When we added support for AES, we also added support for integrity checks on the encrypted components.
It turns out the code that verifies the integrity checks was broken in 2 ways:
1. it wasn't accurately operating when AES was being used (the if statement wasn't actually triggering for AES_CBC because we were looking for AES in the wrong field).
2. password update did not update the integrity checks in the correct location, meaning any database which AES encrypted keys, and which had their password updated will not be able to validate their keys.
While we found this in a previous rebase, the patch had not been pushed upstream.
The attached patch needs sqlite3 to run the tests.
Differential Revision: https://phabricator.services.mozilla.com/D120011
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
invalid algorithms.
Our QA is quite extensive on handling of alert corner cases. Our code that checks if a signature algorithm is supported ignores the role of policy. If SHA1 is turned off by policy, for instance, we only detect that late in the game. This shows up in our test cases as decrypt_alerts rather than illegal_parameter or handshake_error alerts. It also shows up in us apparently accepting a client auth request which only has invalid alerts.
We also don't handle filtering out signature algorithms that are illegal in tls 13 mode.
This patch not only fixes these issues, but also issues where we proposing signature algorithms in server mode that we don't support by policy.
This patch includes:
In gtests:
1) adding support for policy in ssl_gtests. Currently both the server an client will run with the same policy. The patch allows us to set policy on one and keeping the old policy on the other.
2) Update extension tests which failed in tls 1.3 because the patch now correctly rejects illegal tls 1.3 auth values. The test was updated to use a legal auth value in tls 1.3 (so we are correctly testing the format issue.
3) Update extension tests to handle the case where we try to use an illegal value for tls 1.3.
4) add tests to ssl_auth_unittests.cc to make sure we can properly connect even when several auth methods are turned off by policy (make sure we don't advertize them on the client side, and that the server doesn't select them when the client doesn't advertize them).
5) add tests to ssl_auth_unittests.cc to make sure we don't send empty client auth requests when the requester only sends invalid auth requests.
patch itself:
1) The handling of policy checks for ssl schemes were scattered in various locations. I've consolidated them into a single function. That function now checks for NSS_ALG_USE_IN_ANY_SIGNATURE as if this is off by policy, we will fail if we try to use the algorithm in a signature in any case. NSS now supports policy on all signature algorithms, not just DSA, so we need to check the policy of all the algorithms.
2) to support the policy check on the signature algorithms, I added a new ssl_AuthTypeToOID, which also replaces our switch in checking if the SPKI matches our auth type.
3) ssl_SignatureSchemeValid now accepts an spkiOid of SEC_OID_UNKNOWN. To allow us to filter signature schemes based on version and policy restrictions before we try to select a certificate. This prevents us from sending empty client auth messages when we are presented with only invalid signature schemes.
4) We filter supported algorithms against policy early, preventing us from sending, or even setting invalid algorithms if they are turned off by policy.
5) ssl ConsumeSignatureScheme was handling alerts inconsistently. The Consume could send an allert in it's failure case, but the check of scheme validity wouldn't sent an alert. The collers were inconstent as well. Now ssl_ConsumeSignatureScheme always sends and alert on failure, and the callers do not.
Differential Revision: https://phabricator.services.mozilla.com/D120392
|
| | |
|
| |
|
|
|
|
|
|
|
|
| |
https://sqlite.org/forum/info/42cf8e985bb051a2
sqlite is now permissive on opening a readonly file even if you ask for the file to be opened R/W.
normally sqlite is very conservative in changing it's underlying semantics, but evidently they chose convience over compatibility. NSS now needs to check the file permissions itself to preserve nss semantics.
Differential Revision: https://phabricator.services.mozilla.com/D120406
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
all.sh reports.
This patch includes the updated .sed script, and an experiment using bash instead to see how hard it would be to make a more robust parser.
The robust parser generates identical output as sed, but takes about 30x longer, so instead of subsecond operations, it takes almost half a minute. With that result, I think we can stay with sed and continue to update when we get new versions of gtests. (sigh).
time cat report.xml.0 | sed -f parsegtestreport.sed > r1
real 0m0.710s
user 0m0.705s
sys 0m0.008s
time cat report.xml.0 | sh parsegtestreport.sh > r2
real 0m25.066s
user 0m17.759s
sys 0m9.506s
[rrelyea@localhost common]$ diff r1 r2
updated: with review comments from Martin and move the report parsing to the common code so it can be shared with both ssl_gtests and gtests shell scripts.
Differential Revision: https://phabricator.services.mozilla.com/D120028
|
| |
|
|
|
|
| |
When NSS is in FIPS mode, it should reject all primes smaller than 2048. The ike 1536 prime is in the accepted primes table. In FIPS mode it should be rejected.
Differential Revision: https://phabricator.services.mozilla.com/D119895
|
| |
|
|
|
|
|
|
| |
Some of our servers could cause random failures when trying to generate many key pairs from multiple threads. This is caused because some threads would starve long enough for them to give up on getting a begin transaction on sqlite. sqlite only allows one transaction at a time.
Also, there were some bugs in error handling of the broken transaction case where NSS would try to cancel a transation after the begin failed (most cases were correct, but one case in particular was problematic).
Differential Revision: https://phabricator.services.mozilla.com/D120032
|
| |
|
|
|
|
| |
A number of coverity/scanner issues were found in the kdf code which was added in nss 3.44 and the fixes never upstreamed, as well as coverity/scanner errors in nss 3.66. Not all errors were fixed, those errors which were determined to be false positives were just recorded. No attempt has been made to fix coverity/scanner errors in gtests.
Differential Revision: https://phabricator.services.mozilla.com/D119829
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D119912
|
| |
|
|
| |
Last rebase we submitted a patch that used a subdirectory to measure the performance for the SQLite patch. This code wasn't active by default on linux, however, because of a typo in the build system. This is a low priority issue since NSS does not default to measure, so the patch only affects older versions of RHEL or users that have explicitly asked for 'measure' semantics.
|
| |
|
|
| |
The "gtar" command doesn't usually exist on Linux.
|
| | |
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D119045
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D118368
|
| |
|
|
|
| |
patch by Christoph Walcher
r=rrelyea, bbeurdouche
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D115969
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
r=bbeurdouche
Before applying (on Ryzen 9 3900X)
```
# mode in opreps cxreps context op time(sec) thrgput
sha256_e 1Gb 208Mb 23M 0 0.000 10000.000 10.000 123Mb 301Kb
```
After applying
```
# mode in opreps cxreps context op time(sec) thrgput
sha256_e 5Gb 797Mb 110M 0 0.000 10000.000 10.000 591Mb 769Kb
```
Differential Revision: https://phabricator.services.mozilla.com/D116962
|
| |
|
|
|
|
|
|
|
| |
This validates that they are LDH (with underscore because we don't hate
freedom), but that they are not IP addresses. This invokes the horrible WhatWG
IP parsing routines, so that it recognizes a vast array of crazy address formats
(thanks 1980s design).
Differential Revision: https://phabricator.services.mozilla.com/D116343
|
| |
|
|
|
|
|
|
| |
r=bbeurdouche,keeler
We need this function for the rewrite of CertVerifier::VerifyCertificateTransparencyPolicy in order to calculate a certificate's lifetime in months.
Differential Revision: https://phabricator.services.mozilla.com/D118525
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
We've recently learned the value of ALPN and SNI when it comes to protecting
against cross-protocol attacks. However, some protocols don't have ALPN yet.
For servers that terminate connections for those connections, validating that
the client has not offered ALPN provides a way to protect against cross-protocol
attacks. If the cross-protocol attack uses a protocol that does include ALPN,
being able to reject those connections safely reduces exposure.
This modifies SSL_SetNextProtoNego() to accept a zero-length buffer as an
argument. Previously, this would have crashed. Now it causes the server to
reject a handshake if ALPN is offered by the client.
It was always possible to implement this by passing a function that always
returns SECFailure to SSL_SetNextProtoCallback(). This approach has the
advantage that the server generates a no_application_protocol alert, which is
not something that user-provided code can do.
Differential Revision: https://phabricator.services.mozilla.com/D110887
|
| |
|
|
|
|
| |
incremented in 3.66). r=beurdouche
Differential Revision: https://phabricator.services.mozilla.com/D117400
|
| |
|
|
|
|
| |
r=bbeurdouche
Differential Revision: https://phabricator.services.mozilla.com/D116922
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D116297
|
| |
|
|
|
|
| |
255-byte r=beurdouche
Differential Revision: https://phabricator.services.mozilla.com/D116274
|
| | |
|
| |
|
|
|
|
| |
Depends on D115882
Differential Revision: https://phabricator.services.mozilla.com/D115883
|
| |
|
|
|
|
|
|
| |
Certification Authority. r=bwilson
Depends on D115877
Differential Revision: https://phabricator.services.mozilla.com/D115882
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D115877
|
| |
|
|
|
|
| |
Depends on D115890
Differential Revision: https://phabricator.services.mozilla.com/D115891
|
| |
|
|
|
|
| |
Depends on D115889
Differential Revision: https://phabricator.services.mozilla.com/D115890
|
| |
|
|
|
|
| |
Depends on D115888
Differential Revision: https://phabricator.services.mozilla.com/D115889
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D115888
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
the default database.
This patch does 2 things:
1) update certutil.xml pk12util.xml modutil.xml and signver.xml to reflect the fact
the the sql database is default. Many of these also has examples of specifying
sql:dirname which is now the default. I did not replace them with dbm:dirname since
we don't want to encourage regressing back. The one exception is in the paragraph
explaining how to get to the old database format.
2) I ran make in the diretory to update the .1 and .html files generated from the .xml
files. There are a number of old updates to the .xml files which haven't been picked
up in their corresponding html or man page files. This updates are included in this
patch.
It is really only necessary to review the changes to the .xml files, the rest were
reviewed when their patches were applied.
bob
Differential Revision: https://phabricator.services.mozilla.com/D115658
|
| |
|
|
|
|
|
|
| |
r=bbeurdouche
Like intel-gcm.s.
Differential Revision: https://phabricator.services.mozilla.com/D115664
|
| |
|
|
|
|
|
|
|
|
|
| |
includes blapi.h
in blapi.h, strict prototypes compiles fail on:
extern BLAKE2BContext *BLAKE2B_NewContext();
This patch fixes that problem.
Differential Revision: https://phabricator.services.mozilla.com/D115659
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
| |
Differential Revision: https://phabricator.services.mozilla.com/D114660
|
| | |
|
| | |
|
| | |
|
