Re: [Openvpn-devel] [PATCH] Check --ncp-ciphers list on startup

[Selva N. <sel...@gm...>]

Hi,

To nit-pick,

On Tue, Oct 11, 2016 at 3:35 PM, Steffan Karger <st...@ka...> wrote:

> +bool
> +tls_check_ncp_cipher_list(const char *list) {
> +  char *tmp_ciphers = string_alloc (list, NULL);
> +  char *tmp_ciphers_orig = tmp_ciphers;
> +  bool unsupported_cipher_found = false;
> +
> +  const char *token = strtok (tmp_ciphers, ":");
> +  while (token)
> +    {
> +      if (!cipher_kt_get (translate_cipher_name_from_openvpn (token)))
> +       {
> +         msg (M_WARN, "Unsupported cipher in --ncp-ciphers: %s", token);
> +         unsupported_cipher_found = true;
> +       }
> +      token = strtok (NULL, ":");
> +    }
> +  free (tmp_ciphers_orig);
>

the extra "tmp_ciphers_orig" is redundant. Just "free (tmp_ciphers)" will
do.



> +
> +  return list && 0 < strlen(list) && !unsupported_cipher_found;
>

Checking for list == NULL here looks too late. If list is null, so is
tmp_ciphers and then the first call to strtok will not reset the internal
state of strtok(). The behaviour of strtok then depends on whether any
previous use of it was was iterated to the end or not.

If list could be NULL, better check it and do an early return from the top.

+}
> +
>

Selva