731798 - Add a function to report whether the last handshake on a socket resumed a previous SSL session

Status: RESOLVED WONTFIX

(NSS :: Libraries, enhancement, P5)

Description

[Wan-Teh Chang]

Attachment: Add SSL_HandshakeResumedSession

Add a function to report whether the last handshake on a socket resumed a
previous SSL session.  The attached patch adds a SSL_HandshakeResumedSession
function to do this.

An alternative design is to export this info in SSL_GetChannelInfo.

Bob, Brian, which do you prefer?

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Right now, you can tell whether the handshake resumed a session by remembering whether or not your auth. certificate hook was called since the last time your handshake callback was called. So, currently, this function isn't strictly needed. However, I think it is a good idea to add it, as I would like to add the option to have the auth certificate hook called during resumption handshakes too in the future.

It is only safe to call this function from within a callback function or when access to the socket is serialized. (Otherwise, another thread could do a renegotiation on the same socket at the same time, potentially making the result incorrect.) You should document this.

> SSL_HandshakeResumedSession(PRFileDesc *fd, PRBool *handshake_resumed) {

The brace should be on its own line.

Comment 2

[Patrick McManus [:mcmanus]]

I'd be interested in using this in a psm patch. 

Wan-Teh - can we set some review flags and get it going again? Thanks!

Comment 3

[Patrick McManus [:mcmanus]]

Comment on attachment 601772 [details] [diff] [review]
Add SSL_HandshakeResumedSession

I'm not a peer so my review doesn't help process wise - but I certainly support this. I just wrote a psm patch based on the "did callback on a cert" logic but I would be happy to follow it up to use this if it were merged. it would certainly be more clear.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 601772 [details] [diff] [review]
Add SSL_HandshakeResumedSession

Review of attachment 601772 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozilla/security/nss/lib/ssl/sslsock.c
@@ +1507,5 @@
>      return SECSuccess;
>  }
>  
> +SECStatus
> +SSL_HandshakeResumedSession(PRFileDesc *fd, PRBool *handshake_resumed) {

The brace should be on its own line.

Since you're not acquiring any locks here, that must mean you are assuming that the locks are already acquired because this is being called only in a callback, and/or the locks are not needed. Please document in the header file when it is safe to call this function. (Only in the handshake callback?)

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.