diff options
author | android-build-team Robot <android-build-team-robot@google.com> | 2017-09-03 07:32:55 +0000 |
---|---|---|
committer | android-build-team Robot <android-build-team-robot@google.com> | 2017-09-03 07:32:55 +0000 |
commit | d3a4c5684f96eac702d8d34159cf4592d2e18eea (patch) | |
tree | ffc81c26951e6108a88a3924c3b27774be532217 | |
parent | 22f76d41b2a86daab8c0d1124bd540a1c3b87e21 (diff) | |
parent | 882b6c0b2f90afb318f4d0ea6a463dcfecf85abf (diff) | |
download | boringssl-android-security-8.1.0_r87.tar.gz |
release-request-957cd691-fb71-4770-8ff7-a3b9602655a5-for-git_oc-mr1-release-4314464 snap-temp-L54400000099147910android-wear-8.1.0_r1android-vts-8.1_r9android-vts-8.1_r8android-vts-8.1_r7android-vts-8.1_r6android-vts-8.1_r5android-vts-8.1_r4android-vts-8.1_r3android-vts-8.1_r14android-vts-8.1_r13android-vts-8.1_r12android-vts-8.1_r11android-vts-8.1_r10android-security-8.1.0_r93android-security-8.1.0_r92android-security-8.1.0_r91android-security-8.1.0_r90android-security-8.1.0_r89android-security-8.1.0_r88android-security-8.1.0_r87android-security-8.1.0_r86android-security-8.1.0_r85android-security-8.1.0_r84android-security-8.1.0_r83android-security-8.1.0_r82android-cts-8.1_r9android-cts-8.1_r8android-cts-8.1_r7android-cts-8.1_r6android-cts-8.1_r5android-cts-8.1_r4android-cts-8.1_r3android-cts-8.1_r25android-cts-8.1_r24android-cts-8.1_r23android-cts-8.1_r22android-cts-8.1_r21android-cts-8.1_r20android-cts-8.1_r2android-cts-8.1_r19android-cts-8.1_r18android-cts-8.1_r17android-cts-8.1_r16android-cts-8.1_r15android-cts-8.1_r14android-cts-8.1_r13android-cts-8.1_r12android-cts-8.1_r11android-cts-8.1_r10android-cts-8.1_r1android-8.1.0_r9android-8.1.0_r81android-8.1.0_r80android-8.1.0_r8android-8.1.0_r79android-8.1.0_r78android-8.1.0_r77android-8.1.0_r76android-8.1.0_r75android-8.1.0_r74android-8.1.0_r73android-8.1.0_r72android-8.1.0_r71android-8.1.0_r70android-8.1.0_r7android-8.1.0_r69android-8.1.0_r68android-8.1.0_r67android-8.1.0_r66android-8.1.0_r65android-8.1.0_r64android-8.1.0_r63android-8.1.0_r62android-8.1.0_r61android-8.1.0_r60android-8.1.0_r6android-8.1.0_r53android-8.1.0_r52android-8.1.0_r51android-8.1.0_r50android-8.1.0_r5android-8.1.0_r48android-8.1.0_r47android-8.1.0_r46android-8.1.0_r45android-8.1.0_r43android-8.1.0_r42android-8.1.0_r41android-8.1.0_r40android-8.1.0_r4android-8.1.0_r39android-8.1.0_r38android-8.1.0_r37android-8.1.0_r36android-8.1.0_r35android-8.1.0_r33android-8.1.0_r32android-8.1.0_r31android-8.1.0_r30android-8.1.0_r3android-8.1.0_r29android-8.1.0_r28android-8.1.0_r27android-8.1.0_r26android-8.1.0_r25android-8.1.0_r23android-8.1.0_r22android-8.1.0_r21android-8.1.0_r20android-8.1.0_r2android-8.1.0_r19android-8.1.0_r18android-8.1.0_r17android-8.1.0_r16android-8.1.0_r15android-8.1.0_r14android-8.1.0_r13android-8.1.0_r12android-8.1.0_r11android-8.1.0_r10android-8.1.0_r1security-oc-mr1-releaseoreo-mr1-wear-releaseoreo-mr1-vts-releaseoreo-mr1-security-releaseoreo-mr1-s1-releaseoreo-mr1-releaseoreo-mr1-cuttlefish-testingoreo-mr1-cts-releaseoreo-m8-releaseoreo-m7-releaseoreo-m6-s4-releaseoreo-m6-s3-releaseoreo-m6-s2-releaseoreo-m5-releaseoreo-m4-s9-releaseoreo-m4-s8-releaseoreo-m4-s7-releaseoreo-m4-s6-releaseoreo-m4-s5-releaseoreo-m4-s4-releaseoreo-m4-s3-releaseoreo-m4-s2-releaseoreo-m4-s12-releaseoreo-m4-s11-releaseoreo-m4-s10-releaseoreo-m4-s1-releaseoreo-m3-releaseoreo-m2-s5-releaseoreo-m2-s4-releaseoreo-m2-s3-releaseoreo-m2-s2-releaseoreo-m2-s1-releaseoreo-m2-release
Change-Id: I457f6e087fc7ddd618e28ddf9f4e88bfe5d11f43
-rw-r--r-- | src/PORTING.md | 11 | ||||
-rw-r--r-- | src/ssl/handshake_client.cc | 102 | ||||
-rw-r--r-- | src/ssl/ssl_lib.cc | 1 | ||||
-rw-r--r-- | src/ssl/test/bssl_shim.cc | 253 | ||||
-rw-r--r-- | src/ssl/test/runner/common.go | 8 | ||||
-rw-r--r-- | src/ssl/test/runner/handshake_server.go | 7 | ||||
-rw-r--r-- | src/ssl/test/runner/runner.go | 38 |
7 files changed, 274 insertions, 146 deletions
diff --git a/src/PORTING.md b/src/PORTING.md index b9d67523..9f9fda08 100644 --- a/src/PORTING.md +++ b/src/PORTING.md @@ -128,6 +128,17 @@ Things which do not work: * If a HelloRequest is received while `SSL_write` has unsent application data, the renegotiation is rejected. +* Renegotiation does not participate in session resumption. The client will + not offer a session on renegotiation or resume any session established by a + renegotiation handshake. + +* The server may not change its certificate in the renegotiation. This mitigates + the [triple handshake attack](https://mitls.org/pages/attacks/3SHAKE). Any new + stapled OCSP response and SCT list will be ignored. As no authentication state + may change, BoringSSL will not re-verify the certificate on a renegotiation. + Callbacks such as `SSL_CTX_set_custom_verify` will only run on the initial + handshake. + ### Lowercase hexadecimal BoringSSL's `BN_bn2hex` function uses lowercase hexadecimal digits instead of diff --git a/src/ssl/handshake_client.cc b/src/ssl/handshake_client.cc index 9efbf0ad..0519bedc 100644 --- a/src/ssl/handshake_client.cc +++ b/src/ssl/handshake_client.cc @@ -503,7 +503,10 @@ int ssl3_connect(SSL_HANDSHAKE *hs) { ret = -1; goto end; } - ssl->s3->established_session->not_resumable = 0; + /* Renegotiations do not participate in session resumption. */ + if (!ssl->s3->initial_handshake_complete) { + ssl->s3->established_session->not_resumable = 0; + } SSL_SESSION_free(hs->new_session); hs->new_session = NULL; @@ -513,12 +516,8 @@ int ssl3_connect(SSL_HANDSHAKE *hs) { break; case SSL_ST_OK: { - const int is_initial_handshake = !ssl->s3->initial_handshake_complete; ssl->s3->initial_handshake_complete = 1; - if (is_initial_handshake) { - /* Renegotiations do not participate in session resumption. */ - ssl_update_cache(hs, SSL_SESS_CACHE_CLIENT); - } + ssl_update_cache(hs, SSL_SESS_CACHE_CLIENT); ret = 1; ssl_do_info_callback(ssl, SSL_CB_HANDSHAKE_DONE, 1); @@ -1114,33 +1113,6 @@ static int ssl3_get_server_certificate(SSL_HANDSHAKE *hs) { return -1; } - /* Disallow the server certificate from changing during a renegotiation. See - * https://mitls.org/pages/attacks/3SHAKE. We never resume on renegotiation, - * so this check is sufficient. */ - if (ssl->s3->established_session != NULL) { - if (sk_CRYPTO_BUFFER_num(ssl->s3->established_session->certs) != - sk_CRYPTO_BUFFER_num(hs->new_session->certs)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_SERVER_CERT_CHANGED); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); - return -1; - } - - for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(hs->new_session->certs); i++) { - const CRYPTO_BUFFER *old_cert = - sk_CRYPTO_BUFFER_value(ssl->s3->established_session->certs, i); - const CRYPTO_BUFFER *new_cert = - sk_CRYPTO_BUFFER_value(hs->new_session->certs, i); - if (CRYPTO_BUFFER_len(old_cert) != CRYPTO_BUFFER_len(new_cert) || - OPENSSL_memcmp(CRYPTO_BUFFER_data(old_cert), - CRYPTO_BUFFER_data(new_cert), - CRYPTO_BUFFER_len(old_cert)) != 0) { - OPENSSL_PUT_ERROR(SSL, SSL_R_SERVER_CERT_CHANGED); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); - return -1; - } - } - } - return 1; } @@ -1185,8 +1157,72 @@ static int ssl3_get_cert_status(SSL_HANDSHAKE *hs) { return 1; } +static int copy_bytes(uint8_t **out, size_t *out_len, const uint8_t *in, size_t in_len) { + OPENSSL_free(*out); + *out = nullptr; + *out_len = 0; + + if (in_len > 0) { + *out = (uint8_t *)BUF_memdup(in, in_len); + if (*out == nullptr) { + return 0; + } + *out_len = in_len; + } + + return 1; +} + static int ssl3_verify_server_cert(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; + const SSL_SESSION *prev_session = ssl->s3->established_session; + if (prev_session != NULL) { + /* If renegotiating, the server must not change the server certificate. See + * https://mitls.org/pages/attacks/3SHAKE. We never resume on renegotiation, + * so this check is sufficient to ensure the reported peer certificate never + * changes on renegotiation. */ + assert(!ssl->server); + if (sk_CRYPTO_BUFFER_num(prev_session->certs) != + sk_CRYPTO_BUFFER_num(hs->new_session->certs)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_SERVER_CERT_CHANGED); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return -1; + } + + for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(hs->new_session->certs); i++) { + const CRYPTO_BUFFER *old_cert = + sk_CRYPTO_BUFFER_value(prev_session->certs, i); + const CRYPTO_BUFFER *new_cert = + sk_CRYPTO_BUFFER_value(hs->new_session->certs, i); + if (CRYPTO_BUFFER_len(old_cert) != CRYPTO_BUFFER_len(new_cert) || + OPENSSL_memcmp(CRYPTO_BUFFER_data(old_cert), + CRYPTO_BUFFER_data(new_cert), + CRYPTO_BUFFER_len(old_cert)) != 0) { + OPENSSL_PUT_ERROR(SSL, SSL_R_SERVER_CERT_CHANGED); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return -1; + } + } + + /* The certificate is identical, so we may skip re-verifying the + * certificate. Since we only authenticated the previous one, copy other + * authentication from the established session and ignore what was newly + * received. */ + if (!copy_bytes(&hs->new_session->ocsp_response, + &hs->new_session->ocsp_response_length, + prev_session->ocsp_response, + prev_session->ocsp_response_length) || + !copy_bytes(&hs->new_session->tlsext_signed_cert_timestamp_list, + &hs->new_session->tlsext_signed_cert_timestamp_list_length, + prev_session->tlsext_signed_cert_timestamp_list, + prev_session->tlsext_signed_cert_timestamp_list_length)) { + return -1; + } + + hs->new_session->verify_result = prev_session->verify_result; + return 1; + } + if (!ssl->ctx->x509_method->session_verify_cert_chain(hs->new_session, ssl)) { return -1; } diff --git a/src/ssl/ssl_lib.cc b/src/ssl/ssl_lib.cc index 74419252..b8d4c272 100644 --- a/src/ssl/ssl_lib.cc +++ b/src/ssl/ssl_lib.cc @@ -1811,6 +1811,7 @@ void ssl_update_cache(SSL_HANDSHAKE *hs, int mode) { SSL_CTX *ctx = ssl->session_ctx; /* Never cache sessions with empty session IDs. */ if (ssl->s3->established_session->session_id_length == 0 || + ssl->s3->established_session->not_resumable || (ctx->session_cache_mode & mode) != mode) { return; } diff --git a/src/ssl/test/bssl_shim.cc b/src/ssl/test/bssl_shim.cc index cd846be4..fb210582 100644 --- a/src/ssl/test/bssl_shim.cc +++ b/src/ssl/test/bssl_shim.cc @@ -112,6 +112,9 @@ struct TestState { bool alpn_select_done = false; bool is_resume = false; bool early_callback_ready = false; + // cert_verified is true if certificate verification has been driven to + // completion. This tests that the callback is not called again after this. + bool cert_verified = false; }; static void TestStateExFree(void *parent, void *ptr, CRYPTO_EX_DATA *ad, @@ -684,27 +687,41 @@ static int CertCallback(SSL *ssl, void *arg) { return 1; } -static int VerifySucceed(X509_STORE_CTX *store_ctx, void *arg) { - SSL* ssl = (SSL*)X509_STORE_CTX_get_ex_data(store_ctx, - SSL_get_ex_data_X509_STORE_CTX_idx()); +static bool CheckVerifyCallback(SSL *ssl) { const TestConfig *config = GetTestConfig(ssl); - if (!config->expected_ocsp_response.empty()) { const uint8_t *data; size_t len; SSL_get0_ocsp_response(ssl, &data, &len); if (len == 0) { fprintf(stderr, "OCSP response not available in verify callback\n"); - return 0; + return false; } } - return 1; + if (GetTestState(ssl)->cert_verified) { + fprintf(stderr, "Certificate verified twice.\n"); + return false; + } + + return true; } -static int VerifyFail(X509_STORE_CTX *store_ctx, void *arg) { - store_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION; - return 0; +static int CertVerifyCallback(X509_STORE_CTX *store_ctx, void *arg) { + SSL* ssl = (SSL*)X509_STORE_CTX_get_ex_data(store_ctx, + SSL_get_ex_data_X509_STORE_CTX_idx()); + const TestConfig *config = GetTestConfig(ssl); + if (!CheckVerifyCallback(ssl)) { + return 0; + } + + GetTestState(ssl)->cert_verified = true; + if (config->verify_fail) { + store_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION; + return 0; + } + + return 1; } static int NextProtosAdvertisedCallback(SSL *ssl, const uint8_t **out, @@ -1139,11 +1156,7 @@ static bssl::UniquePtr<SSL_CTX> SetupCtx(SSL_CTX *old_ctx, return nullptr; } - if (config->verify_fail) { - SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), VerifyFail, NULL); - } else { - SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), VerifySucceed, NULL); - } + SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), CertVerifyCallback, NULL); if (!config->signed_cert_timestamps.empty() && !SSL_CTX_set_signed_cert_timestamp_list( @@ -1374,11 +1387,115 @@ static uint16_t GetProtocolVersion(const SSL *ssl) { return 0x0201 + ~version; } +// CheckAuthProperties checks, after the initial handshake is completed or +// after a renegotiation, that authentication-related properties match |config|. +static bool CheckAuthProperties(SSL *ssl, bool is_resume, + const TestConfig *config) { + if (!config->expected_ocsp_response.empty()) { + const uint8_t *data; + size_t len; + SSL_get0_ocsp_response(ssl, &data, &len); + if (config->expected_ocsp_response.size() != len || + OPENSSL_memcmp(config->expected_ocsp_response.data(), data, len) != 0) { + fprintf(stderr, "OCSP response mismatch\n"); + return false; + } + } + + if (!config->expected_signed_cert_timestamps.empty()) { + const uint8_t *data; + size_t len; + SSL_get0_signed_cert_timestamp_list(ssl, &data, &len); + if (config->expected_signed_cert_timestamps.size() != len || + OPENSSL_memcmp(config->expected_signed_cert_timestamps.data(), data, + len) != 0) { + fprintf(stderr, "SCT list mismatch\n"); + return false; + } + } + + if (config->expect_verify_result) { + int expected_verify_result = config->verify_fail ? + X509_V_ERR_APPLICATION_VERIFICATION : + X509_V_OK; + + if (SSL_get_verify_result(ssl) != expected_verify_result) { + fprintf(stderr, "Wrong certificate verification result\n"); + return false; + } + } + + if (!config->expect_peer_cert_file.empty()) { + bssl::UniquePtr<X509> expect_leaf; + bssl::UniquePtr<STACK_OF(X509)> expect_chain; + if (!LoadCertificate(&expect_leaf, &expect_chain, + config->expect_peer_cert_file)) { + return false; + } + + // For historical reasons, clients report a chain with a leaf and servers + // without. + if (!config->is_server) { + if (!sk_X509_insert(expect_chain.get(), expect_leaf.get(), 0)) { + return false; + } + X509_up_ref(expect_leaf.get()); // sk_X509_push takes ownership. + } + + bssl::UniquePtr<X509> leaf(SSL_get_peer_certificate(ssl)); + STACK_OF(X509) *chain = SSL_get_peer_cert_chain(ssl); + if (X509_cmp(leaf.get(), expect_leaf.get()) != 0) { + fprintf(stderr, "Received a different leaf certificate than expected.\n"); + return false; + } + + if (sk_X509_num(chain) != sk_X509_num(expect_chain.get())) { + fprintf(stderr, "Received a chain of length %zu instead of %zu.\n", + sk_X509_num(chain), sk_X509_num(expect_chain.get())); + return false; + } + + for (size_t i = 0; i < sk_X509_num(chain); i++) { + if (X509_cmp(sk_X509_value(chain, i), + sk_X509_value(expect_chain.get(), i)) != 0) { + fprintf(stderr, "Chain certificate %zu did not match.\n", + i + 1); + return false; + } + } + } + + 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)->certs != nullptr) { + fprintf(stderr, "Have both client cert and SHA-256 hash: is_resume:%d.\n", + is_resume); + return false; + } + + return true; +} + // CheckHandshakeProperties checks, immediately after |ssl| completes its // initial handshake (or False Starts), whether all the properties are // consistent with the test configuration and invariants. static bool CheckHandshakeProperties(SSL *ssl, bool is_resume, const TestConfig *config) { + if (!CheckAuthProperties(ssl, is_resume, config)) { + return false; + } + if (SSL_get_current_cipher(ssl) == nullptr) { fprintf(stderr, "null cipher after handshake\n"); return false; @@ -1504,40 +1621,6 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume, return false; } - if (!config->expected_ocsp_response.empty()) { - const uint8_t *data; - size_t len; - SSL_get0_ocsp_response(ssl, &data, &len); - if (config->expected_ocsp_response.size() != len || - OPENSSL_memcmp(config->expected_ocsp_response.data(), data, len) != 0) { - fprintf(stderr, "OCSP response mismatch\n"); - return false; - } - } - - if (!config->expected_signed_cert_timestamps.empty()) { - const uint8_t *data; - size_t len; - SSL_get0_signed_cert_timestamp_list(ssl, &data, &len); - if (config->expected_signed_cert_timestamps.size() != len || - OPENSSL_memcmp(config->expected_signed_cert_timestamps.data(), data, - len) != 0) { - fprintf(stderr, "SCT list mismatch\n"); - return false; - } - } - - if (config->expect_verify_result) { - int expected_verify_result = config->verify_fail ? - X509_V_ERR_APPLICATION_VERIFICATION : - X509_V_OK; - - if (SSL_get_verify_result(ssl) != expected_verify_result) { - fprintf(stderr, "Wrong certificate verification result\n"); - return false; - } - } - if (config->expect_peer_signature_algorithm != 0 && config->expect_peer_signature_algorithm != SSL_get_peer_signature_algorithm(ssl)) { @@ -1596,65 +1679,6 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume, } } - if (!config->expect_peer_cert_file.empty()) { - bssl::UniquePtr<X509> expect_leaf; - bssl::UniquePtr<STACK_OF(X509)> expect_chain; - if (!LoadCertificate(&expect_leaf, &expect_chain, - config->expect_peer_cert_file)) { - return false; - } - - // For historical reasons, clients report a chain with a leaf and servers - // without. - if (!config->is_server) { - if (!sk_X509_insert(expect_chain.get(), expect_leaf.get(), 0)) { - return false; - } - X509_up_ref(expect_leaf.get()); // sk_X509_push takes ownership. - } - - bssl::UniquePtr<X509> leaf(SSL_get_peer_certificate(ssl)); - STACK_OF(X509) *chain = SSL_get_peer_cert_chain(ssl); - if (X509_cmp(leaf.get(), expect_leaf.get()) != 0) { - fprintf(stderr, "Received a different leaf certificate than expected.\n"); - return false; - } - - if (sk_X509_num(chain) != sk_X509_num(expect_chain.get())) { - fprintf(stderr, "Received a chain of length %zu instead of %zu.\n", - sk_X509_num(chain), sk_X509_num(expect_chain.get())); - return false; - } - - for (size_t i = 0; i < sk_X509_num(chain); i++) { - if (X509_cmp(sk_X509_value(chain, i), - sk_X509_value(expect_chain.get(), i)) != 0) { - fprintf(stderr, "Chain certificate %zu did not match.\n", - i + 1); - return false; - } - } - } - - 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)->certs != nullptr) { - fprintf(stderr, "Have both client cert and SHA-256 hash: is_resume:%d.\n", - is_resume); - return false; - } - if (is_resume && config->expect_ticket_age_skew != 0 && SSL_get_ticket_age_skew(ssl) != config->expect_ticket_age_skew) { fprintf(stderr, "Ticket age skew was %" PRId32 ", wanted %d\n", @@ -2242,6 +2266,21 @@ static bool DoExchange(bssl::UniquePtr<SSL_SESSION> *out_session, SSL *ssl, return false; } + if (SSL_total_renegotiations(ssl) > 0) { + if (!SSL_get_session(ssl)->not_resumable) { + fprintf(stderr, + "Renegotiations should never produce resumable sessions.\n"); + return false; + } + + // Re-check authentication properties after a renegotiation. The reported + // values should remain unchanged even if the server sent different SCT + // lists. + if (!CheckAuthProperties(ssl, is_resume, config)) { + return false; + } + } + if (SSL_total_renegotiations(ssl) != config->expect_total_renegotiations) { fprintf(stderr, "Expected %d renegotiations, got %d\n", config->expect_total_renegotiations, SSL_total_renegotiations(ssl)); diff --git a/src/ssl/test/runner/common.go b/src/ssl/test/runner/common.go index fd9fb3d5..aebc2648 100644 --- a/src/ssl/test/runner/common.go +++ b/src/ssl/test/runner/common.go @@ -1059,11 +1059,19 @@ type ProtocolBugs struct { // supplied SCT list in resumption handshakes. SendSCTListOnResume []byte + // SendSCTListOnRenegotiation, if not nil, causes the server to send the + // supplied SCT list on renegotiation. + SendSCTListOnRenegotiation []byte + // SendOCSPResponseOnResume, if not nil, causes the server to advertise // OCSP stapling in resumption handshakes and, if applicable, send the // supplied stapled response. SendOCSPResponseOnResume []byte + // SendOCSPResponseOnResume, if not nil, causes the server to send the + // supplied OCSP response on renegotiation. + SendOCSPResponseOnRenegotiation []byte + // SendExtensionOnCertificate, if not nil, causes the runner to send the // supplied bytes in the extensions on the Certificate message. SendExtensionOnCertificate []byte diff --git a/src/ssl/test/runner/handshake_server.go b/src/ssl/test/runner/handshake_server.go index b31a562e..cb9bdac5 100644 --- a/src/ssl/test/runner/handshake_server.go +++ b/src/ssl/test/runner/handshake_server.go @@ -1433,6 +1433,10 @@ func (hs *serverHandshakeState) doFullHandshake() error { hs.hello.extensions.sctList = hs.cert.SignedCertificateTimestampList } + if len(c.clientVerify) > 0 && config.Bugs.SendSCTListOnRenegotiation != nil { + hs.hello.extensions.sctList = config.Bugs.SendSCTListOnRenegotiation + } + hs.hello.extensions.ticketSupported = hs.clientHello.ticketSupported && !config.SessionTicketsDisabled && c.vers > VersionSSL30 hs.hello.cipherSuite = hs.suite.id if config.Bugs.SendCipherSuite != 0 { @@ -1479,6 +1483,9 @@ func (hs *serverHandshakeState) doFullHandshake() error { certStatus := new(certificateStatusMsg) certStatus.statusType = statusTypeOCSP certStatus.response = hs.cert.OCSPStaple + if len(c.clientVerify) > 0 && config.Bugs.SendOCSPResponseOnRenegotiation != nil { + certStatus.response = config.Bugs.SendOCSPResponseOnRenegotiation + } hs.writeServerHash(certStatus.marshal()) c.writeRecord(recordTypeHandshake, certStatus.marshal()) } diff --git a/src/ssl/test/runner/runner.go b/src/ssl/test/runner/runner.go index 29747db6..e3521a4d 100644 --- a/src/ssl/test/runner/runner.go +++ b/src/ssl/test/runner/runner.go @@ -199,7 +199,9 @@ var channelIDKey *ecdsa.PrivateKey var channelIDBytes []byte var testOCSPResponse = []byte{1, 2, 3, 4} +var testOCSPResponse2 = []byte{5, 6, 7, 8} var testSCTList = []byte{0, 6, 0, 4, 5, 6, 7, 8} +var testSCTList2 = []byte{0, 6, 0, 4, 1, 2, 3, 4} var testOCSPExtension = append([]byte{byte(extensionStatusRequest) >> 8, byte(extensionStatusRequest), 0, 8, statusTypeOCSP, 0, 0, 4}, testOCSPResponse...) var testSCTExtension = append([]byte{byte(extensionSignedCertificateTimestamp) >> 8, byte(extensionSignedCertificateTimestamp), 0, byte(len(testSCTList))}, testSCTList...) @@ -6381,10 +6383,6 @@ func addExtensionTests() { expectedError: ":UNEXPECTED_EXTENSION:", }) - var differentSCTList []byte - differentSCTList = append(differentSCTList, testSCTList...) - differentSCTList[len(differentSCTList)-1] ^= 1 - // Test that extensions on intermediates are allowed but ignored. testCases = append(testCases, testCase{ name: "IgnoreExtensionsOnIntermediates-TLS13", @@ -6395,8 +6393,8 @@ func addExtensionTests() { // Send different values on the intermediate. This tests // the intermediate's extensions do not override the // leaf's. - SendOCSPOnIntermediates: []byte{1, 3, 3, 7}, - SendSCTOnIntermediates: differentSCTList, + SendOCSPOnIntermediates: testOCSPResponse2, + SendSCTOnIntermediates: testSCTList2, }, }, flags: []string{ @@ -7446,6 +7444,34 @@ func addRenegotiationTests() { shouldFail: true, expectedError: ":UNEXPECTED_EXTENSION:", }) + + // The server may send different stapled OCSP responses or SCT lists on + // renegotiation, but BoringSSL ignores this and reports the old values. + // Also test that non-fatal verify results are preserved. + testCases = append(testCases, testCase{ + testType: clientTest, + name: "Renegotiation-ChangeAuthProperties", + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendOCSPResponseOnRenegotiation: testOCSPResponse2, + SendSCTListOnRenegotiation: testSCTList2, + }, + }, + renegotiate: 1, + flags: []string{ + "-renegotiate-freely", + "-expect-total-renegotiations", "1", + "-enable-ocsp-stapling", + "-expect-ocsp-response", + base64.StdEncoding.EncodeToString(testOCSPResponse), + "-enable-signed-cert-timestamps", + "-expect-signed-cert-timestamps", + base64.StdEncoding.EncodeToString(testSCTList), + "-verify-fail", + "-expect-verify-result", + }, + }) } func addDTLSReplayTests() { |