From 7c310e872e72977432b3520c5d27641e13815548 Mon Sep 17 00:00:00 2001 From: "Dr. David von Oheimb" Date: Tue, 28 Jun 2022 07:53:59 +0200 Subject: [PATCH] libcrypto refactoring: introduce and use ossl_asn1_string_set_bits_left() Reviewed-by: Tomas Mraz Reviewed-by: Shane Lontis Reviewed-by: David von Oheimb (Merged from https://github.com/openssl/openssl/pull/18668) --- crypto/asn1/a_bitstr.c | 3 +-- crypto/asn1/a_sign.c | 6 ++---- crypto/asn1/asn1_gen.c | 7 ++----- crypto/asn1/asn1_lib.c | 6 ++++++ crypto/asn1/asn1_local.h | 2 ++ crypto/cmp/cmp_protect.c | 3 +-- crypto/cms/cms_dh.c | 4 ++-- crypto/cms/cms_ec.c | 4 ++-- crypto/ec/ec_asn1.c | 7 +++---- crypto/x509/v3_addr.c | 10 +++------- crypto/x509/x_pubkey.c | 4 +--- include/crypto/asn1.h | 1 + include/internal/cryptlib.h | 1 + include/openssl/asn1.h.in | 2 +- 14 files changed, 28 insertions(+), 32 deletions(-) diff --git a/crypto/asn1/a_bitstr.c b/crypto/asn1/a_bitstr.c index f8938ad1073..7b3991a0711 100644 --- a/crypto/asn1/a_bitstr.c +++ b/crypto/asn1/a_bitstr.c @@ -110,8 +110,7 @@ ASN1_BIT_STRING *ossl_c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a, * We do this to preserve the settings. If we modify the settings, via * the _set_bit function, we will recalculate on output */ - ret->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); /* clear */ - ret->flags |= (ASN1_STRING_FLAG_BITS_LEFT | i); /* set */ + ossl_asn1_string_set_bits_left(ret, i); if (len-- > 1) { /* using one because of the bits left byte */ s = OPENSSL_malloc((int)len); diff --git a/crypto/asn1/a_sign.c b/crypto/asn1/a_sign.c index fc3f15007ea..a1e2719e64a 100644 --- a/crypto/asn1/a_sign.c +++ b/crypto/asn1/a_sign.c @@ -102,8 +102,7 @@ int ASN1_sign(i2d_of_void *i2d, X509_ALGOR *algor1, X509_ALGOR *algor2, * In the interests of compatibility, I'll make sure that the bit string * has a 'not-used bits' value of 0 */ - signature->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - signature->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(signature, 0); err: EVP_MD_CTX_free(ctx); OPENSSL_clear_free((char *)buf_in, inll); @@ -286,8 +285,7 @@ int ASN1_item_sign_ctx(const ASN1_ITEM *it, X509_ALGOR *algor1, * In the interests of compatibility, I'll make sure that the bit string * has a 'not-used bits' value of 0 */ - signature->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - signature->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(signature, 0); err: OPENSSL_clear_free((char *)buf_in, inl); OPENSSL_clear_free((char *)buf_out, outll); diff --git a/crypto/asn1/asn1_gen.c b/crypto/asn1/asn1_gen.c index 5b5a469fa9f..c590c62fc2b 100644 --- a/crypto/asn1/asn1_gen.c +++ b/crypto/asn1/asn1_gen.c @@ -714,11 +714,8 @@ static ASN1_TYPE *asn1_str2type(const char *str, int format, int utype) goto bad_form; } - if ((utype == V_ASN1_BIT_STRING) && no_unused) { - atmp->value.asn1_string->flags - &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - atmp->value.asn1_string->flags |= ASN1_STRING_FLAG_BITS_LEFT; - } + if ((utype == V_ASN1_BIT_STRING) && no_unused) + ossl_asn1_string_set_bits_left(atmp->value.asn1_string, 0); break; diff --git a/crypto/asn1/asn1_lib.c b/crypto/asn1/asn1_lib.c index 5359cbc1172..55e3ddbafda 100644 --- a/crypto/asn1/asn1_lib.c +++ b/crypto/asn1/asn1_lib.c @@ -248,6 +248,12 @@ int ASN1_object_size(int constructed, int length, int tag) return ret + length; } +void ossl_asn1_string_set_bits_left(ASN1_STRING *str, unsigned int num) +{ + str->flags &= ~0x07; + str->flags |= ASN1_STRING_FLAG_BITS_LEFT | (num & 0x07); +} + int ASN1_STRING_copy(ASN1_STRING *dst, const ASN1_STRING *str) { if (str == NULL) diff --git a/crypto/asn1/asn1_local.h b/crypto/asn1/asn1_local.h index f73bd8fc6a3..10e9fcb7de4 100644 --- a/crypto/asn1/asn1_local.h +++ b/crypto/asn1/asn1_local.h @@ -9,6 +9,8 @@ /* Internal ASN1 structures and functions: not for application use */ +#include "crypto/asn1.h" + typedef const ASN1_VALUE const_ASN1_VALUE; SKM_DEFINE_STACK_OF(const_ASN1_VALUE, const ASN1_VALUE, ASN1_VALUE) diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c index 937b713c232..93b6116ef39 100644 --- a/crypto/cmp/cmp_protect.c +++ b/crypto/cmp/cmp_protect.c @@ -93,8 +93,7 @@ ASN1_BIT_STRING *ossl_cmp_calc_protection(const OSSL_CMP_CTX *ctx, if ((prot = ASN1_BIT_STRING_new()) == NULL) goto end; /* OpenSSL defaults all bit strings to be encoded as ASN.1 NamedBitList */ - prot->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - prot->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(prot, 0); if (!ASN1_BIT_STRING_set(prot, protection, sig_len)) { ASN1_BIT_STRING_free(prot); prot = NULL; diff --git a/crypto/cms/cms_dh.c b/crypto/cms/cms_dh.c index 31082894eb2..ea8b24528f8 100644 --- a/crypto/cms/cms_dh.c +++ b/crypto/cms/cms_dh.c @@ -13,6 +13,7 @@ #include #include #include "internal/sizes.h" +#include "crypto/asn1.h" #include "crypto/evp.h" #include "cms_local.h" @@ -234,8 +235,7 @@ static int dh_cms_encrypt(CMS_RecipientInfo *ri) if (penclen <= 0) goto err; ASN1_STRING_set0(pubkey, penc, penclen); - pubkey->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - pubkey->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(pubkey, 0); penc = NULL; (void)X509_ALGOR_set0(talg, OBJ_nid2obj(NID_dhpublicnumber), diff --git a/crypto/cms/cms_ec.c b/crypto/cms/cms_ec.c index e82115934e1..808b3bf1ae2 100644 --- a/crypto/cms/cms_ec.c +++ b/crypto/cms/cms_ec.c @@ -12,6 +12,7 @@ #include #include #include "internal/sizes.h" +#include "crypto/asn1.h" #include "crypto/evp.h" #include "cms_local.h" @@ -277,8 +278,7 @@ static int ecdh_cms_encrypt(CMS_RecipientInfo *ri) penclen = EVP_PKEY_get1_encoded_public_key(pkey, &penc); ASN1_STRING_set0(pubkey, penc, penclen); - pubkey->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - pubkey->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(pubkey, 0); penc = NULL; (void)X509_ALGOR_set0(talg, OBJ_nid2obj(NID_X9_62_id_ecPublicKey), diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c index e1b6f88d449..3d9fc197e94 100644 --- a/crypto/ec/ec_asn1.c +++ b/crypto/ec/ec_asn1.c @@ -19,6 +19,7 @@ #include #include #include "internal/nelem.h" +#include "crypto/asn1.h" #include "crypto/asn1_dsa.h" #ifndef FIPS_MODULE @@ -358,8 +359,7 @@ static int ec_asn1_group2curve(const EC_GROUP *group, X9_62_CURVE *curve) ERR_raise(ERR_LIB_EC, ERR_R_MALLOC_FAILURE); goto err; } - curve->seed->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - curve->seed->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(curve->seed, 0); if (!ASN1_BIT_STRING_set(curve->seed, group->seed, (int)group->seed_len)) { ERR_raise(ERR_LIB_EC, ERR_R_ASN1_LIB); @@ -1072,8 +1072,7 @@ int i2d_ECPrivateKey(const EC_KEY *a, unsigned char **out) goto err; } - priv_key->publicKey->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - priv_key->publicKey->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(priv_key->publicKey, 0); ASN1_STRING_set0(priv_key->publicKey, pub, publen); pub = NULL; } diff --git a/crypto/x509/v3_addr.c b/crypto/x509/v3_addr.c index c3ccecb2ebf..a490f76ed00 100644 --- a/crypto/x509/v3_addr.c +++ b/crypto/x509/v3_addr.c @@ -407,12 +407,10 @@ static int make_addressPrefix(IPAddressOrRange **result, goto err; if (!ASN1_BIT_STRING_set(aor->u.addressPrefix, addr, bytelen)) goto err; - aor->u.addressPrefix->flags &= ~7; - aor->u.addressPrefix->flags |= ASN1_STRING_FLAG_BITS_LEFT; if (bitlen > 0) { aor->u.addressPrefix->data[bytelen - 1] &= ~(0xFF >> bitlen); - aor->u.addressPrefix->flags |= 8 - bitlen; } + ossl_asn1_string_set_bits_left(aor->u.addressPrefix, 8 - bitlen); *result = aor; return 1; @@ -455,8 +453,7 @@ static int make_addressRange(IPAddressOrRange **result, for (i = length; i > 0 && min[i - 1] == 0x00; --i) ; if (!ASN1_BIT_STRING_set(aor->u.addressRange->min, min, i)) goto err; - aor->u.addressRange->min->flags &= ~7; - aor->u.addressRange->min->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(aor->u.addressRange->min, 0); if (i > 0) { unsigned char b = min[i - 1]; int j = 1; @@ -468,8 +465,7 @@ static int make_addressRange(IPAddressOrRange **result, for (i = length; i > 0 && max[i - 1] == 0xFF; --i) ; if (!ASN1_BIT_STRING_set(aor->u.addressRange->max, max, i)) goto err; - aor->u.addressRange->max->flags &= ~7; - aor->u.addressRange->max->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(aor->u.addressRange->max, 0); if (i > 0) { unsigned char b = max[i - 1]; int j = 1; diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c index 126c2400f6d..c8d76f882e5 100644 --- a/crypto/x509/x_pubkey.c +++ b/crypto/x509/x_pubkey.c @@ -981,9 +981,7 @@ void X509_PUBKEY_set0_public_key(X509_PUBKEY *pub, unsigned char *penc, int penclen) { ASN1_STRING_set0(pub->public_key, penc, penclen); - /* Set number of unused bits to zero */ - pub->public_key->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - pub->public_key->flags |= ASN1_STRING_FLAG_BITS_LEFT; + ossl_asn1_string_set_bits_left(pub->public_key, 0); } int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *aobj, diff --git a/include/crypto/asn1.h b/include/crypto/asn1.h index 26e48ef7175..2308cc0c03f 100644 --- a/include/crypto/asn1.h +++ b/include/crypto/asn1.h @@ -148,5 +148,6 @@ EVP_PKEY * ossl_d2i_PrivateKey_legacy(int keytype, EVP_PKEY **a, X509_ALGOR *ossl_X509_ALGOR_from_nid(int nid, int ptype, void *pval); time_t asn1_string_to_time_t(const char *asn1_string); +void ossl_asn1_string_set_bits_left(ASN1_STRING *str, unsigned int num); #endif /* ndef OSSL_CRYPTO_ASN1_H */ diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h index d821ef2fdda..71b6b125f38 100644 --- a/include/internal/cryptlib.h +++ b/include/internal/cryptlib.h @@ -19,6 +19,7 @@ # endif # include "internal/common.h" +# include "crypto/asn1.h" # include # include diff --git a/include/openssl/asn1.h.in b/include/openssl/asn1.h.in index 550e0c2c25d..a6001d2b03b 100644 --- a/include/openssl/asn1.h.in +++ b/include/openssl/asn1.h.in @@ -135,7 +135,7 @@ extern "C" { -} -# define ASN1_STRING_FLAG_BITS_LEFT 0x08/* Set if 0x07 has bits left value */ +# define ASN1_STRING_FLAG_BITS_LEFT 0x08 /* Set if 0x07 has bits left value */ /* * This indicates that the ASN1_STRING is not a real value but just a place * holder for the location where indefinite length constructed data should be -- 2.47.2