Make SSL False Start work with asynchronous certificate validation

net/third_party/nss/ssl/ssl.h

Index: net/third_party/nss/ssl/ssl.h
===================================================================
--- net/third_party/nss/ssl/ssl.h	(revision 227672)
+++ net/third_party/nss/ssl/ssl.h	(working copy)
@@ -121,14 +121,17 @@
 #define SSL_ENABLE_FALSE_START         22 /* Enable SSL false start (off by */
                                           /* default, applies only to       */
                                           /* clients). False start is a     */
-/* mode where an SSL client will start sending application data before      */
-/* verifying the server's Finished message. This means that we could end up */
-/* sending data to an imposter. However, the data will be encrypted and     */
-/* only the true server can derive the session key. Thus, so long as the    */
-/* cipher isn't broken this is safe. Because of this, False Start will only */
-/* occur on RSA or DH ciphersuites where the cipher's key length is >= 80   */
-/* bits. The advantage of False Start is that it saves a round trip for     */
-/* client-speaks-first protocols when performing a full handshake.          */
+/* mode where an SSL client will start sending application data before
+ * verifying the server's Finished message. This means that we could end up
+ * sending data to an imposter. However, the data will be encrypted and
+ * only the true server can derive the session key. Thus, so long as the
+ * cipher isn't broken this is safe. The advantage of false start is that
+ * it saves a round trip for client-speaks-first protocols when performing a
+ * full handshake.
+ *
+ * In addition to enabling this option, the application must register a
+ * callback using the SSL_SetCanFalseStartCallback function.
+ */
 
 /* For SSL 3.0 and TLS 1.0, by default we prevent chosen plaintext attacks
  * on SSL CBC mode cipher suites (see RFC 4346 Section F.3) by splitting
@@ -741,14 +744,46 @@
 SSL_IMPORT SECStatus SSL_InheritMPServerSIDCache(const char * envString);
 
 /*
-** Set the callback on a particular socket that gets called when we finish

[wtc, 2013/10/17 00:10:53]

Just wondering: did you delete "on a particular socket" because it's
superfluous?

[briansmith, 2013/10/17 01:43:29]

Yes

-** performing a handshake.
+** Set the callback that gets called when the TLS handshake is complete. The

[wtc, 2013/10/17 00:10:53]

Nit: perhaps "the TLS handshake" should be changed to "a TLS handshake is
complete" to allow for renegotiations?

[briansmith, 2013/10/17 01:42:40]

Done.

+** handshake callback is called after verifying the peer's Finished message and
+** before processing incoming application data. If the connection false started

[wtc, 2013/10/17 00:10:53]

Is this also true for the server side?

Also, this isn't accurate for renegotiation.

[briansmith, 2013/10/17 01:42:40]

I tried to make this clearer in the updated patch.

+** (see SSL_ENABLE_FALSE_START), then application data may already have already
+** been sent before the handshake callback is called. If the connection has not
+** false started then the callback will get called before any application data
+** is sent.
 */
 typedef void (PR_CALLBACK *SSLHandshakeCallback)(PRFileDesc *fd,
                                                  void *client_data);
 SSL_IMPORT SECStatus SSL_HandshakeCallback(PRFileDesc *fd, 
 			          SSLHandshakeCallback cb, void *client_data);
 
+/* Applications that wish to enable TLS false start must set this callback
+** function. NSS will invoke the functon to determine if a particular
+** connection should use false start or not. SECSuccess indicates that the
+** callback completed successfully, and if so *canFalseStart indicates if false
+** start can be used. If the callback does not return SECSuccess then the
+** handshake will be canceled. NSS's recommended criteria can be evaluated by
+** calling SSL_RecommendedCanFalseStart from the custom callback; it is
+** recommended that applications consider the recommended criteria as a
+** minimum requirement.

[wtc, 2013/10/17 00:10:53]

1. As I noted before, unless we document exactly what
SSL_RecommendedCanFalseStart does, an application cannot use
SSL_RecommendedCanFalseStart as the minimum requirement and perform additional
checks in its custom callback.

I agree that it is difficult to document the behavior of
SSL_RecommendedCanFalseStart. Therefore, I think SSL_RecommendedCanFalseStart
can only be used as the custom callback, rather than being called by the custom
callback.

2. I think it is useful to document that NSS will require 80 bits of security,
so that people know their custom callback only need to check the bits of
security if they require an even higher level of security.

[briansmith, 2013/10/17 01:42:40]

I think we should definitely improve this, including even improving the criteria
that SSL_RecommendedCanFalseStart actually uses. But, I think we should land
this as-is now and make those improvements later, after we've agreed on exactly
what we want to do. Many existing things are much less well-documented than this
and we do just fine.

[wtc, 2013/10/17 15:28:14]

Agreed.
I was sharing my experience writing a custom callback. I checked the
SSL_RecommendedCanFalseStart code to verify I didn't duplicate its checks.

But you are right. The most similar default callback function is probably
SSL_AuthCertificate, and its comment simply says "An implementation of the
certificate authentication hook".

+**
+** If no false start callback is registered then false start will never be
+** done.

[wtc, 2013/10/17 00:10:53]

The SSL_ENABLE_FALSE_START option should be mentioned somewhere in this comment
block.

[briansmith, 2013/10/17 01:42:40]

Done.

+**/
+typedef SECStatus (PR_CALLBACK *SSLCanFalseStartCallback)(
+    PRFileDesc *fd, void *arg, PRBool *canFalseStart);
+
+SSL_IMPORT SECStatus SSL_SetCanFalseStartCallback(
+    PRFileDesc *fd, SSLCanFalseStartCallback callback, void *arg);
+
+/* This function sets *canFalseStart according to the NSS team's recommended

[wtc, 2013/10/17 00:10:53]

Nit: remove "NSS team's". Perhaps you can rename this function back to
SSL_DefaultCanFalseStart to avoid the issue of "who recommended these criteria".

[briansmith, 2013/10/17 01:42:40]

Done.

+** criteria for false start. This criteria may change from release to release

[wtc, 2013/10/17 00:10:53]

Nit: "criteria" is plural. The singular form is "criterion".

[briansmith, 2013/10/17 01:42:40]

Done.

+** and may depend on which handshake features have been negotiated and/or
+** properties of the certifciates/keys used on the connection.
+*/
+SSL_IMPORT SECStatus SSL_RecommendedCanFalseStart(PRFileDesc *fd,
+                                                  PRBool *canFalseStart);
+
 /*
 ** For the server, request a new handshake.  For the client, begin a new
 ** handshake.  If flushCache is non-zero, the SSL3 cache entry will be