[openssl] master update
shane.lontis at oracle.com
shane.lontis at oracle.com
Thu Jul 9 03:45:10 UTC 2020
The branch master has been updated
via 63794b048cbe46ac9abb883df4dd703f522e4643 (commit)
from eae4a008341149783b540198470f04f85b22730e (commit)
- Log -----------------------------------------------------------------
commit 63794b048cbe46ac9abb883df4dd703f522e4643
Author: Shane Lontis <shane.lontis at oracle.com>
Date: Thu Jul 9 13:43:10 2020 +1000
Add multiple fixes for ffc key generation using invalid p,q,g parameters.
Fixes #11864
- The dsa keygen assumed valid p, q, g values were being passed. If this is not correct then it is
possible that dsa keygen can either hang or segfault.
The fix was to do a partial validation of p, q, and g inside the keygen.
- Fixed a potential double free in the dsa keypair test in the case when in failed (It should never fail!).
It freed internal object members without setting them to NULL.
- Changed the FFC key validation to accept 1024 bit keys in non fips mode.
- Added tests that use both the default provider & fips provider to test these cases.
Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
(Merged from https://github.com/openssl/openssl/pull/12176)
-----------------------------------------------------------------------
Summary of changes:
crypto/dh/dh_key.c | 4 +
crypto/dsa/dsa_key.c | 7 ++
crypto/ffc/ffc_params_generate.c | 11 +-
crypto/ffc/ffc_params_validate.c | 26 ++++
include/internal/ffc.h | 1 +
test/build.info | 6 +-
test/evp_libctx_test.c | 253 ++++++++++++++++++++++++++++++++++++++
test/ffc_internal_test.c | 7 --
test/recipes/30-test_evp_libctx.t | 46 +++++++
9 files changed, 352 insertions(+), 9 deletions(-)
create mode 100644 test/evp_libctx_test.c
create mode 100644 test/recipes/30-test_evp_libctx.t
diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c
index 5d2acca25c..3b4da19cd2 100644
--- a/crypto/dh/dh_key.c
+++ b/crypto/dh/dh_key.c
@@ -287,6 +287,10 @@ static int generate_key(DH *dh)
} else
#endif
{
+ /* Do a partial check for invalid p, q, g */
+ if (!ffc_params_simple_validate(dh->libctx, &dh->params,
+ FFC_PARAM_TYPE_DH))
+ goto err;
/*
* For FFC FIPS 186-4 keygen
* security strength s = 112,
diff --git a/crypto/dsa/dsa_key.c b/crypto/dsa/dsa_key.c
index 7bd9c5ff2e..b537ec0b3c 100644
--- a/crypto/dsa/dsa_key.c
+++ b/crypto/dsa/dsa_key.c
@@ -74,6 +74,11 @@ static int dsa_keygen(DSA *dsa, int pairwise_test)
priv_key = dsa->priv_key;
}
+ /* Do a partial check for invalid p, q, g */
+ if (!ffc_params_simple_validate(dsa->libctx, &dsa->params,
+ FFC_PARAM_TYPE_DSA))
+ goto err;
+
/*
* For FFC FIPS 186-4 keygen
* security strength s = 112,
@@ -110,6 +115,8 @@ static int dsa_keygen(DSA *dsa, int pairwise_test)
if (!ok) {
BN_free(dsa->pub_key);
BN_clear_free(dsa->priv_key);
+ dsa->pub_key = NULL;
+ dsa->priv_key = NULL;
BN_CTX_free(ctx);
return ok;
}
diff --git a/crypto/ffc/ffc_params_generate.c b/crypto/ffc/ffc_params_generate.c
index 325eb6768f..8a0b77e7f8 100644
--- a/crypto/ffc/ffc_params_generate.c
+++ b/crypto/ffc/ffc_params_generate.c
@@ -39,6 +39,11 @@
*/
static int ffc_validate_LN(size_t L, size_t N, int type)
{
+#ifndef FIPS_MODULE
+ if (L == 1024 && N == 160)
+ return 80;
+#endif
+
if (type == FFC_PARAM_TYPE_DH) {
/* Valid DH L,N parameters from SP800-56Ar3 5.5.1 Table 1 */
if (L == 2048 && (N == 224 || N == 256))
@@ -498,6 +503,7 @@ int ffc_params_FIPS186_4_gen_verify(OPENSSL_CTX *libctx, FFC_PARAMS *params,
EVP_MD *md = NULL;
int verify = (mode == FFC_PARAM_MODE_VERIFY);
unsigned int flags = verify ? params->flags : 0;
+ const char *def_name;
*res = 0;
@@ -506,7 +512,10 @@ int ffc_params_FIPS186_4_gen_verify(OPENSSL_CTX *libctx, FFC_PARAMS *params,
} else {
if (N == 0)
N = (L >= 2048 ? SHA256_DIGEST_LENGTH : SHA_DIGEST_LENGTH) * 8;
- md = EVP_MD_fetch(libctx, default_mdname(N), NULL);
+ def_name = default_mdname(N);
+ if (def_name == NULL)
+ goto err;
+ md = EVP_MD_fetch(libctx, def_name, NULL);
}
if (md == NULL)
goto err;
diff --git a/crypto/ffc/ffc_params_validate.c b/crypto/ffc/ffc_params_validate.c
index f3df0c2b39..821ff3e88a 100644
--- a/crypto/ffc/ffc_params_validate.c
+++ b/crypto/ffc/ffc_params_validate.c
@@ -78,3 +78,29 @@ int ffc_params_FIPS186_2_validate(OPENSSL_CTX *libctx, const FFC_PARAMS *params,
FFC_PARAM_MODE_VERIFY, type,
L, N, res, cb);
}
+
+/*
+ * This does a simple check of L and N and partial g.
+ * It makes no attempt to do a full validation of p, q or g since these require
+ * extra parameters such as the digest and seed, which may not be available for
+ * this test.
+ */
+int ffc_params_simple_validate(OPENSSL_CTX *libctx, FFC_PARAMS *params, int type)
+{
+ int ret, res = 0;
+ int save_gindex;
+ unsigned int save_flags;
+
+ if (params == NULL)
+ return 0;
+
+ save_flags = params->flags;
+ save_gindex = params->gindex;
+ params->flags = FFC_PARAM_FLAG_VALIDATE_G;
+ params->gindex = FFC_UNVERIFIABLE_GINDEX;
+
+ ret = ffc_params_FIPS186_4_validate(libctx, params, type, &res, NULL);
+ params->flags = save_flags;
+ params->gindex = save_gindex;
+ return ret != FFC_PARAM_RET_STATUS_FAILED;
+}
diff --git a/include/internal/ffc.h b/include/internal/ffc.h
index 2ed5d72c5c..b352b8d345 100644
--- a/include/internal/ffc.h
+++ b/include/internal/ffc.h
@@ -155,6 +155,7 @@ int ffc_params_FIPS186_2_gen_verify(OPENSSL_CTX *libctx, FFC_PARAMS *params,
int mode, int type, size_t L, size_t N,
int *res, BN_GENCB *cb);
+int ffc_params_simple_validate(OPENSSL_CTX *libctx, FFC_PARAMS *params, int type);
int ffc_params_FIPS186_4_validate(OPENSSL_CTX *libctx, const FFC_PARAMS *params,
int type, int *res, BN_GENCB *cb);
int ffc_params_FIPS186_2_validate(OPENSSL_CTX *libctx, const FFC_PARAMS *params,
diff --git a/test/build.info b/test/build.info
index 88b35d4d3c..ed547d1488 100644
--- a/test/build.info
+++ b/test/build.info
@@ -36,7 +36,7 @@ IF[{- !$disabled{tests} -}]
destest mdc2test \
enginetest exptest \
evp_pkey_provided_test evp_test evp_extra_test evp_extra_test2 \
- evp_fetch_prov_test acvp_test \
+ evp_fetch_prov_test acvp_test evp_libctx_test \
v3nametest v3ext \
evp_pkey_provided_test evp_test evp_extra_test evp_extra_test2 \
evp_fetch_prov_test v3nametest v3ext \
@@ -141,6 +141,10 @@ IF[{- !$disabled{tests} -}]
INCLUDE[evp_extra_test2]=../include ../apps/include
DEPEND[evp_extra_test2]=../libcrypto libtestutil.a
+ SOURCE[evp_libctx_test]=evp_libctx_test.c
+ INCLUDE[evp_libctx_test]=../include ../apps/include
+ DEPEND[evp_libctx_test]=../libcrypto.a libtestutil.a
+
SOURCE[evp_fetch_prov_test]=evp_fetch_prov_test.c
INCLUDE[evp_fetch_prov_test]=../include ../apps/include
DEPEND[evp_fetch_prov_test]=../libcrypto libtestutil.a
diff --git a/test/evp_libctx_test.c b/test/evp_libctx_test.c
new file mode 100644
index 0000000000..77054f93a2
--- /dev/null
+++ b/test/evp_libctx_test.c
@@ -0,0 +1,253 @@
+/*
+ * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License"). You may not use
+ * this file except in compliance with the License. You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+/*
+
+ * These tests are setup to load null into the default library context.
+ * Any tests are expected to use the created 'libctx' to find algorithms.
+ * The framework runs the tests twice using the 'default' provider or
+ * 'fips' provider as inputs.
+ */
+
+/*
+ * DSA/DH low level APIs are deprecated for public use, but still ok for
+ * internal use.
+ */
+#include "internal/deprecated.h"
+#include <openssl/evp.h>
+#include <openssl/provider.h>
+#include <openssl/dsa.h>
+#include "testutil.h"
+#include "internal/nelem.h"
+#include "crypto/bn_dh.h" /* _bignum_ffdhe2048_p */
+
+static OPENSSL_CTX *libctx = NULL;
+static OSSL_PROVIDER *nullprov = NULL;
+static OSSL_PROVIDER *libprov = NULL;
+
+typedef enum OPTION_choice {
+ OPT_ERR = -1,
+ OPT_EOF = 0,
+ OPT_CONFIG_FILE,
+ OPT_PROVIDER_NAME,
+ OPT_TEST_ENUM
+} OPTION_CHOICE;
+
+const OPTIONS *test_get_options(void)
+{
+ static const OPTIONS test_options[] = {
+ OPT_TEST_OPTIONS_DEFAULT_USAGE,
+ { "config", OPT_CONFIG_FILE, '<',
+ "The configuration file to use for the libctx" },
+ { "provider", OPT_PROVIDER_NAME, 's',
+ "The provider to load (The default value is 'default'" },
+ { NULL }
+ };
+ return test_options;
+}
+
+#if !defined(OPENSSL_NO_DSA) || !defined(OPENSSL_NO_DH)
+static const char *getname(int id)
+{
+ const char *name[] = {"p", "q", "g" };
+
+ if (id >= 0 && id < 3)
+ return name[id];
+ return "?";
+}
+#endif
+
+#ifndef OPENSSL_NO_DSA
+
+static int test_dsa_param_keygen(int tstid)
+{
+ int ret = 0;
+ int expected;
+ EVP_PKEY_CTX *gen_ctx = NULL;
+ EVP_PKEY *pkey_parm = NULL;
+ EVP_PKEY *pkey = NULL;
+ DSA *dsa = NULL;
+ int pind, qind, gind;
+ BIGNUM *p = NULL, *q = NULL, *g = NULL;
+
+ /*
+ * Just grab some fixed dh p, q, g values for testing,
+ * these 'safe primes' should not be used normally for dsa *.
+ */
+ static const BIGNUM *bn[] = {
+ &_bignum_dh2048_256_p, &_bignum_dh2048_256_q, &_bignum_dh2048_256_g
+ };
+
+ /*
+ * These tests are using bad values for p, q, g by reusing the values.
+ * A value of 0 uses p, 1 uses q and 2 uses g.
+ * There are 27 different combinations, with only the 1 valid combination.
+ */
+ pind = tstid / 9;
+ qind = (tstid / 3) % 3;
+ gind = tstid % 3;
+ expected = (pind == 0 && qind == 1 && gind == 2);
+
+ TEST_note("Testing with (p, q, g) = (%s, %s, %s)\n", getname(pind),
+ getname(qind), getname(gind));
+
+ if (!TEST_ptr(pkey_parm = EVP_PKEY_new())
+ || !TEST_ptr(dsa = DSA_new())
+ || !TEST_ptr(p = BN_dup(bn[pind]))
+ || !TEST_ptr(q = BN_dup(bn[qind]))
+ || !TEST_ptr(g = BN_dup(bn[gind]))
+ || !TEST_true(DSA_set0_pqg(dsa, p, q, g)))
+ goto err;
+ p = q = g = NULL;
+
+ if (!TEST_true(EVP_PKEY_assign_DSA(pkey_parm, dsa)))
+ goto err;
+ dsa = NULL;
+
+ if (!TEST_ptr(gen_ctx = EVP_PKEY_CTX_new_from_pkey(libctx, pkey_parm, NULL))
+ || !TEST_int_gt(EVP_PKEY_keygen_init(gen_ctx), 0)
+ || !TEST_int_eq(EVP_PKEY_keygen(gen_ctx, &pkey), expected))
+ goto err;
+ ret = 1;
+err:
+ EVP_PKEY_free(pkey);
+ EVP_PKEY_CTX_free(gen_ctx);
+ EVP_PKEY_free(pkey_parm);
+ DSA_free(dsa);
+ BN_free(g);
+ BN_free(q);
+ BN_free(p);
+ return ret;
+}
+#endif /* OPENSSL_NO_DSA */
+
+#ifndef OPENSSL_NO_DH
+static int do_dh_param_keygen(int tstid, const BIGNUM **bn)
+{
+ int ret = 0;
+ int expected;
+ EVP_PKEY_CTX *gen_ctx = NULL;
+ EVP_PKEY *pkey_parm = NULL;
+ EVP_PKEY *pkey = NULL;
+ DH *dh = NULL;
+ int pind, qind, gind;
+ BIGNUM *p = NULL, *q = NULL, *g = NULL;
+
+ /*
+ * These tests are using bad values for p, q, g by reusing the values.
+ * A value of 0 uses p, 1 uses q and 2 uses g.
+ * There are 27 different combinations, with only the 1 valid combination.
+ */
+ pind = tstid / 9;
+ qind = (tstid / 3) % 3;
+ gind = tstid % 3;
+ expected = (pind == 0 && qind == 1 && gind == 2);
+
+ TEST_note("Testing with (p, q, g) = (%s, %s, %s)", getname(pind),
+ getname(qind), getname(gind));
+
+ if (!TEST_ptr(pkey_parm = EVP_PKEY_new())
+ || !TEST_ptr(dh = DH_new())
+ || !TEST_ptr(p = BN_dup(bn[pind]))
+ || !TEST_ptr(q = BN_dup(bn[qind]))
+ || !TEST_ptr(g = BN_dup(bn[gind]))
+ || !TEST_true(DH_set0_pqg(dh, p, q, g)))
+ goto err;
+ p = q = g = NULL;
+
+ if (!TEST_true(EVP_PKEY_assign_DH(pkey_parm, dh)))
+ goto err;
+ dh = NULL;
+
+ if (!TEST_ptr(gen_ctx = EVP_PKEY_CTX_new_from_pkey(libctx, pkey_parm, NULL))
+ || !TEST_int_gt(EVP_PKEY_keygen_init(gen_ctx), 0)
+ || !TEST_int_eq(EVP_PKEY_keygen(gen_ctx, &pkey), expected))
+ goto err;
+ ret = 1;
+err:
+ EVP_PKEY_free(pkey);
+ EVP_PKEY_CTX_free(gen_ctx);
+ EVP_PKEY_free(pkey_parm);
+ DH_free(dh);
+ BN_free(g);
+ BN_free(q);
+ BN_free(p);
+ return ret;
+}
+
+/*
+ * Note that we get the fips186-4 path being run for most of these cases since
+ * the internal code will detect that the p, q, g does not match a safe prime
+ * group (Except for when tstid = 5, which sets the correct p, q, g)
+ */
+static int test_dh_safeprime_param_keygen(int tstid)
+{
+ static const BIGNUM *bn[] = {
+ &_bignum_ffdhe2048_p, &_bignum_ffdhe2048_q, &_bignum_const_2
+ };
+ return do_dh_param_keygen(tstid, bn);
+}
+
+#endif /* OPENSSL_NO_DH */
+
+int setup_tests(void)
+{
+ const char *prov_name = "default";
+ char *config_file = NULL;
+ OPTION_CHOICE o;
+
+ while ((o = opt_next()) != OPT_EOF) {
+ switch (o) {
+ case OPT_PROVIDER_NAME:
+ prov_name = opt_arg();
+ break;
+ case OPT_CONFIG_FILE:
+ config_file = opt_arg();
+ break;
+ case OPT_TEST_CASES:
+ break;
+ default:
+ case OPT_ERR:
+ return 0;
+ }
+ }
+
+ nullprov = OSSL_PROVIDER_load(NULL, "null");
+ if (!TEST_ptr(nullprov))
+ return 0;
+
+ libctx = OPENSSL_CTX_new();
+
+ if (!TEST_ptr(libctx))
+ return 0;
+
+ if (config_file != NULL) {
+ if (!TEST_true(OPENSSL_CTX_load_config(libctx, config_file)))
+ return 0;
+ }
+
+ libprov = OSSL_PROVIDER_load(libctx, prov_name);
+ if (!TEST_ptr(libprov))
+ return 0;
+
+#ifndef OPENSSL_NO_DSA
+ ADD_ALL_TESTS(test_dsa_param_keygen, 3 * 3 * 3);
+#endif
+#ifndef OPENSSL_NO_DH
+ ADD_ALL_TESTS(test_dh_safeprime_param_keygen, 3 * 3 * 3);
+#endif
+ return 1;
+}
+
+void cleanup_tests(void)
+{
+ OSSL_PROVIDER_unload(libprov);
+ OPENSSL_CTX_free(libctx);
+ OSSL_PROVIDER_unload(nullprov);
+}
diff --git a/test/ffc_internal_test.c b/test/ffc_internal_test.c
index 632cead926..1acc342f6e 100644
--- a/test/ffc_internal_test.c
+++ b/test/ffc_internal_test.c
@@ -399,13 +399,6 @@ static int ffc_params_fips186_2_gen_validate_test(void)
FFC_PARAM_TYPE_DH,
&res, NULL)))
goto err;
- /* FIPS 186-4 L,N pair test will fail for DH */
- if (!TEST_false(ffc_params_FIPS186_4_validate(NULL, ¶ms,
- FFC_PARAM_TYPE_DH,
- &res, NULL)))
- goto err;
- if (!TEST_int_eq(res, FFC_CHECK_BAD_LN_PAIR))
- goto err;
/*
* The fips186-2 generation should produce a different q compared to
diff --git a/test/recipes/30-test_evp_libctx.t b/test/recipes/30-test_evp_libctx.t
new file mode 100644
index 0000000000..8fcc71a1cd
--- /dev/null
+++ b/test/recipes/30-test_evp_libctx.t
@@ -0,0 +1,46 @@
+#! /usr/bin/env perl
+# Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the Apache License 2.0 (the "License"). You may not use
+# this file except in compliance with the License. You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+use strict;
+use warnings;
+
+use OpenSSL::Test qw(:DEFAULT bldtop_dir srctop_dir srctop_file bldtop_file);
+use OpenSSL::Test::Utils;
+
+BEGIN {
+ setup("test_evp_libctx");
+}
+
+my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0);
+
+use lib srctop_dir('Configurations');
+use lib bldtop_dir('.');
+use platform;
+
+my $infile = bldtop_file('providers', platform->dso('fips'));
+# If no fips then run the test with no extra arguments.
+my @test_args = ( );
+
+plan tests =>
+ ($no_fips ? 0 : 1) # FIPS install test
+ + 1;
+
+unless ($no_fips) {
+ @test_args = ("-config", srctop_file("test","fips.cnf"),
+ "-provider", "fips");
+
+ ok(run(app(['openssl', 'fipsinstall',
+ '-out', bldtop_file('providers', 'fipsmodule.cnf'),
+ '-module', $infile,
+ '-provider_name', 'fips', '-mac_name', 'HMAC',
+ '-macopt', 'digest:SHA256', '-macopt', 'hexkey:00',
+ '-section_name', 'fips_sect'])),
+ "fipsinstall");
+}
+
+ok(run(test(["evp_libctx_test", @test_args])), "running evp_libctx_test");
More information about the openssl-commits
mailing list