[Openvpn-devel,v2] show extra info for OpenSSL errors

Message ID 20230811121503.4159089-1-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v2] show extra info for OpenSSL errors | expand

Commit Message

Arne Schwabe Aug. 11, 2023, 12:15 p.m. UTC
This also shows the extra data from the OpenSSL error function that
can contain extra information. For example, the command

    openvpn --providers vollbit

will print out (on macOS):

     OpenSSL: error:12800067:DSO support routines::could not load the shared library:filename(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib): dlopen(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib, 0x0002): tried: '/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib' (no such file), '/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib' (no such file)

Patch v2: Format message more like current messages

Change-Id: Ic2ee89937dcd85721bcacd1b700a20c640364f80
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/crypto_openssl.c | 21 +++++++++++++++++++--
 src/openvpn/openssl_compat.h | 12 ++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

Comments

Selva Nair Aug. 11, 2023, 5:56 p.m. UTC | #1
On Fri, Aug 11, 2023 at 8:16 AM Arne Schwabe <arne@rfc2549.org> wrote:

> This also shows the extra data from the OpenSSL error function that
> can contain extra information. For example, the command
>
>     openvpn --providers vollbit
>
> will print out (on macOS):
>
>      OpenSSL: error:12800067:DSO support routines::could not load the
> shared library:filename(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib):
> dlopen(/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib,
> 0x0002): tried: '/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
> (no such file),
> '/System/Volumes/Preboot/Cryptexes/OS/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
> (no such file), '/opt/homebrew/Cellar/openssl@3/3.1.1_1/lib/ossl-modules/vollbit.dylib'
> (no such file)
>
> Patch v2: Format message more like current messages
>
> Change-Id: Ic2ee89937dcd85721bcacd1b700a20c640364f80
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/crypto_openssl.c | 21 +++++++++++++++++++--
>  src/openvpn/openssl_compat.h | 12 ++++++++++++
>  2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
> index b043bb95e..22c6d6840 100644
> --- a/src/openvpn/crypto_openssl.c
> +++ b/src/openvpn/crypto_openssl.c
> @@ -238,9 +238,16 @@ void
>  crypto_print_openssl_errors(const unsigned int flags)
>  {
>      unsigned long err = 0;
> +    int line, errflags;
> +    const char *file, *data, *func;
>
> -    while ((err = ERR_get_error()))
> +    while ((err = ERR_get_error_all(&file, &line, &func, &data,
> &errflags)) != 0)
>      {
> +        if (!(errflags & ERR_TXT_STRING))
> +        {
> +            data = "";
> +        }
> +
>          /* Be more clear about frequently occurring "no shared cipher"
> error */
>          if (ERR_GET_REASON(err) == SSL_R_NO_SHARED_CIPHER)
>          {
> @@ -258,7 +265,17 @@ crypto_print_openssl_errors(const unsigned int flags)
>                  "tls-version-min 1.0 to the client configuration to use
> TLS 1.0+ "
>                  "instead of TLS 1.0 only");
>          }
> -        msg(flags, "OpenSSL: %s", ERR_error_string(err, NULL));
> +
> +        /* print file and line if verb >=8 */
> +        if (!check_debug_level(D_TLS_DEBUG_MED))
> +        {
> +            msg(flags, "OpenSSL: %s:%s", ERR_error_string(err, NULL),
> data);
> +        }
> +        else
> +        {
> +            msg(flags, "OpenSSL: %s:%s:%s:%d:%s", ERR_error_string(err,
> NULL),
> +                data, file, line, func);
> +        }
>      }
>  }
>
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index ffb64adf6..736ce1bd5 100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -43,6 +43,7 @@
>  #include <openssl/rsa.h>
>  #include <openssl/ssl.h>
>  #include <openssl/x509.h>
> +#include <openssl/err.h>
>
>  /* Functionality missing in 1.1.0 */
>  #if OPENSSL_VERSION_NUMBER < 0x10101000L &&
> !defined(ENABLE_CRYPTO_WOLFSSL)
> @@ -799,6 +800,17 @@ EVP_MD_free(const EVP_MD *md)
>      /* OpenSSL 1.1.1 and lower use only const EVP_MD, nothing to free */
>  }
>
> +static inline unsigned long
> +ERR_get_error_all(const char **file, int *line,
> +                  const char **func,
> +                  const char **data, int *flags)
> +{
> +    static const char *empty = "";
> +    *func = empty;
> +    long err = ERR_get_error_line_data(file, line, data, flags);
>

I think you missed to change that to "unsigned long err = ...."

+    return err;
> +}
> +
>  #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
>
>  #endif /* OPENSSL_COMPAT_H_ */
> --
> 2.39.2 (Apple Git-143)
>

The above could be handled at merge time, so:
Acked-by: Selva Nair <selva.nair@gmail.com>
Gert Doering Aug. 11, 2023, 6:41 p.m. UTC | #2
Thanks, Selva for having an extra eye :-) - I asked for the feature, and
it works beautifully for me, but what do I understand about OpenSSL
internals...  ("unsigned long" fixed on the fly).

Tried on FreeBSD 14 with OpenSSL 3 and a broken provider (which is what
triggered the whole thing):

$ src/openvpn/openvpn --providers legacyXX
2023-08-11 20:19:53 OpenSSL: error:12800067:DSO support routines::could not load the shared library:filename(/usr/lib/ossl-modules/legacyXX.so): /usr/lib/ossl-modules/legacyXX.so: Undefined symbol "ossl_md4_functions"

.. and on Linux with OpenSSL 1.1.1t, passing a wrong passphrase:

2023-08-11 20:17:24 OpenSSL: error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt:
2023-08-11 20:17:24 OpenSSL: error:23077074:PKCS12 routines:PKCS12_pbe_crypt:pkcs12 cipherfinal error:
2023-08-11 20:17:24 OpenSSL: error:2306A075:PKCS12 routines:PKCS12_item_decrypt_d2i:pkcs12 pbe crypt error:
2023-08-11 20:17:24 OpenSSL: error:0907B00D:PEM routines:PEM_read_bio_PrivateKey:ASN1 lib:
2023-08-11 20:17:24 Cannot load private key file [[INLINE]]

(which looks to be "the same what it printed before", so at least it
does not break anything)

With 3.0 it prints, in the same situation...

2023-08-11 20:24:01 OpenSSL: error:1C800064:Provider routines::bad decrypt:
2023-08-11 20:24:01 OpenSSL: error:11800074:PKCS12 routines::pkcs12 cipherfinal error:maybe wrong password

.. or

2023-08-11 20:23:21 OpenSSL: error:0308010C:digital envelope routines::unsupported:Global default library context, Algorithm (DES-CBC : 10), Properties ()

(ahem...)

So, very nice.


For extra sanity checking pushed to GHA first, to get more OpenSSL/OS
combinations tested.


Your patch has been applied to the master and release/2.6 branch.

commit 0f8485f2870277fb7ccdb4097380e35dc35b064e (master)
commit 101499a43d222dcefbf5c6fc6f8b71a4f5d1f533 (release/2.6)
Author: Arne Schwabe
Date:   Fri Aug 11 14:15:03 2023 +0200

     show extra info for OpenSSL errors

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Message-Id: <20230811121503.4159089-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26929.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index b043bb95e..22c6d6840 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -238,9 +238,16 @@  void
 crypto_print_openssl_errors(const unsigned int flags)
 {
     unsigned long err = 0;
+    int line, errflags;
+    const char *file, *data, *func;
 
-    while ((err = ERR_get_error()))
+    while ((err = ERR_get_error_all(&file, &line, &func, &data, &errflags)) != 0)
     {
+        if (!(errflags & ERR_TXT_STRING))
+        {
+            data = "";
+        }
+
         /* Be more clear about frequently occurring "no shared cipher" error */
         if (ERR_GET_REASON(err) == SSL_R_NO_SHARED_CIPHER)
         {
@@ -258,7 +265,17 @@  crypto_print_openssl_errors(const unsigned int flags)
                 "tls-version-min 1.0 to the client configuration to use TLS 1.0+ "
                 "instead of TLS 1.0 only");
         }
-        msg(flags, "OpenSSL: %s", ERR_error_string(err, NULL));
+
+        /* print file and line if verb >=8 */
+        if (!check_debug_level(D_TLS_DEBUG_MED))
+        {
+            msg(flags, "OpenSSL: %s:%s", ERR_error_string(err, NULL), data);
+        }
+        else
+        {
+            msg(flags, "OpenSSL: %s:%s:%s:%d:%s", ERR_error_string(err, NULL),
+                data, file, line, func);
+        }
     }
 }
 
diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
index ffb64adf6..736ce1bd5 100644
--- a/src/openvpn/openssl_compat.h
+++ b/src/openvpn/openssl_compat.h
@@ -43,6 +43,7 @@ 
 #include <openssl/rsa.h>
 #include <openssl/ssl.h>
 #include <openssl/x509.h>
+#include <openssl/err.h>
 
 /* Functionality missing in 1.1.0 */
 #if OPENSSL_VERSION_NUMBER < 0x10101000L && !defined(ENABLE_CRYPTO_WOLFSSL)
@@ -799,6 +800,17 @@  EVP_MD_free(const EVP_MD *md)
     /* OpenSSL 1.1.1 and lower use only const EVP_MD, nothing to free */
 }
 
+static inline unsigned long
+ERR_get_error_all(const char **file, int *line,
+                  const char **func,
+                  const char **data, int *flags)
+{
+    static const char *empty = "";
+    *func = empty;
+    long err = ERR_get_error_line_data(file, line, data, flags);
+    return err;
+}
+
 #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */
 
 #endif /* OPENSSL_COMPAT_H_ */