On 22/01/2019 16:03, Arne Schwabe wrote:
> From: Arne Schwabe <a...@openvpn.net>
> 
> This is useful for features that can use either a persistent
> or an ephemeral key.

Agreed, this is valuable.  But I generally don't like "hiding" this feature
behind an argument to {read,write}_pem_key_file().  I prefer having functions
doing explicit things, not "I can do this, or this, or this" depending on the
arguments given.

Rather add a generate_random_pem_key() which does just that.

> ---
>  src/openvpn/crypto.c    | 23 ++++++++++++++++++++---
>  src/openvpn/crypto.h    |  4 +++-
>  src/openvpn/tls_crypt.c |  5 +++--
>  3 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
> index ff9dbfdc..68a28dee 100644
> --- a/src/openvpn/crypto.c
> +++ b/src/openvpn/crypto.c
> @@ -1885,13 +1885,30 @@ cleanup:
>  
>  bool
>  read_pem_key_file(struct buffer *key, const char *pem_name,
> -                  const char *key_file, const char *key_inline)
> +                  const char *key_file, const char *key_inline,
> +                  bool allow_random)
>  {
>      bool ret = false;
>      struct buffer key_pem = { 0 };
>      struct gc_arena gc = gc_new();
>  
> -    if (strcmp(key_file, INLINE_FILE_TAG))
> +
> +    if (allow_random && key_file == NULL)
> +    {
> +        msg(M_INFO, "Using random %s.", pem_name);
> +        uint8_t rand[BCAP(key)];
> +        if (!rand_bytes(rand, BCAP(key)))
> +        {
> +            msg(M_WARN, "ERROR: could not generate random key");
> +        }
> +        else
> +        {
> +            buf_write(key, rand, BCAP(key));
> +            ret = true;
> +        }

The if (rand_bytes(rand, BCAP(key))) logic could be improved when splitting it
into a separate function.

    if (!rand_bytes(rand, BCAP(key)))
    {
         msg(...);
        return false;
    }
    buf_write(...);
    return true;


[...]

> -    if (strcmp(key_file, INLINE_FILE_TAG))
> +    if (key_file && strcmp(key_file, INLINE_FILE_TAG))

Is this fixing a bug?  I'd recommend putting such fixes in a separate commit.


-- 
kind regards,

David Sommerseth
OpenVPN Inc



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

Reply via email to