Check CA names during the handshake.
Rather than store CA names and only find out that they're unparsable when we're asked for a |STACK_OF(X509_NAME)|, check that we can parse them all during the handshake. This avoids changing the semantics with the previous change that kept CA names as |CRYPTO_BUFFER|s. Change-Id: I0fc7a4e6ab01685347e7a5be0d0579f45b8a4818 Reviewed-on: https://boringssl-review.googlesource.com/13969 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
34b4c829fd
commit
0bdef09263
@ -1446,6 +1446,11 @@ struct ssl_protocol_method_st {
|
||||
};
|
||||
|
||||
struct ssl_x509_method_st {
|
||||
/* check_client_CA_list returns one if |names| is a good list of X.509
|
||||
* distinguished names and zero otherwise. This is used to ensure that we can
|
||||
* reject unparsable values at handshake time when using crypto/x509. */
|
||||
int (*check_client_CA_list)(STACK_OF(CRYPTO_BUFFER) *names);
|
||||
|
||||
/* cert_clear frees and NULLs all X509-related state. */
|
||||
void (*cert_clear)(CERT *cert);
|
||||
/* cert_flush_cached_chain drops any cached |X509|-based certificate chain
|
||||
|
@ -726,6 +726,12 @@ STACK_OF(CRYPTO_BUFFER) *
|
||||
}
|
||||
}
|
||||
|
||||
if (!ssl->ctx->x509_method->check_client_CA_list(ret)) {
|
||||
*out_alert = SSL_AD_INTERNAL_ERROR;
|
||||
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
|
||||
goto err;
|
||||
}
|
||||
|
||||
return ret;
|
||||
|
||||
err:
|
||||
|
@ -331,6 +331,23 @@ static void ssl_crypto_x509_flush_cached_chain(CERT *cert) {
|
||||
cert->x509_chain = NULL;
|
||||
}
|
||||
|
||||
static int ssl_crypto_x509_check_client_CA_list(
|
||||
STACK_OF(CRYPTO_BUFFER) *names) {
|
||||
for (size_t i = 0; i < sk_CRYPTO_BUFFER_num(names); i++) {
|
||||
const CRYPTO_BUFFER *buffer = sk_CRYPTO_BUFFER_value(names, i);
|
||||
const uint8_t *inp = CRYPTO_BUFFER_data(buffer);
|
||||
X509_NAME *name = d2i_X509_NAME(NULL, &inp, CRYPTO_BUFFER_len(buffer));
|
||||
const int ok = name != NULL && inp == CRYPTO_BUFFER_data(buffer) +
|
||||
CRYPTO_BUFFER_len(buffer);
|
||||
X509_NAME_free(name);
|
||||
if (!ok) {
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
static void ssl_crypto_x509_clear(CERT *cert) {
|
||||
ssl_crypto_x509_flush_cached_leaf(cert);
|
||||
ssl_crypto_x509_flush_cached_chain(cert);
|
||||
@ -427,6 +444,7 @@ static void ssl_crypto_x509_ssl_ctx_flush_cached_client_CA(SSL_CTX *ctx) {
|
||||
}
|
||||
|
||||
const SSL_X509_METHOD ssl_crypto_x509_method = {
|
||||
ssl_crypto_x509_check_client_CA_list,
|
||||
ssl_crypto_x509_clear,
|
||||
ssl_crypto_x509_flush_cached_chain,
|
||||
ssl_crypto_x509_flush_cached_leaf,
|
||||
|
@ -258,6 +258,11 @@ const SSL_METHOD *TLS_client_method(void) {
|
||||
return TLS_method();
|
||||
}
|
||||
|
||||
static int ssl_noop_x509_check_client_CA_names(
|
||||
STACK_OF(CRYPTO_BUFFER) *names) {
|
||||
return 1;
|
||||
}
|
||||
|
||||
static void ssl_noop_x509_clear(CERT *cert) {}
|
||||
static void ssl_noop_x509_flush_cached_leaf(CERT *cert) {}
|
||||
static void ssl_noop_x509_flush_cached_chain(CERT *cert) {}
|
||||
@ -275,6 +280,7 @@ static void ssl_noop_x509_ssl_flush_cached_client_CA(SSL *ssl) {}
|
||||
static void ssl_noop_x509_ssl_ctx_flush_cached_client_CA(SSL_CTX *ctx) {}
|
||||
|
||||
const SSL_X509_METHOD ssl_noop_x509_method = {
|
||||
ssl_noop_x509_check_client_CA_names,
|
||||
ssl_noop_x509_clear,
|
||||
ssl_noop_x509_flush_cached_chain,
|
||||
ssl_noop_x509_flush_cached_leaf,
|
||||
|
Loading…
Reference in New Issue
Block a user