From: jsing <jsing@openbsd.org>
Date: Sat, 23 Apr 2022 18:56:54 +0000 (+0000)
Subject: Rewrite c2i_ASN1_BIT_STRING() using CBS.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=eabc055742cf7c577918b94ee1a4522ed6e2ce5e;p=openbsd

Rewrite c2i_ASN1_BIT_STRING() using CBS.

Also switch to freeing and allocating, rather than attempting to recycle.
While here, factor out the flags ASN1_STRING_FLAG_BITS_LEFT bit bashing
and use the name "unused bits" rather than "bits left", to be more inline
with X.690 wording.

ok inoguchi@ tb@
---

diff --git a/lib/libcrypto/asn1/a_bitstr.c b/lib/libcrypto/asn1/a_bitstr.c
index 4ffafd5f86b..52fe0eb9cdb 100644
--- a/lib/libcrypto/asn1/a_bitstr.c
+++ b/lib/libcrypto/asn1/a_bitstr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_bitstr.c,v 1.33 2021/12/25 08:52:44 jsing Exp $ */
+/* $OpenBSD: a_bitstr.c,v 1.34 2022/04/23 18:56:54 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -56,6 +56,7 @@
  * [including the GNU Public Licence.]
  */
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -65,6 +66,8 @@
 #include <openssl/err.h>
 #include <openssl/x509v3.h>
 
+#include "bytestring.h"
+
 const ASN1_ITEM ASN1_BIT_STRING_it = {
 	.itype = ASN1_ITYPE_PRIMITIVE,
 	.utype = V_ASN1_BIT_STRING,
@@ -83,6 +86,25 @@ ASN1_BIT_STRING_free(ASN1_BIT_STRING *a)
 	ASN1_item_free((ASN1_VALUE *)a, &ASN1_BIT_STRING_it);
 }
 
+static void
+asn1_abs_clear_unused_bits(ASN1_BIT_STRING *abs)
+{
+	abs->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07);
+}
+
+static int
+asn1_abs_set_unused_bits(ASN1_BIT_STRING *abs, uint8_t unused_bits)
+{
+	if (unused_bits > 7)
+		return 0;
+
+	asn1_abs_clear_unused_bits(abs);
+
+	abs->flags |= ASN1_STRING_FLAG_BITS_LEFT | unused_bits;
+
+	return 1;
+}
+
 int
 ASN1_BIT_STRING_set(ASN1_BIT_STRING *x, unsigned char *d, int len)
 {
@@ -104,7 +126,7 @@ ASN1_BIT_STRING_set_bit(ASN1_BIT_STRING *a, int n, int value)
 	if (a == NULL)
 		return 0;
 
-	a->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); /* clear, set on write */
+	asn1_abs_clear_unused_bits(a);
 
 	if ((a->length < (w + 1)) || (a->data == NULL)) {
 		if (!value)
@@ -269,68 +291,85 @@ i2c_ASN1_BIT_STRING(ASN1_BIT_STRING *a, unsigned char **pp)
 	return (ret);
 }
 
-ASN1_BIT_STRING *
-c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a, const unsigned char **pp, long len)
+static int
+c2i_ASN1_BIT_STRING_cbs(ASN1_BIT_STRING **out_abs, CBS *cbs)
 {
-	ASN1_BIT_STRING *ret = NULL;
-	const unsigned char *p;
-	unsigned char *s;
-	int i;
+	ASN1_BIT_STRING *abs = NULL;
+	uint8_t *data = NULL;
+	size_t data_len = 0;
+	uint8_t unused_bits;
+	int ret = 0;
 
-	if (len < 1) {
+	if (out_abs == NULL || *out_abs != NULL)
+		goto err;
+
+	if (!CBS_get_u8(cbs, &unused_bits)) {
 		ASN1error(ASN1_R_STRING_TOO_SHORT);
 		goto err;
 	}
 
-	if (a == NULL || *a == NULL) {
-		if ((ret = ASN1_BIT_STRING_new()) == NULL)
-			return (NULL);
-	} else
-		ret = *a;
+	if (!CBS_stow(cbs, &data, &data_len))
+		goto err;
+	if (data_len > INT_MAX)
+		goto err;
 
-	p = *pp;
-	i = *(p++);
-	if (i > 7) {
-		ASN1error(ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
+	if ((abs = ASN1_BIT_STRING_new()) == NULL)
 		goto err;
-	}
+
+	abs->data = data;
+	abs->length = (int)data_len;
+	data = NULL;
 
 	/*
 	 * We do this to preserve the settings. If we modify the settings,
 	 * via the _set_bit function, we will recalculate on output.
 	 */
-	ret->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); /* clear */
-	ret->flags |= (ASN1_STRING_FLAG_BITS_LEFT | i); /* set */
+	if (!asn1_abs_set_unused_bits(abs, unused_bits)) {
+		ASN1error(ASN1_R_INVALID_BIT_STRING_BITS_LEFT);
+		goto err;
+	}
+	if (abs->length > 0)
+		abs->data[abs->length - 1] &= 0xff << unused_bits;
 
-	/* using one because of the bits left byte */
-	if (len-- > 1) {
-		if ((s = malloc(len)) == NULL) {
-			ASN1error(ERR_R_MALLOC_FAILURE);
-			goto err;
-		}
-		memcpy(s, p, len);
-		s[len - 1] &= (0xff << i);
-		p += len;
-	} else
-		s = NULL;
+	*out_abs = abs;
+	abs = NULL;
 
-	free(ret->data);
-	ret->data = s;
-	ret->length = (int)len;
-	ret->type = V_ASN1_BIT_STRING;
+	ret = 1;
 
-	if (a != NULL)
-		*a = ret;
+ err:
+	ASN1_BIT_STRING_free(abs);
+	freezero(data, data_len);
 
-	*pp = p;
+	return ret;
+}
 
-	return (ret);
+ASN1_BIT_STRING *
+c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **out_abs, const unsigned char **pp, long len)
+{
+	ASN1_BIT_STRING *abs = NULL;
+	CBS content;
 
- err:
-	if (a == NULL || *a != ret)
-		ASN1_BIT_STRING_free(ret);
+	if (out_abs != NULL) {
+		ASN1_BIT_STRING_free(*out_abs);
+		*out_abs = NULL;
+	}
+
+	if (len < 0) {
+		ASN1error(ASN1_R_LENGTH_ERROR);
+		return NULL;
+	}
+
+	CBS_init(&content, *pp, len);
+
+	if (!c2i_ASN1_BIT_STRING_cbs(&abs, &content))
+		return NULL;
+
+	*pp = CBS_data(&content);
+
+	if (out_abs != NULL)
+		*out_abs = abs;
 
-	return (NULL);
+	return abs;
 }
 
 int