542510 - Move certificate verification into HandleServerHelloDone

Status: RESOLVED WONTFIX

(NSS :: Libraries, defect)

Description

[Adam Langley]

Attachment: patch v1

Currently, the certificate verification callbacks are made in HandleCertificate. With OCSP stapling, a CertificateStatus message may come after the Certificate mesage. However, we would rather have the OCSP stapling information available at certificate validation time.

This patch moves the verification callbacks into HandleServerHelloDone for the client. This means that any OCSP stapled data will be ready by the time that the callbacks are made. (Note that this is only the first patch in a series - on OCSP stapling code is inclued).

I'm most worried about the badCertHandler. It can return SECWouldBlock and cause the handshake to pause. Previously, a client would pause before sending the CSS, Finished messages etc. Whilst with this patch it will do so afterwards. I'm not sure of the use case here and if this changes anything important.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

I'll guess you haven't considered the impact on the callers of 
SSL_RestartHandshakeAfterServerCert().

Comment 2

[Adam Langley]

> I'll guess you haven't considered the impact on the callers of 
> SSL_RestartHandshakeAfterServerCert().

Isn't that what I was getting at in my 3rd paragraph, above?

If the sending of the additional messages is a problem for these clients then:

1) This patch can be dropped completely. Clients that want OCSP stapling data can check the certificate in the handshake complete callback (as Chromium does).

2) HandleServerHelloDone can be split in two: the bottom half sends the messages only if the badCertHandler doesn't return SECWouldBlock. Restarting the handshake then needs to call this bottom half in addition to its current work.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Adam, 
I think the goal you're trying to achieve is worthy, and we should pursue it.
I don't wish to "throw out" your patch.  It may require a little more work 
to figure out what to do for callers of that one function.  I don't think the problem is intractable.

Comment 4

[Kai Engert (:KaiE:)]

I believe this bug can be marked as obsolete.

The SSL client protocol patches in bug 360420 will ensure that a potential cert status message gets processed prior to certificate authentication.