Re: [Openvpn-devel] [PATCH] Make return code external tls key match docs

2018-03-04 Thread Steffan Karger
Hi,

On 28-02-18 14:52, Joost Rijneveld wrote:
> In tls_ctx_use_external_private_key, the return codes were inverted
> compared to what is documented in ssl_backend.h (and what can
> reasonably be expected). Internally the return code is never checked,
> so this did not directly result in any change of behavior.
> ---
>  src/openvpn/ssl_mbedtls.c | 6 +++---
>  src/openvpn/ssl_openssl.c | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> index 3906cd55..8e31980a 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -630,7 +630,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
>  
>  if (ctx->crt_chain == NULL)
>  {
> -return 0;
> +return 1;
>  }
>  
>  ALLOC_OBJ_CLEAR(ctx->external_key, struct external_context);
> @@ -640,10 +640,10 @@ tls_ctx_use_external_private_key(struct tls_root_ctx 
> *ctx,
>  if (!mbed_ok(mbedtls_pk_setup_rsa_alt(ctx->priv_key, ctx->external_key,
>NULL, external_pkcs1_sign, 
> external_key_len)))
>  {
> -return 0;
> +return 1;
>  }
>  
> -return 1;
> +return 0;
>  }
>  #endif /* ifdef MANAGMENT_EXTERNAL_KEY */
>  
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index d91458b0..8ef68ebd 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -1327,11 +1327,11 @@ tls_ctx_use_external_private_key(struct tls_root_ctx 
> *ctx,
>  goto err;
>  }
>  #endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev */
> -return 1;
> +return 0;
>  
>  err:
>  crypto_msg(M_FATAL, "Cannot enable SSL external private key capability");
> -return 0;
> +return 1;
>  }
>  
>  #endif /* ifdef MANAGMENT_EXTERNAL_KEY */
> 

Thanks for the patch, and pointing out this inconsistency.  Changes make
sense and make this function follow what seems to be the default pattern
in the surrounding code.

Acked-by: Steffan Karger 

(What bothers me more is that we don't actually check the return values,
but I'll send a follow-up patch to fix that.)

-Steffan

--
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] Make return code external tls key match docs

2018-02-28 Thread Joost Rijneveld
In tls_ctx_use_external_private_key, the return codes were inverted
compared to what is documented in ssl_backend.h (and what can
reasonably be expected). Internally the return code is never checked,
so this did not directly result in any change of behavior.
---
 src/openvpn/ssl_mbedtls.c | 6 +++---
 src/openvpn/ssl_openssl.c | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 3906cd55..8e31980a 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -630,7 +630,7 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
 
 if (ctx->crt_chain == NULL)
 {
-return 0;
+return 1;
 }
 
 ALLOC_OBJ_CLEAR(ctx->external_key, struct external_context);
@@ -640,10 +640,10 @@ tls_ctx_use_external_private_key(struct tls_root_ctx *ctx,
 if (!mbed_ok(mbedtls_pk_setup_rsa_alt(ctx->priv_key, ctx->external_key,
   NULL, external_pkcs1_sign, 
external_key_len)))
 {
-return 0;
+return 1;
 }
 
-return 1;
+return 0;
 }
 #endif /* ifdef MANAGMENT_EXTERNAL_KEY */
 
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index d91458b0..8ef68ebd 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1327,11 +1327,11 @@ tls_ctx_use_external_private_key(struct tls_root_ctx 
*ctx,
 goto err;
 }
 #endif /* OPENSSL_VERSION_NUMBER > 1.1.0 dev */
-return 1;
+return 0;
 
 err:
 crypto_msg(M_FATAL, "Cannot enable SSL external private key capability");
-return 0;
+return 1;
 }
 
 #endif /* ifdef MANAGMENT_EXTERNAL_KEY */
-- 
2.16.2


--
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