Improve ciphersuite order stability when disabling ciphersuites.
Change ssl_create_cipher_list() to prefer ephemeral ECDH over
ephemeral DH.
diff --git a/CHANGES b/CHANGES
index 44200d6..837cce4 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,27 @@
Changes between 0.9.8e and 0.9.9 [xx XXX xxxx]
+ *) Change ssl_cipher_apply_rule(), the internal function that does
+ the work each time a ciphersuite string requests enabling
+ ("foo+bar"), moving ("+foo+bar"), disabling ("-foo+bar", or
+ removing ("!foo+bar") a class of ciphersuites: Now it maintains
+ the order of disabled ciphersuites such that those ciphersuites
+ that most recently went from enabled to disabled not only stay
+ in order with respect to each other, but also have higher priority
+ than other disabled ciphersuites the next time ciphersuites are
+ enabled again.
+
+ This means that you can now say, e.g., "PSK:-PSK:HIGH" to enable
+ the same ciphersuites as with "HIGH" alone, but in a specific
+ order where the PSK ciphersuites come first (since they are the
+ most recently disabled ciphersuites when "HIGH" is parsed).
+
+ Also, change ssl_create_cipher_list() (using this new
+ funcionality) such that between otherwise identical
+ cihpersuites, ephemeral ECDH is preferred over ephemeral DH in
+ the default order.
+ [Bodo Moeller]
+
*) Change ssl_create_cipher_list() so that it automatically
arranges the ciphersuites in reasonable order before starting
to process the rule string. Thus, the definition for "DEFAULT"
diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c
index a9d11ca..fdfc0e8 100644
--- a/ssl/ssl_ciph.c
+++ b/ssl/ssl_ciph.c
@@ -476,7 +476,7 @@
*head=curr->next;
if (curr->prev != NULL)
curr->prev->next=curr->next;
- if (curr->next != NULL) /* should always be true */
+ if (curr->next != NULL)
curr->next->prev=curr->prev;
(*tail)->next=curr;
curr->prev= *tail;
@@ -484,6 +484,22 @@
*tail=curr;
}
+static void ll_append_head(CIPHER_ORDER **head, CIPHER_ORDER *curr,
+ CIPHER_ORDER **tail)
+ {
+ if (curr == *head) return;
+ if (curr == *tail)
+ *tail=curr->prev;
+ if (curr->next != NULL)
+ curr->next->prev=curr->prev;
+ if (curr->prev != NULL)
+ curr->prev->next=curr->next;
+ (*head)->prev=curr;
+ curr->next= *head;
+ curr->prev=NULL;
+ *head=curr;
+ }
+
static void ssl_cipher_get_disabled(unsigned long *mkey, unsigned long *auth, unsigned long *enc, unsigned long *mac, unsigned long *ssl)
{
*mkey = 0;
@@ -586,19 +602,27 @@
/*
* Prepare linked list from list entries
*/
- for (i = 1; i < co_list_num - 1; i++)
- {
- co_list[i].prev = &(co_list[i-1]);
- co_list[i].next = &(co_list[i+1]);
- }
if (co_list_num > 0)
{
- (*head_p) = &(co_list[0]);
- (*head_p)->prev = NULL;
- (*head_p)->next = &(co_list[1]);
- (*tail_p) = &(co_list[co_list_num - 1]);
- (*tail_p)->prev = &(co_list[co_list_num - 2]);
- (*tail_p)->next = NULL;
+ co_list[0].prev = NULL;
+
+ if (co_list_num > 1)
+ {
+ co_list[0].next = &co_list[1];
+
+ for (i = 1; i < co_list_num - 1; i++)
+ {
+ co_list[i].prev = &co_list[i - 1];
+ co_list[i].next = &co_list[i + 1];
+ }
+
+ co_list[co_list_num - 1].prev = &co_list[co_list_num - 2];
+ }
+
+ co_list[co_list_num - 1].next = NULL;
+
+ *head_p = &co_list[0];
+ *tail_p = &co_list[co_list_num - 1];
}
}
@@ -679,22 +703,38 @@
int rule, int strength_bits,
CIPHER_ORDER **head_p, CIPHER_ORDER **tail_p)
{
- CIPHER_ORDER *head, *tail, *curr, *curr2, *tail2;
+ CIPHER_ORDER *head, *tail, *curr, *curr2, *last;
SSL_CIPHER *cp;
+ int reverse = 0;
#ifdef CIPHER_DEBUG
printf("Applying rule %d with %08lx/%08lx/%08lx/%08lx/%08lx %08lx (%d)\n",
rule, alg_mkey, alg_auth, alg_enc, alg_mac, alg_ssl, algo_strength, strength_bits);
#endif
- curr = head = *head_p;
- curr2 = head;
- tail2 = tail = *tail_p;
+ if (rule == CIPHER_DEL)
+ reverse = 1; /* needed to maintain sorting between currently deleted ciphers */
+
+ head = *head_p;
+ tail = *tail_p;
+
+ if (reverse)
+ {
+ curr = tail;
+ last = head;
+ }
+ else
+ {
+ curr = head;
+ last = tail;
+ }
+
+ curr2 = curr;
for (;;)
{
- if ((curr == NULL) || (curr == tail2)) break;
+ if ((curr == NULL) || (curr == last)) break;
curr = curr2;
- curr2 = curr->next;
+ curr2 = reverse ? curr->prev : curr->next;
cp = curr->cipher;
@@ -736,6 +776,7 @@
/* add the cipher if it has not been added yet. */
if (rule == CIPHER_ADD)
{
+ /* reverse == 0 */
if (!curr->active)
{
ll_append_tail(&head, curr, &tail);
@@ -745,15 +786,27 @@
/* Move the added cipher to this location */
else if (rule == CIPHER_ORD)
{
+ /* reverse == 0 */
if (curr->active)
{
ll_append_tail(&head, curr, &tail);
}
}
else if (rule == CIPHER_DEL)
- curr->active = 0;
+ {
+ /* reverse == 1 */
+ if (curr->active)
+ {
+ /* most recently deleted ciphersuites get best positions
+ * for any future CIPHER_ADD (note that the CIPHER_DEL loop
+ * works in reverse to maintain the order) */
+ ll_append_head(&head, curr, &tail);
+ curr->active = 0;
+ }
+ }
else if (rule == CIPHER_KILL)
{
+ /* reverse == 0 */
if (head == curr)
head = curr->next;
else
@@ -1123,7 +1176,11 @@
/* Now arrange all ciphers by preference: */
- /* Temporarily enabled AES first (preferred cipher) */
+ /* Everything else being equal, prefer ephemeral ECDH over other key exchange mechanisms */
+ ssl_cipher_apply_rule(0, SSL_kEECDH, 0, 0, 0, 0, 0, CIPHER_ADD, -1, &head, &tail);
+ ssl_cipher_apply_rule(0, SSL_kEECDH, 0, 0, 0, 0, 0, CIPHER_DEL, -1, &head, &tail);
+
+ /* Temporarily enable AES first (preferred cipher) */
ssl_cipher_apply_rule(0, 0, 0, SSL_AES, 0, 0, 0, CIPHER_ADD, -1, &head, &tail);
/* Temporarily enable everything else */
