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 <stef...@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
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to