162033 - certificate manager should not do certificate verification within the paint routine

Status: RESOLVED WORKSFORME

(Core Graveyard :: Security: UI, enhancement, P2)

Description

[Julien Pierre]

Currently, when you scroll, resize, or move another window over the certificate
manager window, the repaint message causes PSM to recompute the validation
status of every certificate.

This ends up calling CERT_VerifyCert over again for the same certificate. This
function is not guaranteed to return immediately, and can in fact be very long
lived if it needs to lookup CRLs. For that reason, it should not be called
during the paint routine.

The result currently is that you end up with an unpainted window and an
hourglass. You can't do anything else in mozilla, and you have to wait for the
verification of all certificates. If you resize the window, and move the mouse,
it will send several repaint messages and this will happen over and over again.

On my 450 MHz system with my database and my CRLs (just Thawte and AOL CRLs and
about 12 user certs), the repaint alone currently takes 20 seconds in the
Mozilla 1.1 tree, all spent in certificate verification, even with the NSS 3.6
optimizations. So you can see what this does to the Mozilla user interface
responsiveness - it's totally blocked for that duration. 

On OS/2, the behavior even worse - the whole Presentation manager UI is blocked
during the 20 seconds of the cert verification, no other GUI application can be
used on the machine. The rule in OS/2 PM applications is that you must never
perform any operation that takes more than 10 milliseconds to complete within a
paint message winproc - you must use other threads to perform them. Mozilla
blatantly breaks this rule here. If you have a multiprocessor machine, as I do
at home, this is even more frustrating, as only one CPU is busy doing the
certificate verification, and the second one should be usable for other Mozilla
browsing, or running other applications, but the entire Mozilla UI is still blocked.

So in summary, this doesn't just hurt the UI responsiveness on OS/2 but for
Mozilla on all platforms. In OS/2 the whole OS UI is blocked, while on other
platforms it's "just" all the Mozilla windows that are blocked. 

There is another good reason to move the certificate verification out of the
paint message processing : OCSP. Currently, we disable OCSP completely for the
certificate list window. Eventually, we should enable that feature. It is a
security issue - because other non-UI threads in mozilla that are still running
and possibly doing SSL I/O suddenly stop doing OCSP validation. However, the
OCSP query can take a very long amount of time, so we definitely can never
afford to do it in the paint message processing, as it would block all of the
Mozilla UI.

Here are my suggestions of an implementation to fix this problem :
1. first, you need to stop doing everything over again in the paint message, so
that a simple resize or click on the scroll bar doesn't cause all the certs to
be reverified. The processing below should be done only the first time the
certificate manager is open, or when the user changes the tab that is being
displayed (eg. from "other people" to "your certificates"). For the later case,
we may not even want to verify the certs again if they have been recently
verified (ie. in the last couple of minutes) so that we don't incur overhead for
users who quickly switch between tabs
2.  When displaying the list of certs in the tab, fill in the "verified" and
"purposes" fields with a string saying something like "computing status". All
the other fields can be queried from the CERTCertificate quickly without
overhead, so they can be painted
3. when a new tab is selected, start a thread to verify all the certs in that
tab. The thread will then update the "verified" and "purposes" for each cert.
This verification status should be saved in a PSM structure wrapping the
CERTCertificate, not just displayed in the GUI, which should of course also be
done when the status is available. The GUI update could be done after each cert
is verified one at a time, or after all the certs in the tab are verified. This
thread needs to be designed carefully so that we can let go of it when PSM is
closed or tabs are changed. The earliest that can be done is after the current
cert verification, so there should be some notification in the loop.

Comment 1

[Kai Engert (:KaiE:)]

I agree. What you describe would be the correct way to implement it.

I suspect it has not been done initially, because the current approach was much
simpler to implement.

We could split this bug into two separate bugs. I believe you are suggesting:

1.) Verify only once while cert manager is displayed. Cache the verification
results. When required to paint, use the cached information. Clear the cache
when cert manager is closed.

2.) Enhance cert manager to actually do OCSP if it is enabled. Redesign cert
manager behaviour. Dynamically update contens of "verified purposes" column as
verification results become available. If information is not yet available,
display "n/a".

Comment 2

[Julien Pierre]

Kai,

Yes, this is an accurate summary of what I was proposing.

Comment 3

[Stephane Saux]

cc rangan. Rangan, please track this issue in blackflag if necessary.

Comment 4

[Stephane Saux]

Kai

Comment 5

[Kai Engert (:KaiE:)]

This bug no longer applies.

We no longer do OCSP verification for all certs shown in the lists. We only verify them when a user clicks "view" or when we actually use a cert.

Also, the OCSP verification triggered from certificate viewer is now done asynchronously.