From a59b14b2d3f8047fe5b687d37304433773603a3f Mon Sep 17 00:00:00 2001 From: jsing <> Date: Mon, 21 Feb 2022 18:22:20 +0000 Subject: Factor out alert handing code in the legacy stack. Pull out the code that processes incoming alerts - a chunk of the complexity is due to the fact that in TLSv1.2 and earlier, alerts can be fragmented across multiple records or multiple alerts can be delivered in a single record. In DTLS there is no way that we can reassemble fragmented alerts (although the RFC is silent on this), however we could have multiple alerts in the same record. This change means that we will handle this situation more appropriately and if we encounter a fragmented alert we will now treat this as a decode error (instead of silently ignoring it). ok beck@ tb@ --- src/lib/libssl/ssl_pkt.c | 122 +++++++++++++++++++++++++++-------------------- 1 file changed, 71 insertions(+), 51 deletions(-) (limited to 'src/lib/libssl/ssl_pkt.c') diff --git a/src/lib/libssl/ssl_pkt.c b/src/lib/libssl/ssl_pkt.c index 3374713644..e3b2034eb4 100644 --- a/src/lib/libssl/ssl_pkt.c +++ b/src/lib/libssl/ssl_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_pkt.c,v 1.53 2022/02/05 14:54:10 jsing Exp $ */ +/* $OpenBSD: ssl_pkt.c,v 1.54 2022/02/21 18:22:20 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -685,6 +685,73 @@ ssl3_write_pending(SSL *s, int type, const unsigned char *buf, unsigned int len) } } +int +ssl3_read_alert(SSL *s) +{ + SSL3_RECORD_INTERNAL *rr = &s->s3->rrec; + uint8_t alert_level, alert_descr; + + /* + * TLSv1.2 permits an alert to be fragmented across multiple records or + * for multiple alerts to be be coalesced into a single alert record. + * In the case of DTLS, there is no way to reassemble an alert + * fragmented across multiple records, hence a full alert must be + * available in the record. + */ + while (rr->length > 0 && + s->s3->alert_fragment_len < sizeof(s->s3->alert_fragment)) { + s->s3->alert_fragment[s->s3->alert_fragment_len++] = + rr->data[rr->off++]; + rr->length--; + } + if (s->s3->alert_fragment_len < sizeof(s->s3->alert_fragment)) { + if (SSL_is_dtls(s)) { + SSLerror(s, SSL_R_BAD_LENGTH); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + return -1; + } + return 1; + } + + ssl_msg_callback(s, 0, SSL3_RT_ALERT, s->s3->alert_fragment, 2); + + alert_level = s->s3->alert_fragment[0]; + alert_descr = s->s3->alert_fragment[1]; + s->s3->alert_fragment_len = 0; + + ssl_info_callback(s, SSL_CB_READ_ALERT, + (alert_level << 8) | alert_descr); + + if (alert_level == SSL3_AL_WARNING) { + s->s3->warn_alert = alert_descr; + if (alert_descr == SSL_AD_CLOSE_NOTIFY) { + s->internal->shutdown |= SSL_RECEIVED_SHUTDOWN; + return 0; + } + /* We requested renegotiation and the peer rejected it. */ + if (alert_descr == SSL_AD_NO_RENEGOTIATION) { + SSLerror(s, SSL_R_NO_RENEGOTIATION); + ssl3_send_alert(s, SSL3_AL_FATAL, + SSL_AD_HANDSHAKE_FAILURE); + return -1; + } + } else if (alert_level == SSL3_AL_FATAL) { + s->internal->rwstate = SSL_NOTHING; + s->s3->fatal_alert = alert_descr; + SSLerror(s, SSL_AD_REASON_OFFSET + alert_descr); + ERR_asprintf_error_data("SSL alert number %d", alert_descr); + s->internal->shutdown |= SSL_RECEIVED_SHUTDOWN; + SSL_CTX_remove_session(s->ctx, s->session); + return 0; + } else { + SSLerror(s, SSL_R_UNKNOWN_ALERT_TYPE); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + return -1; + } + + return 1; +} + /* Return up to 'len' payload bytes received in 'type' records. * 'type' is one of the following: * @@ -875,10 +942,6 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) dest_maxlen = sizeof s->s3->handshake_fragment; dest = s->s3->handshake_fragment; dest_len = &s->s3->handshake_fragment_len; - } else if (rr->type == SSL3_RT_ALERT) { - dest_maxlen = sizeof s->s3->alert_fragment; - dest = s->s3->alert_fragment; - dest_len = &s->s3->alert_fragment_len; } if (dest_maxlen > 0) { /* available space in 'dest' */ @@ -966,53 +1029,10 @@ ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek) ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION); goto start; } - if (s->s3->alert_fragment_len >= 2) { - int alert_level = s->s3->alert_fragment[0]; - int alert_descr = s->s3->alert_fragment[1]; - - s->s3->alert_fragment_len = 0; - - ssl_msg_callback(s, 0, SSL3_RT_ALERT, - s->s3->alert_fragment, 2); - - ssl_info_callback(s, SSL_CB_READ_ALERT, - (alert_level << 8) | alert_descr); - - if (alert_level == SSL3_AL_WARNING) { - s->s3->warn_alert = alert_descr; - if (alert_descr == SSL_AD_CLOSE_NOTIFY) { - s->internal->shutdown |= SSL_RECEIVED_SHUTDOWN; - return (0); - } - /* This is a warning but we receive it if we requested - * renegotiation and the peer denied it. Terminate with - * a fatal alert because if application tried to - * renegotiatie it presumably had a good reason and - * expects it to succeed. - * - * In future we might have a renegotiation where we - * don't care if the peer refused it where we carry on. - */ - else if (alert_descr == SSL_AD_NO_RENEGOTIATION) { - al = SSL_AD_HANDSHAKE_FAILURE; - SSLerror(s, SSL_R_NO_RENEGOTIATION); - goto fatal_err; - } - } else if (alert_level == SSL3_AL_FATAL) { - s->internal->rwstate = SSL_NOTHING; - s->s3->fatal_alert = alert_descr; - SSLerror(s, SSL_AD_REASON_OFFSET + alert_descr); - ERR_asprintf_error_data("SSL alert number %d", - alert_descr); - s->internal->shutdown |= SSL_RECEIVED_SHUTDOWN; - SSL_CTX_remove_session(s->ctx, s->session); - return (0); - } else { - al = SSL_AD_ILLEGAL_PARAMETER; - SSLerror(s, SSL_R_UNKNOWN_ALERT_TYPE); - goto fatal_err; - } + if (rr->type == SSL3_RT_ALERT) { + if ((ret = ssl3_read_alert(s)) <= 0) + return ret; goto start; } -- cgit v1.2.3-55-g6feb