Hi,
Attached a v3 of the patch. This one also makes sure
tls_ctx_restrict_ciphers is always called, to prepare for the defaults
settings of 6/6 to always apply. In the previous version of the patch
--show-tls would ignore the default settings.
-Steffan
On 03-01-14 21:12, Steffan Karger wrote:
> Hi,
>
> Attached a v2 of the patch below, that removes the else to make the diff
> a lot smaller and changes a //-style comment to /* */-style.
>
> -Steffan
>
> On 01-01-14 21:10, Steffan Karger wrote:
>> This diff look like a lot has changed, but this just adds some ifs to check
>> for NULL in tls_ctx_restrict_ciphers() to prepare for disabling export
>> ciphers by default in OpenVPN 2.4+.
>>
>> Signed-off-by: Steffan Karger <stef...@karger.me>
>> ---
>> src/openvpn/ssl.c | 5 +-
>> src/openvpn/ssl_backend.h | 5 +-
>> src/openvpn/ssl_openssl.c | 130
>> ++++++++++++++++++++++++++-------------------
>> src/openvpn/ssl_polarssl.c | 7 ++-
>> 4 files changed, 85 insertions(+), 62 deletions(-)
>>
>> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
>> index bd19d75..93222c4 100644
>> --- a/src/openvpn/ssl.c
>> +++ b/src/openvpn/ssl.c
>> @@ -543,10 +543,7 @@ init_ssl (const struct options *options, struct
>> tls_root_ctx *new_ctx)
>> }
>>
>> /* Allowable ciphers */
>> - if (options->cipher_list)
>> - {
>> - tls_ctx_restrict_ciphers(new_ctx, options->cipher_list);
>> - }
>> + tls_ctx_restrict_ciphers(new_ctx, options->cipher_list);
>>
>> #ifdef ENABLE_CRYPTO_POLARSSL
>> /* Personalise the random by mixing in the certificate */
>> diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
>> index 54383fe..a6fc3bd 100644
>> --- a/src/openvpn/ssl_backend.h
>> +++ b/src/openvpn/ssl_backend.h
>> @@ -167,8 +167,9 @@ void tls_ctx_set_options (struct tls_root_ctx *ctx,
>> unsigned int ssl_flags);
>> /**
>> * Restrict the list of ciphers that can be used within the TLS context.
>> *
>> - * @param ctx TLS context to restrict
>> - * @param ciphers String containing : delimited cipher names.
>> + * @param ctx TLS context to restrict, must be valid.
>> + * @param ciphers String containing : delimited cipher names, or NULL to
>> use
>> + * sane defaults.
>> */
>> void tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char
>> *ciphers);
>>
>> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
>> index 08327a1..5f6c270 100644
>> --- a/src/openvpn/ssl_openssl.c
>> +++ b/src/openvpn/ssl_openssl.c
>> @@ -217,71 +217,91 @@ 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)
>> {
>> - size_t begin_of_cipher, end_of_cipher;
>> -
>> - const char *current_cipher;
>> - size_t current_cipher_len;
>> + if (ciphers == NULL)
>> + {
>> + /* Nothing to do */
>> + return;
>> + }
>> + else
>> + {
>> + /* Parse supplied cipher list and pass on to OpenSSL */
>> + size_t begin_of_cipher, end_of_cipher;
>>
>> - const tls_cipher_name_pair *cipher_pair;
>> + const char *current_cipher;
>> + size_t current_cipher_len;
>>
>> - char openssl_ciphers[4096];
>> - size_t openssl_ciphers_len = 0;
>> - openssl_ciphers[0] = '\0';
>> + const tls_cipher_name_pair *cipher_pair;
>>
>> - ASSERT(NULL != ctx);
>> + char openssl_ciphers[4096];
>> + size_t openssl_ciphers_len = 0;
>> + openssl_ciphers[0] = '\0';
>>
>> - // 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) {
>> - end_of_cipher += strcspn(&ciphers[begin_of_cipher], ":");
>> - cipher_pair = tls_get_cipher_name_pair(&ciphers[begin_of_cipher],
>> end_of_cipher - begin_of_cipher);
>> + ASSERT(NULL != ctx);
>>
>> - if (NULL == cipher_pair)
>> + // 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)
>> {
>> - // No translation found, use original
>> - current_cipher = &ciphers[begin_of_cipher];
>> - current_cipher_len = end_of_cipher - begin_of_cipher;
>> -
>> - // Issue warning on missing translation
>> - // %.*s format specifier expects length of type int, so guarantee
>> - // that length is small enough and cast to int.
>> - msg (M_WARN, "No valid translation found for TLS cipher '%.*s'",
>> - constrain_int(current_cipher_len, 0, 256), current_cipher);
>> - }
>> - else
>> - {
>> - // Use OpenSSL name
>> - current_cipher = cipher_pair->openssl_name;
>> - current_cipher_len = strlen(current_cipher);
>> -
>> - if (end_of_cipher - begin_of_cipher == current_cipher_len &&
>> - 0 == memcmp (&ciphers[begin_of_cipher],
>> cipher_pair->openssl_name, end_of_cipher - begin_of_cipher))
>> - {
>> - // Non-IANA name used, show warning
>> - msg (M_WARN, "Deprecated TLS cipher name '%s', please use IANA
>> name '%s'", cipher_pair->openssl_name, cipher_pair->iana_name);
>> - }
>> - }
>> -
>> - // Make sure new cipher name fits in cipher string
>> - if (((sizeof(openssl_ciphers)-1) - openssl_ciphers_len) <
>> current_cipher_len) {
>> - msg(M_SSLERR, "Failed to set restricted TLS cipher list, too long
>> (>%d).", (int)sizeof(openssl_ciphers)-1);
>> - }
>> + end_of_cipher += strcspn(&ciphers[begin_of_cipher], ":");
>> + cipher_pair = tls_get_cipher_name_pair(&ciphers[begin_of_cipher],
>> + end_of_cipher - begin_of_cipher);
>>
>> - // Concatenate cipher name to OpenSSL cipher string
>> - memcpy(&openssl_ciphers[openssl_ciphers_len], current_cipher,
>> current_cipher_len);
>> - openssl_ciphers_len += current_cipher_len;
>> - openssl_ciphers[openssl_ciphers_len] = ':';
>> - openssl_ciphers_len++;
>> + if (NULL == cipher_pair)
>> + {
>> + // No translation found, use original
>> + current_cipher = &ciphers[begin_of_cipher];
>> + current_cipher_len = end_of_cipher - begin_of_cipher;
>> +
>> + // Issue warning on missing translation
>> + // %.*s format specifier expects length of type int, so
>> guarantee
>> + // that length is small enough and cast to int.
>> + msg (M_WARN, "No valid translation found for TLS cipher
>> '%.*s'",
>> + constrain_int(current_cipher_len, 0, 256),
>> current_cipher);
>> + }
>> + else
>> + {
>> + // Use OpenSSL name
>> + current_cipher = cipher_pair->openssl_name;
>> + current_cipher_len = strlen(current_cipher);
>> +
>> + if (end_of_cipher - begin_of_cipher == current_cipher_len &&
>> + 0 == memcmp (&ciphers[begin_of_cipher],
>> + cipher_pair->openssl_name,
>> + end_of_cipher - begin_of_cipher))
>> + {
>> + // Non-IANA name used, show warning
>> + msg (M_WARN, "Deprecated TLS cipher name '%s', "
>> + "please use IANA name '%s'",
>> cipher_pair->openssl_name,
>> + cipher_pair->iana_name);
>> + }
>> + }
>>
>> - end_of_cipher++;
>> - }
>> + // Make sure new cipher name fits in cipher string
>> + if (((sizeof(openssl_ciphers)-1) - openssl_ciphers_len) <
>> + current_cipher_len) {
>> + msg(M_SSLERR,
>> + "Failed to set restricted TLS cipher list, too long (>%d).",
>> + (int)sizeof(openssl_ciphers)-1);
>> + }
>> +
>> + // Concatenate cipher name to OpenSSL cipher string
>> + memcpy(&openssl_ciphers[openssl_ciphers_len], current_cipher,
>> + current_cipher_len);
>> + openssl_ciphers_len += current_cipher_len;
>> + openssl_ciphers[openssl_ciphers_len] = ':';
>> + openssl_ciphers_len++;
>> +
>> + end_of_cipher++;
>> + }
>>
>> - if (openssl_ciphers_len > 0)
>> - openssl_ciphers[openssl_ciphers_len-1] = '\0';
>> + if (openssl_ciphers_len > 0)
>> + openssl_ciphers[openssl_ciphers_len-1] = '\0';
>>
>> - // Set OpenSSL cipher list
>> - if(!SSL_CTX_set_cipher_list(ctx->ctx, openssl_ciphers))
>> - msg(M_SSLERR, "Failed to set restricted TLS cipher list: %s",
>> openssl_ciphers);
>> + // Set OpenSSL cipher list
>> + if(!SSL_CTX_set_cipher_list(ctx->ctx, openssl_ciphers))
>> + msg(M_SSLERR, "Failed to set restricted TLS cipher list: %s",
>> + openssl_ciphers);
>> + }
>> }
>>
>> void
>> diff --git a/src/openvpn/ssl_polarssl.c b/src/openvpn/ssl_polarssl.c
>> index 551c352..d964b91 100644
>> --- a/src/openvpn/ssl_polarssl.c
>> +++ b/src/openvpn/ssl_polarssl.c
>> @@ -173,7 +173,12 @@ 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 = strlen (ciphers);
>> + int ciphers_len;
>> +
>> + if (NULL == ciphers)
>> + return; // Nothing to do
>> +
>> + ciphers_len = strlen (ciphers);
>>
>> ASSERT (NULL != ctx);
>> ASSERT (0 != ciphers_len);
>>
>From 8fe4c2668aac6f69374682af2eb1579e100f2978 Mon Sep 17 00:00:00 2001
From: Steffan Karger <stef...@karger.me>
List-Post: openvpn-devel@lists.sourceforge.net
Date: Fri, 3 Jan 2014 21:03:02 +0100
Subject: [PATCH 1/2] Make tls_ctx_restrict_ciphers accept NULL as char
*cipher_list.
This adds some ifs to check for NULL in tls_ctx_restrict_ciphers() to prepare
for disabling export ciphers by default in OpenVPN 2.4+.
Also let tls_ctx_restrict_ciphers always be called, also when *cipher_list is
NULL.
Signed-off-by: Steffan Karger <stef...@karger.me>
---
src/openvpn/ssl.c | 5 +----
src/openvpn/ssl_backend.h | 5 +++--
src/openvpn/ssl_openssl.c | 10 ++++++++--
src/openvpn/ssl_polarssl.c | 16 ++++++++++++----
4 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index bd19d75..93222c4 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -543,10 +543,7 @@ init_ssl (const struct options *options, struct tls_root_ctx *new_ctx)
}
/* Allowable ciphers */
- if (options->cipher_list)
- {
- tls_ctx_restrict_ciphers(new_ctx, options->cipher_list);
- }
+ tls_ctx_restrict_ciphers(new_ctx, options->cipher_list);
#ifdef ENABLE_CRYPTO_POLARSSL
/* Personalise the random by mixing in the certificate */
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 54383fe..a6fc3bd 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -167,8 +167,9 @@ void tls_ctx_set_options (struct tls_root_ctx *ctx, unsigned int ssl_flags);
/**
* Restrict the list of ciphers that can be used within the TLS context.
*
- * @param ctx TLS context to restrict
- * @param ciphers String containing : delimited cipher names.
+ * @param ctx TLS context to restrict, must be valid.
+ * @param ciphers String containing : delimited cipher names, or NULL to use
+ * sane defaults.
*/
void tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const char *ciphers);
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 08327a1..7ad7eab 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -217,6 +217,13 @@ 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)
{
+ if (ciphers == NULL)
+ {
+ /* Nothing to do */
+ return;
+ }
+
+ /* Parse supplied cipher list and pass on to OpenSSL */
size_t begin_of_cipher, end_of_cipher;
const char *current_cipher;
@@ -1272,8 +1279,7 @@ show_available_tls_ciphers (const char *cipher_list)
if (!ssl)
msg (M_SSLERR, "Cannot create SSL object");
- if (cipher_list)
- tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
+ tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
printf ("Available TLS Ciphers,\n");
printf ("listed in order of preference:\n\n");
diff --git a/src/openvpn/ssl_polarssl.c b/src/openvpn/ssl_polarssl.c
index 551c352..47fb62a 100644
--- a/src/openvpn/ssl_polarssl.c
+++ b/src/openvpn/ssl_polarssl.c
@@ -173,7 +173,12 @@ 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 = strlen (ciphers);
+ int ciphers_len;
+
+ if (NULL == ciphers)
+ return; /* Nothing to do */
+
+ ciphers_len = strlen (ciphers);
ASSERT (NULL != ctx);
ASSERT (0 != ciphers_len);
@@ -1038,10 +1043,11 @@ show_available_tls_ciphers (const char *cipher_list)
struct tls_root_ctx tls_ctx;
const int *ciphers = ssl_list_ciphersuites();
- if (cipher_list) {
- tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
+ tls_ctx_server_new(&tls_ctx);
+ tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
+
+ if (tls_ctx.allowed_ciphers)
ciphers = tls_ctx.allowed_ciphers;
- }
#ifndef ENABLE_SMALL
printf ("Available TLS Ciphers,\n");
@@ -1054,6 +1060,8 @@ show_available_tls_ciphers (const char *cipher_list)
ciphers++;
}
printf ("\n");
+
+ tls_ctx_free(&tls_ctx);
}
void
--
1.8.3.2
