diff mbox series

[Openvpn-devel,1/3] Factor out convert_tls_list_to_openssl method

Message ID 20181006080617.18136-1-arne@rfc2549.org
State Accepted
Headers
Series [Openvpn-devel,1/3] Factor out convert_tls_list_to_openssl method |

Commit Message

Arne Schwabe Oct. 5, 2018, 10:06 p.m. UTC 
  This makes the tls_ctx_restrict_ciphers function more readable and
clean ups the code a bit more.
---
 src/openvpn/ssl_openssl.c | 57 +++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

Comments

Steffan Karger Oct. 5, 2018, 11:04 p.m. UTC | #1 
Hi,

Two minor nits:

On 06-10-18 10:06, Arne Schwabe wrote:
> This makes the tls_ctx_restrict_ciphers function more readable and
> clean ups the code a bit more.

The signed-off-by tag is missing.

>                  "Failed to set restricted TLS cipher list, too long (>%d).",
> -                (int)sizeof(openssl_ciphers)-1);
> +                  (int)(len - 1));

This breaks the indenting alignment, there should be two less spaces.

Otherwise this makes sense and does not introduce any logical changes.

Acked-by: Steffan Karger <steffan@karger.me>

-Steffan
Gert Doering Oct. 6, 2018, 2:48 a.m. UTC | #2 
Your patch has been applied to the master branch.

Whitespace and signed-off-by line have been adjusted as instructed.

commit 3b9d4d2a9aa89f9c21870a97bcdb42bb007e3ac0
Author: Arne Schwabe
Date:   Sat Oct 6 10:06:15 2018 +0200

     Factor out convert_tls_list_to_openssl method

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Steffan Karger <steffan.karger@fox-it.com>
     Message-Id: <20181006080617.18136-1-arne@rfc2549.org>
     URL: https://www.mail-archive.com/search?l=mid&q=20181006080617.18136-1-arne@rfc2549.org
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 3aac216c..1008396d 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -323,28 +323,8 @@  tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags)
 }
 
 void
-tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
+convert_tls_list_to_openssl(char* openssl_ciphers, size_t len,const char *ciphers)
 {
-    if (ciphers == NULL)
-    {
-        /* Use sane default TLS cipher list */
-        if (!SSL_CTX_set_cipher_list(ctx->ctx,
-                                     /* Use openssl's default list as a basis */
-                                     "DEFAULT"
-                                     /* Disable export ciphers and openssl's 'low' and 'medium' ciphers */
-                                     ":!EXP:!LOW:!MEDIUM"
-                                     /* Disable static (EC)DH keys (no forward secrecy) */
-                                     ":!kDH:!kECDH"
-                                     /* Disable DSA private keys */
-                                     ":!DSS"
-                                     /* Disable unsupported TLS modes */
-                                     ":!PSK:!SRP:!kRSA"))
-        {
-            crypto_msg(M_FATAL, "Failed to set default TLS cipher list.");
-        }
-        return;
-    }
-
     /* Parse supplied cipher list and pass on to OpenSSL */
     size_t begin_of_cipher, end_of_cipher;
 
@@ -353,12 +333,9 @@  tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
     const tls_cipher_name_pair *cipher_pair;
 
-    char openssl_ciphers[4096];
     size_t openssl_ciphers_len = 0;
     openssl_ciphers[0] = '\0';
 
-    ASSERT(NULL != ctx);
-
     /* Translate IANA cipher suite names to OpenSSL names */
     begin_of_cipher = end_of_cipher = 0;
     for (; begin_of_cipher < strlen(ciphers); begin_of_cipher = end_of_cipher)
@@ -395,11 +372,11 @@  tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
 
         /* Make sure new cipher name fits in cipher string */
         if ((SIZE_MAX - openssl_ciphers_len) < current_cipher_len
-            || ((sizeof(openssl_ciphers)-1) < openssl_ciphers_len + current_cipher_len))
+            || (len - 1) < (openssl_ciphers_len + current_cipher_len))
         {
             msg(M_FATAL,
                 "Failed to set restricted TLS cipher list, too long (>%d).",
-                (int)sizeof(openssl_ciphers)-1);
+                  (int)(len - 1));
         }
 
         /* Concatenate cipher name to OpenSSL cipher string */
@@ -415,6 +392,34 @@  tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
     {
         openssl_ciphers[openssl_ciphers_len-1] = '\0';
     }
+}
+void
+tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers)
+{
+    if (ciphers == NULL)
+    {
+        /* Use sane default TLS cipher list */
+        if (!SSL_CTX_set_cipher_list(ctx->ctx,
+                                     /* Use openssl's default list as a basis */
+                                     "DEFAULT"
+                                     /* Disable export ciphers and openssl's 'low' and 'medium' ciphers */
+                                     ":!EXP:!LOW:!MEDIUM"
+                                     /* Disable static (EC)DH keys (no forward secrecy) */
+                                     ":!kDH:!kECDH"
+                                     /* Disable DSA private keys */
+                                     ":!DSS"
+                                     /* Disable unsupported TLS modes */
+                                     ":!PSK:!SRP:!kRSA"))
+        {
+            crypto_msg(M_FATAL, "Failed to set default TLS cipher list.");
+        }
+        return;
+    }
+
+    char openssl_ciphers[4096];
+    convert_tls_list_to_openssl(openssl_ciphers, sizeof(openssl_ciphers), ciphers);
+
+    ASSERT(NULL != ctx);
 
     /* Set OpenSSL cipher list */
     if (!SSL_CTX_set_cipher_list(ctx->ctx, openssl_ciphers))