406999 - Enhance EV performance; Never combine EV with Cert Overrides

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P2)

Description

[Kai Engert (:KaiE:)]

As of today, PSM will call into NSS for EV verification (function CERT_PKIXVerifyCert) quite often. It will make such a call whenever the Mozilla UI requests information related to EV.

Right now that means:
- twice for every page load
- again twice each time you click a link, 
  even though you remain on the same site

This should be optimized.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: caching v1

This is a patch to implement caching of EV status.
The cache is limited to the lifetime of a wrapper object around a raw NSS cert object.

This means we'll call into CERT_PKIXVerifyCert only once for each page load (where the server cert has any EV Oid).

Comment 2

[Kai Engert (:KaiE:)]

Attachment: performance v1

As of today, each time we navigate from one web page to the next, we throw away the cert from the current page, receive new callbacks from NSS, will create a new PSM wrapper object around the cert, and associate it with the new page.

With such a simple implementation it's impossible to make use of caching in the PSM object.

This patch implements a mechanism to extend the lifetime of PSM cert objects.

You should know that each SSL socket will produce a socket object having a reference to the associated cert. It is possible to query information about the "container" where this socket lives in, where "container" is the document window.

The idea is, each time we produce a new socket object, we obtain the certificate object currently associated to our parent document window. The socket object will remember a pointer to that current cert.

Later, when the handshake is completed, NSS will notify us with the cert used by this socket. At that point we can compare the certs. If equal, we may continue to use the old PSM wrapper object, and keep all the data it may have cached (including EV status).

Comment 3

[Kai Engert (:KaiE:)]

Attachment: Patch v1

This patch combines "caching v1" and "performance v1" patches.

I'm using this one for approvals and reviews.
You might want to look at the individual patches for easier reviewing.

Comment 4

[Kai Engert (:KaiE:)]

Merging conflicts strike again!!!
I intended to fix the following as a separate bug, but the changes conflict, so I'm adding it to the scope of this bug... :-/


=======
Request : Never show EV UI when a cert override is effective
=======

Now you might wonder:
EV blessing depends on information added at compile time and thus must refer to one of the certs that are built-in. So, how could you ever have an exception for a server cert issued by one of the built-in roots?

Well, an end user might decide to edit CA trust for whatever reason. He might decide to revoke the trust for Verisign. But for some reason, the same user decides to make an exception for PayPal and adds an override.

As of today, you'll get the EV UI treatment.

This is probably a strange edge case. But if a "manual trust override" if effective, we should probably not show the EV UI.

Comment 5

[Kai Engert (:KaiE:)]

Attachment: pseudo patch, never combine ev with cert errors

This is not a real patch, it contains only the snippets that are used especially to achieve:
  "never combine EV with cert overrides"

Only used for assisting reviewing.

Comment 6

[Kai Engert (:KaiE:)]

Attachment: Patch v2

This is the full set of all changes proposed, it's the combination of the 3 individual patches.

Comment 8

[Kai Engert (:KaiE:)]

Requesting blocking1.9, because Dan had requested that for bug 406034.

Comment 9

[Robert Relyea]

Comment on attachment 291708 [details] [diff] [review]
Patch v2

r+

but I would like more explanation of what is going on in the 'performance code'.

particularly how prevCert and mServerCert are used.

bob

Comment 10

[Kai Engert (:KaiE:)]

(In reply to comment #9)
> but I would like more explanation of what is going on in the 'performance
> code'.
> particularly how prevCert and mServerCert are used.

The only strategy to enhance performance is the use of caching.

nsNSSCertificate is a PSM wrapper object around a CERTCertificate (with additional state). The PSM wrapper objects have a shorter lifetime than the NSS cert objects.

With this patch we attempt to extend the lifetime of the PSM wrapper object. When navigating between browser pages, we remember the currently used server cert (variable prevCert) and when we learn the new server cert, we do a comparison. If both previous and new page use the same SSL server cert, we keep using the PSM wrapper object (and avoid destroying it, creating a new one).

With this strategy, the EV verification state stored in the PSM wrapper object is preserved. Thus, future calls to obtain the EV state can make use of the cached state, avoiding to calculate the state again.

mServerCert is simply the member variable of a SSL status object, which belongs to a browser window.

Comment 11

[Kai Engert (:KaiE:)]

checked in, marking fixed.