Attention is currently required from: flichtenheld, plaisthos.
Hello flichtenheld,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/373?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review-1 by flichtenheld
Change subject: Remove openssl engine method for loading the key
......................................................................
Remove openssl engine method for loading the key
This is a contribution for loading engine key. OpenSSL engine is
deprecated since OpenSSL 3.0 and James Bottomley has not agreed
to the proposed license change. He is also okay removing
with the feature from the current code base as it is obsolete with
OpenSSL 3.0.
The main commits that are reverted by this commit are
8155f8aa0 and 542c69c37 (unit test)
Change-Id: I2d353a0cea0a62f289b8c1060244df66dd7a14cb
---
M .gitignore
M configure.ac
M src/openvpn/crypto_openssl.c
M src/openvpn/crypto_openssl.h
M src/openvpn/ssl_openssl.c
M tests/unit_tests/Makefile.am
D tests/unit_tests/engine-key/Makefile.am
D tests/unit_tests/engine-key/check_engine_keys.sh
D tests/unit_tests/engine-key/libtestengine.c
D tests/unit_tests/engine-key/openssl.cnf.in
10 files changed, 0 insertions(+), 279 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/73/373/2
diff --git a/.gitignore b/.gitignore
index ed03aaa..a1da366 100644
--- a/.gitignore
+++ b/.gitignore
@@ -57,10 +57,6 @@
t_client.rc
t_client_ips.rc
tests/unit_tests/**/*_testdriver
-tests/unit_tests/engine-key/client.key
-tests/unit_tests/engine-key/log.txt
-tests/unit_tests/engine-key/openssl.cnf
-tests/unit_tests/engine-key/passwd
src/openvpn/openvpn
include/openvpn-plugin.h
diff --git a/configure.ac b/configure.ac
index 266b66f..128ab86 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1532,7 +1532,6 @@
tests/unit_tests/openvpn/Makefile
tests/unit_tests/plugins/Makefile
tests/unit_tests/plugins/auth-pam/Makefile
- tests/unit_tests/engine-key/Makefile
sample/Makefile
])
AC_CONFIG_FILES([tests/t_client.sh], [chmod +x tests/t_client.sh])
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 22c6d68..fe1254f 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -1374,66 +1374,6 @@
return CRYPTO_memcmp(a, b, size);
}
-#if HAVE_OPENSSL_ENGINE
-static int
-ui_reader(UI *ui, UI_STRING *uis)
-{
- SSL_CTX *ctx = UI_get0_user_data(ui);
-
- if (UI_get_string_type(uis) == UIT_PROMPT)
- {
- pem_password_cb *cb = SSL_CTX_get_default_passwd_cb(ctx);
- void *d = SSL_CTX_get_default_passwd_cb_userdata(ctx);
- char password[64];
-
- cb(password, sizeof(password), 0, d);
- UI_set_result(ui, uis, password);
-
- return 1;
- }
- return 0;
-}
-#endif
-
-EVP_PKEY *
-engine_load_key(const char *file, SSL_CTX *ctx)
-{
-#if HAVE_OPENSSL_ENGINE
- UI_METHOD *ui;
- EVP_PKEY *pkey;
-
- if (!engine_persist)
- {
- return NULL;
- }
-
- /* this will print out the error from BIO_read */
- crypto_msg(M_INFO, "PEM_read_bio failed, now trying engine method to load
private key");
-
- ui = UI_create_method("openvpn");
- if (!ui)
- {
- crypto_msg(M_FATAL, "Engine UI creation failed");
- return NULL;
- }
-
- UI_method_set_reader(ui, ui_reader);
-
- ENGINE_init(engine_persist);
- pkey = ENGINE_load_private_key(engine_persist, file, ui, ctx);
- ENGINE_finish(engine_persist);
- if (!pkey)
- {
- crypto_msg(M_FATAL, "Engine could not load key file");
- }
-
- UI_destroy_method(ui);
- return pkey;
-#else /* if HAVE_OPENSSL_ENGINE */
- return NULL;
-#endif /* if HAVE_OPENSSL_ENGINE */
-}
-
#if (OPENSSL_VERSION_NUMBER >= 0x10100000L) &&
!defined(LIBRESSL_VERSION_NUMBER)
bool
ssl_tls1_PRF(const uint8_t *seed, int seed_len, const uint8_t *secret,
diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h
index 32849fd..c5a5393 100644
--- a/src/openvpn/crypto_openssl.h
+++ b/src/openvpn/crypto_openssl.h
@@ -118,16 +118,4 @@
msg((flags), __VA_ARGS__); \
} while (false)
-/**
- * Load a key file from an engine
- *
- * @param file The engine file to load
- * @param ui The UI method for the password prompt
- * @param data The data to pass to the UI method
- *
- * @return The private key if successful or NULL if not
- */
-EVP_PKEY *
-engine_load_key(const char *file, SSL_CTX *ctx);
-
#endif /* CRYPTO_OPENSSL_H_ */
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 4c08add..23e7623 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1057,10 +1057,6 @@
pkey = PEM_read_bio_PrivateKey(in, NULL,
SSL_CTX_get_default_passwd_cb(ctx->ctx),
SSL_CTX_get_default_passwd_cb_userdata(ctx->ctx));
- if (!pkey)
- {
- pkey = engine_load_key(priv_key_file, ctx->ctx);
- }
if (!pkey || !SSL_CTX_use_PrivateKey(ssl_ctx, pkey))
{
diff --git a/tests/unit_tests/Makefile.am b/tests/unit_tests/Makefile.am
index f27cd90..33fefaa 100644
--- a/tests/unit_tests/Makefile.am
+++ b/tests/unit_tests/Makefile.am
@@ -2,7 +2,4 @@
if ENABLE_UNITTESTS
SUBDIRS = example_test openvpn plugins
-if OPENSSL_ENGINE
-SUBDIRS += engine-key
-endif
endif
diff --git a/tests/unit_tests/engine-key/Makefile.am
b/tests/unit_tests/engine-key/Makefile.am
deleted file mode 100644
index 0c28885..0000000
--- a/tests/unit_tests/engine-key/Makefile.am
+++ /dev/null
@@ -1,31 +0,0 @@
-AUTOMAKE_OPTIONS = foreign
-
-check_LTLIBRARIES = libtestengine.la
-conffiles = openssl.cnf
-EXTRA_DIST = \
- openssl.cnf.in \
- check_engine_keys.sh
-
-TESTS_ENVIRONMENT = srcdir="$(abs_srcdir)"; \
- builddir="$(abs_builddir)"; \
- top_builddir="$(top_builddir)"; \
- top_srcdir="$(top_srcdir)"; \
- export srcdir builddir top_builddir top_srcdir;
-
-if !CROSS_COMPILING
-TESTS = check_engine_keys.sh
-endif
-check_engine_keys.sh: $(conffiles)
-
-CLEANFILES = \
- client.key \
- passwd \
- log.txt \
- $(conffiles)
-
-openssl.cnf: $(srcdir)/openssl.cnf.in
- sed "s|ABSBUILDDIR|$(abs_builddir)|" < $(srcdir)/openssl.cnf.in > $@
-
-libtestengine_la_SOURCES = libtestengine.c
-libtestengine_la_LDFLAGS = @TEST_LDFLAGS@ -rpath /lib -shrext .so
-libtestengine_la_CFLAGS = @TEST_CFLAGS@ -I$(openvpn_srcdir) -I$(compat_srcdir)
diff --git a/tests/unit_tests/engine-key/check_engine_keys.sh
b/tests/unit_tests/engine-key/check_engine_keys.sh
deleted file mode 100755
index 12dd230..0000000
--- a/tests/unit_tests/engine-key/check_engine_keys.sh
+++ /dev/null
@@ -1,36 +0,0 @@
-#!/bin/sh
-
-OPENSSL_CONF="${builddir}/openssl.cnf"
-export OPENSSL_CONF
-
-password='AT3S4PASSWD'
-
-key="${builddir}/client.key"
-pwdfile="${builddir}/passwd"
-
-# create an engine key for us
-sed 's/PRIVATE KEY/TEST ENGINE KEY/' <
${top_srcdir}/sample/sample-keys/client.key > ${key}
-echo "$password" > $pwdfile
-
-# our version of grep to output log.txt on failure in case it's an openssl
-# error mismatch and the grep expression needs updating
-loggrep() {
- egrep -q "$1" log.txt || { echo '---- begin log.txt ----'; cat log.txt;
echo '--- end log.txt ---'; return 1; }
-}
-
-# note here we've induced a mismatch in the client key and the server
-# cert which openvpn should report and die. Check that it does. Note
-# also that this mismatch depends on openssl not openvpn, so it is
-# somewhat fragile
-${top_builddir}/src/openvpn/openvpn --cd ${top_srcdir}/sample --config
sample-config-files/loopback-server --engine testengine --key ${key} --askpass
$pwdfile > log.txt 2>&1
-
-# first off check we died because of a key mismatch. If this doesn't
-# pass, suspect openssl of returning different messages and update the
-# test accordingly
-loggrep '(x509 certificate routines:(X509_check_private_key)?:key values
mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not
detected"; exit 1; }
-
-# now look for the engine prints (these are under our control)
-loggrep 'ENGINE: engine_init called' || { echo "Engine initialization not
detected"; exit 1; }
-loggrep 'ENGINE: engine_load_key called' || { echo "Key was not loaded from
engine"; exit 1; }
-loggrep "ENGINE: engine_load_key got password ${password}" || { echo "Key
password was not retrieved by the engine"; exit 1; }
-exit 0
diff --git a/tests/unit_tests/engine-key/libtestengine.c
b/tests/unit_tests/engine-key/libtestengine.c
deleted file mode 100644
index 8bcfa92..0000000
--- a/tests/unit_tests/engine-key/libtestengine.c
+++ /dev/null
@@ -1,116 +0,0 @@
-#include <string.h>
-#include <openssl/engine.h>
-#include <openssl/evp.h>
-#include <openssl/pem.h>
-
-static char *engine_id = "testengine";
-static char *engine_name = "Engine for testing openvpn engine key support";
-
-static int is_initialized = 0;
-
-static int
-engine_init(ENGINE *e)
-{
- is_initialized = 1;
- fprintf(stderr, "ENGINE: engine_init called\n");
- return 1;
-}
-
-static int
-engine_finish(ENGINE *e)
-{
- fprintf(stderr, "ENGINE: engine_finsh called\n");
- is_initialized = 0;
- return 1;
-}
-
-static EVP_PKEY *
-engine_load_key(ENGINE *e, const char *key_id,
- UI_METHOD *ui_method, void *cb_data)
-{
- BIO *b;
- EVP_PKEY *pkey;
- PKCS8_PRIV_KEY_INFO *p8inf;
- UI *ui;
- char auth[256];
-
- fprintf(stderr, "ENGINE: engine_load_key called\n");
-
- if (!is_initialized)
- {
- fprintf(stderr, "Load Key called without correct initialization\n");
- return NULL;
- }
- b = BIO_new_file(key_id, "r");
- if (!b)
- {
- fprintf(stderr, "File %s does not exist or cannot be read\n", key_id);
- return 0;
- }
- /* Basically read an EVP_PKEY private key file with different
- * PEM guards --- we are a test engine */
- p8inf = PEM_ASN1_read_bio((d2i_of_void *)d2i_PKCS8_PRIV_KEY_INFO,
- "TEST ENGINE KEY", b,
- NULL, NULL, NULL);
- BIO_free(b);
- if (!p8inf)
- {
- fprintf(stderr, "Failed to read engine private key\n");
- return NULL;
- }
- pkey = EVP_PKCS82PKEY(p8inf);
-
- /* now we have a private key, pretend it had a password
- * this verifies the password makes it through openvpn OK */
- ui = UI_new();
-
- if (ui_method)
- {
- UI_set_method(ui, ui_method);
- }
-
- UI_add_user_data(ui, cb_data);
-
- if (UI_add_input_string(ui, "enter test engine key",
- UI_INPUT_FLAG_DEFAULT_PWD,
- auth, 0, sizeof(auth)) == 0)
- {
- fprintf(stderr, "UI_add_input_string failed\n");
- goto out;
- }
-
- if (UI_process(ui))
- {
- fprintf(stderr, "UI_process failed\n");
- goto out;
- }
-
- fprintf(stderr, "ENGINE: engine_load_key got password %s\n", auth);
-
-out:
- UI_free(ui);
-
- return pkey;
-}
-
-
-static int
-engine_bind_fn(ENGINE *e, const char *id)
-{
- if (id && strcmp(id, engine_id) != 0)
- {
- return 0;
- }
- if (!ENGINE_set_id(e, engine_id)
- || !ENGINE_set_name(e, engine_name)
- || !ENGINE_set_init_function(e, engine_init)
- || !ENGINE_set_finish_function(e, engine_finish)
- || !ENGINE_set_load_privkey_function(e, engine_load_key))
- {
- return 0;
- }
- return 1;
-}
-
-IMPLEMENT_DYNAMIC_CHECK_FN()
-IMPLEMENT_DYNAMIC_BIND_FN(engine_bind_fn)
diff --git a/tests/unit_tests/engine-key/openssl.cnf.in
b/tests/unit_tests/engine-key/openssl.cnf.in
deleted file mode 100644
index 5eda9fa..0000000
--- a/tests/unit_tests/engine-key/openssl.cnf.in
+++ /dev/null
@@ -1,12 +0,0 @@
-HOME = .
-openssl_conf = openssl_init
-
-[req]
-[openssl_init]
-engines = engines_section
-
-[engines_section]
-testengine = testengine_section
-
-[testengine_section]
-dynamic_path = ABSBUILDDIR/.libs/libtestengine.so
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/373?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I2d353a0cea0a62f289b8c1060244df66dd7a14cb
Gerrit-Change-Number: 373
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
