From afc64c240f2a99a6cd30a0384b5296ad9f0f7597 Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Sat, 15 Feb 2025 15:57:48 +1100 Subject: [PATCH] Address non-FP coverity nits Reviewed-by: Neil Horman Reviewed-by: Tomas Mraz Reviewed-by: Tim Hudson (Merged from https://github.com/openssl/openssl/pull/26765) --- apps/dgst.c | 2 +- crypto/ml_kem/ml_kem.c | 14 ++++++++------ providers/implementations/keymgmt/ml_kem_kmgmt.c | 3 ++- providers/implementations/keymgmt/mlx_kmgmt.c | 3 ++- ssl/t1_lib.c | 7 +++---- test/ml_dsa_test.c | 4 ++-- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/apps/dgst.c b/apps/dgst.c index 7992cab796..5ee85ecce7 100644 --- a/apps/dgst.c +++ b/apps/dgst.c @@ -711,7 +711,7 @@ static int do_fp_oneshot_sign(BIO *out, EVP_MD_CTX *ctx, BIO *in, int sep, int b buflen = bio_to_mem(&buf, maxlen, in); if (buflen <= 0) { BIO_printf(bio_err, "Read error in %s\n", file); - goto end; + return ret; } if (sigin != NULL) { res = EVP_DigestVerify(ctx, sigin, siglen, buf, buflen); diff --git a/crypto/ml_kem/ml_kem.c b/crypto/ml_kem/ml_kem.c index 4b9d3b6a3a..a37951f428 100644 --- a/crypto/ml_kem/ml_kem.c +++ b/crypto/ml_kem/ml_kem.c @@ -1821,8 +1821,7 @@ int ossl_ml_kem_genkey(uint8_t *pubenc, size_t publen, ML_KEM_KEY *key) return 0; vinfo = key->vinfo; - if ((pubenc != NULL && publen != vinfo->pubkey_bytes) - || (mdctx = EVP_MD_CTX_new()) == NULL) + if (pubenc != NULL && publen != vinfo->pubkey_bytes) return 0; if (ossl_ml_kem_have_seed(key)) { @@ -1834,6 +1833,9 @@ int ossl_ml_kem_genkey(uint8_t *pubenc, size_t publen, ML_KEM_KEY *key) return 0; } + if ((mdctx = EVP_MD_CTX_new()) == NULL) + return 0; + /* * Data derived from (d, z) defaults secret, and to avoid side-channel * leaks should not influence control flow. @@ -1872,14 +1874,14 @@ int ossl_ml_kem_encap_seed(uint8_t *ctext, size_t clen, EVP_MD_CTX *mdctx; int ret = 0; - if (!ossl_ml_kem_have_pubkey(key)) + if (key == NULL || !ossl_ml_kem_have_pubkey(key)) return 0; vinfo = key->vinfo; if (ctext == NULL || clen != vinfo->ctext_bytes || shared_secret == NULL || slen != ML_KEM_SHARED_SECRET_BYTES || entropy == NULL || elen != ML_KEM_RANDOM_BYTES - || key == NULL || (mdctx = EVP_MD_CTX_new()) == NULL) + || (mdctx = EVP_MD_CTX_new()) == NULL) return 0; /* * Data derived from the encap entropy defaults secret, and to avoid @@ -1953,8 +1955,8 @@ int ossl_ml_kem_decap(uint8_t *shared_secret, size_t slen, if (shared_secret == NULL || slen != ML_KEM_SHARED_SECRET_BYTES || ctext == NULL || clen != vinfo->ctext_bytes || (mdctx = EVP_MD_CTX_new()) == NULL) { - RAND_bytes_ex(key->libctx, shared_secret, - ML_KEM_SHARED_SECRET_BYTES, vinfo->secbits); + (void)RAND_bytes_ex(key->libctx, shared_secret, + ML_KEM_SHARED_SECRET_BYTES, vinfo->secbits); return 0; } #if defined(OPENSSL_CONSTANT_TIME_VALIDATION) diff --git a/providers/implementations/keymgmt/ml_kem_kmgmt.c b/providers/implementations/keymgmt/ml_kem_kmgmt.c index 2853698cc2..cd11659eff 100644 --- a/providers/implementations/keymgmt/ml_kem_kmgmt.c +++ b/providers/implementations/keymgmt/ml_kem_kmgmt.c @@ -690,13 +690,14 @@ static void *ml_kem_gen(void *vgctx, OSSL_CALLBACK *osslcb, void *cbarg) PROV_ML_KEM_GEN_CTX *gctx = vgctx; ML_KEM_KEY *key; uint8_t *nopub = NULL; - uint8_t *seed = gctx->seed; + uint8_t *seed; int genok = 0; if (gctx == NULL || (gctx->selection & OSSL_KEYMGMT_SELECT_KEYPAIR) == OSSL_KEYMGMT_SELECT_PUBLIC_KEY) return NULL; + seed = gctx->seed; key = ml_kem_new(gctx->provctx, gctx->propq, gctx->evp_type); if (key == NULL) return NULL; diff --git a/providers/implementations/keymgmt/mlx_kmgmt.c b/providers/implementations/keymgmt/mlx_kmgmt.c index 5b9fb61247..4bc8b7c1e2 100644 --- a/providers/implementations/keymgmt/mlx_kmgmt.c +++ b/providers/implementations/keymgmt/mlx_kmgmt.c @@ -687,7 +687,7 @@ static void *mlx_kem_gen(void *vgctx, OSSL_CALLBACK *osslcb, void *cbarg) { PROV_ML_KEM_GEN_CTX *gctx = vgctx; MLX_KEY *key; - char *propq = gctx->propq; + char *propq; if (gctx == NULL || (gctx->selection & OSSL_KEYMGMT_SELECT_KEYPAIR) == @@ -695,6 +695,7 @@ static void *mlx_kem_gen(void *vgctx, OSSL_CALLBACK *osslcb, void *cbarg) return NULL; /* Lose ownership of propq */ + propq = gctx->propq; gctx->propq = NULL; if ((key = mlx_kem_key_new(gctx->evp_type, gctx->libctx, propq)) == NULL) return NULL; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 3d8c5fde76..40abf27d40 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -999,11 +999,10 @@ int tls1_get0_implemented_groups(int min_proto_version, int max_proto_version, int ret = 0; size_t ix; - if ((collect = sk_TLS_GROUP_IX_new(tls_group_ix_cmp)) == NULL) - return 0; - if (grps == NULL || out == NULL) return 0; + if ((collect = sk_TLS_GROUP_IX_new(tls_group_ix_cmp)) == NULL) + return 0; for (ix = 0; ix < num; ++ix, ++grps) { if (grps->mintls > 0 && max_proto_version > 0 && grps->mintls > max_proto_version) @@ -1030,7 +1029,7 @@ int tls1_get0_implemented_groups(int min_proto_version, int max_proto_version, if (sk_OPENSSL_CSTRING_push(out, gix->grp->tlsname) <= 0) goto end; } - return 1; + ret = 1; end: sk_TLS_GROUP_IX_pop_free(collect, free_wrapper); diff --git a/test/ml_dsa_test.c b/test/ml_dsa_test.c index 826bb46e39..2ff835f777 100644 --- a/test/ml_dsa_test.c +++ b/test/ml_dsa_test.c @@ -43,8 +43,8 @@ static EVP_PKEY *do_gen_key(const char *alg, || !TEST_int_eq(EVP_PKEY_keygen_init(ctx), 1) || !TEST_int_eq(EVP_PKEY_CTX_set_params(ctx, params), 1) || !TEST_int_eq(EVP_PKEY_generate(ctx, &pkey), 1)) - goto err; -err: + pkey = NULL; + EVP_PKEY_CTX_free(ctx); return pkey; }