diff mbox series

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

Message ID 20180228135240.22945-1-joost@joostrijneveld.nl
State Accepted
Headers
Series [Openvpn-devel] Make return code external tls key match docs |

Commit Message

Joost Rijneveld Feb. 28, 2018, 2:52 a.m. UTC 
  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(-)

Comments

Steffan Karger March 3, 2018, 10:38 p.m. UTC | #1 
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 <steffan@karger.me>

(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
Gert Doering March 3, 2018, 10:58 p.m. UTC | #2 
Your patch has been applied to the master and release/2.4 branch
(with some conflicts due to changed context in master, but that was
easy enough to resolve).

Thanks.

commit 6bee1a1fc01f3d3ddf114b48e52e5b10d57033cb (master)
commit 1f342aad6a13aaae1cc54f632498e0646a1bfe1a (release/2.4)
Author: Joost Rijneveld
Date:   Wed Feb 28 14:52:40 2018 +0100

     Make return code external tls key match docs

     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20180228135240.22945-1-joost@joostrijneveld.nl>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16577.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff mbox series

Patch

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