From 7dc2597b4f99adcd7f0885ae3e1ec4dbe0d861e2 Mon Sep 17 00:00:00 2001 From: tb Date: Thu, 13 Jul 2023 20:59:10 +0000 Subject: [PATCH] Refactor ASN1_item_sign_ctx() Oh, joy! The muppets had a feast: they could combine the horrors of EVP with X.509... Return values between -1 and 3 indicating how much work needs to be done, depending on whether methods are present or absent. Needless to say that RSA and EdDSA had inconsistent return values until recently. Instead of interleaving if/else branches, split out two helper functions that do essentially independent things, which results in something that isn't entirely bad. Well, at least not compared to the surrounding code. asn1_item_set_algorithm_identifiers() extracts the signature algorithm from the digest and pkey if known, and sets it on the two X509_ALGOR that may or may not have been passed in. asn1_item_sign() converts data into der and signs. Of course there were also a few leaks and missing error checks. ok jsing --- lib/libcrypto/asn1/asn1_item.c | 143 ++++++++++++++++++++------------- 1 file changed, 89 insertions(+), 54 deletions(-) diff --git a/lib/libcrypto/asn1/asn1_item.c b/lib/libcrypto/asn1/asn1_item.c index 4458ceb866a..87f426c6f86 100644 --- a/lib/libcrypto/asn1/asn1_item.c +++ b/lib/libcrypto/asn1/asn1_item.c @@ -1,4 +1,4 @@ -/* $OpenBSD: asn1_item.c,v 1.16 2023/07/07 19:37:52 beck Exp $ */ +/* $OpenBSD: asn1_item.c,v 1.17 2023/07/13 20:59:10 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -230,73 +230,59 @@ ASN1_item_sign(const ASN1_ITEM *it, X509_ALGOR *algor1, X509_ALGOR *algor2, return ASN1_item_sign_ctx(it, algor1, algor2, signature, asn, &ctx); } -int -ASN1_item_sign_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1, X509_ALGOR *algor2, - ASN1_BIT_STRING *signature, void *asn, EVP_MD_CTX *ctx) +static int +asn1_item_set_algorithm_identifiers(EVP_MD_CTX *ctx, X509_ALGOR *algor1, + X509_ALGOR *algor2) { - const EVP_MD *type; EVP_PKEY *pkey; - unsigned char *in = NULL, *out = NULL; - size_t out_len = 0; - int in_len = 0; - int signid, paramtype; - int rv = 2; - int ret = 0; + ASN1_OBJECT *aobj; + const EVP_MD *md; + int sign_id, sign_param; if ((pkey = EVP_PKEY_CTX_get0_pkey(ctx->pctx)) == NULL) { ASN1error(ASN1_R_CONTEXT_NOT_INITIALISED); return 0; } - if (pkey->ameth == NULL) { - ASN1error(ASN1_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED); + if ((md = EVP_MD_CTX_md(ctx)) == NULL) { + ASN1error(ASN1_R_CONTEXT_NOT_INITIALISED); return 0; } - if (pkey->ameth->item_sign != NULL) { - rv = pkey->ameth->item_sign(ctx, it, asn, algor1, algor2, - signature); - if (rv == 1) { - out_len = signature->length; - goto done; - } - /* Return value meanings: - * <=0: error. - * 1: method does everything. - * 2: carry on as normal. - * 3: ASN1 method sets algorithm identifiers: just sign. - */ - if (rv <= 0) { - ASN1error(ERR_R_EVP_LIB); - goto err; - } + if (!OBJ_find_sigid_by_algs(&sign_id, EVP_MD_nid(md), + pkey->ameth->pkey_id)) { + ASN1error(ASN1_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED); + return 0; + } + if ((aobj = OBJ_nid2obj(sign_id)) == NULL) { + ASN1error(ASN1_R_UNKNOWN_OBJECT_TYPE); + return 0; } - if (rv == 2) { - if ((type = EVP_MD_CTX_md(ctx)) == NULL) { - ASN1error(ASN1_R_CONTEXT_NOT_INITIALISED); - return 0; - } + sign_param = V_ASN1_UNDEF; + if (pkey->ameth->pkey_flags & ASN1_PKEY_SIGPARAM_NULL) + sign_param = V_ASN1_NULL; - if (!OBJ_find_sigid_by_algs(&signid, EVP_MD_nid(type), - pkey->ameth->pkey_id)) { - ASN1error(ASN1_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED); + if (algor1 != NULL) { + if (!X509_ALGOR_set0(algor1, aobj, sign_param, NULL)) return 0; - } - - if (pkey->ameth->pkey_flags & ASN1_PKEY_SIGPARAM_NULL) - paramtype = V_ASN1_NULL; - else - paramtype = V_ASN1_UNDEF; + } + if (algor2 != NULL) { + if (!X509_ALGOR_set0(algor2, aobj, sign_param, NULL)) + return 0; + } - if (algor1) - X509_ALGOR_set0(algor1, - OBJ_nid2obj(signid), paramtype, NULL); - if (algor2) - X509_ALGOR_set0(algor2, - OBJ_nid2obj(signid), paramtype, NULL); + return 1; +} - } +static int +asn1_item_sign(EVP_MD_CTX *ctx, const ASN1_ITEM *it, void *asn, + ASN1_BIT_STRING *signature) +{ + unsigned char *in = NULL, *out = NULL; + size_t out_len = 0; + int in_len = 0; + int ret = 0; if ((in_len = ASN1_item_i2d(asn, &in, it)) <= 0) { in_len = 0; @@ -325,20 +311,69 @@ ASN1_item_sign_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1, X509_ALGOR *algor2, out = NULL; if (!asn1_abs_set_unused_bits(signature, 0)) { + ASN1_STRING_set0(signature, NULL, 0); ASN1error(ERR_R_ASN1_LIB); goto err; } - done: - ret = out_len; + ret = 1; + err: - EVP_MD_CTX_cleanup(ctx); freezero(in, in_len); freezero(out, out_len); return ret; } +int +ASN1_item_sign_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1, X509_ALGOR *algor2, + ASN1_BIT_STRING *signature, void *asn, EVP_MD_CTX *ctx) +{ + EVP_PKEY *pkey; + int rv; + int ret = 0; + + if ((pkey = EVP_PKEY_CTX_get0_pkey(ctx->pctx)) == NULL) { + ASN1error(ASN1_R_CONTEXT_NOT_INITIALISED); + goto err; + } + if (pkey->ameth == NULL) { + ASN1error(ASN1_R_DIGEST_AND_KEY_TYPE_NOT_SUPPORTED); + goto err; + } + + /* + * API insanity ahead. If the item_sign() method is absent or if it + * returns 2, this means: do all the work here. If it returns 3, only + * sign. If it returns 1, then there's nothing to do but to return + * the signature's length. Everything else is an error. + */ + + rv = 2; + if (pkey->ameth->item_sign != NULL) + rv = pkey->ameth->item_sign(ctx, it, asn, algor1, algor2, + signature); + if (rv <= 0 || rv > 3) + goto err; + if (rv == 1) + goto done; + if (rv == 2) { + if (!asn1_item_set_algorithm_identifiers(ctx, algor1, algor2)) + goto err; + } + + if (!asn1_item_sign(ctx, it, asn, signature)) + goto err; + + done: + ret = signature->length; + + err: + EVP_MD_CTX_cleanup(ctx); + + return ret; +} + int ASN1_item_verify(const ASN1_ITEM *it, X509_ALGOR *a, ASN1_BIT_STRING *signature, void *asn, EVP_PKEY *pkey) -- 2.20.1