wbrawner/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Ensure that we write out alerts correctly after early_data

Browse source 
If we sent early_data and then received back an HRR, the enc_write_ctx
was stale resulting in errors if an alert needed to be sent.

Thanks to Quarkslab for reporting this.

In any case it makes little sense to encrypt alerts using the
client_early_traffic_secret, so we add special handling for alerts sent
after early_data. All such alerts are sent in plaintext.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6887)
This commit is contained in:
Matt Caswell 2018-08-07 10:25:54 +01:00
parent b4f001eb1a
commit 7426cd343d
7 changed files with 36 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
ssl/record/rec_layer_s3.c
View file

@ -829,7 +829,10 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
* In TLSv1.3, once encrypting, we always use application data for the * In TLSv1.3, once encrypting, we always use application data for the
* record type * record type
*/ */
if (SSL_TREAT_AS_TLS13(s) && s->enc_write_ctx != NULL) if (SSL_TREAT_AS_TLS13(s)
&& s->enc_write_ctx != NULL
&& (s->statem.enc_write_state != ENC_WRITE_STATE_WRITE_PLAIN_ALERTS
|| type != SSL3_RT_ALERT))
rectype = SSL3_RT_APPLICATION_DATA; rectype = SSL3_RT_APPLICATION_DATA;
else else
rectype = type; rectype = type;
@ -892,7 +895,10 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
SSL3_RECORD_reset_input(&wr[j]); SSL3_RECORD_reset_input(&wr[j]);
} }
if (SSL_TREAT_AS_TLS13(s) && s->enc_write_ctx != NULL) { if (SSL_TREAT_AS_TLS13(s)
&& s->enc_write_ctx != NULL
&& (s->statem.enc_write_state != ENC_WRITE_STATE_WRITE_PLAIN_ALERTS
|| type != SSL3_RT_ALERT)) {
size_t rlen, max_send_fragment; size_t rlen, max_send_fragment;
if (!WPACKET_put_bytes_u8(thispkt, type)) { if (!WPACKET_put_bytes_u8(thispkt, type)) {
@ -981,8 +987,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
SSL3_RECORD_set_length(thiswr, len); SSL3_RECORD_set_length(thiswr, len);
} }
if (s->early_data_state == SSL_EARLY_DATA_WRITING if (s->statem.enc_write_state == ENC_WRITE_STATE_WRITE_PLAIN_ALERTS) {
|| s->early_data_state == SSL_EARLY_DATA_WRITE_RETRY) {
/* /*
* We haven't actually negotiated the version yet, but we're trying to * We haven't actually negotiated the version yet, but we're trying to
* send early data - so we need to use the tls13enc function. * send early data - so we need to use the tls13enc function.

5
ssl/record/ssl3_record_tls13.c
View file

@ -52,7 +52,10 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
seq = RECORD_LAYER_get_read_sequence(&s->rlayer); seq = RECORD_LAYER_get_read_sequence(&s->rlayer);
} }
if (ctx == NULL) { if (ctx == NULL
|| (rec->type == SSL3_RT_ALERT
&& s->statem.enc_write_state
== ENC_WRITE_STATE_WRITE_PLAIN_ALERTS)) {
memmove(rec->data, rec->input, rec->length); memmove(rec->data, rec->input, rec->length);
rec->input = rec->data; rec->input = rec->data;
return 1; return 1;

4
ssl/s3_enc.c
View file

@ -155,7 +155,7 @@ int ssl3_change_cipher_state(SSL *s, int which)
RECORD_LAYER_reset_read_sequence(&s->rlayer); RECORD_LAYER_reset_read_sequence(&s->rlayer);
mac_secret = &(s->s3->read_mac_secret[0]); mac_secret = &(s->s3->read_mac_secret[0]);
} else { } else {
s->statem.invalid_enc_write_ctx = 1; s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
if (s->enc_write_ctx != NULL) { if (s->enc_write_ctx != NULL) {
reuse_dd = 1; reuse_dd = 1;
} else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL) { } else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL) {
@ -238,7 +238,7 @@ int ssl3_change_cipher_state(SSL *s, int which)
goto err; goto err;
} }
s->statem.invalid_enc_write_ctx = 0; s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
OPENSSL_cleanse(exp_key, sizeof(exp_key)); OPENSSL_cleanse(exp_key, sizeof(exp_key));
OPENSSL_cleanse(exp_iv, sizeof(exp_iv)); OPENSSL_cleanse(exp_iv, sizeof(exp_iv));
return 1; return 1;

3
ssl/statem/statem.c
View file

@ -123,7 +123,8 @@ void ossl_statem_fatal(SSL *s, int al, int func, int reason, const char *file,
s->statem.in_init = 1; s->statem.in_init = 1;
s->statem.state = MSG_FLOW_ERROR; s->statem.state = MSG_FLOW_ERROR;
ERR_put_error(ERR_LIB_SSL, func, reason, file, line); ERR_put_error(ERR_LIB_SSL, func, reason, file, line);
if (al != SSL_AD_NO_ALERT && !s->statem.invalid_enc_write_ctx) if (al != SSL_AD_NO_ALERT
&& s->statem.enc_write_state != ENC_WRITE_STATE_INVALID)
ssl3_send_alert(s, SSL3_AL_FATAL, al); ssl3_send_alert(s, SSL3_AL_FATAL, al);
} }

11
ssl/statem/statem.h
View file

@ -71,6 +71,15 @@ typedef enum {
WRITE_STATE_POST_WORK WRITE_STATE_POST_WORK
} WRITE_STATE; } WRITE_STATE;
typedef enum {
/* The enc_write_ctx can be used normally */
ENC_WRITE_STATE_VALID,
/* The enc_write_ctx cannot be used */
ENC_WRITE_STATE_INVALID,
/* Write alerts in plaintext, but otherwise use the enc_write_ctx */
ENC_WRITE_STATE_WRITE_PLAIN_ALERTS
} ENC_WRITE_STATES;
/***************************************************************************** /*****************************************************************************
* * * *
* This structure should be considered "opaque" to anything outside of the * * This structure should be considered "opaque" to anything outside of the *
@ -100,7 +109,7 @@ struct ossl_statem_st {
/* Should we skip the CertificateVerify message? */ /* Should we skip the CertificateVerify message? */
unsigned int no_cert_verify; unsigned int no_cert_verify;
int use_timer; int use_timer;
int invalid_enc_write_ctx; ENC_WRITE_STATES enc_write_state;
}; };
typedef struct ossl_statem_st OSSL_STATEM; typedef struct ossl_statem_st OSSL_STATEM;

4
ssl/t1_enc.c
View file

@ -154,7 +154,7 @@ int tls1_change_cipher_state(SSL *s, int which)
mac_secret = &(s->s3->read_mac_secret[0]); mac_secret = &(s->s3->read_mac_secret[0]);
mac_secret_size = &(s->s3->read_mac_secret_size); mac_secret_size = &(s->s3->read_mac_secret_size);
} else { } else {
s->statem.invalid_enc_write_ctx = 1; s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
if (s->ext.use_etm) if (s->ext.use_etm)
s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC_WRITE; s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC_WRITE;
else else
@ -316,7 +316,7 @@ int tls1_change_cipher_state(SSL *s, int which)
ERR_R_INTERNAL_ERROR); ERR_R_INTERNAL_ERROR);
goto err; goto err;
} }
s->statem.invalid_enc_write_ctx = 0; s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
#ifdef SSL_DEBUG #ifdef SSL_DEBUG
printf("which = %04X\nkey=", which); printf("which = %04X\nkey=", which);

11
ssl/tls13_enc.c
View file

@ -425,7 +425,7 @@ int tls13_change_cipher_state(SSL *s, int which)
RECORD_LAYER_reset_read_sequence(&s->rlayer); RECORD_LAYER_reset_read_sequence(&s->rlayer);
} else { } else {
s->statem.invalid_enc_write_ctx = 1; s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
if (s->enc_write_ctx != NULL) { if (s->enc_write_ctx != NULL) {
EVP_CIPHER_CTX_reset(s->enc_write_ctx); EVP_CIPHER_CTX_reset(s->enc_write_ctx);
} else { } else {
@ -648,7 +648,10 @@ int tls13_change_cipher_state(SSL *s, int which)
goto err; goto err;
} }
s->statem.invalid_enc_write_ctx = 0; if (!s->server && label == client_early_traffic)
s->statem.enc_write_state = ENC_WRITE_STATE_WRITE_PLAIN_ALERTS;
else
s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
ret = 1; ret = 1;
err: err:
OPENSSL_cleanse(secret, sizeof(secret)); OPENSSL_cleanse(secret, sizeof(secret));
@ -671,7 +674,7 @@ int tls13_update_key(SSL *s, int sending)
insecret = s->client_app_traffic_secret; insecret = s->client_app_traffic_secret;
if (sending) { if (sending) {
s->statem.invalid_enc_write_ctx = 1; s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
iv = s->write_iv; iv = s->write_iv;
ciph_ctx = s->enc_write_ctx; ciph_ctx = s->enc_write_ctx;
RECORD_LAYER_reset_write_sequence(&s->rlayer); RECORD_LAYER_reset_write_sequence(&s->rlayer);
@ -692,7 +695,7 @@ int tls13_update_key(SSL *s, int sending)
memcpy(insecret, secret, hashlen); memcpy(insecret, secret, hashlen);
s->statem.invalid_enc_write_ctx = 0; s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
ret = 1; ret = 1;
err: err:
OPENSSL_cleanse(secret, sizeof(secret)); OPENSSL_cleanse(secret, sizeof(secret));