Attention is currently required from: flichtenheld.
Hello flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/363?usp=email
to review the following change.
Change subject: Log OpenSSL errors on failure to set certificate
......................................................................
Log OpenSSL errors on failure to set certificate
Currently we log a bogus error message saying private key password verification
failed when SSL_CTX_use_cert_and_key() fails in pkcs11_openssl.c. Instead print
OpenSSL error queue and exit promptly.
Also log OpenSSL errors when SSL_CTX_use_certiifcate() fails in cryptoapi.c
and elsewhere. Such logging could be useful especially when the ceritficate is
rejected by OpenSSL due to stricter security restrictions in recent versions
of the library.
Change-Id: Ic7ec25ac0503a91d5869b8da966d0065f264af22
Signed-off-by: Selva Nair <[email protected]>
---
M src/openvpn/cryptoapi.c
M src/openvpn/pkcs11_openssl.c
M src/openvpn/ssl_openssl.c
M tests/unit_tests/openvpn/test_cryptoapi.c
M tests/unit_tests/openvpn/test_pkcs11.c
5 files changed, 30 insertions(+), 2 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/63/363/1
diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 3b92e48..f7e5b67 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -51,6 +51,7 @@
#include "openssl_compat.h"
#include "win32.h"
#include "xkey_common.h"
+#include "crypto_openssl.h"
#ifndef HAVE_XKEY_PROVIDER
@@ -505,6 +506,7 @@
if (SSL_CTX_use_certificate(ssl_ctx, cert)
&& SSL_CTX_use_PrivateKey(ssl_ctx, privkey))
{
+ crypto_print_openssl_errors(M_WARN);
ret = 1;
}
diff --git a/src/openvpn/pkcs11_openssl.c b/src/openvpn/pkcs11_openssl.c
index 40080ef..aa0819f 100644
--- a/src/openvpn/pkcs11_openssl.c
+++ b/src/openvpn/pkcs11_openssl.c
@@ -302,7 +302,8 @@
if (!SSL_CTX_use_cert_and_key(ctx->ctx, x509, pkey, NULL, 0))
{
- msg(M_WARN, "PKCS#11: Failed to set cert and private key for OpenSSL");
+ crypto_print_openssl_errors(M_WARN);
+ msg(M_FATAL, "PKCS#11: Failed to set cert and private key for
OpenSSL");
goto cleanup;
}
ret = 1;
@@ -369,7 +370,8 @@
if (!SSL_CTX_use_certificate(ssl_ctx->ctx, x509))
{
- msg(M_WARN, "PKCS#11: Cannot set certificate for openssl");
+ crypto_print_openssl_errors(M_WARN);
+ msg(M_FATAL, "PKCS#11: Cannot set certificate for openssl");
goto cleanup;
}
ret = 0;
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 0b310de..b5cc9a7 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -857,6 +857,7 @@
/* Load Certificate */
if (!SSL_CTX_use_certificate(ctx->ctx, cert))
{
+ crypto_print_openssl_errors(M_WARN);
crypto_msg(M_FATAL, "Cannot use certificate");
}
@@ -1007,6 +1008,7 @@
end:
if (!ret)
{
+ crypto_print_openssl_errors(M_WARN);
if (cert_file_inline)
{
crypto_msg(M_FATAL, "Cannot load inline certificate file");
diff --git a/tests/unit_tests/openvpn/test_cryptoapi.c
b/tests/unit_tests/openvpn/test_cryptoapi.c
index 008f41c..d90bfc3 100644
--- a/tests/unit_tests/openvpn/test_cryptoapi.c
+++ b/tests/unit_tests/openvpn/test_cryptoapi.c
@@ -58,6 +58,17 @@
return NULL;
}
+/* replacement for crypto_print_openssl_errors() */
+void
+crypto_print_openssl_errors(const unsigned int flags)
+{
+ unsigned long e;
+ while ((e = ERR_get_error()))
+ {
+ msg(flags, "OpenSSL error %lu: %s\n", e, ERR_error_string(e, NULL));
+ }
+}
+
/* tls_libctx is defined in ssl_openssl.c which we do not want to compile in */
OSSL_LIB_CTX *tls_libctx;
diff --git a/tests/unit_tests/openvpn/test_pkcs11.c
b/tests/unit_tests/openvpn/test_pkcs11.c
index 235cc43..b6c130e 100644
--- a/tests/unit_tests/openvpn/test_pkcs11.c
+++ b/tests/unit_tests/openvpn/test_pkcs11.c
@@ -44,6 +44,17 @@
struct management *management; /* global */
+/* replacement for crypto_print_openssl_errors() */
+void
+crypto_print_openssl_errors(const unsigned int flags)
+{
+ unsigned long e;
+ while ((e = ERR_get_error()))
+ {
+ msg(flags, "OpenSSL error %lu: %s\n", e, ERR_error_string(e, NULL));
+ }
+}
+
/* stubs for some unused functions instead of pulling in too many dependencies
*/
int
parse_line(const char *line, char **p, const int n, const char *file,
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/363?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: Ic7ec25ac0503a91d5869b8da966d0065f264af22
Gerrit-Change-Number: 363
Gerrit-PatchSet: 1
Gerrit-Owner: selvanair <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
