commit ce8c9d2b41442abf25132144b220a90670d4e733
author David Benjamin <davidben@google.com> Mon Nov 14 10:45:16 2016 +0900
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Tue Dec 06 19:36:27 2016 +0000
tree cbcf37ca8a19ac8fbcc7fc9096f38ddadacd6eab
parent 7d7597840fdfc3821196e928242f06463837662f

Maintain SSL_HANDSHAKE lifetime outside of handshake_func.

We currently look up SSL_HANDSHAKE off of ssl->s3->hs everywhere, but
this is a little dangerous. Unlike ssl->s3->tmp, ssl->s3->hs may not be
present. Right now we just know not to call some functions outside the
handshake.

Instead, code which expects to only be called during a handshake should
take an explicit SSL_HANDSHAKE * parameter and can assume it non-NULL.
This replaces the SSL * parameter. Instead, that is looked up from
hs->ssl.

Code which is called in both cases, reads from ssl->s3->hs. Ultimately,
we should get to the point that all direct access of ssl->s3->hs needs
to be NULL-checked.

As a start, manage the lifetime of the ssl->s3->hs in SSL_do_handshake.
This allows the top-level handshake_func hooks to be passed in the
SSL_HANDSHAKE *. Later work will route it through the stack. False Start
is a little wonky, but I think this is cleaner overall.

Change-Id: I26dfeb95f1bc5a0a630b5c442c90c26a6b9e2efe
Reviewed-on: https://boringssl-review.googlesource.com/12236
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 5893d4d..af833fb 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -878,15 +878,18 @@
   ssl_hs_private_key_operation,
 };
 
-typedef struct ssl_handshake_st {
-  /* wait contains the operation |do_handshake| is currently blocking on or
-   * |ssl_hs_ok| if none. */
+struct ssl_handshake_st {
+  /* ssl is a non-owning pointer to the parent |SSL| object. */
+  SSL *ssl;
+
+  /* wait contains the operation |do_tls13_handshake| is currently blocking on
+   * or |ssl_hs_ok| if none. */
   enum ssl_hs_wait_t wait;
 
-  /* do_handshake runs the handshake. On completion, it returns |ssl_hs_ok|.
-   * Otherwise, it returns a value corresponding to what operation is needed to
-   * progress. */
-  enum ssl_hs_wait_t (*do_handshake)(SSL *ssl);
+  /* do_tls13_handshake runs the TLS 1.3 handshake. On completion, it returns
+   * |ssl_hs_ok|. Otherwise, it returns a value corresponding to what operation
+   * is needed to progress. */
+  enum ssl_hs_wait_t (*do_tls13_handshake)(SSL *ssl);
 
   int state;
 
@@ -1022,9 +1025,9 @@
 
   /* hostname, on the server, is the value of the SNI extension. */
   char *hostname;
-} SSL_HANDSHAKE;
+} /* SSL_HANDSHAKE */;
 
-SSL_HANDSHAKE *ssl_handshake_new(enum ssl_hs_wait_t (*do_handshake)(SSL *ssl));
+SSL_HANDSHAKE *ssl_handshake_new(SSL *ssl);
 
 /* ssl_handshake_free releases all memory associated with |hs|. */
 void ssl_handshake_free(SSL_HANDSHAKE *hs);
@@ -1033,7 +1036,7 @@
  * 0 on error. */
 int tls13_handshake(SSL *ssl);
 
-/* The following are implementations of |do_handshake| for the client and
+/* The following are implementations of |do_tls13_handshake| for the client and
  * server. */
 enum ssl_hs_wait_t tls13_client_handshake(SSL *ssl);
 enum ssl_hs_wait_t tls13_server_handshake(SSL *ssl);
@@ -1760,8 +1763,8 @@
 
 int ssl3_new(SSL *ssl);
 void ssl3_free(SSL *ssl);
-int ssl3_accept(SSL *ssl);
-int ssl3_connect(SSL *ssl);
+int ssl3_accept(SSL_HANDSHAKE *hs);
+int ssl3_connect(SSL_HANDSHAKE *hs);
 
 int ssl3_init_message(SSL *ssl, CBB *cbb, CBB *body, uint8_t type);
 int ssl3_finish_message(SSL *ssl, CBB *cbb, uint8_t **out_msg, size_t *out_len);