Add `for_comp` flag when retrieving certs for compression

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18186)
This commit is contained in:
Todd Short 2022-08-29 17:00:07 -04:00 committed by Todd Short
parent 7e3cacac94
commit 72620ac791
10 changed files with 52 additions and 56 deletions

View File

@ -280,6 +280,7 @@ typedef int (*tls_session_secret_cb_fn)(SSL *s, void *secret, int *secret_len,
#define SSL_EXT_TLS1_3_CERTIFICATE 0x1000
#define SSL_EXT_TLS1_3_NEW_SESSION_TICKET 0x2000
#define SSL_EXT_TLS1_3_CERTIFICATE_REQUEST 0x4000
#define SSL_EXT_TLS1_3_CERTIFICATE_COMPRESSION 0x8000
/* Typedefs for handling custom extensions */

View File

@ -198,7 +198,6 @@ static size_t ssl_get_cert_to_compress(SSL *ssl, CERT_PKEY *cpk, unsigned char *
WPACKET tmppkt;
BUF_MEM buf = { 0 };
size_t ret = 0;
const SSL_METHOD *method = NULL;
if (sc == NULL
|| cpk == NULL
@ -215,26 +214,14 @@ static size_t ssl_get_cert_to_compress(SSL *ssl, CERT_PKEY *cpk, unsigned char *
goto out;
/*
* ssl3_output_cert_chain() may generate an SSLfata() error,
* for this case, we want to ignore it
* ssl3_output_cert_chain() may generate an SSLfatal() error,
* for this case, we want to ignore it, argument for_comp = 1
*/
sc->statem.ignore_fatal = 1;
ERR_set_mark();
/* Must get the certificate as TLSv1.3, restore before returning */
method = SSL_get_ssl_method(ssl);
if (!SSL_set_ssl_method(ssl, tlsv1_3_server_method()))
goto out;
if (!ssl3_output_cert_chain(sc, &tmppkt, cpk))
if (!ssl3_output_cert_chain(sc, &tmppkt, cpk, 1))
goto out;
WPACKET_get_total_written(&tmppkt, &ret);
out:
/* Don't leave errors in the queue */
ERR_pop_to_mark();
sc->statem.ignore_fatal = 0;
if (method != NULL && !SSL_set_ssl_method(ssl, method))
ret = 0;
WPACKET_cleanup(&tmppkt);
if (ret != 0 && data != NULL)
*data = (unsigned char *)buf.data;

View File

@ -2613,7 +2613,7 @@ __owur int ssl3_finish_mac(SSL_CONNECTION *s, const unsigned char *buf,
size_t len);
void ssl3_free_digest_list(SSL_CONNECTION *s);
__owur unsigned long ssl3_output_cert_chain(SSL_CONNECTION *s, WPACKET *pkt,
CERT_PKEY *cpk);
CERT_PKEY *cpk, int for_comp);
__owur const SSL_CIPHER *ssl3_choose_cipher(SSL_CONNECTION *s,
STACK_OF(SSL_CIPHER) *clnt,
STACK_OF(SSL_CIPHER) *srvr);

View File

@ -832,6 +832,7 @@ int tls_construct_extensions(SSL_CONNECTION *s, WPACKET *pkt,
size_t i;
int min_version, max_version = 0, reason;
const EXTENSION_DEFINITION *thisexd;
int for_comp = (context & SSL_EXT_TLS1_3_CERTIFICATE_COMPRESSION) != 0;
if (!WPACKET_start_sub_packet_u16(pkt)
/*
@ -842,15 +843,17 @@ int tls_construct_extensions(SSL_CONNECTION *s, WPACKET *pkt,
|| ((context &
(SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO)) != 0
&& !WPACKET_set_flags(pkt,
WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH))) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH))) {
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
if ((context & SSL_EXT_CLIENT_HELLO) != 0) {
reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);
if (reason != 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, reason);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, reason);
return 0;
}
}
@ -894,7 +897,8 @@ int tls_construct_extensions(SSL_CONNECTION *s, WPACKET *pkt,
}
if (!WPACKET_close(pkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}

View File

@ -178,6 +178,7 @@ int custom_ext_add(SSL_CONNECTION *s, int context, WPACKET *pkt, X509 *x,
custom_ext_method *meth;
size_t i;
int al;
int for_comp = (context & SSL_EXT_TLS1_3_CERTIFICATE_COMPRESSION) != 0;
for (i = 0; i < exts->meths_count; i++) {
const unsigned char *out = NULL;
@ -211,7 +212,8 @@ int custom_ext_add(SSL_CONNECTION *s, int context, WPACKET *pkt, X509 *x,
meth->add_arg);
if (cb_retval < 0) {
SSLfatal(s, al, SSL_R_CALLBACK_FAILED);
if (!for_comp)
SSLfatal(s, al, SSL_R_CALLBACK_FAILED);
return 0; /* error */
}
if (cb_retval == 0)
@ -222,7 +224,8 @@ int custom_ext_add(SSL_CONNECTION *s, int context, WPACKET *pkt, X509 *x,
|| !WPACKET_start_sub_packet_u16(pkt)
|| (outlen > 0 && !WPACKET_memcpy(pkt, out, outlen))
|| !WPACKET_close(pkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
if ((context & SSL_EXT_CLIENT_HELLO) != 0) {
@ -230,7 +233,8 @@ int custom_ext_add(SSL_CONNECTION *s, int context, WPACKET *pkt, X509 *x,
* We can't send duplicates: code logic should prevent this.
*/
if (!ossl_assert((meth->ext_flags & SSL_EXT_FLAG_SENT) == 0)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
/*

View File

@ -130,7 +130,6 @@ void ossl_statem_clear(SSL_CONNECTION *s)
s->statem.hand_state = TLS_ST_BEFORE;
ossl_statem_set_in_init(s, 1);
s->statem.no_cert_verify = 0;
s->statem.ignore_fatal = 0;
}
/*
@ -144,15 +143,6 @@ void ossl_statem_set_renegotiate(SSL_CONNECTION *s)
void ossl_statem_send_fatal(SSL_CONNECTION *s, int al)
{
/*
* Some public APIs may call internal functions that fatal error,
* which doesn't make sense outside the state machine. Those APIs
* that can handle a failure set this flag to avoid errors sending
* alerts. Example: getting a wire-formatted certificate for
* compression.
*/
if (s->statem.ignore_fatal)
return;
/* We shouldn't call SSLfatal() twice. Once is enough */
if (s->statem.in_init && s->statem.state == MSG_FLOW_ERROR)
return;

View File

@ -96,7 +96,6 @@ struct ossl_statem_st {
OSSL_HANDSHAKE_STATE request_state;
int in_init;
int read_state_first_init;
int ignore_fatal;
/* true when we are actually in SSL_accept() or SSL_connect() */
int in_handshake;
/*

View File

@ -3628,7 +3628,7 @@ CON_FUNC_RETURN tls_construct_client_certificate(SSL_CONNECTION *s,
}
if (!ssl3_output_cert_chain(s, pkt,
(s->s3.tmp.cert_req == 2) ? NULL
: s->cert->key)) {
: s->cert->key, 0)) {
/* SSLfatal() already called */
return CON_FUNC_ERROR;
}
@ -3676,7 +3676,7 @@ CON_FUNC_RETURN tls_construct_client_compressed_certificate(SSL_CONNECTION *sc,
} else if (!WPACKET_sub_memcpy_u8(&tmppkt, sc->pha_context, sc->pha_context_len))
goto err;
if (!ssl3_output_cert_chain(sc, &tmppkt, sc->cert->key)) {
if (!ssl3_output_cert_chain(sc, &tmppkt, sc->cert->key, 0)) {
/* SSLfatal() already called */
goto out;
}

View File

@ -906,25 +906,30 @@ CON_FUNC_RETURN tls_construct_change_cipher_spec(SSL_CONNECTION *s, WPACKET *pkt
/* Add a certificate to the WPACKET */
static int ssl_add_cert_to_wpacket(SSL_CONNECTION *s, WPACKET *pkt,
X509 *x, int chain)
X509 *x, int chain, int for_comp)
{
int len;
unsigned char *outbytes;
int context = SSL_EXT_TLS1_3_CERTIFICATE;
if (for_comp)
context |= SSL_EXT_TLS1_3_CERTIFICATE_COMPRESSION;
len = i2d_X509(x, NULL);
if (len < 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_BUF_LIB);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_BUF_LIB);
return 0;
}
if (!WPACKET_sub_allocate_bytes_u24(pkt, len, &outbytes)
|| i2d_X509(x, &outbytes) != len) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
if (SSL_CONNECTION_IS_TLS13(s)
&& !tls_construct_extensions(s, pkt, SSL_EXT_TLS1_3_CERTIFICATE, x,
chain)) {
if ((SSL_CONNECTION_IS_TLS13(s) || for_comp)
&& !tls_construct_extensions(s, pkt, context, x, chain)) {
/* SSLfatal() already called */
return 0;
}
@ -933,7 +938,7 @@ static int ssl_add_cert_to_wpacket(SSL_CONNECTION *s, WPACKET *pkt,
}
/* Add certificate chain to provided WPACKET */
static int ssl_add_cert_chain(SSL_CONNECTION *s, WPACKET *pkt, CERT_PKEY *cpk)
static int ssl_add_cert_chain(SSL_CONNECTION *s, WPACKET *pkt, CERT_PKEY *cpk, int for_comp)
{
int i, chain_count;
X509 *x;
@ -967,12 +972,14 @@ static int ssl_add_cert_chain(SSL_CONNECTION *s, WPACKET *pkt, CERT_PKEY *cpk)
sctx->propq);
if (xs_ctx == NULL) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_X509_LIB);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_X509_LIB);
return 0;
}
if (!X509_STORE_CTX_init(xs_ctx, chain_store, x, NULL)) {
X509_STORE_CTX_free(xs_ctx);
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_X509_LIB);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_X509_LIB);
return 0;
}
/*
@ -994,14 +1001,15 @@ static int ssl_add_cert_chain(SSL_CONNECTION *s, WPACKET *pkt, CERT_PKEY *cpk)
ERR_raise(ERR_LIB_SSL, SSL_R_CA_MD_TOO_WEAK);
#endif
X509_STORE_CTX_free(xs_ctx);
SSLfatal(s, SSL_AD_INTERNAL_ERROR, i);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, i);
return 0;
}
chain_count = sk_X509_num(chain);
for (i = 0; i < chain_count; i++) {
x = sk_X509_value(chain, i);
if (!ssl_add_cert_to_wpacket(s, pkt, x, i)) {
if (!ssl_add_cert_to_wpacket(s, pkt, x, i, for_comp)) {
/* SSLfatal() already called */
X509_STORE_CTX_free(xs_ctx);
return 0;
@ -1011,16 +1019,17 @@ static int ssl_add_cert_chain(SSL_CONNECTION *s, WPACKET *pkt, CERT_PKEY *cpk)
} else {
i = ssl_security_cert_chain(s, extra_certs, x, 0);
if (i != 1) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, i);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, i);
return 0;
}
if (!ssl_add_cert_to_wpacket(s, pkt, x, 0)) {
if (!ssl_add_cert_to_wpacket(s, pkt, x, 0, for_comp)) {
/* SSLfatal() already called */
return 0;
}
for (i = 0; i < sk_X509_num(extra_certs); i++) {
x = sk_X509_value(extra_certs, i);
if (!ssl_add_cert_to_wpacket(s, pkt, x, i + 1)) {
if (!ssl_add_cert_to_wpacket(s, pkt, x, i + 1, for_comp)) {
/* SSLfatal() already called */
return 0;
}
@ -1030,18 +1039,20 @@ static int ssl_add_cert_chain(SSL_CONNECTION *s, WPACKET *pkt, CERT_PKEY *cpk)
}
unsigned long ssl3_output_cert_chain(SSL_CONNECTION *s, WPACKET *pkt,
CERT_PKEY *cpk)
CERT_PKEY *cpk, int for_comp)
{
if (!WPACKET_start_sub_packet_u24(pkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
if (!ssl_add_cert_chain(s, pkt, cpk))
if (!ssl_add_cert_chain(s, pkt, cpk, for_comp))
return 0;
if (!WPACKET_close(pkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
if (!for_comp)
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}

View File

@ -3733,7 +3733,7 @@ CON_FUNC_RETURN tls_construct_server_certificate(SSL_CONNECTION *s, WPACKET *pkt
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return CON_FUNC_ERROR;
}
if (!ssl3_output_cert_chain(s, pkt, cpk)) {
if (!ssl3_output_cert_chain(s, pkt, cpk, 0)) {
/* SSLfatal() already called */
return CON_FUNC_ERROR;
}