From 9d9e8f706b3af4adf9e24c929cfdb9e7f936c7e7 Mon Sep 17 00:00:00 2001 From: jsing Date: Mon, 16 May 2022 20:06:15 +0000 Subject: [PATCH] Rewrite asn1_item_ex_d2i_sequence() using CBS and readable variable names. Now that combine no longer exists, we can also free and reallocate. ok tb@ --- lib/libcrypto/asn1/tasn_dec.c | 244 ++++++++++++++++------------------ 1 file changed, 113 insertions(+), 131 deletions(-) diff --git a/lib/libcrypto/asn1/tasn_dec.c b/lib/libcrypto/asn1/tasn_dec.c index 48ac38a883b..744dde23c01 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.67 2022/05/12 20:06:46 jsing Exp $ */ +/* $OpenBSD: tasn_dec.c,v 1.68 2022/05/16 20:06:15 jsing Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2000. */ @@ -129,13 +129,13 @@ asn1_item_ex_d2i_choice(ASN1_VALUE **pval, CBS *cbs, const ASN1_ITEM *it, int tag_number, int tag_class, char optional, int depth) { const ASN1_TEMPLATE *tt, *errtt = NULL; - const ASN1_AUX *aux = it->funcs; + const ASN1_AUX *aux; ASN1_aux_cb *asn1_cb = NULL; ASN1_VALUE *achoice = NULL; ASN1_VALUE **pchptr; int i, ret; - if (aux != NULL) + if ((aux = it->funcs) != NULL) asn1_cb = aux->asn1_cb; if (it->itype != ASN1_ITYPE_CHOICE) @@ -171,11 +171,8 @@ asn1_item_ex_d2i_choice(ASN1_VALUE **pval, CBS *cbs, const ASN1_ITEM *it, /* Mark field as OPTIONAL so its absence can be identified. */ ret = asn1_template_ex_d2i_cbs(pchptr, cbs, tt, 1, depth); - - /* If field not present, try the next one */ if (ret == -1) continue; - if (ret != 1) { ASN1error(ERR_R_NESTED_ASN1_ERROR); errtt = tt; @@ -220,190 +217,175 @@ asn1_item_ex_d2i_choice(ASN1_VALUE **pval, CBS *cbs, const ASN1_ITEM *it, } static int -asn1_item_ex_d2i_sequence(ASN1_VALUE **pval, const unsigned char **in, long len, - const ASN1_ITEM *it, int tag, int aclass, char opt, int depth) +asn1_item_ex_d2i_sequence(ASN1_VALUE **pval, CBS *cbs, const ASN1_ITEM *it, + int tag_number, int tag_class, char optional, int depth) { - const ASN1_TEMPLATE *tt, *errtt = NULL; - const ASN1_AUX *aux = it->funcs; + CBS cbs_seq, cbs_seq_content, cbs_object; + char constructed, indefinite, optional_field; + const ASN1_TEMPLATE *errtt = NULL; + const ASN1_TEMPLATE *seqtt, *tt; ASN1_aux_cb *asn1_cb = NULL; - char seq_eoc, seq_nolen, cst, isopt; - const unsigned char *p = NULL, *q; - CBS cbs; - int i; + const ASN1_AUX *aux; + ASN1_VALUE *aseq = NULL; + ASN1_VALUE **pseqval; + int eoc_needed, i; + size_t length; int ret = 0; + CBS_init(&cbs_seq, CBS_data(cbs), CBS_len(cbs)); + + if ((aux = it->funcs) != NULL) + asn1_cb = aux->asn1_cb; + if (it->itype != ASN1_ITYPE_NDEF_SEQUENCE && it->itype != ASN1_ITYPE_SEQUENCE) goto err; - if (aux && aux->asn1_cb) - asn1_cb = aux->asn1_cb; - - p = *in; + if (*pval != NULL) { + ASN1_item_ex_free(pval, it); + *pval = NULL; + } - /* If no IMPLICIT tagging set to SEQUENCE, UNIVERSAL */ - if (tag == -1) { - tag = V_ASN1_SEQUENCE; - aclass = V_ASN1_UNIVERSAL; + /* If no IMPLICIT tagging use UNIVERSAL/SEQUENCE. */ + if (tag_number == -1) { + tag_class = V_ASN1_UNIVERSAL; + tag_number = V_ASN1_SEQUENCE; } - /* Get SEQUENCE length and update len, p */ - ret = asn1_check_tag(&len, NULL, NULL, &seq_eoc, &cst, &p, len, - tag, aclass, opt); - if (!ret) { + + /* Read ASN.1 SEQUENCE header. */ + ret = asn1_check_tag_cbs(&cbs_seq, &length, NULL, NULL, &indefinite, + &constructed, tag_number, tag_class, optional); + if (ret == -1) + return -1; + if (ret != 1) { ASN1error(ERR_R_NESTED_ASN1_ERROR); goto err; - } else if (ret == -1) - return -1; - seq_nolen = seq_eoc; - if (!cst) { + } + + if (!constructed) { ASN1error(ASN1_R_SEQUENCE_NOT_CONSTRUCTED); goto err; } - if (!*pval && !ASN1_item_ex_new(pval, it)) { + if (indefinite) { + eoc_needed = 1; + CBS_init(&cbs_seq_content, CBS_data(&cbs_seq), CBS_len(&cbs_seq)); + } else { + eoc_needed = 0; + if (!CBS_get_bytes(&cbs_seq, &cbs_seq_content, length)) + goto err; + } + + if (!ASN1_item_ex_new(&aseq, it)) { ASN1error(ERR_R_NESTED_ASN1_ERROR); goto err; } - if (asn1_cb && !asn1_cb(ASN1_OP_D2I_PRE, pval, it, NULL)) - goto auxerr; - - /* Free up and zero any ADB found */ - for (i = 0, tt = it->templates; i < it->tcount; i++, tt++) { - if (tt->flags & ASN1_TFLG_ADB_MASK) { - const ASN1_TEMPLATE *seqtt; - ASN1_VALUE **pseqval; - seqtt = asn1_do_adb(pval, tt, 1); - if (!seqtt) - goto err; - pseqval = asn1_get_field_ptr(pval, seqtt); - ASN1_template_free(pseqval, seqtt); - } + if (asn1_cb != NULL && !asn1_cb(ASN1_OP_D2I_PRE, &aseq, it, NULL)) { + ASN1error(ASN1_R_AUX_ERROR); + goto err; } - /* Get each field entry */ for (i = 0, tt = it->templates; i < it->tcount; i++, tt++) { - const ASN1_TEMPLATE *seqtt; - ASN1_VALUE **pseqval; - seqtt = asn1_do_adb(pval, tt, 1); - if (!seqtt) - goto err; - pseqval = asn1_get_field_ptr(pval, seqtt); - /* Have we ran out of data? */ - if (!len) - break; - q = p; - if (asn1_check_eoc(&p, len)) { - if (!seq_eoc) { + if (asn1_check_eoc_cbs(&cbs_seq_content)) { + if (!indefinite) { ASN1error(ASN1_R_UNEXPECTED_EOC); goto err; } - len -= p - q; - seq_eoc = 0; - q = p; + eoc_needed = 0; break; } - /* This determines the OPTIONAL flag value. The field - * cannot be omitted if it is the last of a SEQUENCE - * and there is still data to be read. This isn't - * strictly necessary but it increases efficiency in - * some cases. - */ - if (i == (it->tcount - 1)) - isopt = 0; - else - isopt = (char)(seqtt->flags & ASN1_TFLG_OPTIONAL); - /* attempt to read in field, allowing each to be - * OPTIONAL */ - - ret = asn1_template_ex_d2i(pseqval, &p, len, - seqtt, isopt, depth); - if (!ret) { - errtt = seqtt; + if (CBS_len(&cbs_seq_content) == 0) + break; + + if ((seqtt = asn1_do_adb(&aseq, tt, 1)) == NULL) goto err; - } else if (ret == -1) { - /* OPTIONAL component absent. - * Free and zero the field. - */ + + pseqval = asn1_get_field_ptr(&aseq, seqtt); + + /* + * This was originally implemented to "increase efficiency", + * however it currently needs to remain since it papers over + * the use of ASN.1 ANY with OPTIONAL in SEQUENCEs (which + * asn1_d2i_ex_primitive() currently rejects). + */ + optional_field = (seqtt->flags & ASN1_TFLG_OPTIONAL) != 0; + if (i == it->tcount - 1) + optional_field = 0; + + ret = asn1_template_ex_d2i_cbs(pseqval, &cbs_seq_content, + seqtt, optional_field, depth); + if (ret == -1) { + /* Absent OPTIONAL component. */ ASN1_template_free(pseqval, seqtt); continue; } - /* Update length */ - len -= p - q; + if (ret != 1) { + errtt = seqtt; + goto err; + } } - /* Check for EOC if expecting one */ - if (seq_eoc && !asn1_check_eoc(&p, len)) { + if (eoc_needed && !asn1_check_eoc_cbs(&cbs_seq_content)) { ASN1error(ASN1_R_MISSING_EOC); goto err; } - /* Check all data read */ - if (!seq_nolen && len) { + + if (indefinite) { + if (!CBS_skip(&cbs_seq, CBS_offset(&cbs_seq_content))) + goto err; + } else if (CBS_len(&cbs_seq_content) != 0) { ASN1error(ASN1_R_SEQUENCE_LENGTH_MISMATCH); goto err; } - /* If we get here we've got no more data in the SEQUENCE, - * however we may not have read all fields so check all - * remaining are OPTIONAL and clear any that are. + /* + * There is no more data in the ASN.1 SEQUENCE, however we may not have + * populated all fields - check that any remaining are OPTIONAL. */ for (; i < it->tcount; tt++, i++) { - const ASN1_TEMPLATE *seqtt; - seqtt = asn1_do_adb(pval, tt, 1); - if (!seqtt) + if ((seqtt = asn1_do_adb(&aseq, tt, 1)) == NULL) goto err; - if (seqtt->flags & ASN1_TFLG_OPTIONAL) { - ASN1_VALUE **pseqval; - pseqval = asn1_get_field_ptr(pval, seqtt); - ASN1_template_free(pseqval, seqtt); - } else { - errtt = seqtt; + + if ((seqtt->flags & ASN1_TFLG_OPTIONAL) == 0) { ASN1error(ASN1_R_FIELD_MISSING); + errtt = seqtt; goto err; } + + /* XXX - this is probably unnecessary with earlier free. */ + pseqval = asn1_get_field_ptr(&aseq, seqtt); + ASN1_template_free(pseqval, seqtt); } - /* Save encoding */ - CBS_init(&cbs, *in, p - *in); - if (!asn1_enc_save(pval, &cbs, it)) { + + if (!CBS_get_bytes(cbs, &cbs_object, CBS_offset(&cbs_seq))) + goto err; + + if (!asn1_enc_save(&aseq, &cbs_object, it)) { ASN1error(ERR_R_MALLOC_FAILURE); goto err; } - *in = p; - if (asn1_cb && !asn1_cb(ASN1_OP_D2I_POST, pval, it, NULL)) - goto auxerr; + + if (asn1_cb != NULL && !asn1_cb(ASN1_OP_D2I_POST, &aseq, it, NULL)) { + ASN1error(ASN1_R_AUX_ERROR); + goto err; + } + + *pval = aseq; + aseq = NULL; + return 1; - auxerr: - ASN1error(ASN1_R_AUX_ERROR); err: - ASN1_item_ex_free(pval, it); + ASN1_item_ex_free(&aseq, it); - if (errtt) + if (errtt != NULL) ERR_asprintf_error_data("Field=%s, Type=%s", errtt->field_name, it->sname); else ERR_asprintf_error_data("Type=%s", it->sname); - return 0; -} -static int -asn1_item_ex_d2i_sequence_cbs(ASN1_VALUE **pval, CBS *cbs, const ASN1_ITEM *it, - int tag_number, int tag_class, char optional, int depth) -{ - const unsigned char *p; - int ret; - - if (CBS_len(cbs) > LONG_MAX) - return 0; - - p = CBS_data(cbs); - ret = asn1_item_ex_d2i_sequence(pval, &p, (long)CBS_len(cbs), it, - tag_number, tag_class, optional, depth); - if (ret == 1) { - if (!CBS_skip(cbs, p - CBS_data(cbs))) - return 0; - } - return ret; + return 0; } /* @@ -502,7 +484,7 @@ asn1_item_ex_d2i_cbs(ASN1_VALUE **pval, CBS *cbs, const ASN1_ITEM *it, case ASN1_ITYPE_NDEF_SEQUENCE: case ASN1_ITYPE_SEQUENCE: - return asn1_item_ex_d2i_sequence_cbs(pval, cbs, it, tag_number, + return asn1_item_ex_d2i_sequence(pval, cbs, it, tag_number, tag_class, optional, depth); default: -- 2.20.1