Certificate chain is verified twice for EV certificates

RESOLVED WORKSFORME

Status

()

Core
Security: PSM
RESOLVED WORKSFORME
7 years ago
4 years ago

People

(Reporter: briansmith, Unassigned)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla6
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Currently, we verify the certificate chain at least twice for EV certs; once in AuthCertificateCallback, and then later in getValidEVOidTag (which is eventually called from AuthCertificateCallback, indirectly). Instead, we should do the EV policy OID check before the first verification, choose the appropriate (non-EV vs. EV) input parameters, and then verify the certificate chain only once with those input parameters, when we are using libpkix.
No longer blocks: 479393
No longer blocks: 651246

Comment 1

7 years ago
What is the motivation for fixing this?  Performance?  Ensuring the consistency of the site identity indicators?  (I don't see anything that can go wrong here currently, but after fixing bug 601608 we would want the trust anchor shown to be the one responsible for EV.)

The input parameters to CERT_PKIXVerifyCert for an EV verification consist of the following:
a. The EV policy OID.  (We already know it is in the EE cert.  Is passing it here redundant or is CERT_PKIXVerifyCert checking policy constraints in the chain?)
b. Revocation configuration.
c. The list of trust anchors accepted for EV with the given policy OID.

After the verification, hasValidEVOidTag rechecks that the (policy OID, actual trust anchor) pair is accepted for EV.  I believe this is redundant because the trust anchor will be one from the list in (c), which only contains accepted trust anchors.

The current implementation has the property that EV problems never interfere with the use of a certificate for basic server authentication of SSL connections (SSL_AuthCertificate).  It would be nice to maintain this so webmasters don't have to worry that they might lose customers by adopting EV.  The way to do so would be to have an automatic fallback to non-EV verification.  (It might be preferable to do a single verification with success+EV, success, and failure outcomes, but even if libpkix were hacked to drop the EV constraints and try to proceed when the first error occurs, it's not clear if the EV constraints might affect decision making prior to the first definite error or force selection of a chain that will fail other checks.  A fallback is the obviously correct solution.)

The other "certificate verified twice" issue is bug 650296, and bug 650296 comment 1 is relevant to this issue as well.  An alternative to putting the EV verification with fallback in SSL_AuthCertificate would be to try the EV verification first and, on succeeds, pass the already verified chain to SSL_AuthCertificate for further checks.  If SSL_AuthCertificate then fails, it might conceivably be necessary to try again without EV and see if SSL_AuthCertificate can find a different chain that is acceptable; I don't believe this can happen now, but it could happen with schemes like DANE.

Another approach to the entire problem is to do SSL_AuthCertificate and, on success, verify the /same chain/ for EV.  This is closer to the current design.  It ensures the consistency of indicators, but it doesn't help the performance and it may fail to recognize EV status if there is an acceptable non-EV chain and SSL_AuthCertificate chooses it.
We must continue validate with the non-libpkix validation logic in addition to the libpkix version until all the bugs that are blocking libpkix by default are fixed, especially bug 470994.
No longer blocks: 481656
Depends on: 470994, 651246
Duplicate of this bug: 705265
Assignee: bsmith → nobody

Comment 4

6 years ago
Just to echo what Matt said in Comment 1, the benefit of the two-pass approach as implemented is that it allows for EV-policy failures to fall back to DV-based verification, by virtue of making a second pass.

It's unfortunately common to see situations where a user configures an EV server as such:
  EV Cert -> Old Intermediate
With the constructed chain leading to
  EV Cert -> Old Intermediate -> Old (non-EV) root
rather than the desired EV chain
  EV Cert -> New Intermediate -> EV Root

When building the chain for EV, and performing policy OID matching, EV Cert -> Old Intermediate fails. If New Intermediate is not available (eg: no AIA / local user issues with AIA fetching), then such a chain would fail for policy mismatch.

Meanwhile, if constructed solely for DV, it would work.

Note: I use policy OIDs here, but any other 'more stringent' check during EV path building (eg: revocation check issues) can potentially lead to the same outcome.

For comparison sake:
- On OS X, Apple currently makes a two-pass implementation. However, if they detect an EV OID within the leaf, they'll try to prime the intermediates/roots to have path building go quicker. So it's also "two-pass"
- On Windows, Microsoft splits the API, so that path building/discovery is separate from policy enforcement. The path building API can return multiple chains (eg: EV -> Old -> Root, EV -> New -> Root), and will prioritize them based on criteria matching (eg: EV -> New -> Root, since leaf policy OIDs overlap with intermediate policy OIDs). The policy enforcement is then implemented as two separate verification passes, where EV may fail, but DV can find a valid chain in the list of candidate chains.

I don't believe it's possible to implement a sane one-pass verification for what is essentially two different RFC5280 inputs.

That said, much can be done with libpkix to aid in path building, such as:
- Have libpkix respect hint-certs supplied. You can then do a first pass with the DV certs, then have a second pass that is optimized by trying that chain first. Currently, libpkix resets the entire algorithm state and has to re-discover.
- Have libpkix prioritize candidate certs for exploration in a way similar to legacy code does. Currently, libpkix performs a simple bubble sort based on dates ( http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/libpkix/pkix/top/pkix_build.c&rev=1.65&mark=1832-1841#1832 / http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/libpkix/pkix/top/pkix_build.c&rev=1.65&mark=672-698#672 ).
No longer depends on: 651246
Camilo, there isn't anything more to do here since we did this as part of fixing bug 813418, right?
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Depends on: 813418
No longer depends on: 470994
Flags: needinfo?(cviecco)
Resolution: --- → WORKSFORME
Yes.. The new logic evaluates an EV once on success.
Flags: needinfo?(cviecco)
You need to log in before you can comment on or make changes to this bug.