Add |SSL_set_retain_only_sha256_of_client_certs|.
Previously the option to retain only the SHA-256 hash of client
certificates could only be set at the |SSL_CTX| level. This change makes
|SSL| objects inherit the setting from the |SSL_CTX|, but allows it to
be overridden on a per-|SSL| basis.
Change-Id: Id435934af3d425d5f008d2f3b9751d1d0884ee55
Reviewed-on: https://boringssl-review.googlesource.com/12182
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>
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index a0562d8..3b66ab7 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -482,7 +482,7 @@
/* If we aren't retaining peer certificates then we can discard it
* now. */
if (ssl->s3->new_session != NULL &&
- ssl->ctx->retain_only_sha256_of_client_certs) {
+ ssl->retain_only_sha256_of_client_certs) {
X509_free(ssl->s3->new_session->x509_peer);
ssl->s3->new_session->x509_peer = NULL;
sk_X509_pop_free(ssl->s3->new_session->x509_chain, X509_free);
@@ -1313,7 +1313,7 @@
CBS_init(&certificate_msg, ssl->init_msg, ssl->init_num);
uint8_t alert;
STACK_OF(X509) *chain = ssl_parse_cert_chain(
- ssl, &alert, ssl->ctx->retain_only_sha256_of_client_certs
+ ssl, &alert, ssl->retain_only_sha256_of_client_certs
? ssl->s3->new_session->peer_sha256
: NULL,
&certificate_msg);
@@ -1352,7 +1352,7 @@
ssl->s3->new_session->verify_result = X509_V_OK;
} else {
/* The hash would have been filled in. */
- if (ssl->ctx->retain_only_sha256_of_client_certs) {
+ if (ssl->retain_only_sha256_of_client_certs) {
ssl->s3->new_session->peer_sha256_valid = 1;
}
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 982cb1a..001928f 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -412,6 +412,8 @@
assert(ssl->sid_ctx_length <= sizeof ssl->sid_ctx);
memcpy(&ssl->sid_ctx, &ctx->sid_ctx, sizeof(ssl->sid_ctx));
ssl->verify_callback = ctx->default_verify_callback;
+ ssl->retain_only_sha256_of_client_certs =
+ ctx->retain_only_sha256_of_client_certs;
ssl->param = X509_VERIFY_PARAM_new();
if (!ssl->param) {
@@ -2908,6 +2910,10 @@
return ssl->s3->tmp.new_cipher;
}
+void SSL_set_retain_only_sha256_of_client_certs(SSL *ssl, int enabled) {
+ ssl->retain_only_sha256_of_client_certs = !!enabled;
+}
+
void SSL_CTX_set_retain_only_sha256_of_client_certs(SSL_CTX *ctx, int enabled) {
ctx->retain_only_sha256_of_client_certs = !!enabled;
}
diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index a28876b..a6b669d 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -641,7 +641,13 @@
* version. */
ssl->version == session->ssl_version &&
/* Only resume if the session's cipher matches the negotiated one. */
- ssl->s3->tmp.new_cipher == session->cipher;
+ ssl->s3->tmp.new_cipher == session->cipher &&
+ /* If the session contains a client certificate (either the full
+ * certificate or just the hash) then require that the form of the
+ * certificate matches the current configuration. */
+ ((session->x509_peer == NULL && !session->peer_sha256_valid) ||
+ session->peer_sha256_valid ==
+ ssl->retain_only_sha256_of_client_certs);
}
/* ssl_lookup_session looks up |session_id| in the session cache and sets
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index d1ccdb4..97860c4 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1436,6 +1436,25 @@
}
}
+ bool expected_sha256_client_cert = config->expect_sha256_client_cert_initial;
+ if (is_resume) {
+ expected_sha256_client_cert = config->expect_sha256_client_cert_resume;
+ }
+
+ if (SSL_get_session(ssl)->peer_sha256_valid != expected_sha256_client_cert) {
+ fprintf(stderr,
+ "Unexpected SHA-256 client cert state: expected:%d is_resume:%d.\n",
+ expected_sha256_client_cert, is_resume);
+ return false;
+ }
+
+ if (expected_sha256_client_cert &&
+ SSL_get_session(ssl)->x509_peer != nullptr) {
+ fprintf(stderr, "Have both client cert and SHA-256 hash: is_resume:%d.\n",
+ is_resume);
+ return false;
+ }
+
return true;
}
@@ -1595,6 +1614,12 @@
if (config->max_cert_list > 0) {
SSL_set_max_cert_list(ssl.get(), config->max_cert_list);
}
+ if (!is_resume && config->retain_only_sha256_client_cert_initial) {
+ SSL_set_retain_only_sha256_of_client_certs(ssl.get(), 1);
+ }
+ if (is_resume && config->retain_only_sha256_client_cert_resume) {
+ SSL_set_retain_only_sha256_of_client_certs(ssl.get(), 1);
+ }
int sock = Connect(config->port);
if (sock == -1) {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index c6b18b1..3877a8b 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -9532,6 +9532,87 @@
}
}
+func addRetainOnlySHA256ClientCertTests() {
+ for _, ver := range tlsVersions {
+ // Test that enabling
+ // SSL_CTX_set_retain_only_sha256_of_client_certs without
+ // actually requesting a client certificate is a no-op.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RetainOnlySHA256-NoCert-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{
+ "-retain-only-sha256-client-cert-initial",
+ "-retain-only-sha256-client-cert-resume",
+ },
+ resumeSession: true,
+ })
+
+ // Test that when retaining only a SHA-256 certificate is
+ // enabled, the hash appears as expected.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RetainOnlySHA256-Cert-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Certificates: []Certificate{rsaCertificate},
+ },
+ flags: []string{
+ "-verify-peer",
+ "-retain-only-sha256-client-cert-initial",
+ "-retain-only-sha256-client-cert-resume",
+ "-expect-sha256-client-cert-initial",
+ "-expect-sha256-client-cert-resume",
+ },
+ resumeSession: true,
+ })
+
+ // Test that when the config changes from on to off, a
+ // resumption is rejected because the server now wants the full
+ // certificate chain.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RetainOnlySHA256-OnOff-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Certificates: []Certificate{rsaCertificate},
+ },
+ flags: []string{
+ "-verify-peer",
+ "-retain-only-sha256-client-cert-initial",
+ "-expect-sha256-client-cert-initial",
+ },
+ resumeSession: true,
+ expectResumeRejected: true,
+ })
+
+ // Test that when the config changes from off to on, a
+ // resumption is rejected because the server now wants just the
+ // hash.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "RetainOnlySHA256-OffOn-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Certificates: []Certificate{rsaCertificate},
+ },
+ flags: []string{
+ "-verify-peer",
+ "-retain-only-sha256-client-cert-resume",
+ "-expect-sha256-client-cert-resume",
+ },
+ resumeSession: true,
+ expectResumeRejected: true,
+ })
+ }
+}
+
func worker(statusChan chan statusMsg, c chan *testCase, shimPath string, wg *sync.WaitGroup) {
defer wg.Done()
@@ -9658,6 +9739,7 @@
addPeekTests()
addRecordVersionTests()
addCertificateTests()
+ addRetainOnlySHA256ClientCertTests()
var wg sync.WaitGroup
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 4f40df9..940e676 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -108,6 +108,14 @@
{ "-peek-then-read", &TestConfig::peek_then_read },
{ "-enable-grease", &TestConfig::enable_grease },
{ "-use-exporter-between-reads", &TestConfig::use_exporter_between_reads },
+ { "-retain-only-sha256-client-cert-initial",
+ &TestConfig::retain_only_sha256_client_cert_initial },
+ { "-retain-only-sha256-client-cert-resume",
+ &TestConfig::retain_only_sha256_client_cert_resume },
+ { "-expect-sha256-client-cert-initial",
+ &TestConfig::expect_sha256_client_cert_initial },
+ { "-expect-sha256-client-cert-resume",
+ &TestConfig::expect_sha256_client_cert_resume },
};
const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 5b1e0e8..ed1a47b 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -122,6 +122,10 @@
int expect_cipher_no_aes = 0;
std::string expect_peer_cert_file;
int resumption_delay = 0;
+ bool retain_only_sha256_client_cert_initial = false;
+ bool retain_only_sha256_client_cert_resume = false;
+ bool expect_sha256_client_cert_initial = false;
+ bool expect_sha256_client_cert_resume = false;
};
bool ParseConfig(int argc, char **argv, TestConfig *out_config);
diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c
index ab09240..9e4d450 100644
--- a/ssl/tls13_both.c
+++ b/ssl/tls13_both.c
@@ -175,7 +175,7 @@
}
const int retain_sha256 =
- ssl->server && ssl->ctx->retain_only_sha256_of_client_certs;
+ ssl->server && ssl->retain_only_sha256_of_client_certs;
int ret = 0;
STACK_OF(X509) *chain = sk_X509_new_null();