Re: [Openvpn-devel] [PATCH v3] Improve management-external-key/cert error handling

[Steffan Karger]

Hi,

On 03-04-18 04:53, Selva Nair wrote:
> But I can't believe I missed this in the last round. This else clause
> will now get executed not only if options->cert_file is false, but
> also if its true and the call to tls_ctx_use_external_private_key()
> succeeds! That would be wrong and is not what we want.

Thanks for catching this before it went into master!

For archiving purposes: this patch is no longer needed because it has
been superseded by the refactoring in "Do not load certificate from
tls_ctx_use_external_private_key()" (Message-Id:
<1536916459-25900-1-git-send-email-steffan.kar...@fox-it.com>).

I'll register it like so in patchwork too.

-Steffan


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3] Improve management-external-key/cert error handling

[Selva Nair]

Hi,

This one applies cleanly on top of master.

On Mon, Apr 2, 2018 at 7:44 AM, Steffan Karger  wrote:
>
> Check the return values of management_query_cert() and
> tls_ctx_use_external_private_key(), and error out with a more descriptive
> error message.  To do so, we make the openssl-backed implementation of
> tls_ctx_use_external_private_key() not throw fatal error anymore.
>
> (And fix line wrapping while touching this code.)
>
> Signed-off-by: Steffan Karger 
> ---
> v2: error out with M_FATAL as suggested by Selva.
> v3: rebase on master (without extra patches)
>
>  src/openvpn/ssl.c | 28 
>  src/openvpn/ssl_openssl.c |  2 +-
>  2 files changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 669f941b..b06820ba 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -660,18 +660,30 @@ init_ssl(const struct options *options, struct 
> tls_root_ctx *new_ctx)
>  else if ((options->management_flags & MF_EXTERNAL_KEY)
>   && (options->cert_file || options->management_flags & 
> MF_EXTERNAL_CERT))
>  {
> -if (options->cert_file)
> +if (options->cert_file
> +&& 0 != tls_ctx_use_external_private_key(new_ctx,
> + options->cert_file,
> + 
> options->cert_file_inline))
>  {
> -tls_ctx_use_external_private_key(new_ctx, options->cert_file,
> - options->cert_file_inline);
> +msg(M_FATAL, "Failed to initialize management-external-key");
>  }
>  else

But I can't believe I missed this in the last round. This else clause
will now get executed not only if options->cert_file is false, but
also if its true and the call to tls_ctx_use_external_private_key()
succeeds! That would be wrong and is not what we want.

>
>  {
> -char *external_certificate = management_query_cert(management,
> -   
> options->management_certificate);
> -tls_ctx_use_external_private_key(new_ctx, INLINE_FILE_TAG,
> - external_certificate);
> -free(external_certificate);
> +char *external_cert = management_query_cert(
> +management, options->management_certificate);
> +
>
> +if (!external_cert)
> +{
> +msg(M_FATAL, "Failed to initialize 
> management-external-cert");
> +}
> +
> +if (0 != tls_ctx_use_external_private_key(new_ctx, 
> INLINE_FILE_TAG,
> +  external_cert))
> +{
> +msg(M_FATAL, "Failed to initialize management-external-key");
> +}
> +
> +free(external_cert);
>  }
>  }
>  #endif
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 8ef68ebd..4d434fa2 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -1330,7 +1330,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx 
> *ctx,
>  return 0;
>
>  err:
> -crypto_msg(M_FATAL, "Cannot enable SSL external private key capability");
> +crypto_msg(M_WARN, "Cannot enable SSL external private key capability");
>  return 1;
>  }


Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] Improve management-external-key/cert error handling

[Steffan Karger]

Check the return values of management_query_cert() and
tls_ctx_use_external_private_key(), and error out with a more descriptive
error message.  To do so, we make the openssl-backed implementation of
tls_ctx_use_external_private_key() not throw fatal error anymore.

(And fix line wrapping while touching this code.)

Signed-off-by: Steffan Karger 
---
v2: error out with M_FATAL as suggested by Selva.
v3: rebase on master (without extra patches)

 src/openvpn/ssl.c | 28 
 src/openvpn/ssl_openssl.c |  2 +-
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 669f941b..b06820ba 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -660,18 +660,30 @@ init_ssl(const struct options *options, struct 
tls_root_ctx *new_ctx)
 else if ((options->management_flags & MF_EXTERNAL_KEY)
  && (options->cert_file || options->management_flags & 
MF_EXTERNAL_CERT))
 {
-if (options->cert_file)
+if (options->cert_file
+&& 0 != tls_ctx_use_external_private_key(new_ctx,
+ options->cert_file,
+ 
options->cert_file_inline))
 {
-tls_ctx_use_external_private_key(new_ctx, options->cert_file,
- options->cert_file_inline);
+msg(M_FATAL, "Failed to initialize management-external-key");
 }
 else
 {
-char *external_certificate = management_query_cert(management,
-   
options->management_certificate);
-tls_ctx_use_external_private_key(new_ctx, INLINE_FILE_TAG,
- external_certificate);
-free(external_certificate);
+char *external_cert = management_query_cert(
+management, options->management_certificate);
+
+if (!external_cert)
+{
+msg(M_FATAL, "Failed to initialize management-external-cert");
+}
+
+if (0 != tls_ctx_use_external_private_key(new_ctx, INLINE_FILE_TAG,
+  external_cert))
+{
+msg(M_FATAL, "Failed to initialize management-external-key");
+}
+
+free(external_cert);
 }
 }
 #endif
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 8ef68ebd..4d434fa2 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1330,7 +1330,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
 return 0;
 
 err:
-crypto_msg(M_FATAL, "Cannot enable SSL external private key capability");
+crypto_msg(M_WARN, "Cannot enable SSL external private key capability");
 return 1;
 }
 
-- 
2.14.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel