Remove the ASN.1 decoder tag/length cache (TLC).

Currently, every time an ASN.1 identifier and length is decoded it is
stored in a tag/length cache for potential reuse. However, the only time
this is actually of benefit is when decoding CHOICE or SEQUENCE with
OPTIONAL fields (or MSTRING and ANY due to less than ideal
implementation). For CHOICE and SEQUENCE with OPTIONAL fields the
current code attempts to decode the first option and if that fails, it
moves onto the next option and attempts to decode it, repeating until
it succeeds (or runs out of options).

There are a number of problems with the cache. Firstly, it adds complexity
to the ASN.1 decoder since it has to be passed up and down through the
various layers. Secondly, there is nothing that keeps the cached data in
synchronisation with the input stream. This makes it fragile and a
potential security risk. Thirdly, the type is in the public headers and
API, meaning that we cannot readily change the types or fields to improve
the code.

Testing also suggests that in typical decoding cases we actually get a
small performance increase by removing the cache. There are also several
other options that would improve decoding performance, which we can visit
once we have simpler and more robust code.

ok beck@ inoguchi@ tb@

1 files changed, 37 insertions, 90 deletions

diff --git a/lib/libcrypto/asn1/tasn_dec.c b/lib/libcrypto/asn1/tasn_dec.c
index 5c688344610..3936ecba634 100644
--- a/lib/libcrypto/asn1/tasn_dec.c
+++ b/lib/libcrypto/asn1/tasn_dec.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tasn_dec.c,v 1.51 2022/04/26 20:00:18 jsing Exp $ */
+/* $OpenBSD: tasn_dec.c,v 1.52 2022/04/27 17:28:34 jsing Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2000.
  */
@@ -83,40 +83,28 @@ static int asn1_collect(CBB *cbb, const unsigned char **in, long len,
 
 static int asn1_check_tlen(long *olen, int *otag, unsigned char *oclass,
     char *inf, char *cst, const unsigned char **in, long len, int exptag,
-    int expclass, char opt, ASN1_TLC *ctx);
+    int expclass, char opt);
 
 static int asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in,
-    long len, const ASN1_ITEM *it, int tag, int aclass, char opt, ASN1_TLC *ctx,
-    int depth);
+    long len, const ASN1_ITEM *it, int tag, int aclass, char opt, int depth);
 static int asn1_template_ex_d2i(ASN1_VALUE **pval, const unsigned char **in,
-    long len, const ASN1_TEMPLATE *tt, char opt, ASN1_TLC *ctx, int depth);
+    long len, const ASN1_TEMPLATE *tt, char opt, int depth);
 static int asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in,
-    long len, const ASN1_TEMPLATE *tt, char opt, ASN1_TLC *ctx, int depth);
+    long len, const ASN1_TEMPLATE *tt, char opt, int depth);
 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);
+    long len, const ASN1_ITEM *it, int tag, int aclass, char opt);
 static int asn1_ex_c2i(ASN1_VALUE **pval, CBS *content, int utype,
     const ASN1_ITEM *it);
 
-static void
-asn1_tlc_invalidate(ASN1_TLC *ctx)
-{
-	if (ctx != NULL)
-		ctx->valid = 0;
-}
-
 ASN1_VALUE *
 ASN1_item_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
     const ASN1_ITEM *it)
 {
 	ASN1_VALUE *ptmpval = NULL;
-	ASN1_TLC ctx;
-
-	asn1_tlc_invalidate(&ctx);
 
 	if (pval == NULL)
 		pval = &ptmpval;
-	if (asn1_item_ex_d2i(pval, in, len, it, -1, 0, 0, &ctx, 0) <= 0)
+	if (asn1_item_ex_d2i(pval, in, len, it, -1, 0, 0, 0) <= 0)
 		return NULL;
 
 	return *pval;
@@ -126,11 +114,7 @@ int
 ASN1_template_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
     const ASN1_TEMPLATE *tt)
 {
-	ASN1_TLC ctx;
-
-	asn1_tlc_invalidate(&ctx);
-
-	return asn1_template_ex_d2i(pval, in, len, tt, 0, &ctx, 0);
+	return asn1_template_ex_d2i(pval, in, len, tt, 0, 0);
 }
 
 /* Decode an item, taking care of IMPLICIT tagging, if any.
@@ -139,13 +123,13 @@ ASN1_template_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 
 static int
 asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
-    const ASN1_ITEM *it, int tag, int aclass, char opt, ASN1_TLC *ctx,
-    int depth)
+    const ASN1_ITEM *it, int tag, int aclass, char opt, int depth)
 {
 	const ASN1_TEMPLATE *tt, *errtt = NULL;
 	const ASN1_EXTERN_FUNCS *ef;
 	const ASN1_AUX *aux = it->funcs;
 	ASN1_aux_cb *asn1_cb = NULL;
+	ASN1_TLC ctx = { 0 };
 	const unsigned char *p = NULL, *q;
 	unsigned char oclass;
 	char seq_eoc, seq_nolen, cst, isopt;
@@ -184,10 +168,10 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 				goto err;
 			}
 			return asn1_template_ex_d2i(pval, in, len,
-			    it->templates, opt, ctx, depth);
+			    it->templates, opt, depth);
 		}
 		return asn1_d2i_ex_primitive(pval, in, len, it,
-		    tag, aclass, opt, ctx);
+		    tag, aclass, opt);
 		break;
 
 	case ASN1_ITYPE_MSTRING:
@@ -204,7 +188,7 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 		p = *in;
 		/* Just read in tag and class */
 		ret = asn1_check_tlen(NULL, &otag, &oclass, NULL, NULL,
-		    &p, len, -1, 0, 1, ctx);
+		    &p, len, -1, 0, 1);
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			goto err;
@@ -227,13 +211,12 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 			goto err;
 		}
 		return asn1_d2i_ex_primitive(pval, in, len,
-		    it, otag, 0, 0, ctx);
+		    it, otag, 0, 0);
 
 	case ASN1_ITYPE_EXTERN:
 		/* Use new style d2i */
 		ef = it->funcs;
-		return ef->asn1_ex_d2i(pval, in, len,
-		    it, tag, aclass, opt, ctx);
+		return ef->asn1_ex_d2i(pval, in, len, it, tag, aclass, opt, &ctx);
 
 	case ASN1_ITYPE_CHOICE:
 		/*
@@ -269,8 +252,8 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 			/* We mark field as OPTIONAL so its absence
 			 * can be recognised.
 			 */
-			ret = asn1_template_ex_d2i(pchptr, &p, len, tt, 1, ctx,
-				depth);
+			ret = asn1_template_ex_d2i(pchptr, &p, len, tt, 1,
+			    depth);
 			/* If field not present, try the next one */
 			if (ret == -1)
 				continue;
@@ -313,7 +296,7 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 		}
 		/* Get SEQUENCE length and update len, p */
 		ret = asn1_check_tlen(&len, NULL, NULL, &seq_eoc, &cst,
-		    &p, len, tag, aclass, opt, ctx);
+		    &p, len, tag, aclass, opt);
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			goto err;
@@ -388,7 +371,7 @@ asn1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 			 * OPTIONAL */
 
 			ret = asn1_template_ex_d2i(pseqval, &p, len,
-			    seqtt, isopt, ctx, depth);
+			    seqtt, isopt, depth);
 			if (!ret) {
 				errtt = seqtt;
 				goto err;
@@ -464,7 +447,7 @@ int
 ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
     const ASN1_ITEM *it, int tag, int aclass, char opt, ASN1_TLC *ctx)
 {
-	return asn1_item_ex_d2i(pval, in, len, it, tag, aclass, opt, ctx, 0);
+	return asn1_item_ex_d2i(pval, in, len, it, tag, aclass, opt, 0);
 }
 
 /* Templates are handled with two separate functions.
@@ -473,7 +456,7 @@ ASN1_item_ex_d2i(ASN1_VALUE **pval, const unsigned char **in, long len,
 
 static int
 asn1_template_ex_d2i(ASN1_VALUE **val, const unsigned char **in, long inlen,
-    const ASN1_TEMPLATE *tt, char opt, ASN1_TLC *ctx, int depth)
+    const ASN1_TEMPLATE *tt, char opt, int depth)
 {
 	int flags, aclass;
 	int ret;
@@ -496,7 +479,7 @@ asn1_template_ex_d2i(ASN1_VALUE **val, const unsigned char **in, long inlen,
 		 * get the info.
 		 */
 		ret = asn1_check_tlen(&len, NULL, NULL, &exp_eoc, &cst,
-		    &p, inlen, tt->tag, aclass, opt, ctx);
+		    &p, inlen, tt->tag, aclass, opt);
 		q = p;
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
@@ -508,7 +491,7 @@ asn1_template_ex_d2i(ASN1_VALUE **val, const unsigned char **in, long inlen,
 			return 0;
 		}
 		/* We've found the field so it can't be OPTIONAL now */
-		ret = asn1_template_noexp_d2i(val, &p, len, tt, 0, ctx, depth);
+		ret = asn1_template_noexp_d2i(val, &p, len, tt, 0, depth);
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			return 0;
@@ -530,8 +513,7 @@ asn1_template_ex_d2i(ASN1_VALUE **val, const unsigned char **in, long inlen,
 			}
 		}
 	} else
-		return asn1_template_noexp_d2i(val, in, inlen, tt, opt, ctx,
-			depth);
+		return asn1_template_noexp_d2i(val, in, inlen, tt, opt, depth);
 
 	*in = p;
 	return 1;
@@ -543,7 +525,7 @@ asn1_template_ex_d2i(ASN1_VALUE **val, const unsigned char **in, long inlen,
 
 static int
 asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in, long len,
-    const ASN1_TEMPLATE *tt, char opt, ASN1_TLC *ctx, int depth)
+    const ASN1_TEMPLATE *tt, char opt, int depth)
 {
 	int flags, aclass;
 	int ret;
@@ -574,7 +556,7 @@ asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in, long len,
 		}
 		/* Get the tag */
 		ret = asn1_check_tlen(&len, NULL, NULL, &sk_eoc, NULL,
-		    &p, len, sktag, skaclass, opt, ctx);
+		    &p, len, sktag, skaclass, opt);
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			return 0;
@@ -615,7 +597,7 @@ asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in, long len,
 			}
 			skfield = NULL;
 			if (!asn1_item_ex_d2i(&skfield, &p, len,
-			    tt->item, -1, 0, 0, ctx, depth)) {
+			    tt->item, -1, 0, 0, depth)) {
 				ASN1error(ERR_R_NESTED_ASN1_ERROR);
 				goto err;
 			}
@@ -633,7 +615,7 @@ asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in, long len,
 	} else if (flags & ASN1_TFLG_IMPTAG) {
 		/* IMPLICIT tagging */
 		ret = asn1_item_ex_d2i(val, &p, len,
-		    tt->item, tt->tag, aclass, opt, ctx, depth);
+		    tt->item, tt->tag, aclass, opt, depth);
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			goto err;
@@ -642,7 +624,7 @@ asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in, long len,
 	} else {
 		/* Nothing special */
 		ret = asn1_item_ex_d2i(val, &p, len, tt->item,
-		    -1, tt->flags & ASN1_TFLG_COMBINE, opt, ctx, depth);
+		    -1, tt->flags & ASN1_TFLG_COMBINE, opt, depth);
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			goto err;
@@ -660,7 +642,7 @@ asn1_template_noexp_d2i(ASN1_VALUE **val, const unsigned char **in, long len,
 
 static int
 asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
-    const ASN1_ITEM *it, int tag, int aclass, char opt, ASN1_TLC *ctx)
+    const ASN1_ITEM *it, int tag, int aclass, char opt)
 {
 	int ret = 0, utype;
 	long plen;
@@ -699,7 +681,7 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
 		}
 		p = *in;
 		ret = asn1_check_tlen(NULL, &utype, &oclass, NULL, NULL,
-		    &p, inlen, -1, 0, 0, ctx);
+		    &p, inlen, -1, 0, 0);
 		if (!ret) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			return 0;
@@ -714,7 +696,7 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
 	p = *in;
 	/* Check header */
 	ret = asn1_check_tlen(&plen, NULL, NULL, &inf, &cst,
-	    &p, inlen, tag, aclass, opt, ctx);
+	    &p, inlen, tag, aclass, opt);
 	if (!ret) {
 		ASN1error(ERR_R_NESTED_ASN1_ERROR);
 		return 0;
@@ -724,12 +706,7 @@ asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in, long inlen,
 	/* SEQUENCE, SET and "OTHER" are left in encoded form */
 	if ((utype == V_ASN1_SEQUENCE) || (utype == V_ASN1_SET) ||
 	    (utype == V_ASN1_OTHER)) {
-		/* Clear context cache for type OTHER because the auto clear
-		 * when we have a exact match wont work
-		 */
-		if (utype == V_ASN1_OTHER) {
-			asn1_tlc_invalidate(ctx);
-		} else if (!cst) {
+		if (utype != V_ASN1_OTHER && !cst) {
 			/* SEQUENCE and SET must be constructed */
 			ASN1error(ASN1_R_TYPE_NOT_CONSTRUCTED);
 			return 0;
@@ -964,7 +941,7 @@ asn1_find_end(const unsigned char **in, long len, char inf)
 		q = p;
 		/* Just read in a header: only care about the length */
 		if (!asn1_check_tlen(&plen, NULL, NULL, &inf, NULL, &p, len,
-		    -1, 0, 0, NULL)) {
+		    -1, 0, 0)) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			return 0;
 		}
@@ -1027,7 +1004,7 @@ asn1_collect(CBB *cbb, const unsigned char **in, long len, char inf,
 		}
 
 		if (!asn1_check_tlen(&plen, NULL, NULL, &ininf, &cst, &p,
-		    len, tag, aclass, 0, NULL)) {
+		    len, tag, aclass, 0)) {
 			ASN1error(ERR_R_NESTED_ASN1_ERROR);
 			return 0;
 		}
@@ -1079,7 +1056,7 @@ asn1_check_eoc(const unsigned char **in, long len)
 static int
 asn1_check_tlen(long *olen, int *otag, unsigned char *oclass, char *inf,
     char *cst, const unsigned char **in, long len, int exptag, int expclass,
-    char opt, ASN1_TLC *ctx)
+    char opt)
 {
 	int i;
 	int ptag, pclass;
@@ -1089,35 +1066,9 @@ asn1_check_tlen(long *olen, int *otag, unsigned char *oclass, char *inf,
 	p = *in;
 	q = p;
 
-	if (ctx && ctx->valid) {
-		i = ctx->ret;
-		plen = ctx->plen;
-		pclass = ctx->pclass;
-		ptag = ctx->ptag;
-		p += ctx->hdrlen;
-	} else {
-		i = ASN1_get_object(&p, &plen, &ptag, &pclass, len);
-		if (ctx) {
-			ctx->ret = i;
-			ctx->plen = plen;
-			ctx->pclass = pclass;
-			ctx->ptag = ptag;
-			ctx->hdrlen = p - q;
-			ctx->valid = 1;
-			/* If definite length, and no error, length +
-			 * header can't exceed total amount of data available.
-			 */
-			if (!(i & 0x81) && ((plen + ctx->hdrlen) > len)) {
-				ASN1error(ASN1_R_TOO_LONG);
-				asn1_tlc_invalidate(ctx);
-				return 0;
-			}
-		}
-	}
-
+	i = ASN1_get_object(&p, &plen, &ptag, &pclass, len);
 	if (i & 0x80) {
 		ASN1error(ASN1_R_BAD_OBJECT_HEADER);
-		asn1_tlc_invalidate(ctx);
 		return 0;
 	}
 	if (exptag >= 0) {
@@ -1127,13 +1078,9 @@ asn1_check_tlen(long *olen, int *otag, unsigned char *oclass, char *inf,
 			 */
 			if (opt)
 				return -1;
-			asn1_tlc_invalidate(ctx);
 			ASN1error(ASN1_R_WRONG_TAG);
 			return 0;
 		}
-		/* We have a tag and class match:
-		 * assume we are going to do something with it */
-		asn1_tlc_invalidate(ctx);
 	}
 
 	if (i & 1)