Rely on tlsext_parse() to set a decode_error alert

Instead of setting the alert manually in various parse handlers, we can
make use of the fact that tlsext_parse() sets the alert to decode_error
by default. This simplifies the code quite a bit.

ok jsing

1 files changed, 47 insertions, 79 deletions

diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index ab6450deab0..033608e03eb 100644
--- a/lib/libssl/ssl_tlsext.c
+++ b/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.126 2022/07/22 13:10:31 tb Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.127 2022/07/24 10:52:51 tb Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -92,10 +92,10 @@ tlsext_alpn_server_parse(SSL *s, uint16_t msg_types, CBS *cbs, int *alert)
 	int r;
 
 	if (!CBS_get_u16_length_prefixed(cbs, &alpn))
-		goto err;
+		return 0;
 
 	if (!tlsext_alpn_check_format(&alpn))
-		goto err;
+		return 0;
 
 	if (s->ctx->internal->alpn_select_cb == NULL)
 		return 1;
@@ -132,10 +132,6 @@ tlsext_alpn_server_parse(SSL *s, uint16_t msg_types, CBS *cbs, int *alert)
 	SSLerror(s, SSL_R_NO_APPLICATION_PROTOCOL);
 
 	return 0;
-
- err:
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 int
@@ -176,24 +172,20 @@ tlsext_alpn_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	}
 
 	if (!CBS_get_u16_length_prefixed(cbs, &list))
-		goto err;
+		return 0;
 
 	if (!CBS_get_u8_length_prefixed(&list, &proto))
-		goto err;
+		return 0;
 
 	if (CBS_len(&list) != 0)
-		goto err;
+		return 0;
 	if (CBS_len(&proto) == 0)
-		goto err;
+		return 0;
 
 	if (!CBS_stow(&proto, &s->s3->alpn_selected, &s->s3->alpn_selected_len))
-		goto err;
+		return 0;
 
 	return 1;
-
- err:
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 /*
@@ -246,11 +238,11 @@ tlsext_supportedgroups_server_parse(SSL *s, uint16_t msg_type, CBS *cbs,
 	int i;
 
 	if (!CBS_get_u16_length_prefixed(cbs, &grouplist))
-		goto err;
+		return 0;
 
 	groups_len = CBS_len(&grouplist);
 	if (groups_len == 0 || groups_len % 2 != 0)
-		goto err;
+		return 0;
 	groups_len /= 2;
 
 	if (s->internal->hit)
@@ -271,7 +263,7 @@ tlsext_supportedgroups_server_parse(SSL *s, uint16_t msg_type, CBS *cbs,
 	}
 
 	if (s->session->tlsext_supportedgroups != NULL)
-		goto err;
+		return 0; /* XXX internal error? */
 
 	if ((groups = reallocarray(NULL, groups_len, sizeof(uint16_t))) == NULL) {
 		*alert = SSL_AD_INTERNAL_ERROR;
@@ -281,23 +273,19 @@ tlsext_supportedgroups_server_parse(SSL *s, uint16_t msg_type, CBS *cbs,
 	for (i = 0; i < groups_len; i++) {
 		if (!CBS_get_u16(&grouplist, &groups[i])) {
 			free(groups);
-			goto err;
+			return 0;
 		}
 	}
 
 	if (CBS_len(&grouplist) != 0) {
 		free(groups);
-		goto err;
+		return 0;
 	}
 
 	s->session->tlsext_supportedgroups = groups;
 	s->session->tlsext_supportedgroups_length = groups_len;
 
 	return 1;
-
- err:
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 /* This extension is never used by the server. */
@@ -456,8 +444,10 @@ tlsext_ri_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 {
 	CBS reneg;
 
-	if (!CBS_get_u8_length_prefixed(cbs, &reneg))
-		goto err;
+	if (!CBS_get_u8_length_prefixed(cbs, &reneg)) {
+		SSLerror(s, SSL_R_RENEGOTIATION_ENCODING_ERR);
+		return 0;
+	}
 
 	if (!CBS_mem_equal(&reneg, s->s3->previous_client_finished,
 	    s->s3->previous_client_finished_len)) {
@@ -470,11 +460,6 @@ tlsext_ri_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	s->s3->send_connection_binding = 1;
 
 	return 1;
-
- err:
-	SSLerror(s, SSL_R_RENEGOTIATION_ENCODING_ERR);
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 int
@@ -520,16 +505,24 @@ tlsext_ri_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 		return 0;
 	}
 
-	if (!CBS_get_u8_length_prefixed(cbs, &reneg))
-		goto err;
+	if (!CBS_get_u8_length_prefixed(cbs, &reneg)) {
+		SSLerror(s, SSL_R_RENEGOTIATION_ENCODING_ERR);
+		return 0;
+	}
 	if (!CBS_get_bytes(&reneg, &prev_client,
-	    s->s3->previous_client_finished_len))
-		goto err;
+	    s->s3->previous_client_finished_len)) {
+		SSLerror(s, SSL_R_RENEGOTIATION_ENCODING_ERR);
+		return 0;
+	}
 	if (!CBS_get_bytes(&reneg, &prev_server,
-	    s->s3->previous_server_finished_len))
-		goto err;
-	if (CBS_len(&reneg) != 0)
-		goto err;
+	    s->s3->previous_server_finished_len)) {
+		SSLerror(s, SSL_R_RENEGOTIATION_ENCODING_ERR);
+		return 0;
+	}
+	if (CBS_len(&reneg) != 0) {
+		SSLerror(s, SSL_R_RENEGOTIATION_ENCODING_ERR);
+		return 0;
+	}
 
 	if (!CBS_mem_equal(&prev_client, s->s3->previous_client_finished,
 	    s->s3->previous_client_finished_len)) {
@@ -548,11 +541,6 @@ tlsext_ri_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	s->s3->send_connection_binding = 1;
 
 	return 1;
-
- err:
-	SSLerror(s, SSL_R_RENEGOTIATION_ENCODING_ERR);
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 /*
@@ -862,10 +850,8 @@ tlsext_sni_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 			return 0;
 		}
 	} else {
-		if (s->session->tlsext_hostname != NULL) {
-			*alert = SSL_AD_DECODE_ERROR;
+		if (s->session->tlsext_hostname != NULL)
 			return 0;
-		}
 		if ((s->session->tlsext_hostname =
 		    strdup(s->tlsext_hostname)) == NULL) {
 			*alert = SSL_AD_INTERNAL_ERROR;
@@ -1303,7 +1289,6 @@ tlsext_srtp_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 
 	if (!CBS_get_u8_length_prefixed(cbs, &mki) || CBS_len(&mki) != 0) {
 		SSLerror(s, SSL_R_BAD_SRTP_MKI_VALUE);
-		*alert = SSL_AD_DECODE_ERROR;
 		goto done;
 	}
 
@@ -1319,8 +1304,7 @@ tlsext_srtp_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	if ((srvr = SSL_get_srtp_profiles(s)) == NULL)
 		goto err;
 	for (i = 0; i < sk_SRTP_PROTECTION_PROFILE_num(srvr); i++) {
-		if ((sprof = sk_SRTP_PROTECTION_PROFILE_value(srvr, i))
-		    == NULL)
+		if ((sprof = sk_SRTP_PROTECTION_PROFILE_value(srvr, i)) == NULL)
 			goto err;
 
 		for (j = 0; j < sk_SRTP_PROTECTION_PROFILE_num(clnt); j++) {
@@ -1342,7 +1326,6 @@ tlsext_srtp_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 
  err:
 	SSLerror(s, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
-	*alert = SSL_AD_DECODE_ERROR;
 
  done:
 	sk_SRTP_PROTECTION_PROFILE_free(clnt);
@@ -1390,12 +1373,12 @@ tlsext_srtp_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 
 	if (!CBS_get_u16_length_prefixed(cbs, &profile_ids)) {
 		SSLerror(s, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
-		goto err;
+		return 0;
 	}
 
 	if (!CBS_get_u16(&profile_ids, &id) || CBS_len(&profile_ids) != 0) {
 		SSLerror(s, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
-		goto err;
+		return 0;
 	}
 
 	if (!CBS_get_u8_length_prefixed(cbs, &mki) || CBS_len(&mki) != 0) {
@@ -1406,14 +1389,14 @@ tlsext_srtp_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 
 	if ((clnt = SSL_get_srtp_profiles(s)) == NULL) {
 		SSLerror(s, SSL_R_NO_SRTP_PROFILES);
-		goto err;
+		return 0;
 	}
 
 	for (i = 0; i < sk_SRTP_PROTECTION_PROFILE_num(clnt); i++) {
 		if ((prof = sk_SRTP_PROTECTION_PROFILE_value(clnt, i))
 		    == NULL) {
 			SSLerror(s, SSL_R_NO_SRTP_PROFILES);
-			goto err;
+			return 0;
 		}
 
 		if (prof->id == id) {
@@ -1423,8 +1406,7 @@ tlsext_srtp_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	}
 
 	SSLerror(s, SSL_R_BAD_SRTP_PROTECTION_PROFILE_LIST);
- err:
-	*alert = SSL_AD_DECODE_ERROR;
+
 	return 0;
 }
 
@@ -1635,11 +1617,11 @@ tlsext_versions_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	min = s->s3->hs.our_min_tls_version;
 
 	if (!CBS_get_u8_length_prefixed(cbs, &versions))
-		goto err;
+		return 0;
 
 	while (CBS_len(&versions) > 0) {
 		if (!CBS_get_u16(&versions, &version))
-			goto err;
+			return 0;
 		/*
 		 * XXX What is below implements client preference, and
 		 * ignores any server preference entirely.
@@ -1656,10 +1638,6 @@ tlsext_versions_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 
 	*alert = SSL_AD_PROTOCOL_VERSION;
 	return 0;
-
- err:
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 int
@@ -1679,10 +1657,8 @@ tlsext_versions_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 {
 	uint16_t selected_version;
 
-	if (!CBS_get_u16(cbs, &selected_version)) {
-		*alert = SSL_AD_DECODE_ERROR;
+	if (!CBS_get_u16(cbs, &selected_version))
 		return 0;
-	}
 
 	/* XXX - need to fix for DTLS 1.3 */
 	if (selected_version < TLS1_3_VERSION) {
@@ -1732,10 +1708,10 @@ tlsext_cookie_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	CBS cookie;
 
 	if (!CBS_get_u16_length_prefixed(cbs, &cookie))
-		goto err;
+		return 0;
 
 	if (CBS_len(&cookie) != s->s3->hs.tls13.cookie_len)
-		goto err;
+		return 0;
 
 	/*
 	 * Check provided cookie value against what server previously
@@ -1750,10 +1726,6 @@ tlsext_cookie_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	}
 
 	return 1;
-
- err:
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 int
@@ -1804,17 +1776,13 @@ tlsext_cookie_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 	}
 
 	if (!CBS_get_u16_length_prefixed(cbs, &cookie))
-		goto err;
+		return 0;
 
 	if (!CBS_stow(&cookie, &s->s3->hs.tls13.cookie,
 	    &s->s3->hs.tls13.cookie_len))
-		goto err;
+		return 0;
 
 	return 1;
-
- err:
-	*alert = SSL_AD_DECODE_ERROR;
-	return 0;
 }
 
 /*