From: tb <tb@openbsd.org>
Date: Fri, 22 Jul 2022 13:10:31 +0000 (+0000)
Subject: Remove redundant length checks in parse functions
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=616ed5c69523d875ca0571ef1d43646fd3b8369f;p=openbsd

Remove redundant length checks in parse functions

The main parsing function already checks that the entire extension data
was consumed, so the length checks inside some of the parse handlers are
redundant. They were also not done everywhere, so this makes the parse
handlers more consistent.

Similar diff was sent by jsing a long while back

ok jsing
---

diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index d802a6e1355..ab6450deab0 100644
--- a/lib/libssl/ssl_tlsext.c
+++ b/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.125 2022/07/20 15:16:06 tb Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.126 2022/07/22 13:10:31 tb Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -93,8 +93,6 @@ tlsext_alpn_server_parse(SSL *s, uint16_t msg_types, CBS *cbs, int *alert)
 
 	if (!CBS_get_u16_length_prefixed(cbs, &alpn))
 		goto err;
-	if (CBS_len(cbs) != 0)
-		goto err;
 
 	if (!tlsext_alpn_check_format(&alpn))
 		goto err;
@@ -179,8 +177,6 @@ tlsext_alpn_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 
 	if (!CBS_get_u16_length_prefixed(cbs, &list))
 		goto err;
-	if (CBS_len(cbs) != 0)
-		goto err;
 
 	if (!CBS_get_u8_length_prefixed(&list, &proto))
 		goto err;
@@ -251,8 +247,6 @@ tlsext_supportedgroups_server_parse(SSL *s, uint16_t msg_type, CBS *cbs,
 
 	if (!CBS_get_u16_length_prefixed(cbs, &grouplist))
 		goto err;
-	if (CBS_len(cbs) != 0)
-		goto err;
 
 	groups_len = CBS_len(&grouplist);
 	if (groups_len == 0 || groups_len % 2 != 0)
@@ -377,8 +371,6 @@ tlsext_ecpf_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 		return 0;
 	if (CBS_len(&ecpf) == 0)
 		return 0;
-	if (CBS_len(cbs) != 0)
-		return 0;
 
 	/* Must contain uncompressed (0) - RFC 8422, section 5.1.2. */
 	if (!CBS_contains_zero_byte(&ecpf)) {
@@ -466,8 +458,6 @@ tlsext_ri_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 
 	if (!CBS_get_u8_length_prefixed(cbs, &reneg))
 		goto err;
-	if (CBS_len(cbs) != 0)
-		goto err;
 
 	if (!CBS_mem_equal(&reneg, s->s3->previous_client_finished,
 	    s->s3->previous_client_finished_len)) {
@@ -540,8 +530,6 @@ tlsext_ri_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 		goto err;
 	if (CBS_len(&reneg) != 0)
 		goto err;
-	if (CBS_len(cbs) != 0)
-		goto err;
 
 	if (!CBS_mem_equal(&prev_client, s->s3->previous_client_finished,
 	    s->s3->previous_client_finished_len)) {
@@ -833,8 +821,6 @@ tlsext_sni_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 		*alert = SSL_AD_ILLEGAL_PARAMETER;
 		goto err;
 	}
-	if (CBS_len(cbs) != 0)
-		goto err;
 
 	return 1;
 
@@ -1014,10 +1000,6 @@ tlsext_ocsp_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 			goto err;
 	}
 
-	/* should be nothing left */
-	if (CBS_len(cbs) > 0)
-		goto err;
-
 	ret = 1;
  err:
 	if (ret == 0)
@@ -1324,8 +1306,6 @@ tlsext_srtp_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert)
 		*alert = SSL_AD_DECODE_ERROR;
 		goto done;
 	}
-	if (CBS_len(cbs) != 0)
-		goto err;
 
 	/*
 	 * Per RFC 5764 section 4.1.1