When navigating to an about: page that doesn't exist (e.g.
"about:somethingthatdoesnotexist"), the docShell will call
nsSecureBrowserUIImpl::OnLocationChange with a request that is null.
Consequently, we can't use that to QueryInterface to a nsISecurityEventSink to
call OnSecurityChange. The previous implementation would use the prior
request's nsISecurityEventSink, which was a bug but luckily this produced the
correct behavior. Since the original docShell the nsSecureBrowserUIImpl was
initialized with is what needs to be notified, we can just QueryInterface that
to an nsISecurityEventSink and call OnSecurityChange directly instead.
Differential Revision: https://phabricator.services.mozilla.com/D6951
--HG--
rename : browser/base/content/test/siteIdentity/browser_tls_handshake_failure.js => browser/base/content/test/siteIdentity/browser_navigation_failures.js
extra : moz-landing-system : lando
This also removes the (afaict, unused) stub implementation from TabParent. The netwerk header
inclusions were necessary because those files included TabParent.h and through it,
nsISecureBrowserUI, but now TabParent.h no longer does that.
Differential Revision: https://phabricator.services.mozilla.com/D6829
--HG--
extra : moz-landing-system : lando
The site identity security indicator machinery treats connections where the TLS
handshake failed as insecure (also referred to as "unknown identity"). Before
bug 1468222, such cases were easily detectable as the SSLStatus field of the
relevant nsITransportSecurityInfo would be null. When we merged nsISSLStatus
into nsITransportSecurityInfo, we didn't take this differentiation into account.
This patch brings back the prior behavior by checking if the securityInfo's
securityState indicates that the handshake failed (i.e. it is
STATE_IS_INSECURE).
Differential Revision: https://phabricator.services.mozilla.com/D6316
--HG--
extra : moz-landing-system : lando
Everything that goes in a PLDHashtable (and its derivatives, like
nsTHashtable) needs to inherit from PLDHashEntryHdr. But through a lack
of enforcement, copy constructors for these derived classes didn't
explicitly invoke the copy constructor for PLDHashEntryHdr (and the
compiler didn't invoke the copy constructor for us). Instead,
PLDHashTable explicitly copied around the bits that the copy constructor
would have.
The current setup has two problems:
1) Derived classes should be using move construction, not copy
construction, since anything that's shuffling hash table keys/entries
around will be using move construction.
2) Derived classes should take responsibility for transferring bits of
superclass state around, and not rely on something else to handle that.
The second point is not a huge problem for PLDHashTable (PLDHashTable
only has to copy PLDHashEntryHdr's bits in a single place), but future
hash table implementations that might move entries around more
aggressively would have to insert compensation code all over the
place. Additionally, if moving entries is implemented via memcpy (which
is quite common), PLDHashTable copying around bits *again* is
inefficient.
Let's fix all these problems in one go, by:
1) Explicitly declaring the set of constructors that PLDHashEntryHdr
implements (and does not implement). In particular, the copy
constructor is deleted, so any derived classes that attempt to make
themselves copyable will be detected at compile time: the compiler
will complain that the superclass type is not copyable.
This change on its own will result in many compiler errors, so...
2) Change any derived classes to implement move constructors instead of
copy constructors. Note that some of these move constructors are,
strictly speaking, unnecessary, since the relevant classes are moved
via memcpy in nsTHashtable and its derivatives.
A previous patch in this bug made the incorrect assumption that we had disabled
the family safety root detection/importing feature by default. In reality, we
enabled it by default in bug 1282871.
In bug 1487258 we moved enterprise root loading to a background thread so as to
not block the main thread. This patch does the same with the family safety
feature.
Differential Revision: https://phabricator.services.mozilla.com/D5484
--HG--
extra : moz-landing-system : lando
I also changed security/certverifier/moz.build a bit while I am here:
* Using '-Xclang' to pass through '-Wall' on clang-cl.
* Now clang-cl will take clang/gcc path because most '-wd****' options have no
effect on clang-cl. '-wd4010' will have an effect, but we already have the
corresponding clang/gcc option ('-Wno-unused-parameter').
--HG--
extra : source : df566a1bd9087cc0bfc03fe19fd9d21bf58f5d9c
nsIAssociatedContentSecurity and nsISecurityInfoProvider are unused as of
bug 832834, so this patch removes them.
Differential Revision: https://phabricator.services.mozilla.com/D5693
--HG--
extra : moz-landing-system : lando
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
Background here is that we are disabling a piece of the downgrade protection in TLS 1.3 and we want to turn it on. We don't know if that is safe, so a pref (and an associated experiment) seems prudent. This is that pref.
Differential Revision: https://phabricator.services.mozilla.com/D4629
--HG--
extra : moz-landing-system : lando
Several source files use DLL_PREFIX/DLL_SUFFIX defines, and they all set
them in moz.build using `DEFINES`. This is problematic for the WSL
build because the quoting gets lost somewhere between bash and cl.exe.
We cannot simply set them globally in moz.configure because their
stringified definitions would conflict with the `set_config` of
DLL_PREFIX/DLL_SUFFIX. Therefore, we globally define
MOZ_DLL_PREFIX/MOZ_DLL_SUFFIX and change all define-related uses of
DLL_PREFIX/DLL_SUFFIX to use their MOZ-equivalents instead.
Move all fields of nsISSLStatus to nsITransportSecurityProvider
Remove nsISSLStatus interface and definition
Update all code and test references to nsISSLStatus
Maintain ability to read in older version of serialized nsISSLStatus. This
is verified with psm_DeserializeCert gtest.
Differential Revision: https://phabricator.services.mozilla.com/D3704
--HG--
extra : moz-landing-system : lando
Move all fields of nsISSLStatus to nsITransportSecurityProvider
Remove nsISSLStatus interface and definition
Update all code and test references to nsISSLStatus
Maintain ability to read in older version of serialized nsISSLStatus. This
is verified with psm_DeserializeCert gtest.
Differential Revision: https://phabricator.services.mozilla.com/D3704
--HG--
extra : moz-landing-system : lando
As of bug 1346297, we don't collect telemetry for the family safety root
feature. At this point, it makes the most sense to disable the entire feature by
default.
Differential Revision: https://phabricator.services.mozilla.com/D4994
--HG--
extra : moz-landing-system : lando
As of bug 1346297, we don't collect telemetry for the family safety root
feature. At this point, it makes the most sense to disable the entire feature by
default.
Differential Revision: https://phabricator.services.mozilla.com/D4994
--HG--
extra : moz-landing-system : lando
Loading enterprise roots could potentially take a while, so we certainly
shouldn't do it on the main thread at startup. Note that this doesn't address
the case where a user enables the feature while Firefox is running. This isn't
great but since it's an about:config preference rather than a first-class
preference exposed in about:preferences, we can probably get away with it for
now.
Differential Revision: https://phabricator.services.mozilla.com/D4708
--HG--
extra : moz-landing-system : lando
It turns out nsSecureBrowserUIImpl is considerably more complicated than it
needs to be. This patch reimplements it in terms of OnLocationChange only, which
is all it needs to produce the same behavior as before.
Differential Revision: https://phabricator.services.mozilla.com/D3548
--HG--
extra : moz-landing-system : lando
Most of the times when we automatically create nsThread wrappers for threads
that don't already have them, we don't actually need the event targets, since
those threads don't run XPCOM event loops. Aside from wasting memory, actually
creating these event loops can lead to leaks if a thread tries to dispatch a
runnable to the queue which creates a reference cycle with the thread.
Not creating the event queues for threads that don't actually need them helps
avoid those foot guns, and also makes it easier to figure out which treads
actually run XPCOM event loops.
MozReview-Commit-ID: Arck4VQqdne
--HG--
extra : source : a03a61d6d724503c3b7c5e31fe32ced1f5d1c219
extra : intermediate-source : 5152af6ab3e399216ef6db8f060c257b2ffbd330
extra : histedit_source : ef06000344416e0919f536d5720fa979d2d29c66%2C4671676b613dc3e3ec762edf5d72a2ffbe6fca3f
Most of the times when we automatically create nsThread wrappers for threads
that don't already have them, we don't actually need the event targets, since
those threads don't run XPCOM event loops. Aside from wasting memory, actually
creating these event loops can lead to leaks if a thread tries to dispatch a
runnable to the queue which creates a reference cycle with the thread.
Not creating the event queues for threads that don't actually need them helps
avoid those foot guns, and also makes it easier to figure out which treads
actually run XPCOM event loops.
MozReview-Commit-ID: Arck4VQqdne
--HG--
extra : rebase_source : fcf8fa50e748c4b54c3bb1997575d9ffd4cbaae1
extra : source : a03a61d6d724503c3b7c5e31fe32ced1f5d1c219
In bug 1279479 and bug 1316300 we hid some ciphersuites from TLS 1.3
handshakes, assuming we would fall back to TLS 1.2 if the peer needed them.
However, as of bug 1479501, we don't fall back by default, so this just means
we can't negotiate these ciphersuites. This patch un-hides these ciphersuites
from the TLS 1.3 handshake.
Differential Revision: https://phabricator.services.mozilla.com/D4725
--HG--
extra : moz-landing-system : lando
There is a late-breaking EV compatibility concern with cross signatures for EV
certificates:
Firefox's EV handling code always validates EV using the first EV policy OID
expressed in a certificate. For compatibility certificates issued under a cross-
signed root, if the first EV policy OID matches the original Symantec EV policy
OID, then Firefox will attempt to verify that the root CA matches the original
Symantec EV CA -- which it won't, as the root will be one of DigiCert's. Without
a patch, EV treatment will break.
This patch removes all EV policy OIDs for roots mentioned in TrustOverride-
SymantecData.inc, letting the moz::pkix algorithm pick other EV policy OIDs to
validate. I verified that I removed all affected OIDs using the BASH shell
commands:
$ cd security/certverifier
$ grep "CN=" TrustOverride-SymantecData.inc | sed -e 's/.*\(CN=.*\).*/\1/' |
sort | uniq | while read r; do
echo $r; grep "$r" ExtendedValidation.cpp;
done
Reviewers should help me ensure that I did not remove any unexpected EV policy
OIDs.
Differential Revision: https://phabricator.services.mozilla.com/D4709
--HG--
extra : moz-landing-system : lando
This does not change the outward behavior of LossyUTF8ToUTF16(). Both
ToNewUnicode() and CopyASCIItoUTF16() convert from Latin1 to UTF-16.
MozReview-Commit-ID: 8SDgvoGaN4A
Differential Revision: https://phabricator.services.mozilla.com/D4639
--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
As initially implemented, nsITLSServerSocket by default enabled the use of the
TLS session cache provided by NSS. However, no consumers of nsITLSServerSocket
actually used it. Because it was an option, though, PSM had to jump through some
hoops to a) make it work in the first place and b) not have NSS panic on
shutdown. Furthermore, it meant increased memory usage for every user of Firefox
(and again, nothing actually used the feature, so this was for naught).
In bug 1479918, we discovered that if PSM shut down before Necko, NSS could
attempt to acquire a lock on the session cache that had been deleted, causing a
shutdown hang. We probably should make it less easy to make this mistake in NSS,
but in the meantime bug 1479918 needs uplifting and this workaround is the
safest, most straight-forward way to achieve this.
Differential Revision: https://phabricator.services.mozilla.com/D3919
--HG--
extra : moz-landing-system : lando
This allows JS callers to automatically get the correct types during
interation, without having to explicitly specify them.
Differential Revision: https://phabricator.services.mozilla.com/D3728
--HG--
extra : rebase_source : b708f382d8ea571d199c669bfed5b5a7ca9ffac4
extra : histedit_source : 7df6feb82088c8a5ca45dc28fe4d2b852c177fee
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
Add StartOpenBSDSandbox method calling pledge() syscall,
and use it where we're sandboxing processes.
The pledge subsets are coming from two new prefs:
- security.sandbox.pledge.content for the content process
- security.sandbox.pledge.main for the main process
--HG--
extra : rebase_source : 60da70e2d335755fda6126a6b7de7aad41eebb7e
- Remove the viewCert method from nsICertificateDialogs
- Remove all associated C++ code
- Directly invoke UI window where it was previous called.
- Update tests
MozReview-Commit-ID: 9b62Go0DjE9
Differential Revision: https://phabricator.services.mozilla.com/D3358
--HG--
extra : moz-landing-system : lando
Closures are nice but -- as pointed out in bug 1481978 comment #2 --
it's a footgun to take a std::function argument in a context where heap
allocation isn't safe.
Fortunately, non-capturing closures convert to C function pointers,
so a C-style interface with a void* context can still be relatively
ergonomic.
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
When the HSTS preload script was reworked to use async/await in bug 1436369,
`fetchstatus` would create an asynchronous xml http request and then attempt to
access a response header from it. However, there was nothing to ensure that the
request had completed before this code ran. This patch ensures that the request
has completed before the response header is used.
This patch also replaces a lingering instance of `Ci.nsISSLStatusProvider` that
should have been changed to `Ci.nsITransportSecurityInfo` in bug 1475647.
Finally, this patch removes the old, redundant getHSTSPreloadList.js in
security/manager/tools as well as the unused nsSTSPreloadList.errors file in
security/manager/ssl.
Differential Revision: https://phabricator.services.mozilla.com/D2807
--HG--
extra : moz-landing-system : lando
This patch implements the Symantec distrust plan on Nightly only for now.
Differential Revision: https://phabricator.services.mozilla.com/D2959
--HG--
extra : moz-landing-system : lando
- enhance nsIX509CertDB.importPKCS12File to accept a password and return error code.
- enhance nsIX509CertDB.exportPKCS12File to accept a password and return error code.
- remove password and error prompts being invoked in C++ layer to Javascript layer.
- update unit tests
- add unit test for importing certs with empty string password and no passwords.
- remove unused code
MozReview-Commit-ID: 23ypAzBarOp
--HG--
extra : rebase_source : df608a240c6fa7ce4278145861e57882f0803e02
Right now, a lot of test code relies on side-effects of SpecialPowers being
loaded into frame script globals. In particular:
- It forces permissive COWs from those scopes, which allows frame scripts to
pass objects from those scopes to unprivileged content that they otherwise
wouldn't.
- It imports a bunch of helper modules and WebIDL globals which would
otherwise not be available.
Fortunately, this seems to only impact test code at this point. But there's a
real down-the-road risk of it impacting shipping code, which ends up working
in automation due to the side-effects of SpecialPowers, but failing in real
world use.
MozReview-Commit-ID: G27eSSOHymX
--HG--
extra : rebase_source : 1702e63fed719fc92def2bdbbb8a7c53572432db
extra : source : 41bedc526dd6ec6b7e8c7be1c832ac60c81d6263
Summary:
The plan is to also expose perfecthash.py from this module on the python path.
This also allows us to stop using explicit module loading to load make_dafsa.py.
make_dafsa.py was moved into tools/ to avoid any extra python files from
accidentally ending up on the python path.
Reviewers: froydnj!
Tags: #secure-revision
Bug #: 1479484
Differential Revision: https://phabricator.services.mozilla.com/D2614
--HG--
rename : xpcom/ds/make_dafsa.py => xpcom/ds/tools/make_dafsa.py
In some cases, nsNSSComponent functions were acquiring nsNSSComponent's mMutex
to check mNSSInitialized to see if it had been initialized. It turns out this is
unnecessary in some cases because those functions are only callable if
nsNSSComponent has been initialized. This fixes those instances and renames
'mNSSInitialized' to 'mNonIdempotentCleanupMustHappen' to make it clear exactly
what that boolean represents.
Differential Revision: https://phabricator.services.mozilla.com/D2577
--HG--
extra : moz-landing-system : lando
OS key-store adapter for Windows Credential Manager.
It looks like Windows doesn't allow locking the credential manager without locking the desktop. So `lock` and `unlock` are no-ops here.
Depends on D2487.
Differential Revision: https://phabricator.services.mozilla.com/D2550
--HG--
extra : moz-landing-system : lando
Ehsan Akhgari
Noemi Erli
J.C. Jones
Dana Keeler
Coroiu Cristina
ffxbld
Boris Zbarsky
shindli
Gijs Kruitbosch
Mark Goodwin
Andrew McCreight
Haik Aftandilian
Dragana Damjanovic
Gurzau Raul
Andreea Pavel
Nathan Froyd
Bogdan Tara
Jed Davis
Masatoshi Kimura
Martin Thomson
Ted Mielczarek
Cosmin Sabou
Dipen Patel
Daniel Varga
Csoregi Natalia
Mike Hommey
Tiberius Oros
Jonathan Kingston
Brindusan Cristian
Kris Maglione
trisha
Kai Engert
Dave Townsend
arthur.iakab
Narcis Beleuzu
Henri Sivonen
Alexander Surkov
Jim Mathies
Nicholas Nethercote
Johann Hofmann
Brendan Dahl
Landry Breuil
Robert Helmer
Gian-Carlo Pascutto
Franziskus Kiefer
Dorel Luca
Ciure Andrei
Nika Layzell