Closed
Bug 406999
Opened 17 years ago
Closed 17 years ago
Enhance EV performance; Never combine EV with Cert Overrides
Categories
(Core :: Security: PSM, defect, P2)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
People
(Reporter: KaiE, Assigned: KaiE)
References
Details
(Keywords: perf)
Attachments
(4 files, 1 obsolete file)
6.85 KB,
patch
|
Details | Diff | Splinter Review | |
6.82 KB,
patch
|
Details | Diff | Splinter Review | |
2.65 KB,
patch
|
Details | Diff | Splinter Review | |
14.41 KB,
patch
|
rrelyea
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•17 years ago
|
||
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).
Assignee | ||
Comment 2•17 years ago
|
||
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).
Assignee | ||
Comment 3•17 years ago
|
||
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.
Attachment #291698 -
Flags: review?(rrelyea)
Assignee | ||
Comment 4•17 years ago
|
||
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.
Summary: Enhance EV performance → Enhance EV performance; Never combine EV with Cert Overrides
Assignee | ||
Comment 5•17 years ago
|
||
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.
Assignee | ||
Comment 6•17 years ago
|
||
This is the full set of all changes proposed, it's the combination of the 3 individual patches.
Attachment #291698 -
Attachment is obsolete: true
Attachment #291708 -
Flags: review?(rrelyea)
Attachment #291698 -
Flags: review?(rrelyea)
Assignee | ||
Comment 8•17 years ago
|
||
Requesting blocking1.9, because Dan had requested that for bug 406034.
Flags: blocking1.9?
Updated•17 years ago
|
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Comment 9•17 years ago
|
||
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
Attachment #291708 -
Flags: review?(rrelyea) → review+
Assignee | ||
Comment 10•17 years ago
|
||
(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.
Assignee | ||
Comment 11•17 years ago
|
||
checked in, marking fixed.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•