Each instance of CERTCertList creates a PLArena with a chunk size of 2048 bytes,
but only needs space for 3 pointers per certificate in the list. The majority of
the time Gecko uses CERTCertList, we'll store ~3 certificates (although in some
cases we do store a few hundred, such as in tests or the certificate manager).
This is fairly inefficient. This patch starts the process of avoiding using
CERTCertList in Gecko by converting nsNSSCertList (i.e. nsIX509CertList) (as
well as nsNSSCertListEnumerator) to use a more efficient data structure to hold
references to certificates long-term. Future follow-up patches could (and
should) update certificate verification APIs in PSM to avoid CERTCertList as
well.
Depends on D5096
Differential Revision: https://phabricator.services.mozilla.com/D5097
--HG--
extra : moz-landing-system : lando
nsIX509CertList.getRawCertList is only used once and doesn't provide
particularly unique functionality (its one use can easily be re-worked in terms
of other APIs). Removing this API will ease refactoring work to avoid holding
long-lived references to CERTCertList instances in nsNSSCertList.
Differential Revision: https://phabricator.services.mozilla.com/D5096
--HG--
extra : moz-landing-system : lando
There are surprisingly many of them.
(Plus a couple of unnecessary checks after `new` calls that were nearby.)
--HG--
extra : rebase_source : 47b6d5d7c5c99b1b50b396daf7a3b67abfd74fc1
In order to allow JS callers to use nsISimpleEnumerator instances with the JS
iteration protocol, we'll need to additional methods to every instance. Since
we currently have a large number of unrelated implementations, it would be
best if they could share the same implementation for the JS portion of the
protocol.
This patch adds a stub nsSimpleEnumerator base class, and updates all existing
implementations to inherit from it. A follow-up will add a new base interface
to this class, and implement the additional functionality required for JS
iteration.
Differential Revision: https://phabricator.services.mozilla.com/D3725
--HG--
extra : rebase_source : ad66d7b266856d5a750c772e4710679fab9434b1
extra : histedit_source : a83ebffbf2f0b191ba7de9007f73def6b9a955b8
Correctness improvements:
* UTF errors are handled safely per spec instead of dangerously truncating
strings.
* There are fewer converter implementations.
Performance improvements:
* The old code did exact buffer length math, which meant doing UTF math twice
on each input string (once for length calculation and another time for
conversion). Exact length math is more complicated when handling errors
properly, which the old code didn't do. The new code does UTF math on the
string content only once (when converting) but risks allocating more than
once. There are heuristics in place to lower the probability of
reallocation in cases where the double math avoidance isn't enough of a
saving to absorb an allocation and memcpy.
* Previously, in UTF-16 <-> UTF-8 conversions, an ASCII prefix was optimized
but a single non-ASCII code point pessimized the rest of the string. The
new code tries to get back on the fast ASCII path.
* UTF-16 to Latin1 conversion guarantees less about handling of out-of-range
input to eliminate an operation from the inner loop on x86/x86_64.
* When assigning to a pre-existing string, the new code tries to reuse the
old buffer instead of first releasing the old buffer and then allocating a
new one.
* When reallocating from the new code, the memcpy covers only the data that
is part of the logical length of the old string instead of memcpying the
whole capacity. (For old callers old excess memcpy behavior is preserved
due to bogus callers. See bug 1472113.)
* UTF-8 strings in XPConnect that are in the Latin1 range are passed to
SpiderMonkey as Latin1.
New features:
* Conversion between UTF-8 and Latin1 is added in order to enable faster
future interop between Rust code (or otherwise UTF-8-using code) and text
node and SpiderMonkey code that uses Latin1.
MozReview-Commit-ID: JaJuExfILM9
Before this patch, nsNSSComponent initialization would call PK11_ConfigurePKCS11
with some localized strings, which contributed to startup time. Also,
PK11_UnconfigurePKCS11 was never called, so the memory allocated to these
strings would stick around forever. This patch addresses both of these problems
by not calling PK11_ConfigurePKCS11. This means that some properties of NSS'
internal "PKCS#11 slots/tokens" have to be localized when displaying them to the
user.
MozReview-Commit-ID: BbAgbgpFfFG
--HG--
extra : rebase_source : b633da8fea683675d0c0514a378954332afeb024
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
At this point, all uses of GetPIPNSSBundleString *should* be on the main thread,
so we can just remove the nsINSSComponent version and rely on the
nsNSSCertHelper instance.
MozReview-Commit-ID: Lt7AgokGKRH
--HG--
extra : rebase_source : 95d3cf6e011468e2aa9df9bb69372ac4d3430286
In debug builds, we assert if any UTF8-to-UTF16 conversion fails. If we have
invalid UTF8 in a certificate, we don't want to assert. So, we now lossily
convert invalid UTF8 in certificates for any display purposes.
This also handles fields that are supposed to be ASCII in a similar way.
MozReview-Commit-ID: 6TdVPDTmNlh
--HG--
extra : rebase_source : 17000bd0671551bbdae534a4eaf4946c1b0beb83
These functions cause main-thread certificate verifications, which is bad for
performance. In general, nsIX509CertDB.asyncVerifyCertAtTime should be used
instead.
MozReview-Commit-ID: 9nkUDmyFY0k
--HG--
extra : rebase_source : d3e8a02e2d21e5507e71681b88f0360edf64b790
This patch moves all TLS error string handling to the frontend.
Dev-tools doesn't show the same error code as the page does anymore but only the error code as string.
All logging of these error messages has been removed.
Bug #: 1415279
Differential Revision: https://phabricator.services.mozilla.com/D607
--HG--
extra : rebase_source : 61e2d94cb21ef4c02b81448531609205c85a9707
This change is to use the higher-level structure nsNSSCertList when checking
IsChainValid so that we can use the more powerful (and tested) methods of that
object instead of the ad-hoc iterators.
This will also permit the Symantec Distrust code in Bug 1434300 to use these
methods, which keeps the code the same from the earlier Bug 1409259.
MozReview-Commit-ID: B5KmDa1JLE
--HG--
extra : rebase_source : 397d3ef7189eb6f81a1ceaf920464d9e842a8981
extra : histedit_source : 26b22257cb5fcc3389630dd0a1aba24095c46158
This adds another utility method to nsNSSCertList to perform CERT_LIST_TAIL on
the underlying certificate list and return the last entry -- e.g., the root.
This is a convenience method to let other parts of the certificate verifier
continue to work with the higher-level nsNSSCertificate objects instead of
having to convert them.
MozReview-Commit-ID: EEi9L5Iepc6
--HG--
extra : rebase_source : 2836767a7186f65debf338f8d1f2a981636ed29b
extra : histedit_source : 5b87ec6c522ac1b84d91052e21184f3c03d9ea52
As of bug 1417680, the NSS shutdown tracking infrastructure is unnecessary (and
does nothing anyway). This series of changesets removes the remaining pieces in
a way that is hopefully easy to confirm is correct.
MozReview-Commit-ID: 8Y5wpsyNlGc
--HG--
extra : rebase_source : ef6b481510d949e404a4ef5615097d66e566c947
nsIX509Cert::GetCert() returns a CERTCertificate whose reference count has
already been increased. Before this patch, when IsCertificateDistrustImminent
called CertDNIsInList(rootCert->GetCert(), RootSymantecDNs) and
CertDNIsInList(aCert->GetCert(), RootAppleAndGoogleDNs), the reference count on
those certificates would never get a corresponding decrement, so we would keep
those certificates alive until shut down. A reasonable and consistent solution
is to introduce a UniqueCERTCertificate handle in each case to own the
reference.
The status of this fix can be verified by setting MOZ_LOG="pipnss:4", running
Firefox, connecting to any host, and then shutting down. If an NSS resource
reference has been leaked, "[Main Thread]: E/pipnss NSS SHUTDOWN FAILURE" will
be in the console output. Otherwise,
"[Main Thread]: D/pipnss NSS shutdown =====>> OK <<=====" will be in the console
output.
This patch also removes nsIX509CertList::DeleteCert because it would also leak a
reference. Luckily, nothing was using it.
This patch also clarifies the implementation of nsIX509CertList::AddCert by
making the ownership transfers explicit.
MozReview-Commit-ID: 2qHo3DmhTPz
--HG--
extra : rebase_source : 42cd42d082431b4637733d8f94fcd560bdea8a44
SegmentCertificateChain, when provided a cert chain from nsISSLStatus, delivers
the EE as the Root, the Root as the EE, and the intermediates in reverse order.
Basically, now that Bug 1406856 landed, it's clear this was backward in its
thinking, so reverse it for the common case.
MozReview-Commit-ID: Ahtv9U9A9oS
--HG--
extra : rebase_source : 75c8688c5041652fd966babe91cb8c6287e19ad0
This adds two methods to nsNSSCertList: ForEachCertificateInChain, and
SegmentCertificateChain. The ForEach method calls a supplied function for each
certificate in the chain, one by one.
That method is then used by the Segment method, which (assuming the chain is
ordered) splits it into Root, End Entity, and everything in-between as a list of
Intermediates.
This patch does _not_ try to add these methods to the IDL, as it's not
straightforward to me on how to handle the nsCOMPtr or std::function arguments.
These methods will be first used by Bug 1409259.
(Update to fix gtest bustage on Linux)
MozReview-Commit-ID: 8qjwF3juLTr
--HG--
extra : rebase_source : 3dee871a4622b8ad84cca247dc9a9f3ceb3b4bd9
This adds two methods to nsNSSCertList: ForEachCertificateInChain, and
SegmentCertificateChain. The ForEach method calls a supplied function for each
certificate in the chain, one by one.
That method is then used by the Segment method, which (assuming the chain is
ordered) splits it into Root, End Entity, and everything in-between as a list of
Intermediates.
This patch does _not_ try to add these methods to the IDL, as it's not
straightforward to me on how to handle the nsCOMPtr or std::function arguments.
These methods will be first used by Bug 1409259.
MozReview-Commit-ID: 8qjwF3juLTr
--HG--
extra : rebase_source : 39e2e8530ac23c6b96eb73f406bca32a59bcccf5
This lets us replace moz_xstrdup() of string literals with AssignLiteral(),
among other improvements.
--HG--
extra : rebase_source : 9994d8ccb4f196cf63564b0dac2ae6c4370defb4
Incidentally, this means we can remove certificateUsageVerifyCA and
certificateUsageStatusResponder from CertVerifier, since we no longer use them.
MozReview-Commit-ID: Bbqn8fShfTm
--HG--
extra : rebase_source : 012cb08dcbe33fe889c9f6824959b1a02cd0bdc7
Bug 1364159 introduced an optimization that attempted to avoid reading from the
user's cached certificate database as much as possible when building a verified
certificate chain. Unfortunately this had the side-effect of not preferring root
certificates in path building, which can result in unnecessarily long chains
(which rather defeats the purpose, since it means more signature verifications).
This patch reverts the functionality changes from that bug but keeps the test
that was added (the test didn't directly test the functionality changes - it's
more of a check that path building will query the cached certificate db when
necessary).
MozReview-Commit-ID: I56THTLUytH
--HG--
extra : rebase_source : 7db9597e25b98942450840519d707046cc660781
These are all easy cases where an nsXPIDLCString local variable is set via
getter_Copies() and then is only used in ways that nsCStrings can also be used
(i.e. no null checks or implicit conversions to |char*|).
In every case the patch trivially replaces the nsXPIDLCString with an
nsCString. (Also, there are a couple of unused nsXPIDLCString variables that
the patch simply removes.)
In a profile, loading the loadable roots PKCS#11 module (i.e. the built-in root
CA module) accounted for about 60% of the time to initialize PSM/NSS. Since we
only need the roots module loaded when we're actually looking for an issuing
certificate or querying a certificate's trust, we can do the load
asynchronously (where it hopefully finishes before we actually need it, because
otherwise we'll have to wait anyway).
MozReview-Commit-ID: JyY6NtpQAUj
--HG--
extra : rebase_source : f63a697b18a409dd042289afa2b727b09f81f19f
It's silly to use prmem.h within Firefox code given that in our configuration
its functions are just wrappers for malloc() et al. (Indeed, in some places we
mix PR_Malloc() with free(), or malloc() with PR_Free().)
This patch removes all uses, except for the places where we need to use
PR_Free() to free something allocated by another NSPR function; in those cases
I've added a comment explaining which function did the allocation.
--HG--
extra : rebase_source : 0f781bca68b5bf3c4c191e09e277dfc8becffa09
CERT_CreateSubjectCertList is not an inexpensive function call, since it
enumerates the certificate database (i.e. reads from disk a lot). If we're
verifying for a TLS handshake, however, we should already have in memory a
certificate chain sent by the peer (there are some cases where we won't, such as
session resumption (see bug 731478)). If we can, we should use those
certificates before falling back to calling CERT_CreateSubjectCertList.
MozReview-Commit-ID: ASjVGsELb1O
--HG--
extra : rebase_source : 1efc635d4a98079c87f77ef3794e4b2f20eec59f
There are a few places where we can use the safer functionality provided by the
Mozilla string classes instead.
Also fixes Bug 1268657 (remove vestigial
TransportSecurityInfo::SetShortSecurityDescription declaration).
MozReview-Commit-ID: Cxv5B4bsDua
--HG--
extra : rebase_source : 074a154c9000807d6dd466f23e92289e0d4c76d8
nsIX509Cert.getAllTokenNames() is only used (improperly) to determine if a
certificate is a built-in. nsIX509Cert.isBuiltInRoot should be used instead.
MozReview-Commit-ID: LBwI8nTc05C
--HG--
extra : rebase_source : 9494cd1243395b0d293022e981f64be560a54dec
This removes findCertByNickname, findEmailEncryptionCert, and
findEmailSigningCert.
MozReview-Commit-ID: KOxWHJm3GNX
--HG--
extra : rebase_source : c67a65ce71b25c6502bad012c48aa1c30e71f334
In general, any code that was using nsIX509Cert.nickname should be able to use
the attribute displayName (if using nickname for display purposes) or the
attribute dbKey (if using nickname as a unique identifier for a certificate).
MozReview-Commit-ID: G9CfMJDfLqe
--HG--
extra : rebase_source : 1c464dab8f028568cedd5a42cf87428b8bb63fc0
This patch makes the following changes:
1. renames nsIX509Cert.windowTitle to displayName
2. removes (most of*) displayName's use of the NSS certificate nickname API
3. adds additional fields that displayName will attempt to use to find a good
name to return (namely, organizationalUnitName and organizationName)
*except for built-in roots, where we have a good hard-coded value for the name
MozReview-Commit-ID: Bt864GnOu7D
--HG--
extra : rebase_source : 5d85aaf68fd4fbe563596354a5ed50e541689934
When doing TLS session resumption, the AuthCertificate hook is bypassed, which
means that the front-end doesn't know whether or not to show the EV indicator.
To deal with this, the platform attempts an EV verification. Before this patch,
this verification lacked much of the original context (e.g. stapled OCSP
responses, SCTs, the hostname, and in particular the first-party origin key).
Furthermore, it was unclear from a code architecture standpoint that a full
verification was even occurring. This patch brings the necessary context to the
verification and makes it much more clear that it is happening. It also takes
the opportunity to remove some unnecessary EV-related fields and information in
code and data structures that don't require it.
MozReview-Commit-ID: LTmZU4Z1YXL
--HG--
extra : rebase_source : 7db702f2037fae83c87fbb6aca75b4420544dff9
Previously PSM would load EV information on-demand (i.e. just before verifying a
certificate). This simplifies this operation, removes a dubious optimization
(loading the EV information on another thread while opening a network
connection), and relocates the loading operation to when we are likely to have
good disk locality (i.e. when we've just loaded the built-in roots module).
This also removes the now-unused MOZ_NO_EV_CERTS build flag.
MozReview-Commit-ID: 8Rnl4ozF95V
--HG--
extra : rebase_source : 5b2e76079c256f7e3c55b1d4ec0d9f654fec44f6
Previously PSM would load EV information on-demand (i.e. just before verifying a
certificate). This simplifies this operation, removes a dubious optimization
(loading the EV information on another thread while opening a network
connection), and relocates the loading operation to when we are likely to have
good disk locality (i.e. when we've just loaded the built-in roots module).
This also removes the now-unused MOZ_NO_EV_CERTS build flag.
MozReview-Commit-ID: 8Rnl4ozF95V
--HG--
extra : rebase_source : 344b68c81af1ed3fb038e4e96c3c50e939d32c3d
The PR_SetError() + PR_GetError() pattern currently used is error prone and
unnecessary. The functions involved can instead return mozilla::pkix::Result,
which is equally expressive and more robust.
MozReview-Commit-ID: Hkd39eqTvds
--HG--
extra : rebase_source : f09e37c6a3a930c30cce003139df86bc84d771ee
Ehsan Akhgari
Sylvestre Ledru
Dana Keeler
Franziskus Kiefer
Nicholas Nethercote
Kris Maglione
Henri Sivonen
Csoregi Natalia
Emilio Cobos Álvarez
J.C. Jones
Sebastian Hengst
Masatoshi Kimura
Cykesiopka
Tom Tromey
Jonathan Hao
Wes Kocher