commit 87e4acd2f5543c3498b979a97f9c9904c849f60c
author David Benjamin <davidben@chromium.org> Thu Apr 02 19:57:35 2015 -0400
committer Adam Langley <agl@google.com> Mon Apr 06 17:39:46 2015 +0000
tree c1a0d66318c6808db76c108a4fe48fe3692aafdf
parent 513f0ea8cd1ca57ec833ef6343b44656e96cd4b7

Test the interaction of SSL_CB_HANDSHAKE_DONE and False Start.

Based on whether -false-start is passed, we expect SSL_CB_HANDSHAKE_DONE to or
not to fire. Also add a flag that asserts SSL_CB_HANDSHAKE_DONE does *not* fire
in any False Start test where the handshake fails after SSL_connect returns.

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

ssl/test/bssl_shim.cc

diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index d79074b..b22819e 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -73,6 +73,7 @@
   ScopedSSL_SESSION session;
   ScopedSSL_SESSION pending_session;
   bool early_callback_called = false;
+  bool handshake_done = false;
 };
 
 static void TestStateExFree(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
@@ -88,7 +89,7 @@
   return SSL_set_ex_data(ssl, g_config_index, (void *)config) == 1;
 }
 
-static const TestConfig *GetConfigPtr(SSL *ssl) {
+static const TestConfig *GetConfigPtr(const SSL *ssl) {
   return (const TestConfig *)SSL_get_ex_data(ssl, g_config_index);
 }
 
@@ -96,7 +97,7 @@
   return SSL_set_ex_data(ssl, g_clock_index, (void *)clock) == 1;
 }
 
-static OPENSSL_timeval *GetClockPtr(SSL *ssl) {
+static OPENSSL_timeval *GetClockPtr(const SSL *ssl) {
   return (OPENSSL_timeval *)SSL_get_ex_data(ssl, g_clock_index);
 }
 
@@ -108,7 +109,7 @@
   return false;
 }
 
-static TestState *GetTestState(SSL *ssl) {
+static TestState *GetTestState(const SSL *ssl) {
   return (TestState *)SSL_get_ex_data(ssl, g_state_index);
 }
 
@@ -321,6 +322,18 @@
   return 1;
 }
 
+static void InfoCallback(const SSL *ssl, int type, int val) {
+  if (type == SSL_CB_HANDSHAKE_DONE) {
+    if (GetConfigPtr(ssl)->handshake_never_done) {
+      fprintf(stderr, "handshake completed\n");
+      // Abort before any expected error code is printed, to ensure the overall
+      // test fails.
+      abort();
+    }
+    GetTestState(ssl)->handshake_done = true;
+  }
+}
+
 // Connect returns a new socket connected to localhost on |port| or -1 on
 // error.
 static int Connect(uint16_t port) {
@@ -435,6 +448,8 @@
 
   ssl_ctx->current_time_cb = CurrentTimeCallback;
 
+  SSL_CTX_set_info_callback(ssl_ctx.get(), InfoCallback);
+
   return ssl_ctx;
 }
 
@@ -686,6 +701,13 @@
       return false;
     }
 
+    bool expect_handshake_done = is_resume || !config->false_start;
+    if (expect_handshake_done != GetTestState(ssl.get())->handshake_done) {
+      fprintf(stderr, "handshake was%s completed\n",
+              GetTestState(ssl.get())->handshake_done ? "" : " not");
+      return false;
+    }
+
     if (config->is_server && !GetTestState(ssl.get())->early_callback_called) {
       fprintf(stderr, "early callback not called\n");
       return false;
@@ -877,6 +899,14 @@
         fprintf(stderr, "Invalid SSL_get_error output\n");
         return false;
       }
+
+      // After a successful read, with or without False Start, the handshake
+      // must be complete.
+      if (!GetTestState(ssl.get())->handshake_done) {
+        fprintf(stderr, "handshake was not completed after SSL_read\n");
+        return false;
+      }
+
       for (int i = 0; i < n; i++) {
         buf[i] ^= 0xff;
       }

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index cdeacee..0d20770 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -781,6 +781,7 @@
 		},
 		flags: []string{
 			"-false-start",
+			"-handshake-never-done",
 			"-advertise-alpn", "\x03foo",
 		},
 		shimWritesFirst: true,
@@ -801,6 +802,7 @@
 		flags: []string{
 			"-implicit-handshake",
 			"-false-start",
+			"-handshake-never-done",
 			"-advertise-alpn", "\x03foo",
 		},
 		shouldFail:    true,
@@ -943,6 +945,7 @@
 		},
 		flags: []string{
 			"-false-start",
+			"-handshake-never-done",
 			"-advertise-alpn", "\x03foo",
 		},
 		shimWritesFirst: true,

ssl/test/test_config.cc

diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 72a8dae..253c6a1 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -78,6 +78,7 @@
   { "-install-ddos-callback", &TestConfig::install_ddos_callback },
   { "-fail-ddos-callback", &TestConfig::fail_ddos_callback },
   { "-fail-second-ddos-callback", &TestConfig::fail_second_ddos_callback },
+  { "-handshake-never-done", &TestConfig::handshake_never_done },
 };
 
 const Flag<std::string> kStringFlags[] = {

ssl/test/test_config.h

diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index ecd3f51..478a1e7 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -72,6 +72,7 @@
   bool fail_ddos_callback = false;
   bool fail_second_ddos_callback = false;
   std::string cipher;
+  bool handshake_never_done = false;
 };
 
 bool ParseConfig(int argc, char **argv, TestConfig *out_config);