Tolerate encrypted or plaintext alerts

At certain points in the handshake we could receive either a plaintext or
an encrypted alert from the client. We should tolerate both where
appropriate.

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 12:40:08 +01:00
parent 7426cd343d
commit de9e884b2f
6 changed files with 44 additions and 14 deletions

View File

@ -816,7 +816,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
/* Clear our SSL3_RECORD structures */
memset(wr, 0, sizeof(wr));
for (j = 0; j < numpipes; j++) {
unsigned int version = SSL_TREAT_AS_TLS13(s) ? TLS1_2_VERSION
unsigned int version = (s->version == TLS1_3_VERSION) ? TLS1_2_VERSION
: s->version;
unsigned char *compressdata = NULL;
size_t maxcomplen;

View File

@ -342,7 +342,10 @@ int ssl3_get_record(SSL *s)
if (SSL_IS_TLS13(s) && s->enc_read_ctx != NULL) {
if (thisrr->type != SSL3_RT_APPLICATION_DATA
&& (thisrr->type != SSL3_RT_CHANGE_CIPHER_SPEC
|| !SSL_IS_FIRST_HANDSHAKE(s))) {
|| !SSL_IS_FIRST_HANDSHAKE(s))
&& (thisrr->type != SSL3_RT_ALERT
|| s->statem.enc_read_state
!= ENC_READ_STATE_ALLOW_PLAIN_ALERTS)) {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE,
SSL_F_SSL3_GET_RECORD, SSL_R_BAD_RECORD_TYPE);
return -1;
@ -692,7 +695,9 @@ int ssl3_get_record(SSL *s)
}
}
if (SSL_IS_TLS13(s) && s->enc_read_ctx != NULL) {
if (SSL_IS_TLS13(s)
&& s->enc_read_ctx != NULL
&& thisrr->type != SSL3_RT_ALERT) {
size_t end;
if (thisrr->length == 0

View File

@ -52,10 +52,13 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
seq = RECORD_LAYER_get_read_sequence(&s->rlayer);
}
if (ctx == NULL
|| (rec->type == SSL3_RT_ALERT
&& s->statem.enc_write_state
== ENC_WRITE_STATE_WRITE_PLAIN_ALERTS)) {
/*
* If we're sending an alert and ctx != NULL then we must be forcing
* plaintext alerts. If we're reading and ctx != NULL then we allow
* plaintext alerts at certain points in the handshake. If we've got this
* far then we have already validated that a plaintext alert is ok here.
*/
if (ctx == NULL || rec->type == SSL3_RT_ALERT) {
memmove(rec->data, rec->input, rec->length);
rec->input = rec->data;
return 1;

View File

@ -80,6 +80,13 @@ typedef enum {
ENC_WRITE_STATE_WRITE_PLAIN_ALERTS
} ENC_WRITE_STATES;
typedef enum {
/* The enc_read_ctx can be used normally */
ENC_READ_STATE_VALID,
/* We may receive encrypted or plaintext alerts */
ENC_READ_STATE_ALLOW_PLAIN_ALERTS
} ENC_READ_STATES;
/*****************************************************************************
* *
* This structure should be considered "opaque" to anything outside of the *
@ -110,6 +117,7 @@ struct ossl_statem_st {
unsigned int no_cert_verify;
int use_timer;
ENC_WRITE_STATES enc_write_state;
ENC_READ_STATES enc_read_state;
};
typedef struct ossl_statem_st OSSL_STATEM;

View File

@ -747,6 +747,12 @@ MSG_PROCESS_RETURN tls_process_finished(SSL *s, PACKET *pkt)
/* This is a real handshake so make sure we clean it up at the end */
if (s->server) {
/*
* To get this far we must have read encrypted data from the client. We
* no longer tolerate unencrypted alerts. This value is ignored if less
* than TLSv1.3
*/
s->statem.enc_read_state = ENC_READ_STATE_VALID;
if (s->post_handshake_auth != SSL_PHA_REQUESTED)
s->statem.cleanuphand = 1;
if (SSL_IS_TLS13(s) && !tls13_save_handshake_digest_for_pha(s)) {

View File

@ -848,12 +848,7 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
return WORK_MORE_A;
break;
}
/*
* TODO(TLS1.3): This actually causes a problem. We don't yet know
* whether the next record we are going to receive is an unencrypted
* alert, or an encrypted handshake message. We're going to need
* something clever in the record layer for this.
*/
if (SSL_IS_TLS13(s)) {
if (!s->method->ssl3_enc->setup_key_block(s)
|| !s->method->ssl3_enc->change_cipher_state(s,
@ -868,6 +863,12 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
/* SSLfatal() already called */
return WORK_ERROR;
}
/*
* We don't yet know whether the next record we are going to receive
* is an unencrypted alert, an encrypted alert, or an encrypted
* handshake message. We temporarily tolerate unencrypted alerts.
*/
s->statem.enc_read_state = ENC_READ_STATE_ALLOW_PLAIN_ALERTS;
break;
}
@ -3523,6 +3524,13 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
size_t chainidx;
SSL_SESSION *new_sess = NULL;
/*
* To get this far we must have read encrypted data from the client. We no
* longer tolerate unencrypted alerts. This value is ignored if less than
* TLSv1.3
*/
s->statem.enc_read_state = ENC_READ_STATE_VALID;
if ((sk = sk_X509_new_null()) == NULL) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE,
ERR_R_MALLOC_FAILURE);