Move more side-specific code out of tls13_process_certificate.
tls13_process_certificate can take a boolean for whether anonymous is allowed. This does change the error on the client slightly, but I think this is correct anyway. It is not a syntax error for the server to send no certificates in so far as the Certificate message allows it. It's just illegal. Change-Id: I1af80dacf23f50aad0b1fbd884bc068a40714399 Reviewed-on: https://boringssl-review.googlesource.com/9072 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
parent
bb9e36e005
commit
4087df92f4
@ -919,7 +919,7 @@ int tls13_post_handshake(SSL *ssl);
|
|||||||
* it returns one. Otherwise, it sends an alert and returns zero. */
|
* it returns one. Otherwise, it sends an alert and returns zero. */
|
||||||
int tls13_check_message_type(SSL *ssl, int type);
|
int tls13_check_message_type(SSL *ssl, int type);
|
||||||
|
|
||||||
int tls13_process_certificate(SSL *ssl);
|
int tls13_process_certificate(SSL *ssl, int allow_anonymous);
|
||||||
int tls13_process_certificate_verify(SSL *ssl);
|
int tls13_process_certificate_verify(SSL *ssl);
|
||||||
int tls13_process_finished(SSL *ssl);
|
int tls13_process_finished(SSL *ssl);
|
||||||
|
|
||||||
|
@ -1430,7 +1430,7 @@ func addBasicTests() {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
shouldFail: true,
|
shouldFail: true,
|
||||||
expectedError: ":DECODE_ERROR:",
|
expectedError: ":PEER_DID_NOT_RETURN_A_CERTIFICATE:",
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "TLSFatalBadPackets",
|
name: "TLSFatalBadPackets",
|
||||||
|
@ -181,7 +181,7 @@ err:
|
|||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
int tls13_process_certificate(SSL *ssl) {
|
int tls13_process_certificate(SSL *ssl, int allow_anonymous) {
|
||||||
CBS cbs, context;
|
CBS cbs, context;
|
||||||
CBS_init(&cbs, ssl->init_msg, ssl->init_num);
|
CBS_init(&cbs, ssl->init_msg, ssl->init_num);
|
||||||
if (!CBS_get_u8_length_prefixed(&cbs, &context) ||
|
if (!CBS_get_u8_length_prefixed(&cbs, &context) ||
|
||||||
@ -191,12 +191,12 @@ int tls13_process_certificate(SSL *ssl) {
|
|||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const int retain_sha256 =
|
||||||
|
ssl->server && ssl->ctx->retain_only_sha256_of_client_certs;
|
||||||
int ret = 0;
|
int ret = 0;
|
||||||
uint8_t alert;
|
uint8_t alert;
|
||||||
STACK_OF(X509) *chain = ssl_parse_cert_chain(
|
STACK_OF(X509) *chain = ssl_parse_cert_chain(
|
||||||
ssl, &alert, ssl->ctx->retain_only_sha256_of_client_certs
|
ssl, &alert, retain_sha256 ? ssl->s3->new_session->peer_sha256 : NULL,
|
||||||
? ssl->s3->new_session->peer_sha256
|
|
||||||
: NULL,
|
|
||||||
&cbs);
|
&cbs);
|
||||||
if (chain == NULL) {
|
if (chain == NULL) {
|
||||||
ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
|
ssl3_send_alert(ssl, SSL3_AL_FATAL, alert);
|
||||||
@ -210,15 +210,7 @@ int tls13_process_certificate(SSL *ssl) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (sk_X509_num(chain) == 0) {
|
if (sk_X509_num(chain) == 0) {
|
||||||
/* Clients must receive a certificate from the server. */
|
if (!allow_anonymous) {
|
||||||
if (!ssl->server) {
|
|
||||||
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
|
|
||||||
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
|
|
||||||
goto err;
|
|
||||||
}
|
|
||||||
|
|
||||||
/* Servers may be configured to accept anonymous clients. */
|
|
||||||
if (ssl->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT) {
|
|
||||||
OPENSSL_PUT_ERROR(SSL, SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE);
|
OPENSSL_PUT_ERROR(SSL, SSL_R_PEER_DID_NOT_RETURN_A_CERTIFICATE);
|
||||||
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
|
ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
|
||||||
goto err;
|
goto err;
|
||||||
@ -229,10 +221,7 @@ int tls13_process_certificate(SSL *ssl) {
|
|||||||
goto err;
|
goto err;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (ssl->server && ssl->ctx->retain_only_sha256_of_client_certs) {
|
ssl->s3->new_session->peer_sha256_valid = retain_sha256;
|
||||||
/* The hash was filled in by |ssl_parse_cert_chain|. */
|
|
||||||
ssl->s3->new_session->peer_sha256_valid = 1;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!ssl_verify_cert_chain(ssl, chain)) {
|
if (!ssl_verify_cert_chain(ssl, chain)) {
|
||||||
goto err;
|
goto err;
|
||||||
|
@ -361,7 +361,7 @@ static enum ssl_hs_wait_t do_process_certificate_request(SSL *ssl,
|
|||||||
static enum ssl_hs_wait_t do_process_server_certificate(SSL *ssl,
|
static enum ssl_hs_wait_t do_process_server_certificate(SSL *ssl,
|
||||||
SSL_HANDSHAKE *hs) {
|
SSL_HANDSHAKE *hs) {
|
||||||
if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE) ||
|
if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE) ||
|
||||||
!tls13_process_certificate(ssl) ||
|
!tls13_process_certificate(ssl, 0 /* certificate required */) ||
|
||||||
!ssl->method->hash_current_message(ssl)) {
|
!ssl->method->hash_current_message(ssl)) {
|
||||||
return ssl_hs_error;
|
return ssl_hs_error;
|
||||||
}
|
}
|
||||||
|
@ -458,8 +458,11 @@ static enum ssl_hs_wait_t do_process_client_certificate(SSL *ssl,
|
|||||||
return ssl_hs_ok;
|
return ssl_hs_ok;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const int allow_anonymous =
|
||||||
|
(ssl->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT) == 0;
|
||||||
|
|
||||||
if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE) ||
|
if (!tls13_check_message_type(ssl, SSL3_MT_CERTIFICATE) ||
|
||||||
!tls13_process_certificate(ssl) ||
|
!tls13_process_certificate(ssl, allow_anonymous) ||
|
||||||
!ssl->method->hash_current_message(ssl)) {
|
!ssl->method->hash_current_message(ssl)) {
|
||||||
return ssl_hs_error;
|
return ssl_hs_error;
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user