commit 9e68f19e1b8ac53c301702d1d7079105a99df4e1
author David Benjamin <davidben@google.com> Thu Jun 30 14:55:33 2016 -0400
committer Adam Langley <agl@google.com> Thu Jun 30 23:20:34 2016 +0000
tree 2f86f9e1eefdc12cf4d7f717d1b4b7175bd90ea6
parent 727757694ead0c764930f8ad5e47afc1660a43a4

Add SSL_get_curve_id and SSL_get_dhe_group_size.

This replaces the old key_exchange_info APIs and does not require the
caller be aware of the mess around SSL_SESSION management. They
currently have the same bugs around renegotiation as before, but later
work to fix up SSL_SESSION tracking will fix their internals.

For consistency with the existing functions, I've kept the public API at
'curve' rather than 'group' for now. I think it's probably better to
have only one name with a single explanation in the section header
rather than half and half. (I also wouldn't be surprised if the IETF
ends up renaming 'group' again to 'key exchange' at some point.  We'll
see what happens.)

Change-Id: I8e90a503bc4045d12f30835c86de64ef9f2d07c8
Reviewed-on: https://boringssl-review.googlesource.com/8565
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 99a5b93..8516977 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1137,11 +1137,21 @@
     return false;
   }
 
-  if (config->expect_key_exchange_info != 0) {
-    uint32_t info = SSL_SESSION_get_key_exchange_info(SSL_get_session(ssl));
-    if (static_cast<uint32_t>(config->expect_key_exchange_info) != info) {
-      fprintf(stderr, "key_exchange_info was %" PRIu32 ", wanted %" PRIu32 "\n",
-              info, static_cast<uint32_t>(config->expect_key_exchange_info));
+  if (config->expect_curve_id != 0) {
+    uint16_t curve_id = SSL_get_curve_id(ssl);
+    if (static_cast<uint16_t>(config->expect_curve_id) != curve_id) {
+      fprintf(stderr, "curve_id was %04x, wanted %04x\n", curve_id,
+              static_cast<uint16_t>(config->expect_curve_id));
+      return false;
+    }
+  }
+
+  if (config->expect_dhe_group_size != 0) {
+    unsigned dhe_group_size = SSL_get_dhe_group_size(ssl);
+    if (static_cast<unsigned>(config->expect_dhe_group_size) !=
+        dhe_group_size) {
+      fprintf(stderr, "dhe_group_size was %u, wanted %d\n", dhe_group_size,
+              config->expect_dhe_group_size);
       return false;
     }
   }

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 32a6959..0ede7bb 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5258,7 +5258,7 @@
 				DHGroupPrime: bigFromHex("0215C589A86BE450D1255A86D7A08877A70E124C11F0C75E476BA6A2186B1C830D4A132555973F2D5881D5F737BB800B7F417C01EC5960AEBF79478F8E0BBB6A021269BD10590C64C57F50AD8169D5488B56EE38DC5E02DA1A16ED3B5F41FEB2AD184B78A31F3A5B2BEC8441928343DA35DE3D4F89F0D4CEDE0034045084A0D1E6182E5EF7FCA325DD33CE81BE7FA87D43613E8FA7A1457099AB53"),
 			},
 		},
-		flags: []string{"-expect-key-exchange-info", "1234"},
+		flags: []string{"-expect-dhe-group-size", "1234"},
 	})
 	testCases = append(testCases, testCase{
 		testType: serverTest,
@@ -5268,7 +5268,7 @@
 			CipherSuites: []uint16{TLS_DHE_RSA_WITH_AES_128_GCM_SHA256},
 		},
 		// bssl_shim as a server configures a 2048-bit DHE group.
-		flags: []string{"-expect-key-exchange-info", "2048"},
+		flags: []string{"-expect-dhe-group-size", "2048"},
 	})
 
 	// TODO(davidben): Add TLS 1.3 versions of these tests once the
@@ -5281,7 +5281,7 @@
 			CipherSuites:     []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
 			CurvePreferences: []CurveID{CurveX25519},
 		},
-		flags: []string{"-expect-key-exchange-info", "29", "-enable-all-curves"},
+		flags: []string{"-expect-curve-id", "29", "-enable-all-curves"},
 	})
 	testCases = append(testCases, testCase{
 		testType: serverTest,
@@ -5291,7 +5291,7 @@
 			CipherSuites:     []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
 			CurvePreferences: []CurveID{CurveX25519},
 		},
-		flags: []string{"-expect-key-exchange-info", "29", "-enable-all-curves"},
+		flags: []string{"-expect-curve-id", "29", "-enable-all-curves"},
 	})
 }
 

ssl/test/test_config.cc

diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 536978b..cf3541d 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -147,8 +147,8 @@
   { "-expect-total-renegotiations", &TestConfig::expect_total_renegotiations },
   { "-expect-server-key-exchange-hash",
     &TestConfig::expect_server_key_exchange_hash },
-  { "-expect-key-exchange-info",
-    &TestConfig::expect_key_exchange_info },
+  { "-expect-curve-id", &TestConfig::expect_curve_id },
+  { "-expect-dhe-group-size", &TestConfig::expect_dhe_group_size },
   { "-initial-timeout-duration-ms", &TestConfig::initial_timeout_duration_ms },
 };
 

ssl/test/test_config.h

diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index aff194e..b554f14 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -103,7 +103,8 @@
   bool p384_only = false;
   bool enable_all_curves = false;
   bool use_sparse_dh_prime = false;
-  int expect_key_exchange_info = 0;
+  int expect_curve_id = 0;
+  int expect_dhe_group_size = 0;
   bool use_old_client_cert_callback = false;
   int initial_timeout_duration_ms = 0;
 };