Replace cipherlist test

The old cipherlist test in ssltest.c only tests the internal order of
the cipher table, which is pretty useless.

Replace this test with a test that catches inadvertent changes to the
default cipherlist.

Fix run_tests.pl to correctly filter tests that have "list" in their name.

(Also includes a small drive-by fix in .gitignore.)
Reviewed-by: NRich Salz <rsalz@openssl.org>

.gitignore

@@ -79,6 +79,7 @@ Makefile
 /test/fips_ecdsavs
 /test/fips_rngvs
 /test/fips_test_suite
+/test/ssltest_old
 *.so*
 *.dylib*
 *.dll*

test/build.info

@@ -16,7 +16,7 @@ IF[{- !$disabled{tests} -}]
           constant_time_test verify_extra_test clienthellotest \
           packettest asynctest secmemtest srptest memleaktest \
           dtlsv1listentest ct_test threadstest afalgtest d2i_test \
-          ssl_test_ctx_test ssl_test x509aux
+          ssl_test_ctx_test ssl_test x509aux cipherlist_test
 
   SOURCE[aborttest]=aborttest.c
   INCLUDE[aborttest]={- rel2abs(catdir($builddir,"../include")) -} ../include
@@ -234,6 +234,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[ssl_test]={- rel2abs(catdir($builddir,"../include")) -} .. ../include
   DEPEND[ssl_test]=../libcrypto ../libssl
 
+  SOURCE[cipherlist_test]=cipherlist_test.c testutil.c
+  INCLUDE[cipherlist_test]={- rel2abs(catdir($builddir,"../include")) -} .. ../include
+  DEPEND[cipherlist_test]=../libcrypto ../libssl
+
   INCLUDE[testutil.o]=..
   INCLUDE[ssl_test_ctx.o]={- rel2abs(catdir($builddir,"../include")) -} ../include
   INCLUDE[handshake_helper.o]={- rel2abs(catdir($builddir,"../include")) -} ../include

test/cipherlist_test.c

+/*
+ * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the OpenSSL licenses, (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * https://www.openssl.org/source/license.html
+ * or in the file LICENSE in the source distribution.
+ */
+
+#include <stdio.h>
+
+#include <openssl/opensslconf.h>
+#include <openssl/err.h>
+#include <openssl/e_os2.h>
+#include <openssl/ssl.h>
+#include <openssl/ssl3.h>
+#include <openssl/tls1.h>
+
+#include "e_os.h"
+#include "testutil.h"
+
+typedef struct cipherlist_test_fixture {
+    const char *test_case_name;
+    SSL_CTX *server;
+    SSL_CTX *client;
+} CIPHERLIST_TEST_FIXTURE;
+
+
+static CIPHERLIST_TEST_FIXTURE set_up(const char *const test_case_name)
+{
+    CIPHERLIST_TEST_FIXTURE fixture;
+    fixture.test_case_name = test_case_name;
+    fixture.server = SSL_CTX_new(TLS_server_method());
+    fixture.client = SSL_CTX_new(TLS_client_method());
+    OPENSSL_assert(fixture.client != NULL && fixture.server != NULL);
+    return fixture;
+}
+
+/*
+ * All ciphers in the DEFAULT cipherlist meet the default security level.
+ * However, default supported ciphers exclude SRP and PSK ciphersuites
+ * for which no callbacks have been set up.
+ *
+ * Supported ciphers also exclude TLSv1.2 ciphers if TLSv1.2 is disabled,
+ * and individual disabled algorithms. However, NO_RSA, NO_AES and NO_SHA
+ * are currently broken and should be considered mission impossible in libssl.
+ */
+static const uint32_t default_ciphers_in_order[] = {
+#ifndef OPENSSL_NO_TLS1_2
+# ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+    TLS1_CK_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+# endif
+# ifndef OPENSSL_NO_DH
+    TLS1_CK_DHE_RSA_WITH_AES_256_GCM_SHA384,
+# endif
+
+# if !defined OPENSSL_NO_CHACHA && !defined OPENSSL_NO_POLY1305
+#  ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+    TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305,
+#  endif
+#  ifndef OPENSSL_NO_DH
+    TLS1_CK_DHE_RSA_WITH_CHACHA20_POLY1305,
+#  endif
+# endif  /* !OPENSSL_NO_CHACHA && !OPENSSL_NO_POLY1305 */
+
+# ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+    TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+# endif
+# ifndef OPENSSL_NO_DH
+    TLS1_CK_DHE_RSA_WITH_AES_128_GCM_SHA256,
+# endif
+# ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_AES_256_SHA384,
+    TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384,
+# endif
+# ifndef OPENSSL_NO_DH
+    TLS1_CK_DHE_RSA_WITH_AES_256_SHA256,
+# endif
+# ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_AES_128_SHA256,
+    TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256,
+# endif
+# ifndef OPENSSL_NO_DH
+    TLS1_CK_DHE_RSA_WITH_AES_128_SHA256,
+# endif
+#endif  /* !OPENSSL_NO_TLS1_2 */
+
+#ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
+    TLS1_CK_ECDHE_RSA_WITH_AES_256_CBC_SHA,
+#endif
+#ifndef OPENSSL_NO_DH
+    TLS1_CK_DHE_RSA_WITH_AES_256_SHA,
+#endif
+#ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
+    TLS1_CK_ECDHE_RSA_WITH_AES_128_CBC_SHA,
+#endif
+#ifndef OPENSSL_NO_DH
+    TLS1_CK_DHE_RSA_WITH_AES_128_SHA,
+#endif
+
+#ifndef OPENSSL_NO_DES
+# ifndef OPENSSL_NO_EC
+    TLS1_CK_ECDHE_ECDSA_WITH_DES_192_CBC3_SHA,
+    TLS1_CK_ECDHE_RSA_WITH_DES_192_CBC3_SHA,
+# endif
+# ifndef OPENSSL_NO_DH
+    SSL3_CK_DHE_RSA_DES_192_CBC3_SHA,
+# endif
+#endif  /* !OPENSSL_NO_DES */
+
+#ifndef OPENSSL_NO_TLS1_2
+    TLS1_CK_RSA_WITH_AES_256_GCM_SHA384,
+    TLS1_CK_RSA_WITH_AES_128_GCM_SHA256,
+    TLS1_CK_RSA_WITH_AES_256_SHA256,
+    TLS1_CK_RSA_WITH_AES_128_SHA256,
+#endif
+
+    TLS1_CK_RSA_WITH_AES_256_SHA,
+    TLS1_CK_RSA_WITH_AES_128_SHA,
+#ifndef OPENSSL_NO_DES
+    SSL3_CK_RSA_DES_192_CBC3_SHA,
+#endif
+};
+
+static int test_default_cipherlist(SSL_CTX *ctx)
+{
+    STACK_OF(SSL_CIPHER) *ciphers;
+    SSL *ssl;
+    int i, ret = 0, num_expected_ciphers, num_ciphers;
+    uint32_t expected_cipher_id, cipher_id;
+
+    ssl = SSL_new(ctx);
+    OPENSSL_assert(ssl != NULL);
+
+    ciphers = SSL_get1_supported_ciphers(ssl);
+    OPENSSL_assert(ciphers != NULL);
+    num_expected_ciphers = OSSL_NELEM(default_ciphers_in_order);
+    num_ciphers = sk_SSL_CIPHER_num(ciphers);
+    if (num_ciphers != num_expected_ciphers) {
+        fprintf(stderr, "Expected %d supported ciphers, got %d.\n",
+                num_expected_ciphers, num_ciphers);
+        goto err;
+    }
+
+    for (i = 0; i < num_ciphers; i++) {
+        expected_cipher_id = default_ciphers_in_order[i];
+        cipher_id = SSL_CIPHER_get_id(sk_SSL_CIPHER_value(ciphers, i));
+        if (cipher_id != expected_cipher_id) {
+            fprintf(stderr, "Wrong cipher at position %d: expected %x, "
+                    "got %x\n", i, expected_cipher_id, cipher_id);
+            goto err;
+        }
+    }
+
+    ret = 1;
+
+ err:
+    sk_SSL_CIPHER_free(ciphers);
+    SSL_free(ssl);
+    return ret;
+}
+
+static int execute_test(CIPHERLIST_TEST_FIXTURE fixture)
+{
+    return test_default_cipherlist(fixture.server)
+        && test_default_cipherlist(fixture.client);
+}
+
+static void tear_down(CIPHERLIST_TEST_FIXTURE fixture)
+{
+    SSL_CTX_free(fixture.server);
+    SSL_CTX_free(fixture.client);
+    ERR_print_errors_fp(stderr);
+}
+
+#define SETUP_CIPHERLIST_TEST_FIXTURE() \
+    SETUP_TEST_FIXTURE(CIPHERLIST_TEST_FIXTURE, set_up)
+
+#define EXECUTE_CIPHERLIST_TEST() \
+    EXECUTE_TEST(execute_test, tear_down)
+
+static int test_default_cipherlist_implicit()
+{
+    SETUP_CIPHERLIST_TEST_FIXTURE();
+    EXECUTE_CIPHERLIST_TEST();
+}
+
+static int test_default_cipherlist_explicit()
+{
+    SETUP_CIPHERLIST_TEST_FIXTURE();
+    OPENSSL_assert(SSL_CTX_set_cipher_list(fixture.server, "DEFAULT"));
+    OPENSSL_assert(SSL_CTX_set_cipher_list(fixture.client, "DEFAULT"));
+    EXECUTE_CIPHERLIST_TEST();
+}
+
+int main(int argc, char **argv)
+{
+    int result = 0;
+
+    ADD_TEST(test_default_cipherlist_implicit);
+    ADD_TEST(test_default_cipherlist_explicit);
+
+    result = run_tests(argv[0]);
+
+    return result;
+}

test/recipes/80-test_cipherlist.t

+#! /usr/bin/perl
+
+use strict;
+use warnings;
+
+use OpenSSL::Test::Simple;
+use OpenSSL::Test;
+use OpenSSL::Test::Utils qw(alldisabled available_protocols);
+
+setup("test_cipherlist");
+
+my $no_anytls = alldisabled(available_protocols("tls"));
+
+# If we have no protocols, then we also have no supported ciphers.
+plan skip_all => "No SSL/TLS protocol is supported by this OpenSSL build."
+    if $no_anytls;
+
+simple_test("test_cipherlist", "cipherlist_test", "cipherlist");

test/recipes/80-test_ssl_old.t

@@ -78,7 +78,6 @@ my $client_sess="client.ss";
 # new format in ssl_test.c and add recipes to 80-test_ssl_new.t instead.
 plan tests =>
     1				# For testss
-    + 1				# For ssltest_old -test_cipherlist
     + 14			# For the first testssl
     + 16			# For the first testsslproxy
     + 16			# For the second testsslproxy
@@ -96,21 +95,15 @@ subtest 'test_ss' => sub {
     }
 };
 
-my $check = ok(run(test(["ssltest_old","-test_cipherlist"])), "running ssltest_old");
+note('test_ssl -- key U');
+testssl("keyU.ss", $Ucert, $CAcert);
 
-  SKIP: {
-      skip "ssltest_old ended with error, skipping the rest", 3
-	  if !$check;
-
-      note('test_ssl -- key U');
-      testssl("keyU.ss", $Ucert, $CAcert);
+note('test_ssl -- key P1');
+testsslproxy("keyP1.ss", "certP1.ss", "intP1.ss", "AB");
 
-      note('test_ssl -- key P1');
-      testsslproxy("keyP1.ss", "certP1.ss", "intP1.ss", "AB");
+note('test_ssl -- key P2');
+testsslproxy("keyP2.ss", "certP2.ss", "intP2.ss", "BC");
 
-      note('test_ssl -- key P2');
-      testsslproxy("keyP2.ss", "certP2.ss", "intP2.ss", "BC");
-    }
 
 # -----------
 # subtest functions

test/run_tests.pl

@@ -39,7 +39,7 @@ if (@ARGV) {
     @tests = @ARGV;
 }
 my $list_mode = scalar(grep /^list$/, @tests) != 0;
-if (grep /^alltests|list$/, @tests) {
+if (grep /^(alltests|list)$/, @tests) {
     @tests = grep {
 	basename($_) =~ /^[0-9][0-9]-[^\.]*\.t$/
     } glob(catfile($recipesdir,"*.t"));

test/ssltest_old.c

@@ -799,7 +799,6 @@ int doit_localhost(SSL *s_ssl, SSL *c_ssl, int family,
 int doit_biopair(SSL *s_ssl, SSL *c_ssl, long bytes, clock_t *s_time,
                  clock_t *c_time);
 int doit(SSL *s_ssl, SSL *c_ssl, long bytes);
-static int do_test_cipherlist(void);
 
 static void sv_usage(void)
 {
@@ -870,10 +869,6 @@ static void sv_usage(void)
     fprintf(stderr,
             " -time         - measure processor time used by client and server\n");
     fprintf(stderr, " -zlib         - use zlib compression\n");
-    fprintf(stderr,
-            " -test_cipherlist - Verifies the order of the ssl cipher lists.\n"
-            "                    When this option is requested, the cipherlist\n"
-            "                    tests are run instead of handshake tests.\n");
 #ifndef OPENSSL_NO_NEXTPROTONEG
     fprintf(stderr, " -npn_client - have client side offer NPN\n");
     fprintf(stderr, " -npn_server - have server side offer NPN\n");
@@ -1102,7 +1097,6 @@ int main(int argc, char *argv[])
     COMP_METHOD *cm = NULL;
     STACK_OF(SSL_COMP) *ssl_comp_methods = NULL;
 #endif
-    int test_cipherlist = 0;
 #ifdef OPENSSL_FIPS
     int fips_mode = 0;
 #endif
@@ -1315,11 +1309,9 @@ int main(int argc, char *argv[])
             app_verify_arg.app_verify = 1;
         } else if (strcmp(*argv, "-proxy") == 0) {
             app_verify_arg.allow_proxy_certs = 1;
-        } else if (strcmp(*argv, "-test_cipherlist") == 0) {
-            test_cipherlist = 1;
         }
 #ifndef OPENSSL_NO_NEXTPROTONEG
-        else if (strcmp(*argv, "-npn_client") == 0) {
+          else if (strcmp(*argv, "-npn_client") == 0) {
             npn_client = 1;
         } else if (strcmp(*argv, "-npn_server") == 0) {
             npn_server = 1;
@@ -1454,22 +1446,6 @@ int main(int argc, char *argv[])
         goto end;
     }
 
-    /*
-     * test_cipherlist prevails over protocol switch: we test the cipherlist
-     * for all enabled protocols.
-     */
-    if (test_cipherlist == 1) {
-        /*
-         * ensure that the cipher list are correctly sorted and exit
-         */
-        fprintf(stdout, "Testing cipherlist order only. Ignoring all "
-                "other options.\n");
-        if (do_test_cipherlist() == 0)
-            EXIT(1);
-        ret = 0;
-        goto end;
-    }
-
     if (ssl3 + tls1 + dtls + dtls1 + dtls12 > 1) {
         fprintf(stderr, "At most one of -ssl3, -tls1, -dtls, -dtls1 or -dtls12 should "
                 "be requested.\n");
@@ -3726,33 +3702,3 @@ static unsigned int psk_server_callback(SSL *ssl, const char *identity,
     return psk_len;
 }
 #endif
-
-static int do_test_cipherlist(void)
-{
-#ifndef OPENSSL_NO_TLS
-    int i = 0;
-    const SSL_METHOD *meth;
-    const SSL_CIPHER *ci, *tci = NULL;
-
-    /*
-     * This is required because ssltest "cheats" and uses internal headers to
-     * call functions, thus avoiding auto-init
-     */
-    OPENSSL_init_crypto(0, NULL);
-    OPENSSL_init_ssl(0, NULL);
-
-    meth = TLS_method();
-    tci = NULL;
-    while ((ci = meth->get_cipher(i++)) != NULL) {
-        if (tci != NULL)
-            if (ci->id >= tci->id) {
-                fprintf(stderr, "testing SSLv3 cipher list order: ");
-                fprintf(stderr, "failed %x vs. %x\n", ci->id, tci->id);
-                return 0;
-            }
-        tci = ci;
-    }
-#endif
-
-    return 1;
-}