[openssl] master update
bernd.edlinger at hotmail.de
bernd.edlinger at hotmail.de
Tue Nov 23 05:09:21 UTC 2021
The branch master has been updated
via 2595eef82c2b67ea75cc3368529078b643a1ecb6 (commit)
via e2571e02d2b0cd83ed1c79d384fe941f27e603c0 (commit)
from 4599ea9fe31953c0c50738ed4b91ade76a693356 (commit)
- Log -----------------------------------------------------------------
commit 2595eef82c2b67ea75cc3368529078b643a1ecb6
Author: Bernd Edlinger <bernd.edlinger at hotmail.de>
Date: Fri Nov 19 16:38:55 2021 +0100
Add a test case for duplicate engine loading
Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
(Merged from https://github.com/openssl/openssl/pull/17073)
commit e2571e02d2b0cd83ed1c79d384fe941f27e603c0
Author: Bernd Edlinger <bernd.edlinger at hotmail.de>
Date: Fri Nov 19 11:33:34 2021 +0100
Avoid loading of a dynamic engine twice
Use the address of the bind function as a DYNAMIC_ID,
since the true name of the engine is not known
before the bind function returns,
but invoking the bind function before the engine
is unloaded results in memory corruption.
Fixes #17023
Reviewed-by: Tomas Mraz <tomas at openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
(Merged from https://github.com/openssl/openssl/pull/17073)
-----------------------------------------------------------------------
Summary of changes:
crypto/engine/eng_dyn.c | 4 ++-
crypto/engine/eng_lib.c | 2 ++
crypto/engine/eng_list.c | 87 +++++++++++++++++++++++++++++++++++++++++++++
crypto/engine/eng_local.h | 9 +++++
test/recipes/20-test_dgst.t | 23 ++++++++++--
5 files changed, 122 insertions(+), 3 deletions(-)
diff --git a/crypto/engine/eng_dyn.c b/crypto/engine/eng_dyn.c
index f401063d37..c8a54f7d44 100644
--- a/crypto/engine/eng_dyn.c
+++ b/crypto/engine/eng_dyn.c
@@ -484,7 +484,9 @@ static int dynamic_load(ENGINE *e, dynamic_data_ctx *ctx)
engine_set_all_null(e);
/* Try to bind the ENGINE onto our own ENGINE structure */
- if (!ctx->bind_engine(e, ctx->engine_id, &fns)) {
+ if (!engine_add_dynamic_id(e, (ENGINE_DYNAMIC_ID)ctx->bind_engine, 1)
+ || !ctx->bind_engine(e, ctx->engine_id, &fns)) {
+ engine_remove_dynamic_id(e, 1);
ctx->bind_engine = NULL;
ctx->v_check = NULL;
DSO_free(ctx->dynamic_dso);
diff --git a/crypto/engine/eng_lib.c b/crypto/engine/eng_lib.c
index 44e997e77b..05c6a67c1e 100644
--- a/crypto/engine/eng_lib.c
+++ b/crypto/engine/eng_lib.c
@@ -65,6 +65,7 @@ void engine_set_all_null(ENGINE *e)
e->load_pubkey = NULL;
e->cmd_defns = NULL;
e->flags = 0;
+ e->dynamic_id = NULL;
}
int engine_free_util(ENGINE *e, int not_locked)
@@ -90,6 +91,7 @@ int engine_free_util(ENGINE *e, int not_locked)
*/
if (e->destroy)
e->destroy(e);
+ engine_remove_dynamic_id(e, not_locked);
CRYPTO_free_ex_data(CRYPTO_EX_INDEX_ENGINE, e, &e->ex_data);
OPENSSL_free(e);
return 1;
diff --git a/crypto/engine/eng_list.c b/crypto/engine/eng_list.c
index fec0ef7129..04c73c7628 100644
--- a/crypto/engine/eng_list.c
+++ b/crypto/engine/eng_list.c
@@ -27,6 +27,12 @@
static ENGINE *engine_list_head = NULL;
static ENGINE *engine_list_tail = NULL;
+/*
+ * The linked list of currently loaded dynamic engines.
+ */
+static ENGINE *engine_dyn_list_head = NULL;
+static ENGINE *engine_dyn_list_tail = NULL;
+
/*
* This cleanup function is only needed internally. If it should be called,
* we register it with the "engine_cleanup_int()" stack to be called during
@@ -128,6 +134,85 @@ static int engine_list_remove(ENGINE *e)
return 1;
}
+/* Add engine to dynamic engine list. */
+int engine_add_dynamic_id(ENGINE *e, ENGINE_DYNAMIC_ID dynamic_id,
+ int not_locked)
+{
+ int result = 0;
+ ENGINE *iterator = NULL;
+
+ if (e == NULL)
+ return 0;
+
+ if (e->dynamic_id == NULL && dynamic_id == NULL)
+ return 0;
+
+ if (not_locked && !CRYPTO_THREAD_write_lock(global_engine_lock))
+ return 0;
+
+ if (dynamic_id != NULL) {
+ iterator = engine_dyn_list_head;
+ while (iterator != NULL) {
+ if (iterator->dynamic_id == dynamic_id)
+ goto err;
+ iterator = iterator->next;
+ }
+ if (e->dynamic_id != NULL)
+ goto err;
+ e->dynamic_id = dynamic_id;
+ }
+
+ if (engine_dyn_list_head == NULL) {
+ /* We are adding to an empty list. */
+ if (engine_dyn_list_tail != NULL)
+ goto err;
+ engine_dyn_list_head = e;
+ e->prev_dyn = NULL;
+ } else {
+ /* We are adding to the tail of an existing list. */
+ if (engine_dyn_list_tail == NULL
+ || engine_dyn_list_tail->next_dyn != NULL)
+ goto err;
+ engine_dyn_list_tail->next_dyn = e;
+ e->prev_dyn = engine_dyn_list_tail;
+ }
+
+ engine_dyn_list_tail = e;
+ e->next_dyn = NULL;
+ result = 1;
+
+ err:
+ if (not_locked)
+ CRYPTO_THREAD_unlock(global_engine_lock);
+ return result;
+}
+
+/* Remove engine from dynamic engine list. */
+void engine_remove_dynamic_id(ENGINE *e, int not_locked)
+{
+ if (e == NULL || e->dynamic_id == NULL)
+ return;
+
+ if (not_locked && !CRYPTO_THREAD_write_lock(global_engine_lock))
+ return;
+
+ e->dynamic_id = NULL;
+
+ /* un-link e from the chain. */
+ if (e->next_dyn != NULL)
+ e->next_dyn->prev_dyn = e->prev_dyn;
+ if (e->prev_dyn != NULL)
+ e->prev_dyn->next_dyn = e->next_dyn;
+ /* Correct our head/tail if necessary. */
+ if (engine_dyn_list_head == e)
+ engine_dyn_list_head = e->next_dyn;
+ if (engine_dyn_list_tail == e)
+ engine_dyn_list_tail = e->prev_dyn;
+
+ if (not_locked)
+ CRYPTO_THREAD_unlock(global_engine_lock);
+}
+
/* Get the first/last "ENGINE" type available. */
ENGINE *ENGINE_get_first(void)
{
@@ -278,6 +363,8 @@ static void engine_cpy(ENGINE *dest, const ENGINE *src)
dest->load_pubkey = src->load_pubkey;
dest->cmd_defns = src->cmd_defns;
dest->flags = src->flags;
+ dest->dynamic_id = src->dynamic_id;
+ engine_add_dynamic_id(dest, NULL, 0);
}
ENGINE *ENGINE_by_id(const char *id)
diff --git a/crypto/engine/eng_local.h b/crypto/engine/eng_local.h
index 455dc1fdb7..03a86299cf 100644
--- a/crypto/engine/eng_local.h
+++ b/crypto/engine/eng_local.h
@@ -99,6 +99,11 @@ void engine_pkey_asn1_meths_free(ENGINE *e);
extern CRYPTO_ONCE engine_lock_init;
DECLARE_RUN_ONCE(do_engine_lock_init)
+typedef void (*ENGINE_DYNAMIC_ID)(void);
+int engine_add_dynamic_id(ENGINE *e, ENGINE_DYNAMIC_ID dynamic_id,
+ int not_locked);
+void engine_remove_dynamic_id(ENGINE *e, int not_locked);
+
/*
* This is a structure for storing implementations of various crypto
* algorithms and functions.
@@ -143,6 +148,10 @@ struct engine_st {
/* Used to maintain the linked-list of engines. */
struct engine_st *prev;
struct engine_st *next;
+ /* Used to maintain the linked-list of dynamic engines. */
+ struct engine_st *prev_dyn;
+ struct engine_st *next_dyn;
+ ENGINE_DYNAMIC_ID dynamic_id;
};
typedef struct st_engine_pile ENGINE_PILE;
diff --git a/test/recipes/20-test_dgst.t b/test/recipes/20-test_dgst.t
index 5af74aec2a..e72038d852 100644
--- a/test/recipes/20-test_dgst.t
+++ b/test/recipes/20-test_dgst.t
@@ -12,12 +12,12 @@ use warnings;
use File::Spec;
use File::Basename;
-use OpenSSL::Test qw/:DEFAULT with srctop_file/;
+use OpenSSL::Test qw/:DEFAULT with srctop_file bldtop_file/;
use OpenSSL::Test::Utils;
setup("test_dgst");
-plan tests => 9;
+plan tests => 10;
sub tsignverify {
my $testtext = shift;
@@ -103,6 +103,25 @@ SKIP: {
};
}
+SKIP: {
+ skip "dgst with engine is not supported by this OpenSSL build", 1
+ if disabled("engine") || disabled("dynamic-engine");
+
+ subtest "SHA1 generation by engine with `dgst` CLI" => sub {
+ plan tests => 1;
+
+ my $testdata = srctop_file('test', 'data.bin');
+ # intentionally using -engine twice, please do not remove the duplicate line
+ my @macdata = run(app(['openssl', 'dgst', '-sha1',
+ '-engine', $^O eq 'linux' ? bldtop_file("engines", "ossltest.so") : "ossltest",
+ '-engine', $^O eq 'linux' ? bldtop_file("engines", "ossltest.so") : "ossltest",
+ $testdata]), capture => 1);
+ chomp(@macdata);
+ my $expected = qr/SHA1\(\Q$testdata\E\)= 000102030405060708090a0b0c0d0e0f10111213/;
+ ok($macdata[0] =~ $expected, "SHA1: Check HASH value is as expected ($macdata[0]) vs ($expected)");
+ }
+}
+
subtest "HMAC generation with `dgst` CLI" => sub {
plan tests => 2;
More information about the openssl-commits
mailing list