diff mbox series

[Openvpn-devel,v2,2/3] Refactor counting number of element in a : delimited list into function

Message ID 20200416152619.5465-1-arne@rfc2549.org
State Accepted
Headers
Series None |

Commit Message

Arne Schwabe April 16, 2020, 5:26 a.m. UTC 
  Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/misc.c        | 19 +++++++++++++++++++
 src/openvpn/misc.h        | 14 ++++++++++++++
 src/openvpn/ssl_mbedtls.c | 15 ++-------------
 3 files changed, 35 insertions(+), 13 deletions(-)

Comments

Antonio Quartulli April 18, 2020, 11:11 a.m. UTC | #1 
Hi,

On 16/04/2020 17:26, Arne Schwabe wrote:
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>

Patch looks good and does what it says.

I tested the parser to make sure it still parses the ciphers as
expected, and it does:

./openvpn --tls-cipher
TLS-RSA-PSK-WITH-CHACHA20-POLY1305-SHA256:TLS-RSA-PSK-WITH-AES-256-GCM-SHA384:TLS-RSA-PSK-WITH-AES-256-CBC-SHA384
--show-tls
Available TLS Ciphers, listed in order of preference:

For TLS 1.2 and older (--tls-cipher):

TLS-RSA-PSK-WITH-CHACHA20-POLY1305-SHA256
TLS-RSA-PSK-WITH-AES-256-GCM-SHA384
TLS-RSA-PSK-WITH-AES-256-CBC-SHA384

So all good imho.

Acked-by: Antonio Quartulli <antonio@openvpn.net>

Regards,


> ---
>  src/openvpn/misc.c        | 19 +++++++++++++++++++
>  src/openvpn/misc.h        | 14 ++++++++++++++
>  src/openvpn/ssl_mbedtls.c | 15 ++-------------
>  3 files changed, 35 insertions(+), 13 deletions(-)
> 
> diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
> index 1c17948c..a768f88d 100644
> --- a/src/openvpn/misc.c
> +++ b/src/openvpn/misc.c
> @@ -765,4 +765,23 @@ output_peer_info_env(struct env_set *es, const char *peer_info)
>      }
>  }
>  
> +int
> +get_num_elements(const char *string, char delimiter)
> +{
> +    int string_len = strlen(string);
> +
> +    ASSERT(0 != string_len);
> +
> +    int element_count = 1;
> +    /* Get number of ciphers */
> +    for (int i = 0; i < string_len; i++)
> +    {
> +        if (string[i] == delimiter)
> +        {
> +            element_count++;
> +        }
> +    }
> +
> +    return element_count;
> +}
>  #endif /* P2MP_SERVER */
> diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
> index 991b7df2..2605c6d2 100644
> --- a/src/openvpn/misc.h
> +++ b/src/openvpn/misc.h
> @@ -175,4 +175,18 @@ void output_peer_info_env(struct env_set *es, const char *peer_info);
>  
>  #endif /* P2MP_SERVER */
>  
> +/**
> + * Returns the occurrences of 'delimiter' in a string +1
> + * This is typically used to find out the number elements in a
> + * cipher string or similar that is separated by : like
> + *
> + *   X25519:secp256r1:X448:secp512r1:secp384r1:brainpoolP384r1
> + *
> + * @param string        the string to work on
> + * @param delimiter     the delimiter to count, typically ':'
> + * @return              occrrences of delimiter + 1
> + */
> +int
> +get_num_elements(const char *string, char delimiter);
> +
>  #endif /* ifndef MISC_H */
> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> index 4f194ad7..51669278 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -289,33 +289,22 @@ void
>  tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
>  {
>      char *tmp_ciphers, *tmp_ciphers_orig, *token;
> -    int i, cipher_count;
> -    int ciphers_len;
>  
>      if (NULL == ciphers)
>      {
>          return; /* Nothing to do */
> -
>      }
> -    ciphers_len = strlen(ciphers);
>  
>      ASSERT(NULL != ctx);
> -    ASSERT(0 != ciphers_len);
>  
>      /* Get number of ciphers */
> -    for (i = 0, cipher_count = 1; i < ciphers_len; i++)
> -    {
> -        if (ciphers[i] == ':')
> -        {
> -            cipher_count++;
> -        }
> -    }
> +    int cipher_count = get_num_elements(ciphers, ':');
>  
>      /* Allocate an array for them */
>      ALLOC_ARRAY_CLEAR(ctx->allowed_ciphers, int, cipher_count+1)
>  
>      /* Parse allowed ciphers, getting IDs */
> -    i = 0;
> +    int i = 0;
>      tmp_ciphers_orig = tmp_ciphers = string_alloc(ciphers, NULL);
>  
>      token = strtok(tmp_ciphers, ":");
>
Gert Doering April 19, 2020, 12:15 a.m. UTC | #2 
Your patch has been applied to the master branch.

Nothing to add to Antonio's review :)

commit c577facffb09046da90c52f3ed1af5bdf7b25888
Author: Arne Schwabe
Date:   Thu Apr 16 17:26:18 2020 +0200

     Refactor counting number of element in a : delimited list into function

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Antonio Quartulli <antonio@openvpn.net>
     Message-Id: <20200416152619.5465-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19757.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 1c17948c..a768f88d 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -765,4 +765,23 @@  output_peer_info_env(struct env_set *es, const char *peer_info)
     }
 }
 
+int
+get_num_elements(const char *string, char delimiter)
+{
+    int string_len = strlen(string);
+
+    ASSERT(0 != string_len);
+
+    int element_count = 1;
+    /* Get number of ciphers */
+    for (int i = 0; i < string_len; i++)
+    {
+        if (string[i] == delimiter)
+        {
+            element_count++;
+        }
+    }
+
+    return element_count;
+}
 #endif /* P2MP_SERVER */
diff --git a/src/openvpn/misc.h b/src/openvpn/misc.h
index 991b7df2..2605c6d2 100644
--- a/src/openvpn/misc.h
+++ b/src/openvpn/misc.h
@@ -175,4 +175,18 @@  void output_peer_info_env(struct env_set *es, const char *peer_info);
 
 #endif /* P2MP_SERVER */
 
+/**
+ * Returns the occurrences of 'delimiter' in a string +1
+ * This is typically used to find out the number elements in a
+ * cipher string or similar that is separated by : like
+ *
+ *   X25519:secp256r1:X448:secp512r1:secp384r1:brainpoolP384r1
+ *
+ * @param string        the string to work on
+ * @param delimiter     the delimiter to count, typically ':'
+ * @return              occrrences of delimiter + 1
+ */
+int
+get_num_elements(const char *string, char delimiter);
+
 #endif /* ifndef MISC_H */
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 4f194ad7..51669278 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -289,33 +289,22 @@  void
 tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 {
     char *tmp_ciphers, *tmp_ciphers_orig, *token;
-    int i, cipher_count;
-    int ciphers_len;
 
     if (NULL == ciphers)
     {
         return; /* Nothing to do */
-
     }
-    ciphers_len = strlen(ciphers);
 
     ASSERT(NULL != ctx);
-    ASSERT(0 != ciphers_len);
 
     /* Get number of ciphers */
-    for (i = 0, cipher_count = 1; i < ciphers_len; i++)
-    {
-        if (ciphers[i] == ':')
-        {
-            cipher_count++;
-        }
-    }
+    int cipher_count = get_num_elements(ciphers, ':');
 
     /* Allocate an array for them */
     ALLOC_ARRAY_CLEAR(ctx->allowed_ciphers, int, cipher_count+1)
 
     /* Parse allowed ciphers, getting IDs */
-    i = 0;
+    int i = 0;
     tmp_ciphers_orig = tmp_ciphers = string_alloc(ciphers, NULL);
 
     token = strtok(tmp_ciphers, ":");