From be7b42f7987f9d00704bbfd3d473b8f2256a02af Mon Sep 17 00:00:00 2001
From: jsing <>
Date: Sun, 13 Mar 2022 14:58:14 +0000
Subject: Remove free_cont from asn1_d2i_ex_primitive()/asn1_ex_c2i().
The constructed ASN.1 handling in asn1_d2i_ex_primitive() and asn1_ex_c2i()
currently has code to potentially avoid a malloc/memcpy - this is a less
common code path and it introduces a bunch of complexity for minimal gain.
In particular, we're manually adding a trailing NUL when ASN1_STRING_set()
would already do that for us, plus we currently manually free() the data on
an ASN1_STRING, rather than using freezero().
ok inoguchi@ tb@
---
src/lib/libcrypto/asn1/asn1_locl.h | 4 +-
src/lib/libcrypto/asn1/tasn_dec.c | 77 +++++++++++++++-----------------------
2 files changed, 31 insertions(+), 50 deletions(-)
(limited to 'src/lib')
diff --git a/src/lib/libcrypto/asn1/asn1_locl.h b/src/lib/libcrypto/asn1/asn1_locl.h
index bb6a9fc91a..9a29a2b13f 100644
--- a/src/lib/libcrypto/asn1/asn1_locl.h
+++ b/src/lib/libcrypto/asn1/asn1_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: asn1_locl.h,v 1.21 2022/03/02 11:28:00 jsing Exp $ */
+/* $OpenBSD: asn1_locl.h,v 1.22 2022/03/13 14:58:14 jsing Exp $ */
/* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
* project 2006.
*/
@@ -161,8 +161,6 @@ struct x509_crl_method_st {
int (*crl_verify)(X509_CRL *crl, EVP_PKEY *pk);
};
-int asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype, char *free_cont, const ASN1_ITEM *it);
-
int asn1_get_choice_selector(ASN1_VALUE **pval, const ASN1_ITEM *it);
int asn1_set_choice_selector(ASN1_VALUE **pval, int value, const ASN1_ITEM *it);
diff --git a/src/lib/libcrypto/asn1/tasn_dec.c b/src/lib/libcrypto/asn1/tasn_dec.c
index 8f41deef8d..d475c99676 100644
--- a/src/lib/libcrypto/asn1/tasn_dec.c
+++ b/src/lib/libcrypto/asn1/tasn_dec.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tasn_dec.c,v 1.48 2022/01/07 12:24:17 tb Exp $ */
+/* $OpenBSD: tasn_dec.c,v 1.49 2022/03/13 14:58:14 jsing Exp $ */
/* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
* project 2000.
*/
@@ -95,6 +95,8 @@ static int asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in,
static int asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in,
long len, const ASN1_ITEM *it, int tag, int aclass, char opt,
ASN1_TLC *ctx);
+static int asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *content, int len,
+ int utype, const ASN1_ITEM *it);
static void
asn1_tlc_invalidate(ASN1_TLC *ctx)
@@ -662,9 +664,9 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
{
int ret = 0, utype;
long plen;
- char cst, inf, free_cont = 0;
+ char cst, inf;
const unsigned char *p;
- const unsigned char *cont = NULL;
+ const unsigned char *content = NULL;
uint8_t *data = NULL;
size_t data_len = 0;
CBB cbb;
@@ -732,14 +734,14 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
return 0;
}
- cont = *in;
+ content = *in;
/* If indefinite length constructed find the real end */
if (inf) {
if (!asn1_find_end(&p, plen, inf))
goto err;
- len = p - cont;
+ len = p - content;
} else {
- len = p - cont + plen;
+ len = p - content + plen;
p += plen;
}
} else if (cst) {
@@ -754,29 +756,22 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
goto err;
if (!asn1_collect(&cbb, &p, plen, inf, -1, V_ASN1_UNIVERSAL, 0))
goto err;
-
- /* Append a final NUL to string. */
- if (!CBB_add_u8(&cbb, 0))
- goto err;
-
if (!CBB_finish(&cbb, &data, &data_len))
goto err;
- free_cont = 1;
-
- if (data_len < 1 || data_len > LONG_MAX)
+ if (data_len > LONG_MAX)
goto err;
- cont = data;
- len = data_len - 1;
+ content = data;
+ len = data_len;
} else {
- cont = p;
+ content = p;
len = plen;
p += plen;
}
/* We now have content length and type: translate into a structure */
- if (!asn1_ex_c2i(pval, cont, len, utype, &free_cont, it))
+ if (!asn1_ex_c2i(pval, content, len, utype, it))
goto err;
*in = p;
@@ -784,18 +779,16 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
err:
CBB_cleanup(&cbb);
-
- if (free_cont)
- freezero(data, data_len);
+ freezero(data, data_len);
return ret;
}
/* Translate ASN1 content octets into a structure */
-int
-asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype,
- char *free_cont, const ASN1_ITEM *it)
+static int
+asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *content, int len, int utype,
+ const ASN1_ITEM *it)
{
ASN1_VALUE **opval = NULL;
ASN1_STRING *stmp;
@@ -805,10 +798,11 @@ asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype,
if (it->funcs != NULL) {
const ASN1_PRIMITIVE_FUNCS *pf = it->funcs;
+ char free_content = 0;
if (pf->prim_c2i == NULL)
return 0;
- return pf->prim_c2i(pval, cont, len, utype, free_cont, it);
+ return pf->prim_c2i(pval, content, len, utype, &free_content, it);
}
/* If ANY type clear type and set pointer to internal value */
@@ -828,7 +822,7 @@ asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype,
}
switch (utype) {
case V_ASN1_OBJECT:
- if (!c2i_ASN1_OBJECT((ASN1_OBJECT **)pval, &cont, len))
+ if (!c2i_ASN1_OBJECT((ASN1_OBJECT **)pval, &content, len))
goto err;
break;
@@ -847,19 +841,19 @@ asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype,
} else {
ASN1_BOOLEAN *tbool;
tbool = (ASN1_BOOLEAN *)pval;
- *tbool = *cont;
+ *tbool = *content;
}
break;
case V_ASN1_BIT_STRING:
- if (!c2i_ASN1_BIT_STRING((ASN1_BIT_STRING **)pval, &cont, len))
+ if (!c2i_ASN1_BIT_STRING((ASN1_BIT_STRING **)pval, &content, len))
goto err;
break;
case V_ASN1_INTEGER:
case V_ASN1_ENUMERATED:
tint = (ASN1_INTEGER **)pval;
- if (!c2i_ASN1_INTEGER(tint, &cont, len))
+ if (!c2i_ASN1_INTEGER(tint, &content, len))
goto err;
/* Fixup type to match the expected form */
(*tint)->type = utype | ((*tint)->type & V_ASN1_NEG);
@@ -891,10 +885,9 @@ asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype,
ASN1error(ASN1_R_UNIVERSALSTRING_IS_WRONG_LENGTH);
goto err;
}
- /* All based on ASN1_STRING and handled the same */
- if (!*pval) {
- stmp = ASN1_STRING_type_new(utype);
- if (!stmp) {
+ /* All based on ASN1_STRING and handled the same way. */
+ if (*pval == NULL) {
+ if ((stmp = ASN1_STRING_type_new(utype)) == NULL) {
ASN1error(ERR_R_MALLOC_FAILURE);
goto err;
}
@@ -903,19 +896,10 @@ asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype,
stmp = (ASN1_STRING *)*pval;
stmp->type = utype;
}
- /* If we've already allocated a buffer use it */
- if (*free_cont) {
- free(stmp->data);
- stmp->data = (unsigned char *)cont; /* UGLY CAST! RL */
- stmp->length = len;
- *free_cont = 0;
- } else {
- if (!ASN1_STRING_set(stmp, cont, len)) {
- ASN1error(ERR_R_MALLOC_FAILURE);
- ASN1_STRING_free(stmp);
- *pval = NULL;
- goto err;
- }
+ if (!ASN1_STRING_set(stmp, content, len)) {
+ ASN1_STRING_free(stmp);
+ *pval = NULL;
+ goto err;
}
break;
}
@@ -934,7 +918,6 @@ asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len, int utype,
return ret;
}
-
/* This function finds the end of an ASN1 structure when passed its maximum
* length, whether it is indefinite length and a pointer to the content.
* This is more efficient than calling asn1_collect because it does not
--
cgit v1.2.3-55-g6feb