From e0137ca92b4abf65acde15b255ae58d7e76af22f Mon Sep 17 00:00:00 2001 From: Nicola Tuveri Date: Mon, 29 Jun 2020 00:53:46 +0300 Subject: [PATCH] [EC][ASN1] Detect missing OID when serializing EC parameters and keys The following built-in curves do not have an assigned OID: - Oakley-EC2N-3 - Oakley-EC2N-4 In general we shouldn't assume that an OID is always available. This commit detects such cases, raises an error and returns appropriate return values so that the condition can be detected and correctly handled by the callers, when serializing EC parameters or EC keys with the default `ec_param_enc:named_curve`. Fixes #12306 Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/12313) --- crypto/ec/ec_ameth.c | 9 ++++++++- crypto/ec/ec_asn1.c | 11 +++++++++-- crypto/ec/ec_err.c | 1 + crypto/err/openssl.txt | 2 ++ crypto/pem/pem_lib.c | 2 +- include/openssl/ecerr.h | 1 + providers/common/include/prov/providercommonerr.h | 1 + providers/common/provider_err.c | 1 + providers/implementations/serializers/serializer_ec.c | 8 ++++++++ 9 files changed, 32 insertions(+), 4 deletions(-) diff --git a/crypto/ec/ec_ameth.c b/crypto/ec/ec_ameth.c index 761f697850..8a33b3232c 100644 --- a/crypto/ec/ec_ameth.c +++ b/crypto/ec/ec_ameth.c @@ -43,7 +43,14 @@ static int eckey_param2type(int *pptype, void **ppval, const EC_KEY *ec_key) && (nid = EC_GROUP_get_curve_name(group))) /* we have a 'named curve' => just set the OID */ { - *ppval = OBJ_nid2obj(nid); + ASN1_OBJECT *asn1obj = OBJ_nid2obj(nid); + + if (asn1obj == NULL || OBJ_length(asn1obj) == 0) { + ASN1_OBJECT_free(asn1obj); + ECerr(EC_F_ECKEY_PARAM2TYPE, EC_R_MISSING_OID); + return 0; + } + *ppval = asn1obj; *pptype = V_ASN1_OBJECT; } else { /* explicit parameters */ diff --git a/crypto/ec/ec_asn1.c b/crypto/ec/ec_asn1.c index a53573cc92..654a12ad60 100644 --- a/crypto/ec/ec_asn1.c +++ b/crypto/ec/ec_asn1.c @@ -553,9 +553,16 @@ ECPKPARAMETERS *EC_GROUP_get_ecpkparameters(const EC_GROUP *group, */ tmp = EC_GROUP_get_curve_name(group); if (tmp) { - ret->type = 0; - if ((ret->value.named_curve = OBJ_nid2obj(tmp)) == NULL) + ASN1_OBJECT *asn1obj = OBJ_nid2obj(tmp); + + if (asn1obj == NULL || OBJ_length(asn1obj) == 0) { + ASN1_OBJECT_free(asn1obj); + ECerr(EC_F_EC_GROUP_GET_ECPKPARAMETERS, EC_R_MISSING_OID); ok = 0; + } else { + ret->type = 0; + ret->value.named_curve = asn1obj; + } } else /* we don't know the nid => ERROR */ ok = 0; diff --git a/crypto/ec/ec_err.c b/crypto/ec/ec_err.c index d775ced93a..afb2696285 100644 --- a/crypto/ec/ec_err.c +++ b/crypto/ec/ec_err.c @@ -70,6 +70,7 @@ static const ERR_STRING_DATA EC_str_reasons[] = { {ERR_PACK(ERR_LIB_EC, 0, EC_R_LADDER_POST_FAILURE), "ladder post failure"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_LADDER_PRE_FAILURE), "ladder pre failure"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_LADDER_STEP_FAILURE), "ladder step failure"}, + {ERR_PACK(ERR_LIB_EC, 0, EC_R_MISSING_OID), "missing OID"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_MISSING_PARAMETERS), "missing parameters"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_MISSING_PRIVATE_KEY), "missing private key"}, {ERR_PACK(ERR_LIB_EC, 0, EC_R_NEED_NEW_SETUP_VALUES), diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index bc39b37cd0..579c2dce9a 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -2439,6 +2439,7 @@ EC_R_KEYS_NOT_SET:140:keys not set EC_R_LADDER_POST_FAILURE:136:ladder post failure EC_R_LADDER_PRE_FAILURE:153:ladder pre failure EC_R_LADDER_STEP_FAILURE:162:ladder step failure +EC_R_MISSING_OID:167:missing OID EC_R_MISSING_PARAMETERS:124:missing parameters EC_R_MISSING_PRIVATE_KEY:125:missing private key EC_R_NEED_NEW_SETUP_VALUES:157:need new setup values @@ -2886,6 +2887,7 @@ PROV_R_MISSING_CONSTANT:156:missing constant PROV_R_MISSING_KEY:128:missing key PROV_R_MISSING_MAC:150:missing mac PROV_R_MISSING_MESSAGE_DIGEST:129:missing message digest +PROV_R_MISSING_OID:209:missing OID PROV_R_MISSING_PASS:130:missing pass PROV_R_MISSING_SALT:131:missing salt PROV_R_MISSING_SECRET:132:missing secret diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c index c170f60bcd..bd20bbb783 100644 --- a/crypto/pem/pem_lib.c +++ b/crypto/pem/pem_lib.c @@ -334,7 +334,7 @@ int PEM_ASN1_write_bio(i2d_of_void *i2d, const char *name, BIO *bp, } } - if ((dsize = i2d(x, NULL)) < 0) { + if ((dsize = i2d(x, NULL)) <= 0) { PEMerr(PEM_F_PEM_ASN1_WRITE_BIO, ERR_R_ASN1_LIB); dsize = 0; goto err; diff --git a/include/openssl/ecerr.h b/include/openssl/ecerr.h index 033c94d9a9..b12e222510 100644 --- a/include/openssl/ecerr.h +++ b/include/openssl/ecerr.h @@ -264,6 +264,7 @@ int ERR_load_EC_strings(void); # define EC_R_LADDER_POST_FAILURE 136 # define EC_R_LADDER_PRE_FAILURE 153 # define EC_R_LADDER_STEP_FAILURE 162 +# define EC_R_MISSING_OID 167 # define EC_R_MISSING_PARAMETERS 124 # define EC_R_MISSING_PRIVATE_KEY 125 # define EC_R_NEED_NEW_SETUP_VALUES 157 diff --git a/providers/common/include/prov/providercommonerr.h b/providers/common/include/prov/providercommonerr.h index b7fd2c2bf4..c21537fd4f 100644 --- a/providers/common/include/prov/providercommonerr.h +++ b/providers/common/include/prov/providercommonerr.h @@ -113,6 +113,7 @@ int ERR_load_PROV_strings(void); # define PROV_R_MISSING_KEY 128 # define PROV_R_MISSING_MAC 150 # define PROV_R_MISSING_MESSAGE_DIGEST 129 +# define PROV_R_MISSING_OID 209 # define PROV_R_MISSING_PASS 130 # define PROV_R_MISSING_SALT 131 # define PROV_R_MISSING_SECRET 132 diff --git a/providers/common/provider_err.c b/providers/common/provider_err.c index 08978189b9..7a0e0c595d 100644 --- a/providers/common/provider_err.c +++ b/providers/common/provider_err.c @@ -112,6 +112,7 @@ static const ERR_STRING_DATA PROV_str_reasons[] = { {ERR_PACK(ERR_LIB_PROV, 0, PROV_R_MISSING_MAC), "missing mac"}, {ERR_PACK(ERR_LIB_PROV, 0, PROV_R_MISSING_MESSAGE_DIGEST), "missing message digest"}, + {ERR_PACK(ERR_LIB_PROV, 0, PROV_R_MISSING_OID), "missing OID"}, {ERR_PACK(ERR_LIB_PROV, 0, PROV_R_MISSING_PASS), "missing pass"}, {ERR_PACK(ERR_LIB_PROV, 0, PROV_R_MISSING_SALT), "missing salt"}, {ERR_PACK(ERR_LIB_PROV, 0, PROV_R_MISSING_SECRET), "missing secret"}, diff --git a/providers/implementations/serializers/serializer_ec.c b/providers/implementations/serializers/serializer_ec.c index 4d81651c5a..0dbc889d34 100644 --- a/providers/implementations/serializers/serializer_ec.c +++ b/providers/implementations/serializers/serializer_ec.c @@ -11,6 +11,7 @@ #include "crypto/ec.h" #include "prov/bio.h" /* ossl_prov_bio_printf() */ #include "prov/implementations.h" /* ec_keymgmt_functions */ +#include "prov/providercommonerr.h" /* PROV_R_MISSING_OID */ #include "serializer_local.h" void ec_get_new_free_import(OSSL_FUNC_keymgmt_new_fn **ec_new, @@ -117,6 +118,13 @@ int ossl_prov_prepare_ec_params(const void *eckey, int nid, return 0; } + if (OBJ_length(params) == 0) { + /* Some curves might not have an associated OID */ + ERR_raise(ERR_LIB_PROV, PROV_R_MISSING_OID); + ASN1_OBJECT_free(params); + return 0; + } + *pstr = params; *pstrtype = V_ASN1_OBJECT; return 1; -- 2.39.2