Browse Source

Revert of Determining certificate_auth and key_exchange based on SSL.

Reason for revert:  Right now in TLS 1.3, certificate_auth is exactly
the same as whether we're doing resumption. With the weird reauth
stuff punted to later in the spec, having extra state is just more
room for bugs to creep in.

Original issue's description:
> Determining certificate_auth and key_exchange based on SSL.
> 
> This allows us to switch TLS 1.3 to use non-cipher based negotiation
> without needing to use separate functions between 1.3 and below.
> 
> BUG=77
> 
> Change-Id: I9207e7a6793cb69e8300e2c15afe3548cbf82af2
> Reviewed-on: https://boringssl-review.googlesource.com/10803
> 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>
> 

Change-Id: I240e3ee959ffd1f2481a06eabece3af554d20ffa
Reviewed-on: https://boringssl-review.googlesource.com/11008
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>
kris/onging/CECPQ3_patch15
David Benjamin 8 years ago
committed by CQ bot account: commit-bot@chromium.org
parent
commit
3d458dc048
6 changed files with 14 additions and 32 deletions
  1. +6
    -8
      ssl/handshake_client.c
  2. +3
    -9
      ssl/handshake_server.c
  3. +0
    -2
      ssl/internal.h
  4. +2
    -2
      ssl/t1_lib.c
  5. +1
    -4
      ssl/tls13_client.c
  6. +2
    -7
      ssl/tls13_server.c

+ 6
- 8
ssl/handshake_client.c View File

@@ -264,7 +264,7 @@ int ssl3_connect(SSL *ssl) {
break; break;


case SSL3_ST_CR_CERT_A: case SSL3_ST_CR_CERT_A:
if (ssl->s3->hs->use_cert_auth) {
if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ret = ssl3_get_server_certificate(ssl); ret = ssl3_get_server_certificate(ssl);
if (ret <= 0) { if (ret <= 0) {
goto end; goto end;
@@ -288,7 +288,7 @@ int ssl3_connect(SSL *ssl) {
break; break;


case SSL3_ST_VERIFY_SERVER_CERT: case SSL3_ST_VERIFY_SERVER_CERT:
if (ssl->s3->hs->use_cert_auth) {
if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ret = ssl3_verify_server_cert(ssl); ret = ssl3_verify_server_cert(ssl);
if (ret <= 0) { if (ret <= 0) {
goto end; goto end;
@@ -308,7 +308,7 @@ int ssl3_connect(SSL *ssl) {
break; break;


case SSL3_ST_CR_CERT_REQ_A: case SSL3_ST_CR_CERT_REQ_A:
if (ssl->s3->hs->use_cert_auth) {
if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ret = ssl3_get_certificate_request(ssl); ret = ssl3_get_certificate_request(ssl);
if (ret <= 0) { if (ret <= 0) {
goto end; goto end;
@@ -952,9 +952,6 @@ static int ssl3_get_server_hello(SSL *ssl) {
ssl->s3->new_session->cipher = c; ssl->s3->new_session->cipher = c;
} }
ssl->s3->tmp.new_cipher = c; ssl->s3->tmp.new_cipher = c;
if (ssl_cipher_uses_certificate_auth(c)) {
ssl->s3->hs->use_cert_auth = 1;
}


/* Now that the cipher is known, initialize the handshake hash. */ /* Now that the cipher is known, initialize the handshake hash. */
if (!ssl3_init_handshake_hash(ssl)) { if (!ssl3_init_handshake_hash(ssl)) {
@@ -964,7 +961,8 @@ static int ssl3_get_server_hello(SSL *ssl) {
/* If doing a full handshake, the server may request a client certificate /* If doing a full handshake, the server may request a client certificate
* which requires hashing the handshake transcript. Otherwise, the handshake * which requires hashing the handshake transcript. Otherwise, the handshake
* buffer may be released. */ * buffer may be released. */
if (ssl->session != NULL || !ssl->s3->hs->use_cert_auth) {
if (ssl->session != NULL ||
!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ssl3_free_handshake_buffer(ssl); ssl3_free_handshake_buffer(ssl);
} }


@@ -1286,7 +1284,7 @@ static int ssl3_get_server_key_exchange(SSL *ssl) {
CBS_len(&server_key_exchange_orig) - CBS_len(&server_key_exchange)); CBS_len(&server_key_exchange_orig) - CBS_len(&server_key_exchange));


/* ServerKeyExchange should be signed by the server's public key. */ /* ServerKeyExchange should be signed by the server's public key. */
if (ssl->s3->hs->use_cert_auth) {
if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
pkey = X509_get_pubkey(ssl->s3->new_session->peer); pkey = X509_get_pubkey(ssl->s3->new_session->peer);
if (pkey == NULL) { if (pkey == NULL) {
goto err; goto err;


+ 3
- 9
ssl/handshake_server.c View File

@@ -257,7 +257,7 @@ int ssl3_accept(SSL *ssl) {


case SSL3_ST_SW_CERT_A: case SSL3_ST_SW_CERT_A:
case SSL3_ST_SW_CERT_B: case SSL3_ST_SW_CERT_B:
if (ssl->s3->hs->use_cert_auth) {
if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ret = ssl3_send_server_certificate(ssl); ret = ssl3_send_server_certificate(ssl);
if (ret <= 0) { if (ret <= 0) {
goto end; goto end;
@@ -776,9 +776,6 @@ static int ssl3_get_client_hello(SSL *ssl) {


ssl->s3->tmp.new_cipher = ssl->session->cipher; ssl->s3->tmp.new_cipher = ssl->session->cipher;
ssl->s3->tmp.cert_request = 0; ssl->s3->tmp.cert_request = 0;
if (ssl_cipher_uses_certificate_auth(ssl->session->cipher)) {
ssl->s3->hs->use_cert_auth = 1;
}
} else { } else {
/* Call |cert_cb| to update server certificates if required. */ /* Call |cert_cb| to update server certificates if required. */
if (ssl->cert->cert_cb != NULL) { if (ssl->cert->cert_cb != NULL) {
@@ -804,9 +801,6 @@ static int ssl3_get_client_hello(SSL *ssl) {


ssl->s3->new_session->cipher = c; ssl->s3->new_session->cipher = c;
ssl->s3->tmp.new_cipher = c; ssl->s3->tmp.new_cipher = c;
if (ssl_cipher_uses_certificate_auth(c)) {
ssl->s3->hs->use_cert_auth = 1;
}


/* Determine whether to request a client certificate. */ /* Determine whether to request a client certificate. */
ssl->s3->tmp.cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER); ssl->s3->tmp.cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER);
@@ -816,7 +810,7 @@ static int ssl3_get_client_hello(SSL *ssl) {
ssl->s3->tmp.cert_request = 0; ssl->s3->tmp.cert_request = 0;
} }
/* CertificateRequest may only be sent in certificate-based ciphers. */ /* CertificateRequest may only be sent in certificate-based ciphers. */
if (!ssl->s3->hs->use_cert_auth) {
if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ssl->s3->tmp.cert_request = 0; ssl->s3->tmp.cert_request = 0;
} }


@@ -1053,7 +1047,7 @@ static int ssl3_send_server_key_exchange(SSL *ssl) {
} }


/* Add a signature. */ /* Add a signature. */
if (ssl->s3->hs->use_cert_auth) {
if (ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
if (!ssl_has_private_key(ssl)) { if (!ssl_has_private_key(ssl)) {
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
goto err; goto err;


+ 0
- 2
ssl/internal.h View File

@@ -897,8 +897,6 @@ struct ssl_handshake_st {
uint8_t secret[EVP_MAX_MD_SIZE]; uint8_t secret[EVP_MAX_MD_SIZE];
uint8_t traffic_secret_0[EVP_MAX_MD_SIZE]; uint8_t traffic_secret_0[EVP_MAX_MD_SIZE];


int use_cert_auth;

SSL_ECDH_CTX *groups; SSL_ECDH_CTX *groups;
size_t groups_len; size_t groups_len;
/* retry_group is the group ID selected by the server in HelloRetryRequest. */ /* retry_group is the group ID selected by the server in HelloRetryRequest. */


+ 2
- 2
ssl/t1_lib.c View File

@@ -1191,7 +1191,7 @@ static int ext_ocsp_parse_serverhello(SSL *ssl, uint8_t *out_alert,
} }


/* OCSP stapling is forbidden on a non-certificate cipher. */ /* OCSP stapling is forbidden on a non-certificate cipher. */
if (!ssl->s3->hs->use_cert_auth) {
if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
return 0; return 0;
} }


@@ -1244,7 +1244,7 @@ static int ext_ocsp_parse_clienthello(SSL *ssl, uint8_t *out_alert,
static int ext_ocsp_add_serverhello(SSL *ssl, CBB *out) { static int ext_ocsp_add_serverhello(SSL *ssl, CBB *out) {
if (!ssl->s3->tmp.ocsp_stapling_requested || if (!ssl->s3->tmp.ocsp_stapling_requested ||
ssl->ctx->ocsp_response_length == 0 || ssl->ctx->ocsp_response_length == 0 ||
!ssl->s3->hs->use_cert_auth) {
!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
return 1; return 1;
} }




+ 1
- 4
ssl/tls13_client.c View File

@@ -267,9 +267,6 @@ static enum ssl_hs_wait_t do_process_server_hello(SSL *ssl, SSL_HANDSHAKE *hs) {


ssl->s3->new_session->cipher = cipher; ssl->s3->new_session->cipher = cipher;
ssl->s3->tmp.new_cipher = cipher; ssl->s3->tmp.new_cipher = cipher;
if (ssl_cipher_uses_certificate_auth(cipher)) {
hs->use_cert_auth = 1;
}


/* The PRF hash is now known. Set up the key schedule. */ /* The PRF hash is now known. Set up the key schedule. */
static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0}; static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0};
@@ -383,7 +380,7 @@ static enum ssl_hs_wait_t do_process_certificate_request(SSL *ssl,
ssl->s3->tmp.cert_request = 0; ssl->s3->tmp.cert_request = 0;


/* CertificateRequest may only be sent in certificate-based ciphers. */ /* CertificateRequest may only be sent in certificate-based ciphers. */
if (!ssl->s3->hs->use_cert_auth) {
if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
hs->state = state_process_server_finished; hs->state = state_process_server_finished;
return ssl_hs_ok; return ssl_hs_ok;
} }


+ 2
- 7
ssl/tls13_server.c View File

@@ -230,10 +230,6 @@ static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {


ssl->s3->new_session->cipher = cipher; ssl->s3->new_session->cipher = cipher;
ssl->s3->tmp.new_cipher = cipher; ssl->s3->tmp.new_cipher = cipher;

if (ssl_cipher_uses_certificate_auth(cipher)) {
hs->use_cert_auth = 1;
}
} else { } else {
uint16_t resumption_cipher; uint16_t resumption_cipher;
if (!ssl_cipher_get_ecdhe_psk_cipher(ssl->s3->new_session->cipher, if (!ssl_cipher_get_ecdhe_psk_cipher(ssl->s3->new_session->cipher,
@@ -243,7 +239,6 @@ static enum ssl_hs_wait_t do_select_parameters(SSL *ssl, SSL_HANDSHAKE *hs) {
return ssl_hs_error; return ssl_hs_error;
} }
ssl->s3->tmp.new_cipher = SSL_get_cipher_by_value(resumption_cipher); ssl->s3->tmp.new_cipher = SSL_get_cipher_by_value(resumption_cipher);
hs->use_cert_auth = 0;
} }


ssl->method->received_flight(ssl); ssl->method->received_flight(ssl);
@@ -386,7 +381,7 @@ static enum ssl_hs_wait_t do_send_certificate_request(SSL *ssl,
/* Determine whether to request a client certificate. */ /* Determine whether to request a client certificate. */
ssl->s3->tmp.cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER); ssl->s3->tmp.cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER);
/* CertificateRequest may only be sent in certificate-based ciphers. */ /* CertificateRequest may only be sent in certificate-based ciphers. */
if (!ssl->s3->hs->use_cert_auth) {
if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
ssl->s3->tmp.cert_request = 0; ssl->s3->tmp.cert_request = 0;
} }


@@ -431,7 +426,7 @@ err:


static enum ssl_hs_wait_t do_send_server_certificate(SSL *ssl, static enum ssl_hs_wait_t do_send_server_certificate(SSL *ssl,
SSL_HANDSHAKE *hs) { SSL_HANDSHAKE *hs) {
if (!ssl->s3->hs->use_cert_auth) {
if (!ssl_cipher_uses_certificate_auth(ssl->s3->tmp.new_cipher)) {
hs->state = state_send_server_finished; hs->state = state_send_server_finished;
return ssl_hs_ok; return ssl_hs_ok;
} }


Loading…
Cancel
Save