From b398d16c1d103cff0fee577a9e8c1a7cc8d5242b Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 13 Nov 2014 15:29:09 -0500 Subject: [PATCH] Remove SSL_check_chain and unexport CERT_PKEY flags. Both of these are newly-exported in OpenSSL 1.0.2, so they cannot be used by current consumers. This was added in upstream's 18d7158809c9722f4c6d2a8af7513577274f9b56 to support custom selection of certificates. The intent seems to be that you listen to cert_cb and use SSL_check_chain to lean on OpenSSL to process signature algorithms list for you. Unfortunately, the implementation is slightly suspect: it uses the same function as the codepath which mutates and refers to the CERT_PKEY of the matching type. Some access was guarded by check_flags, but this is too complex. Part of it is also because the matching digest is selected early and we intend to connect this to EVP_PKEY_supports_digest so it is no longer a property of just the key type. Let's remove the hook for now, to unblock removing a lot of complexity. After cleaning up this area, a function like this could be cleaner to support, but we already have a version of this: select_certificate_cb and ssl_early_callback_ctx. Change-Id: I3add425b3996e5e32d4a88e14cc607b4fdaa5aec Reviewed-on: https://boringssl-review.googlesource.com/2283 Reviewed-by: Adam Langley --- include/openssl/ssl.h | 20 ------------ include/openssl/tls1.h | 2 -- ssl/s3_clnt.c | 2 +- ssl/ssl_locl.h | 24 ++++++++++++-- ssl/t1_lib.c | 74 ++++++++++-------------------------------- 5 files changed, 41 insertions(+), 81 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index d357e73f6..bcc9501c0 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -543,26 +543,6 @@ struct ssl_session_st /* Clear verification errors from queue */ #define SSL_BUILD_CHAIN_FLAG_CLEAR_ERROR 0x10 -/* Flags returned by SSL_check_chain */ -/* Certificate can be used with this session */ -#define CERT_PKEY_VALID 0x1 -/* Certificate can also be used for signing */ -#define CERT_PKEY_SIGN 0x2 -/* EE certificate signing algorithm OK */ -#define CERT_PKEY_EE_SIGNATURE 0x10 -/* CA signature algorithms OK */ -#define CERT_PKEY_CA_SIGNATURE 0x20 -/* EE certificate parameters OK */ -#define CERT_PKEY_EE_PARAM 0x40 -/* CA certificate parameters OK */ -#define CERT_PKEY_CA_PARAM 0x80 -/* Client CA issuer names match (always set for server cert) */ -#define CERT_PKEY_ISSUER_NAME 0x200 -/* Cert type matches client types (always set for server cert) */ -#define CERT_PKEY_CERT_TYPE 0x400 -/* Cert chain suitable to Suite B */ -#define CERT_PKEY_SUITEB 0x800 - #define SSL_CONF_FLAG_CMDLINE 0x1 #define SSL_CONF_FLAG_FILE 0x2 #define SSL_CONF_FLAG_CLIENT 0x4 diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index d2682ddff..dc106d2c1 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h @@ -325,8 +325,6 @@ OPENSSL_EXPORT int SSL_get_shared_sigalgs(SSL *s, int idx, int *psign, int *phash, int *psignandhash, unsigned char *rsig, unsigned char *rhash); -OPENSSL_EXPORT int SSL_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain); - #define SSL_set_tlsext_host_name(s,name) \ SSL_ctrl(s,SSL_CTRL_SET_TLSEXT_HOSTNAME,TLSEXT_NAMETYPE_host_name,(char *)name) diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index bade13b38..a49c7697b 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -2351,7 +2351,7 @@ static int ssl3_check_client_certificate(SSL *s) /* If strict mode check suitability of chain before using it. */ if (s->cert->cert_flags & SSL_CERT_FLAGS_CHECK_TLS_STRICT && - !tls1_check_chain(s, NULL, NULL, NULL, -2)) + !tls1_check_chain(s, s->cert->key - s->cert->pkeys)) return 0; return 1; } diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index aca7cbded..950f25176 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -421,6 +421,27 @@ #define SSL_GET_MESSAGE_DONT_HASH_MESSAGE 0 #define SSL_GET_MESSAGE_HASH_MESSAGE 1 +/* Flags for a CERT_PKEY. */ + +/* Certificate can be used with this session */ +#define CERT_PKEY_VALID 0x1 +/* Certificate can also be used for signing */ +#define CERT_PKEY_SIGN 0x2 +/* EE certificate signing algorithm OK */ +#define CERT_PKEY_EE_SIGNATURE 0x10 +/* CA signature algorithms OK */ +#define CERT_PKEY_CA_SIGNATURE 0x20 +/* EE certificate parameters OK */ +#define CERT_PKEY_EE_PARAM 0x40 +/* CA certificate parameters OK */ +#define CERT_PKEY_CA_PARAM 0x80 +/* Client CA issuer names match (always set for server cert) */ +#define CERT_PKEY_ISSUER_NAME 0x200 +/* Cert type matches client types (always set for server cert) */ +#define CERT_PKEY_CERT_TYPE 0x400 +/* Cert chain suitable to Suite B */ +#define CERT_PKEY_SUITEB 0x800 + typedef struct cert_pkey_st { X509 *x509; @@ -1059,8 +1080,7 @@ int tls1_record_handshake_hashes_for_channel_id(SSL *s); int tls1_set_sigalgs_list(CERT *c, const char *str, int client); int tls1_set_sigalgs(CERT *c, const int *salg, size_t salglen, int client); -int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain, - int idx); +int tls1_check_chain(SSL *s, int idx); void tls1_set_cert_validity(SSL *s); /* ssl_ctx_log_rsa_client_key_exchange logs |premaster| to |ctx|, if logging is diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index b228b2afe..2e28a08a4 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -2949,62 +2949,29 @@ static int ssl_check_ca_name(STACK_OF(X509_NAME) *names, X509 *x) return 0; } -/* Check certificate chain is consistent with TLS extensions and is - * usable by server. This servers two purposes: it allows users to - * check chains before passing them to the server and it allows the - * server to check chains before attempting to use them. +/* Check certificate chain is consistent with TLS extensions and is usable by + * server. This allows the server to check chains before attempting to use them. */ -/* Flags which need to be set for a certificate when stict mode not set */ - -#define CERT_PKEY_VALID_FLAGS \ - (CERT_PKEY_EE_SIGNATURE|CERT_PKEY_EE_PARAM) -/* Strict mode flags */ -#define CERT_PKEY_STRICT_FLAGS \ - (CERT_PKEY_VALID_FLAGS|CERT_PKEY_CA_SIGNATURE|CERT_PKEY_CA_PARAM \ - | CERT_PKEY_ISSUER_NAME|CERT_PKEY_CERT_TYPE) - -int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain, - int idx) +int tls1_check_chain(SSL *s, int idx) { size_t i; int rv = 0; int check_flags = 0, strict_mode; CERT_PKEY *cpk = NULL; CERT *c = s->cert; - /* idx == -1 means checking server chains */ - if (idx != -1) - { - /* idx == -2 means checking client certificate chains */ - if (idx == -2) - { - cpk = c->key; - idx = cpk - c->pkeys; - } - else - cpk = c->pkeys + idx; - x = cpk->x509; - pk = cpk->privatekey; - chain = cpk->chain; - strict_mode = c->cert_flags & SSL_CERT_FLAGS_CHECK_TLS_STRICT; - /* If no cert or key, forget it */ - if (!x || !pk) - goto end; - } - else - { - if (!x || !pk) - goto end; - idx = ssl_cert_type(x, pk); - if (idx == -1) - goto end; - cpk = c->pkeys + idx; - if (c->cert_flags & SSL_CERT_FLAGS_CHECK_TLS_STRICT) - check_flags = CERT_PKEY_STRICT_FLAGS; - else - check_flags = CERT_PKEY_VALID_FLAGS; - strict_mode = 1; - } + X509 *x; + EVP_PKEY *pk; + STACK_OF(X509) *chain; + + cpk = c->pkeys + idx; + x = cpk->x509; + pk = cpk->privatekey; + chain = cpk->chain; + strict_mode = c->cert_flags & SSL_CERT_FLAGS_CHECK_TLS_STRICT; + /* If no cert or key, forget it */ + if (!x || !pk) + goto end; /* Check all signature algorithms are consistent with * signature algorithms extension if TLS 1.2 or later @@ -3199,13 +3166,8 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain, /* Set validity of certificates in an SSL structure */ void tls1_set_cert_validity(SSL *s) { - tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_RSA_ENC); - tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_RSA_SIGN); - tls1_check_chain(s, NULL, NULL, NULL, SSL_PKEY_ECC); - } -/* User level utiity function to check a chain is suitable */ -int SSL_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain) - { - return tls1_check_chain(s, x, pk, chain, -1); + tls1_check_chain(s, SSL_PKEY_RSA_ENC); + tls1_check_chain(s, SSL_PKEY_RSA_SIGN); + tls1_check_chain(s, SSL_PKEY_ECC); }