kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Insert a state before cert_cb.

Browse Source 
If cert_cb runs asynchronously, we end up repeating a large part of very
stateful ClientHello processing. This seems to be mostly fine and there
are few users of server-side cert_cb (it's a new API in 1.0.2), but it's
a little scary.

This is also visible to external consumers because some callbacks get
called multiple times. We especially should try to avoid that as there
is no guarantee that these callbacks are idempotent and give the same
answer each time.

Change-Id: I212b2325eae2cfca0fb423dace101e466c5e5d4e
Reviewed-on: https://boringssl-review.googlesource.com/10224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2016-08-09 20:00:32 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent fddbadcba9
commit 25fe85b38c
6 changed files with 249 additions and 158 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
include/openssl/ssl.h
View File

@ -817,7 +817,10 @@ OPENSSL_EXPORT int SSL_clear_chain_certs(SSL *ssl);
*
* On the client, the callback may call |SSL_get0_certificate_types| and
* |SSL_get_client_CA_list| for information on the server's certificate
* request. */
* request.
*
* On the server, the callback will be called on non-resumption handshakes,
* after extensions have been processed. */
OPENSSL_EXPORT void SSL_CTX_set_cert_cb(SSL_CTX *ctx,
int (*cb)(SSL *ssl, void *arg),
void *arg);

1
include/openssl/ssl3.h
View File

@ -351,6 +351,7 @@ OPENSSL_COMPILE_ASSERT(
#define SSL3_ST_SR_CLNT_HELLO_A (0x110 | SSL_ST_ACCEPT)
#define SSL3_ST_SR_CLNT_HELLO_B (0x111 | SSL_ST_ACCEPT)
#define SSL3_ST_SR_CLNT_HELLO_C (0x112 | SSL_ST_ACCEPT)
#define SSL3_ST_SR_CLNT_HELLO_D (0x113 | SSL_ST_ACCEPT)
/* write to client */
#define SSL3_ST_SW_HELLO_REQ_A (0x120 | SSL_ST_ACCEPT)
#define SSL3_ST_SW_HELLO_REQ_B (0x121 | SSL_ST_ACCEPT)

145
ssl/handshake_server.c
View File

@ -226,6 +226,7 @@ int ssl3_accept(SSL *ssl) {
case SSL3_ST_SR_CLNT_HELLO_A:
case SSL3_ST_SR_CLNT_HELLO_B:
case SSL3_ST_SR_CLNT_HELLO_C:
case SSL3_ST_SR_CLNT_HELLO_D:
ret = ssl3_get_client_hello(ssl);
if (ssl->state == SSL_ST_TLS13) {
break;
@ -550,9 +551,53 @@ int ssl_client_cipher_list_contains_cipher(
return 0;
}
static int negotiate_version(
SSL *ssl, int *out_alert,
const struct ssl_early_callback_ctx *client_hello) {
uint16_t min_version, max_version;
if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
*out_alert = SSL_AD_PROTOCOL_VERSION;
return 0;
}
uint16_t client_version =
ssl->method->version_from_wire(client_hello->version);
ssl->client_version = client_hello->version;
/* Select the version to use. */
uint16_t version = client_version;
if (version > max_version) {
version = max_version;
}
if (version < min_version) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
*out_alert = SSL_AD_PROTOCOL_VERSION;
return 0;
}
/* Handle FALLBACK_SCSV. */
if (ssl_client_cipher_list_contains_cipher(client_hello,
SSL3_CK_FALLBACK_SCSV & 0xffff) &&
version < max_version) {
OPENSSL_PUT_ERROR(SSL, SSL_R_INAPPROPRIATE_FALLBACK);
*out_alert = SSL3_AD_INAPPROPRIATE_FALLBACK;
return 0;
}
ssl->version = ssl->method->version_to_wire(version);
ssl->s3->enc_method = ssl3_get_enc_method(version);
assert(ssl->s3->enc_method != NULL);
/* At this point, the connection's version is known and |ssl->version| is
* fixed. Begin enforcing the record-layer version. */
ssl->s3->have_version = 1;
return 1;
}
static int ssl3_get_client_hello(SSL *ssl) {
int al = SSL_AD_INTERNAL_ERROR, ret = -1;
const SSL_CIPHER *c;
SSL_SESSION *session = NULL;
if (ssl->state == SSL3_ST_SR_CLNT_HELLO_A) {
@ -598,52 +643,9 @@ static int ssl3_get_client_hello(SSL *ssl) {
}
}
uint16_t client_version =
ssl->method->version_from_wire(client_hello.version);
ssl->client_version = client_hello.version;
uint16_t min_version, max_version;
if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
al = SSL_AD_PROTOCOL_VERSION;
goto f_err;
}
/* Note: This codepath may run twice if |ssl_get_prev_session| completes
* asynchronously.
*
* TODO(davidben): Clean up the order of events around ClientHello
* processing. */
/* Negotiate the protocol version if we have not done so yet. */
if (!ssl->s3->have_version) {
/* Select the version to use. */
uint16_t version = client_version;
if (version > max_version) {
version = max_version;
}
if (version < min_version) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
al = SSL_AD_PROTOCOL_VERSION;
goto f_err;
}
/* Handle FALLBACK_SCSV. */
if (ssl_client_cipher_list_contains_cipher(
&client_hello, SSL3_CK_FALLBACK_SCSV & 0xffff) &&
version < max_version) {
al = SSL3_AD_INAPPROPRIATE_FALLBACK;
OPENSSL_PUT_ERROR(SSL, SSL_R_INAPPROPRIATE_FALLBACK);
goto f_err;
}
ssl->version = ssl->method->version_to_wire(version);
ssl->s3->enc_method = ssl3_get_enc_method(version);
assert(ssl->s3->enc_method != NULL);
/* At this point, the connection's version is known and |ssl->version| is
* fixed. Begin enforcing the record-layer version. */
ssl->s3->have_version = 1;
} else if (client_version < ssl3_protocol_version(ssl)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_VERSION_NUMBER);
al = SSL_AD_PROTOCOL_VERSION;
if (!negotiate_version(ssl, &al, &client_hello)) {
goto f_err;
}
@ -651,14 +653,18 @@ static int ssl3_get_client_hello(SSL *ssl) {
ssl->state = SSL_ST_TLS13;
return 1;
}
}
if (ssl->state == SSL3_ST_SR_CLNT_HELLO_C) {
/* Load the client random. */
if (client_hello.random_len != SSL3_RANDOM_SIZE) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return -1;
}
memcpy(ssl->s3->client_random, client_hello.random, client_hello.random_len);
memcpy(ssl->s3->client_random, client_hello.random,
client_hello.random_len);
/* Determine whether we are doing session resumption. */
int send_new_ticket = 0;
switch (
ssl_get_prev_session(ssl, &session, &send_new_ticket, &client_hello)) {
@ -695,8 +701,8 @@ static int ssl3_get_client_hello(SSL *ssl) {
}
has_session =
/* Only resume if the session's version matches the negotiated version:
* most clients do not accept a mismatch. */
/* Only resume if the session's version matches the negotiated
* version: most clients do not accept a mismatch. */
ssl->version == session->ssl_version &&
/* If the client offers the EMS extension, but the previous session
* didn't use it, then negotiate a new session. */
@ -728,15 +734,6 @@ static int ssl3_get_client_hello(SSL *ssl) {
goto f_err;
}
/* If it is a hit, check that the cipher is in the list. */
if (ssl->session != NULL &&
!ssl_client_cipher_list_contains_cipher(
&client_hello, (uint16_t)ssl->session->cipher->id)) {
al = SSL_AD_ILLEGAL_PARAMETER;
OPENSSL_PUT_ERROR(SSL, SSL_R_REQUIRED_CIPHER_MISSING);
goto f_err;
}
/* Only null compression is supported. */
if (memchr(client_hello.compression_methods, 0,
client_hello.compression_methods_len) == NULL) {
@ -757,10 +754,27 @@ static int ssl3_get_client_hello(SSL *ssl) {
goto f_err;
}
/* Given ciphers and SSL_get_ciphers, we must pick a cipher */
if (ssl->session == NULL) {
/* Let cert callback update server certificates if required */
if (ssl->cert->cert_cb) {
ssl->state = SSL3_ST_SR_CLNT_HELLO_D;
}
/* Determine the remaining connection parameters. This is a separate state so
* |cert_cb| does not cause earlier logic to run multiple times. */
assert(ssl->state == SSL3_ST_SR_CLNT_HELLO_D);
if (ssl->session != NULL) {
/* Check that the cipher is in the list. */
if (!ssl_client_cipher_list_contains_cipher(
&client_hello, (uint16_t)ssl->session->cipher->id)) {
al = SSL_AD_ILLEGAL_PARAMETER;
OPENSSL_PUT_ERROR(SSL, SSL_R_REQUIRED_CIPHER_MISSING);
goto f_err;
}
ssl->s3->tmp.new_cipher = ssl->session->cipher;
ssl->s3->tmp.cert_request = 0;
} else {
/* Call |cert_cb| to update server certificates if required. */
if (ssl->cert->cert_cb != NULL) {
int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg);
if (rv == 0) {
al = SSL_AD_INTERNAL_ERROR;
@ -773,7 +787,8 @@ static int ssl3_get_client_hello(SSL *ssl) {
}
}
c = ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
const SSL_CIPHER *c =
ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
if (c == NULL) {
al = SSL_AD_HANDSHAKE_FAILURE;
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER);
@ -794,10 +809,6 @@ static int ssl3_get_client_hello(SSL *ssl) {
if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ssl->s3->tmp.cert_request = 0;
}
} else {
/* Session-id reuse */
ssl->s3->tmp.new_cipher = ssl->session->cipher;
ssl->s3->tmp.cert_request = 0;
}
/* Now that the cipher is known, initialize the handshake hash. */

22
ssl/test/bssl_shim.cc
View File

@ -101,6 +101,8 @@ struct TestState {
// operation has been retried.
unsigned private_key_retries = 0;
bool got_new_session = false;
bool ticket_decrypt_done = false;
bool alpn_select_done = false;
};
static void TestStateExFree(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
@ -515,6 +517,13 @@ static int NextProtoSelectCallback(SSL* ssl, uint8_t** out, uint8_t* outlen,
static int AlpnSelectCallback(SSL* ssl, const uint8_t** out, uint8_t* outlen,
const uint8_t* in, unsigned inlen, void* arg) {
if (GetTestState(ssl)->alpn_select_done) {
fprintf(stderr, "AlpnSelectCallback called after completion.\n");
exit(1);
}
GetTestState(ssl)->alpn_select_done = true;
const TestConfig *config = GetTestConfig(ssl);
if (config->decline_alpn) {
return SSL_TLSEXT_ERR_NOACK;
@ -627,6 +636,10 @@ static void InfoCallback(const SSL *ssl, int type, int val) {
abort();
}
GetTestState(ssl)->handshake_done = true;
// Callbacks may be called again on a new handshake.
GetTestState(ssl)->ticket_decrypt_done = false;
GetTestState(ssl)->alpn_select_done = false;
}
}
@ -640,6 +653,15 @@ static int NewSessionCallback(SSL *ssl, SSL_SESSION *session) {
static int TicketKeyCallback(SSL *ssl, uint8_t *key_name, uint8_t *iv,
EVP_CIPHER_CTX *ctx, HMAC_CTX *hmac_ctx,
int encrypt) {
if (!encrypt) {
if (GetTestState(ssl)->ticket_decrypt_done) {
fprintf(stderr, "TicketKeyCallback called after completion.\n");
return -1;
}
GetTestState(ssl)->ticket_decrypt_done = true;
}
// This is just test code, so use the all-zeros key.
static const uint8_t kZeros[16] = {0};

39
ssl/test/runner/runner.go
View File

@ -4338,6 +4338,25 @@ func addExtensionTests() {
resumeSession: resumeSession,
})
// Test ALPN in async mode as well to ensure that extensions callbacks are only
// called once.
testCases = append(testCases, testCase{
testType: serverTest,
name: "ALPNServer-Async-" + ver.name,
config: Config{
MaxVersion: ver.version,
NextProtos: []string{"foo", "bar", "baz"},
},
flags: []string{
"-expect-advertised-alpn", "\x03foo\x03bar\x03baz",
"-select-alpn", "foo",
"-async",
},
expectedNextProto: "foo",
expectedNextProtoType: alpn,
resumeSession: resumeSession,
})
var emptyString string
testCases = append(testCases, testCase{
testType: clientTest,
@ -4502,6 +4521,26 @@ func addExtensionTests() {
resumeSession: true,
})
// Test that the ticket callback is only called once when everything before
// it in the ClientHello is asynchronous. This corrupts the ticket so
// certificate selection callbacks run.
testCases = append(testCases, testCase{
testType: serverTest,
name: "TicketCallback-SingleCall-" + ver.name,
config: Config{
MaxVersion: ver.version,
Bugs: ProtocolBugs{
CorruptTicket: true,
},
},
resumeSession: true,
expectResumeRejected: true,
flags: []string{
"-use-ticket-callback",
"-async",
},
})
// Resume with an oversized session id.
testCases = append(testCases, testCase{
testType: serverTest,

23
ssl/tls13_server.c
View File

@ -29,6 +29,7 @@
enum server_hs_state_t {
state_process_client_hello = 0,
state_select_parameters,
state_send_hello_retry_request,
state_flush_hello_retry_request,
state_process_second_client_hello,
@ -150,9 +151,12 @@ static enum ssl_hs_wait_t do_process_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) {
return ssl_hs_error;
}
/* Let cert callback update server certificates if required.
*
* TODO(davidben): Can this get run earlier? */
hs->state = state_select_parameters;
return ssl_hs_ok;
}
static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {
/* Call |cert_cb| to update server certificates if required. */
if (ssl->cert->cert_cb != NULL) {
int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg);
if (rv == 0) {
@ -161,11 +165,19 @@ static enum ssl_hs_wait_t do_process_client_hello(SSL *ssl, SSL_HANDSHAKE *hs) {
return ssl_hs_error;
}
if (rv < 0) {
hs->state = state_process_client_hello;
hs->state = state_select_parameters;
return ssl_hs_x509_lookup;
}
}
struct ssl_early_callback_ctx client_hello;
if (!ssl_early_callback_init(ssl, &client_hello, ssl->init_msg,
ssl->init_num)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED);
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return ssl_hs_error;
}
const SSL_CIPHER *cipher =
ssl3_choose_cipher(ssl, &client_hello, ssl_get_cipher_preferences(ssl));
if (cipher == NULL) {
@ -529,6 +541,9 @@ enum ssl_hs_wait_t tls13_server_handshake(SSL *ssl) {
case state_process_client_hello:
ret = do_process_client_hello(ssl, hs);
break;
case state_select_parameters:
ret = do_select_parameters(ssl, hs);
break;
case state_send_hello_retry_request:
ret = do_send_hello_retry_request(ssl, hs);
break;