[gnutls-devel] [RFC] Relaxing cipher suite (priority) string requirements
Jouko Orava
jouko.orava at helsinki.fi
Thu Jan 24 02:40:01 CET 2013
Hi!
GnuTLS is very, very picky about the cipher suite strings it accepts.
I wrote a test patch (attached), UNTESTED, that should relax the
requirements. The main points are:
- Make '+' prefix optional
- Allow full cipher names, adding/removing cipher, mac, and kx
(I suspect it would be better to add all three, but
when removing, only remove the cipher.
Currently the patch adds/removes all three.)
The patch also includes a change into lib/gnutls_priority.c:prio_remove(),
that instead of replacing the removed item with the final item in the
array, uses memmove() to shorten the array. For some reason I seem
to believe the order in these priority lists matter.
I'm not exactly sure if this patch is in the right direction,
but I'm hoping to raise some discussion on how to make the
cipher suite string parsing easier for administrators.
Best regards,
Jouko Orava
-------------- next part --------------
commit 4665b1c975aaf0165bda6c02ae59d0f3a2782040
Author: Jouko Orava <jouko.orava at iki.fi>
Date: Thu Jan 24 02:46:43 2013 +0200
Extend cipher suite priority parsing to make '+' prefix optional,
and to allow users to use the full name strings to add or remove
cipher, mac, and key exchanges easier.
diff --git a/lib/gnutls_priority.c b/lib/gnutls_priority.c
index 9f47f0a..67c8c9f 100644
--- a/lib/gnutls_priority.c
+++ b/lib/gnutls_priority.c
@@ -547,8 +547,10 @@ prio_remove (priority_st * priority_list, unsigned int algo)
if (pos >= 0)
{
- priority_list->priority[pos] = priority_list->priority[i - 1];
- priority_list->priority[i - 1] = 0;
+ if (pos < i - 1)
+ memmove (priority_list->priority + pos,
+ priority_list->priority + pos + 1,
+ (i - 1 - pos) * sizeof priority_list->priority[0]);
priority_list->algorithms--;
}
@@ -762,6 +764,8 @@ bulk_rmadd_func *func;
* Special keywords are "!", "-" and "+".
* "!" or "-" appended with an algorithm will remove this algorithm.
* "+" appended with an algorithm will add this algorithm.
+ * Algorithms without special keywords, and beginning with an ASCII
+ * letter, will add this algorithm. (Thus, "+" is optional.)
*
* Check the GnuTLS manual section "Priority strings" for detailed
* information.
@@ -792,8 +796,8 @@ gnutls_priority_init (gnutls_priority_t * priority_cache,
{
char *broken_list[MAX_ELEMENTS];
int broken_list_size = 0, i = 0, j;
- char *darg = NULL;
- int algo;
+ char *darg = NULL, *s;
+ int algo, add;
rmadd_func *fn;
bulk_rmadd_func *bulk_fn;
@@ -850,43 +854,52 @@ gnutls_priority_init (gnutls_priority_t * priority_cache,
continue;
}
else if (broken_list[i][0] == '!' || broken_list[i][0] == '+'
- || broken_list[i][0] == '-')
+ || broken_list[i][0] == '-'
+ || (broken_list[i][0] >= 'A' && broken_list[i] <= 'Z')
+ || (broken_list[i][0] >= 'a' && broken_list[i] <= 'z'))
{
if (broken_list[i][0] == '+')
{
fn = prio_add;
bulk_fn = _set_priority;
+ s = &broken_list[i][1];
+ add = 1;
}
- else
+ else if (broken_list[i][0] == '!' || broken_list[i][0] == '-')
{
fn = prio_remove;
bulk_fn = _clear_priorities;
+ s = &broken_list[i][1];
+ add = 0;
+ }
+ else
+ {
+ fn = prio_add;
+ bulk_fn = _set_priority;
+ s = &broken_list[i][0];
+ add = 1;
}
- if (broken_list[i][0] == '+' && check_level(&broken_list[i][1], *priority_cache, 1) != 0)
+ if (add && check_level(s, *priority_cache, 1) != 0)
{
continue;
}
- else if ((algo =
- gnutls_mac_get_id (&broken_list[i][1])) != GNUTLS_MAC_UNKNOWN)
+ else if ((algo = gnutls_mac_get_id (s)) != GNUTLS_MAC_UNKNOWN)
fn (&(*priority_cache)->mac, algo);
- else if ((algo = gnutls_cipher_get_id (&broken_list[i][1])) !=
- GNUTLS_CIPHER_UNKNOWN)
+ else if ((algo = gnutls_cipher_get_id (s)) != GNUTLS_CIPHER_UNKNOWN)
fn (&(*priority_cache)->cipher, algo);
- else if ((algo = gnutls_kx_get_id (&broken_list[i][1])) !=
- GNUTLS_KX_UNKNOWN)
+ else if ((algo = gnutls_kx_get_id (s)) != GNUTLS_KX_UNKNOWN)
fn (&(*priority_cache)->kx, algo);
- else if (strncasecmp (&broken_list[i][1], "VERS-", 5) == 0)
+ else if (strncasecmp (s, "VERS-", 5) == 0)
{
- if (strncasecmp (&broken_list[i][1], "VERS-TLS-ALL", 12) == 0)
+ if (strncasecmp (s, "VERS-TLS-ALL", 12) == 0)
{
bulk_fn (&(*priority_cache)->protocol,
protocol_priority);
}
else
{
- if ((algo =
- gnutls_protocol_get_id (&broken_list[i][6])) !=
+ if ((algo = gnutls_protocol_get_id (s + 5) !=
GNUTLS_VERSION_UNKNOWN)
fn (&(*priority_cache)->protocol, algo);
else
@@ -894,91 +907,103 @@ gnutls_priority_init (gnutls_priority_t * priority_cache,
}
} /* now check if the element is something like -ALGO */
- else if (strncasecmp (&broken_list[i][1], "COMP-", 5) == 0)
+ else if (strncasecmp (s, "COMP-", 5) == 0)
{
- if (strncasecmp (&broken_list[i][1], "COMP-ALL", 8) == 0)
+ if (strncasecmp (s, "COMP-ALL", 8) == 0)
{
bulk_fn (&(*priority_cache)->compression,
comp_priority);
}
else
{
- if ((algo =
- gnutls_compression_get_id (&broken_list[i][6])) !=
+ if ((algo = gnutls_compression_get_id (s + 5)) !=
GNUTLS_COMP_UNKNOWN)
fn (&(*priority_cache)->compression, algo);
else
goto error;
}
} /* now check if the element is something like -ALGO */
- else if (strncasecmp (&broken_list[i][1], "CURVE-", 6) == 0)
+ else if (strncasecmp (s, "CURVE-", 6) == 0)
{
- if (strncasecmp (&broken_list[i][1], "CURVE-ALL", 9) == 0)
+ if (strncasecmp (s, "CURVE-ALL", 9) == 0)
{
bulk_fn (&(*priority_cache)->supported_ecc,
supported_ecc_normal);
}
else
{
- if ((algo =
- _gnutls_ecc_curve_get_id (&broken_list[i][7])) !=
+ if ((algo = _gnutls_ecc_curve_get_id (s + 6)) !=
GNUTLS_ECC_CURVE_INVALID)
fn (&(*priority_cache)->supported_ecc, algo);
else
goto error;
}
} /* now check if the element is something like -ALGO */
- else if (strncasecmp (&broken_list[i][1], "CTYPE-", 6) == 0)
+ else if (strncasecmp (s, "CTYPE-", 6) == 0)
{
- if (strncasecmp (&broken_list[i][1], "CTYPE-ALL", 9) == 0)
+ if (strncasecmp (s, "CTYPE-ALL", 9) == 0)
{
bulk_fn (&(*priority_cache)->cert_type,
cert_type_priority_all);
}
else
{
- if ((algo =
- gnutls_certificate_type_get_id (&broken_list[i][7])) !=
+ if ((algo = gnutls_certificate_type_get_id (s + 6)) !=
GNUTLS_CRT_UNKNOWN)
fn (&(*priority_cache)->cert_type, algo);
else
goto error;
}
} /* now check if the element is something like -ALGO */
- else if (strncasecmp (&broken_list[i][1], "SIGN-", 5) == 0)
+ else if (strncasecmp (s, "SIGN-", 5) == 0)
{
- if (strncasecmp (&broken_list[i][1], "SIGN-ALL", 8) == 0)
+ if (strncasecmp (s, "SIGN-ALL", 8) == 0)
{
bulk_fn (&(*priority_cache)->sign_algo,
sign_priority_default);
}
else
{
- if ((algo =
- gnutls_sign_get_id (&broken_list[i][6])) !=
+ if ((algo = gnutls_sign_get_id (s + 5)) !=
GNUTLS_SIGN_UNKNOWN)
fn (&(*priority_cache)->sign_algo, algo);
else
goto error;
}
}
- else if (strncasecmp (&broken_list[i][1], "MAC-ALL", 7) == 0)
+ else if (strncasecmp (s, "MAC-ALL", 7) == 0)
{
bulk_fn (&(*priority_cache)->mac,
mac_priority_normal);
}
- else if (strncasecmp (&broken_list[i][1], "CIPHER-ALL", 10) == 0)
+ else if (strncasecmp (s, "CIPHER-ALL", 10) == 0)
{
bulk_fn (&(*priority_cache)->cipher,
cipher_priority_normal);
}
- else if (strncasecmp (&broken_list[i][1], "KX-ALL", 6) == 0)
+ else if (strncasecmp (s, "KX-ALL", 6) == 0)
{
bulk_fn (&(*priority_cache)->kx,
kx_priority_secure);
}
else
- goto error;
+ {
+ for (j = 0; cs_algorithms[j].name != NULL; j++)
+ {
+ if (strcasecmp (s, cs_algorithms[j].name + 7) == 0)
+ {
+ fn (&(*priority_cache)->mac,
+ cs_algorithms[j].mac_algorithm);
+ fn (&(*priority_cache)->cipher,
+ cs_algorithms[j].cipher_algorithm);
+ fn (&(*priority_cache)->kx,
+ cs_algorithms[j].kx_algorithm);
+ break;
+ }
+ }
+ if (cs_algorithms[j].name == NULL)
+ goto error;
+ }
}
else if (broken_list[i][0] == '%')
{
More information about the Gnutls-devel
mailing list