commit fc0c9d96652f317b3c08e71c81f7a7fcb0246584
author David Benjamin <davidben@google.com> Thu Sep 01 01:33:33 2016 -0400
committer Adam Langley <agl@google.com> Mon Sep 12 19:00:50 2016 +0000
tree 56057910fed0ebadd3b685908580ebc444904ed9
parent 5510863fbdb24867d80d607a78a1ac6437c9ba26

Use a common state to begin the handshake.

This simplifies the logic around SSL_clear to reset the state for a new
handshake. The state around here is still a little iffy, but this is a
slight improvement.

The SSL_ST_CONNECT and SSL_ST_ACCEPT states are still kept separate to
avoid problems with the info callback reporting SSL_ST_INIT. Glancing
through info callback consumers, although they're all debugging, they
tend to assume that all intermediate states either have only
SSL_ST_CONNECT set or only SSL_ST_ACCEPT set.

(They also all look identical which makes me think it's copy-and-pasted
from OpenSSL command-line tool or something.)

Change-Id: I55503781e52b51b4ca829256c14de6f5942dae51
Reviewed-on: https://boringssl-review.googlesource.com/10760
Reviewed-by: Adam Langley <agl@google.com>

ssl/handshake_client.c

diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 93562e57..9a5cd02 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -196,6 +196,11 @@
     state = ssl->state;
 
     switch (ssl->state) {
+      case SSL_ST_INIT:
+        ssl->state = SSL_ST_CONNECT;
+        skip = 1;
+        break;
+
       case SSL_ST_CONNECT:
         ssl_do_info_callback(ssl, SSL_CB_HANDSHAKE_START, 1);
 

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 20986ec..c5bd4be 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -197,6 +197,11 @@
     state = ssl->state;
 
     switch (ssl->state) {
+      case SSL_ST_INIT:
+        ssl->state = SSL_ST_ACCEPT;
+        skip = 1;
+        break;
+
       case SSL_ST_ACCEPT:
         ssl_do_info_callback(ssl, SSL_CB_HANDSHAKE_START, 1);
 

ssl/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 4c3b40e..bdc1074 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -393,6 +393,8 @@
   ssl->min_version = ctx->min_version;
   ssl->max_version = ctx->max_version;
 
+  ssl->state = SSL_ST_INIT;
+
   /* RFC 6347 states that implementations SHOULD use an initial timer value of
    * 1 second. */
   ssl->initial_timeout_duration_ms = 1000;
@@ -529,13 +531,11 @@
 
 void SSL_set_connect_state(SSL *ssl) {
   ssl->server = 0;
-  ssl->state = SSL_ST_CONNECT;
   ssl->handshake_func = ssl3_connect;
 }
 
 void SSL_set_accept_state(SSL *ssl) {
   ssl->server = 1;
-  ssl->state = SSL_ST_ACCEPT;
   ssl->handshake_func = ssl3_accept;
 }
 
@@ -681,7 +681,7 @@
 
   /* Begin a new handshake. */
   ssl->s3->total_renegotiations++;
-  ssl->state = SSL_ST_CONNECT;
+  ssl->state = SSL_ST_INIT;
   return 1;
 
 no_renegotiation:
@@ -2887,25 +2887,13 @@
     return 0;
   }
 
-  /* SSL_clear may be called before or after the |ssl| is initialized in either
-   * accept or connect state. In the latter case, SSL_clear should preserve the
-   * half and reset |ssl->state| accordingly. */
-  if (ssl->handshake_func != NULL) {
-    if (ssl->server) {
-      SSL_set_accept_state(ssl);
-    } else {
-      SSL_set_connect_state(ssl);
-    }
-  } else {
-    assert(ssl->state == 0);
-  }
-
   /* TODO(davidben): Some state on |ssl| is reset both in |SSL_new| and
    * |SSL_clear| because it is per-connection state rather than configuration
    * state. Per-connection state should be on |ssl->s3| and |ssl->d1| so it is
    * naturally reset at the right points between |SSL_new|, |SSL_clear|, and
    * |ssl3_new|. */
 
+  ssl->state = SSL_ST_INIT;
   ssl->rwstate = SSL_NOTHING;
 
   BUF_MEM_free(ssl->init_buf);