From: jsing <jsing@openbsd.org>
Date: Sat, 25 Jun 2022 15:39:12 +0000 (+0000)
Subject: Rewrite ASN1_INTEGER_{get,set}() using CBS/CBB
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=90a5dc6e3ed9b79a6aecceb6adf526dc0eefd36d;p=openbsd

Rewrite ASN1_INTEGER_{get,set}() using CBS/CBB

In the process, prepare to provide ASN1_INTEGER_{get,set}_{u,}int64().

ok beck@ tb@
---

diff --git a/lib/libcrypto/asn1/a_int.c b/lib/libcrypto/asn1/a_int.c
index 6ad0df3d1e3..546713ae46d 100644
--- a/lib/libcrypto/asn1/a_int.c
+++ b/lib/libcrypto/asn1/a_int.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: a_int.c,v 1.40 2022/06/25 14:22:54 jsing Exp $ */
+/* $OpenBSD: a_int.c,v 1.41 2022/06/25 15:39:12 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -80,6 +80,16 @@ ASN1_INTEGER_new(void)
 	return (ASN1_INTEGER *)ASN1_item_new(&ASN1_INTEGER_it);
 }
 
+static void
+asn1_aint_clear(ASN1_INTEGER *aint)
+{
+	freezero(aint->data, aint->length);
+
+	memset(aint, 0, sizeof(*aint));
+
+	aint->type = V_ASN1_INTEGER;
+}
+
 void
 ASN1_INTEGER_free(ASN1_INTEGER *a)
 {
@@ -117,83 +127,190 @@ ASN1_INTEGER_cmp(const ASN1_INTEGER *a, const ASN1_INTEGER *b)
 }
 
 int
-ASN1_INTEGER_set(ASN1_INTEGER *a, long v)
+asn1_aint_get_uint64(CBS *cbs, uint64_t *out_val)
 {
-	int j, k;
-	unsigned int i;
-	unsigned char buf[sizeof(long) + 1];
-	long d;
-
-	a->type = V_ASN1_INTEGER;
-	/* XXX ssl/ssl_asn1.c:i2d_SSL_SESSION() depends upon this bound vae */
-	if (a->length < (int)(sizeof(long) + 1)) {
-		free(a->data);
-		a->data = calloc(1, sizeof(long) + 1);
-	}
-	if (a->data == NULL) {
-		ASN1error(ERR_R_MALLOC_FAILURE);
-		return (0);
+	uint64_t val = 0;
+	uint8_t u8;
+
+	*out_val = 0;
+
+	while (CBS_len(cbs) > 0) {
+		if (!CBS_get_u8(cbs, &u8))
+			return 0;
+		if (val > (UINT64_MAX >> 8)) {
+			ASN1error(ASN1_R_TOO_LARGE);
+			return 0;
+		}
+		val = val << 8 | u8;
 	}
-	d = v;
-	if (d < 0) {
-		d = -d;
-		a->type = V_ASN1_NEG_INTEGER;
+
+	*out_val = val;
+
+	return 1;
+}
+
+int
+asn1_aint_set_uint64(uint64_t val, uint8_t **out_data, int *out_len)
+{
+	uint8_t *data = NULL;
+	size_t data_len = 0;
+	int started = 0;
+	uint8_t u8;
+	CBB cbb;
+	int i;
+	int ret = 0;
+
+	if (!CBB_init(&cbb, sizeof(long)))
+		goto err;
+
+	if (out_data == NULL || out_len == NULL)
+		goto err;
+	if (*out_data != NULL || *out_len != 0)
+		goto err;
+
+	for (i = sizeof(uint64_t) - 1; i >= 0; i--) {
+		u8 = (val >> (i * 8)) & 0xff;
+		if (!started && i != 0 && u8 == 0)
+			continue;
+		if (!CBB_add_u8(&cbb, u8))
+			goto err;
+		started = 1;
 	}
 
-	for (i = 0; i < sizeof(long); i++) {
-		if (d == 0)
-			break;
-		buf[i] = (int)d & 0xff;
-		d >>= 8;
+	if (!CBB_finish(&cbb, &data, &data_len))
+		goto err;
+	if (data_len > INT_MAX)
+		goto err;
+
+	*out_data = data;
+	*out_len = (int)data_len;
+	data = NULL;
+
+	ret = 1;
+ err:
+	CBB_cleanup(&cbb);
+	freezero(data, data_len);
+
+	return ret;
+}
+
+int
+asn1_aint_get_int64(CBS *cbs, int negative, int64_t *out_val)
+{
+	uint64_t val;
+
+	if (!asn1_aint_get_uint64(cbs, &val))
+		return 0;
+
+	if (negative) {
+		if (val > (uint64_t)INT64_MIN) {
+			ASN1error(ASN1_R_TOO_SMALL);
+			return 0;
+		}
+		*out_val = -(int64_t)val;
+	} else {
+		if (val > (uint64_t)INT64_MAX) {
+			ASN1error(ASN1_R_TOO_LARGE);
+			return 0;
+		}
+		*out_val = (int64_t)val;
 	}
-	j = 0;
-	for (k = i - 1; k >= 0; k--)
-		a->data[j++] = buf[k];
-	a->length = j;
-	return (1);
+
+	return 1;
 }
 
-/*
- * XXX this particular API is a gibbering eidrich horror that makes it
- * impossible to determine valid return cases from errors.. "a bit
- * ugly" is preserved for posterity, unfortunately this is probably
- * unfixable without changing public API
- */
-long
-ASN1_INTEGER_get(const ASN1_INTEGER *a)
+int
+ASN1_INTEGER_get_uint64(uint64_t *out_val, const ASN1_INTEGER *aint)
 {
-	int neg = 0, i;
-	unsigned long r = 0;
+	uint64_t val;
+	CBS cbs;
 
-	if (a == NULL)
-		return (0L);
-	i = a->type;
-	if (i == V_ASN1_NEG_INTEGER)
-		neg = 1;
-	else if (i != V_ASN1_INTEGER)
-		return -1;
+	*out_val = 0;
 
-	if (!ASN1_INTEGER_valid(a))
-		return -1; /* XXX best effort */
+	if (aint == NULL || aint->length < 0)
+		return 0;
 
-	if (a->length > (int)sizeof(long)) {
-		/* hmm... a bit ugly, return all ones */
-		return -1;
+	if (aint->type == V_ASN1_NEG_INTEGER) {
+		ASN1error(ASN1_R_ILLEGAL_NEGATIVE_VALUE);
+		return 0;
 	}
-	if (a->data == NULL)
+	if (aint->type != V_ASN1_INTEGER) {
+		ASN1error(ASN1_R_WRONG_INTEGER_TYPE);
 		return 0;
+	}
+
+	CBS_init(&cbs, aint->data, aint->length);
+
+	if (!asn1_aint_get_uint64(&cbs, &val))
+		return 0;
+
+	*out_val = val;
+
+	return 1;
+}
+
+int
+ASN1_INTEGER_set_uint64(ASN1_INTEGER *aint, uint64_t val)
+{
+	asn1_aint_clear(aint);
 
-	for (i = 0; i < a->length; i++) {
-		r <<= 8;
-		r |= (unsigned char)a->data[i];
+	return asn1_aint_set_uint64(val, &aint->data, &aint->length);
+}
+
+int
+ASN1_INTEGER_get_int64(int64_t *out_val, const ASN1_INTEGER *aint)
+{
+	CBS cbs;
+
+	*out_val = 0;
+
+	if (aint == NULL || aint->length < 0)
+		return 0;
+
+	if (aint->type != V_ASN1_INTEGER &&
+	    aint->type != V_ASN1_NEG_INTEGER) {
+		ASN1error(ASN1_R_WRONG_INTEGER_TYPE);
+		return 0;
 	}
 
-	if (r > LONG_MAX)
+	CBS_init(&cbs, aint->data, aint->length);
+
+	return asn1_aint_get_int64(&cbs, (aint->type == V_ASN1_NEG_INTEGER),
+	    out_val);
+}
+
+int
+ASN1_INTEGER_set_int64(ASN1_INTEGER *aint, int64_t val)
+{
+	asn1_aint_clear(aint);
+
+	if (val < 0) {
+		aint->type = V_ASN1_NEG_INTEGER;
+		val = -val;
+	}
+
+	return asn1_aint_set_uint64((uint64_t)val, &aint->data, &aint->length);
+}
+
+long
+ASN1_INTEGER_get(const ASN1_INTEGER *aint)
+{
+	int64_t val;
+
+	if (!ASN1_INTEGER_get_int64(&val, aint))
 		return -1;
+	if (val < LONG_MIN || val > LONG_MAX) {
+		/* hmm... a bit ugly, return all ones */
+		return -1;
+	}
+
+	return (long)val;
+}
 
-	if (neg)
-		return -(long)r;
-	return (long)r;
+int
+ASN1_INTEGER_set(ASN1_INTEGER *aint, long val)
+{
+	return ASN1_INTEGER_set_int64(aint, val);
 }
 
 ASN1_INTEGER *
diff --git a/lib/libcrypto/asn1/asn1.h b/lib/libcrypto/asn1/asn1.h
index e569e875c28..d6adb0d22e4 100644
--- a/lib/libcrypto/asn1/asn1.h
+++ b/lib/libcrypto/asn1/asn1.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: asn1.h,v 1.62 2022/01/14 08:53:53 tb Exp $ */
+/* $OpenBSD: asn1.h,v 1.63 2022/06/25 15:39:12 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -748,6 +748,12 @@ int a2d_ASN1_OBJECT(unsigned char *out, int olen, const char *buf, int num);
 ASN1_OBJECT *ASN1_OBJECT_create(int nid, unsigned char *data, int len,
     const char *sn, const char *ln);
 
+#ifdef LIBRESSL_INTERNAL
+int ASN1_INTEGER_get_uint64(uint64_t *out_val, const ASN1_INTEGER *aint);
+int ASN1_INTEGER_set_uint64(ASN1_INTEGER *aint, uint64_t val);
+int ASN1_INTEGER_get_int64(int64_t *out_val, const ASN1_INTEGER *aint);
+int ASN1_INTEGER_set_int64(ASN1_INTEGER *aint, int64_t val);
+#endif
 int ASN1_INTEGER_set(ASN1_INTEGER *a, long v);
 long ASN1_INTEGER_get(const ASN1_INTEGER *a);
 ASN1_INTEGER *BN_to_ASN1_INTEGER(const BIGNUM *bn, ASN1_INTEGER *ai);
@@ -1109,6 +1115,7 @@ void ERR_load_ASN1_strings(void);
 #define ASN1_R_ILLEGAL_HEX				 178
 #define ASN1_R_ILLEGAL_IMPLICIT_TAG			 179
 #define ASN1_R_ILLEGAL_INTEGER				 180
+#define ASN1_R_ILLEGAL_NEGATIVE_VALUE			 226
 #define ASN1_R_ILLEGAL_NESTED_TAGGING			 181
 #define ASN1_R_ILLEGAL_NULL				 125
 #define ASN1_R_ILLEGAL_NULL_VALUE			 182
@@ -1168,7 +1175,9 @@ void ERR_load_ASN1_strings(void);
 #define ASN1_R_TAG_VALUE_TOO_HIGH			 153
 #define ASN1_R_THE_ASN1_OBJECT_IDENTIFIER_IS_NOT_KNOWN_FOR_THIS_MD 154
 #define ASN1_R_TIME_NOT_ASCII_FORMAT			 193
+#define ASN1_R_TOO_LARGE				 223
 #define ASN1_R_TOO_LONG					 155
+#define ASN1_R_TOO_SMALL				 224
 #define ASN1_R_TYPE_NOT_CONSTRUCTED			 156
 #define ASN1_R_UNABLE_TO_DECODE_RSA_KEY			 157
 #define ASN1_R_UNABLE_TO_DECODE_RSA_PRIVATE_KEY		 158
@@ -1186,11 +1195,11 @@ void ERR_load_ASN1_strings(void);
 #define ASN1_R_UNSUPPORTED_ENCRYPTION_ALGORITHM		 166
 #define ASN1_R_UNSUPPORTED_PUBLIC_KEY_TYPE		 167
 #define ASN1_R_UNSUPPORTED_TYPE				 196
+#define ASN1_R_WRONG_INTEGER_TYPE			 225
 #define ASN1_R_WRONG_PUBLIC_KEY_TYPE			 200
 #define ASN1_R_WRONG_TAG				 168
 #define ASN1_R_WRONG_TYPE				 169
 
-
 int ASN1_time_parse(const char *_bytes, size_t _len, struct tm *_tm, int _mode);
 int ASN1_time_tm_cmp(struct tm *_tm1, struct tm *_tm2);
 #ifdef  __cplusplus
diff --git a/lib/libcrypto/asn1/asn1_err.c b/lib/libcrypto/asn1/asn1_err.c
index e2c56deb5ba..98db4f78022 100644
--- a/lib/libcrypto/asn1/asn1_err.c
+++ b/lib/libcrypto/asn1/asn1_err.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: asn1_err.c,v 1.22 2020/12/08 15:06:42 tb Exp $ */
+/* $OpenBSD: asn1_err.c,v 1.23 2022/06/25 15:39:12 jsing Exp $ */
 /* ====================================================================
  * Copyright (c) 1999-2011 The OpenSSL Project.  All rights reserved.
  *
@@ -118,6 +118,7 @@ static ERR_STRING_DATA ASN1_str_reasons[] = {
 	{ERR_REASON(ASN1_R_ILLEGAL_HEX)          , "illegal hex"},
 	{ERR_REASON(ASN1_R_ILLEGAL_IMPLICIT_TAG) , "illegal implicit tag"},
 	{ERR_REASON(ASN1_R_ILLEGAL_INTEGER)      , "illegal integer"},
+	{ERR_REASON(ASN1_R_ILLEGAL_NEGATIVE_VALUE), "illegal negative value"},
 	{ERR_REASON(ASN1_R_ILLEGAL_NESTED_TAGGING), "illegal nested tagging"},
 	{ERR_REASON(ASN1_R_ILLEGAL_NULL)         , "illegal null"},
 	{ERR_REASON(ASN1_R_ILLEGAL_NULL_VALUE)   , "illegal null value"},
@@ -177,7 +178,9 @@ static ERR_STRING_DATA ASN1_str_reasons[] = {
 	{ERR_REASON(ASN1_R_TAG_VALUE_TOO_HIGH)   , "tag value too high"},
 	{ERR_REASON(ASN1_R_THE_ASN1_OBJECT_IDENTIFIER_IS_NOT_KNOWN_FOR_THIS_MD), "the asn1 object identifier is not known for this md"},
 	{ERR_REASON(ASN1_R_TIME_NOT_ASCII_FORMAT), "time not ascii format"},
+	{ERR_REASON(ASN1_R_TOO_LARGE)            , "too large"},
 	{ERR_REASON(ASN1_R_TOO_LONG)             , "too long"},
+	{ERR_REASON(ASN1_R_TOO_SMALL)            , "too small"},
 	{ERR_REASON(ASN1_R_TYPE_NOT_CONSTRUCTED) , "type not constructed"},
 	{ERR_REASON(ASN1_R_UNABLE_TO_DECODE_RSA_KEY), "unable to decode rsa key"},
 	{ERR_REASON(ASN1_R_UNABLE_TO_DECODE_RSA_PRIVATE_KEY), "unable to decode rsa private key"},
@@ -195,12 +198,12 @@ static ERR_STRING_DATA ASN1_str_reasons[] = {
 	{ERR_REASON(ASN1_R_UNSUPPORTED_ENCRYPTION_ALGORITHM), "unsupported encryption algorithm"},
 	{ERR_REASON(ASN1_R_UNSUPPORTED_PUBLIC_KEY_TYPE), "unsupported public key type"},
 	{ERR_REASON(ASN1_R_UNSUPPORTED_TYPE)     , "unsupported type"},
+	{ERR_REASON(ASN1_R_WRONG_INTEGER_TYPE)   , "wrong integer type"},
 	{ERR_REASON(ASN1_R_WRONG_PUBLIC_KEY_TYPE), "wrong public key type"},
 	{ERR_REASON(ASN1_R_WRONG_TAG)            , "wrong tag"},
 	{ERR_REASON(ASN1_R_WRONG_TYPE)           , "wrong type"},
 	{0, NULL}
 };
-
 #endif
 
 void
diff --git a/lib/libcrypto/asn1/asn1_locl.h b/lib/libcrypto/asn1/asn1_locl.h
index db456c8ff27..2d007061f21 100644
--- a/lib/libcrypto/asn1/asn1_locl.h
+++ b/lib/libcrypto/asn1/asn1_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: asn1_locl.h,v 1.32 2022/05/17 09:17:20 tb Exp $ */
+/* $OpenBSD: asn1_locl.h,v 1.33 2022/06/25 15:39:12 jsing Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2006.
  */
@@ -208,6 +208,9 @@ int asn1_tag2charwidth(int tag);
 int asn1_abs_set_unused_bits(ASN1_BIT_STRING *abs, uint8_t unused_bits);
 int c2i_ASN1_BIT_STRING_cbs(ASN1_BIT_STRING **out_abs, CBS *cbs);
 
+int asn1_aint_get_uint64(CBS *cbs, uint64_t *out_val);
+int asn1_aint_set_uint64(uint64_t val, uint8_t **out_data, int *out_len);
+int asn1_aint_get_int64(CBS *cbs, int negative, int64_t *out_val);
 int c2i_ASN1_INTEGER_cbs(ASN1_INTEGER **out_aint, CBS *cbs);
 
 int c2i_ASN1_OBJECT_cbs(ASN1_OBJECT **out_aobj, CBS *content);