Send alert on ssl_get_prev_session failure

ssl_get_prev_session() can fail for various reasons some of which
may be internal_error others decode_error alerts. Propagate the
appropriate alert up to the caller so we can abort the handshake
by sending a fatal alert instead of rudely closing the pipe.

Currently only 28 of 292 test cases of tlsfuzzer's test-extension.py pass.
With this diff, 272 pass. The rest will require fixes elsewhere.

ok beck inoguchi jsing

4 files changed, 32 insertions, 20 deletions

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index e41465419a6..036c1dacb28 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.284 2020/08/30 15:40:20 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.285 2020/08/31 14:04:51 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1161,7 +1161,8 @@ int ssl_cert_add1_chain_cert(CERT *c, X509 *cert);
 SESS_CERT *ssl_sess_cert_new(void);
 void ssl_sess_cert_free(SESS_CERT *sc);
 int ssl_get_new_session(SSL *s, int session);
-int ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block);
+int ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block,
+    int *alert);
 int ssl_cipher_id_cmp(const SSL_CIPHER *a, const SSL_CIPHER *b);
 SSL_CIPHER *OBJ_bsearch_ssl_cipher_id(SSL_CIPHER *key, SSL_CIPHER const *base,
     int num);
@@ -1397,7 +1398,7 @@ int ssl_check_clienthello_tlsext_late(SSL *s);
 int ssl_check_serverhello_tlsext(SSL *s);
 
 int tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block,
-    SSL_SESSION **ret);
+    int *alert, SSL_SESSION **ret);
 
 long ssl_get_algorithm2(SSL *s);
 
diff --git a/lib/libssl/ssl_sess.c b/lib/libssl/ssl_sess.c
index 16b4b75bc4a..827360176b0 100644
--- a/lib/libssl/ssl_sess.c
+++ b/lib/libssl/ssl_sess.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_sess.c,v 1.85 2019/04/22 15:12:20 jsing Exp $ */
+/* $OpenBSD: ssl_sess.c,v 1.86 2020/08/31 14:04:51 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -435,10 +435,10 @@ sess_id_done:
  *     to 1 if the server should issue a new session ticket (to 0 otherwise).
  */
 int
-ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block)
+ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
 {
 	SSL_SESSION *ret = NULL;
-	int fatal = 0;
+	int alert_desc = SSL_AD_INTERNAL_ERROR, fatal = 0;
 	int try_session_cache = 1;
 	int r;
 
@@ -451,7 +451,7 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block)
 		try_session_cache = 0;
 
 	/* Sets s->internal->tlsext_ticket_expected. */
-	r = tls1_process_ticket(s, session_id, ext_block, &ret);
+	r = tls1_process_ticket(s, session_id, ext_block, &alert_desc, &ret);
 	switch (r) {
 	case -1: /* Error during processing */
 		fatal = 1;
@@ -591,9 +591,10 @@ err:
 			s->internal->tlsext_ticket_expected = 1;
 		}
 	}
-	if (fatal)
+	if (fatal) {
+		*alert = alert_desc;
 		return -1;
-	else
+	} else
 		return 0;
 }
 
diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 67671f276cd..745b15aad05 100644
--- a/lib/libssl/ssl_srvr.c
+++ b/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.80 2020/07/03 04:12:50 tb Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.81 2020/08/31 14:04:51 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -920,11 +920,11 @@ ssl3_get_client_hello(SSL *s)
 
 		CBS_dup(&cbs, &ext_block);
 
-		i = ssl_get_prev_session(s, &session_id, &ext_block);
+		i = ssl_get_prev_session(s, &session_id, &ext_block, &al);
 		if (i == 1) { /* previous session */
 			s->internal->hit = 1;
 		} else if (i == -1)
-			goto err;
+			goto f_err;
 		else {
 			/* i == 0 */
 			if (!ssl_get_new_session(s, 1))
diff --git a/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index 1191f9201e9..59146eb767a 100644
--- a/lib/libssl/t1_lib.c
+++ b/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.169 2020/08/09 16:25:54 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.170 2020/08/31 14:04:51 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -122,7 +122,7 @@
 #include "ssl_sigalgs.h"
 #include "ssl_tlsext.h"
 
-static int tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket,
+static int tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert,
     SSL_SESSION **psess);
 
 SSL3_ENC_METHOD TLSv1_enc_data = {
@@ -782,7 +782,8 @@ ssl_check_serverhello_tlsext(SSL *s)
  *   Otherwise, s->internal->tlsext_ticket_expected is set to 0.
  */
 int
-tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, SSL_SESSION **ret)
+tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert,
+    SSL_SESSION **ret)
 {
 	CBS extensions, ext_data;
 	uint16_t ext_type = 0;
@@ -805,13 +806,17 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, SSL_SESSION **ret)
 	if (CBS_len(ext_block) == 0)
 		return 0;
 
-	if (!CBS_get_u16_length_prefixed(ext_block, &extensions))
+	if (!CBS_get_u16_length_prefixed(ext_block, &extensions)) {
+		*alert = SSL_AD_DECODE_ERROR;
 		return -1;
+	}
 
 	while (CBS_len(&extensions) > 0) {
 		if (!CBS_get_u16(&extensions, &ext_type) ||
-		    !CBS_get_u16_length_prefixed(&extensions, &ext_data))
+		    !CBS_get_u16_length_prefixed(&extensions, &ext_data)) {
+			*alert = SSL_AD_DECODE_ERROR;
 			return -1;
+		}
 
 		if (ext_type == TLSEXT_TYPE_session_ticket)
 			break;
@@ -839,7 +844,7 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, SSL_SESSION **ret)
 		return 2;
 	}
 
-	r = tls_decrypt_ticket(s, session_id, &ext_data, ret);
+	r = tls_decrypt_ticket(s, session_id, &ext_data, alert, ret);
 	switch (r) {
 	case 2: /* ticket couldn't be decrypted */
 		s->internal->tlsext_ticket_expected = 1;
@@ -868,7 +873,8 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, SSL_SESSION **ret)
  *    4: same as 3, but the ticket needs to be renewed.
  */
 static int
-tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, SSL_SESSION **psess)
+tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert,
+    SSL_SESSION **psess)
 {
 	CBS ticket_name, ticket_iv, ticket_encdata, ticket_hmac;
 	SSL_SESSION *sess = NULL;
@@ -883,6 +889,7 @@ tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, SSL_SESSION **psess)
 	int slen, hlen;
 	int renew_ticket = 0;
 	int ret = -1;
+	int alert_desc = SSL_AD_INTERNAL_ERROR;
 
 	*psess = NULL;
 
@@ -956,8 +963,10 @@ tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, SSL_SESSION **psess)
 		goto derr;
 	if (!CBS_get_bytes(ticket, &ticket_hmac, hlen))
 		goto derr;
-	if (CBS_len(ticket) != 0)
+	if (CBS_len(ticket) != 0) {
+		alert_desc = SSL_AD_DECODE_ERROR;
 		goto err;
+	}
 
 	/* Check HMAC of encrypted ticket. */
 	if (HMAC_Update(hctx, CBS_data(&ticket_name),
@@ -1020,6 +1029,7 @@ tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, SSL_SESSION **psess)
 	goto done;
 
  err:
+	*alert = alert_desc;
 	ret = -1;
 	goto done;