priority: be backwards compatible with priority strings starting with NONE

That is, we allow priority strings which do not enable any groups to
work, by disabling TLS1.3. For example
'NONE:+VERS-TLS-ALL:+MAC-ALL:+RSA:+AES-128-GCM:+SIGN-ALL:+COMP-NULL'
is still operational, but no TLS1.3 is enabled when specified.

Resolves: #549

Signed-off-by: Nikos Mavrogiannopoulos <nmav@redhat.com>

diff --git a/doc/cha-gtls-app.texi b/doc/cha-gtls-app.texi
index 9a4cf299337b799bfb8b6d276195b2e0aeeab321..c7a87a5a228de7c62975c24d081afda25d7248d6 100644 (file)
--- a/doc/cha-gtls-app.texi
+++ b/doc/cha-gtls-app.texi
@@ -1185,10 +1185,10 @@ verification profile.
 Means nothing is enabled.  This disables even protocol versions.
 It should be followed by the algorithms to be enabled. Note that
 using this option to build a priority string gives detailed control
-into the resulting settings, however it creates non-portable applications.
-With new revisions of the TLS protocol new priority items are routinely added
-requiring such a string to be continuously updated with the library. As
-such, we advice against using that option for applications targetting multiple versions
+into the resulting settings, however with new revisions of the TLS protocol
+new priority items are routinely added, and such strings are not
+forward compatible with new protocols. As such, we
+advice against using that option for applications targetting multiple versions
 of the GnuTLS library, and recommend using the defaults (see above) or
 adjusting the defaults via @funcref{gnutls_set_default_priority_append}.
 

diff --git a/lib/priority.c b/lib/priority.c
index 6a93545cdfddbdca46d6a9548e303a5bf0e8ea11..1991635d8638594b4addd031067b435d8098711d 100644 (file)
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -1201,6 +1201,15 @@ static void add_dh(gnutls_priority_t priority_cache)
        }
 }
 
+#define REMOVE_TLS13_IN_LOOP(vers, i) \
+       if (vers->tls13_sem) { \
+               for (j=i+1;j<priority_cache->protocol.algorithms;j++) \
+                       priority_cache->protocol.priority[j-1] = priority_cache->protocol.priority[j]; \
+               priority_cache->protocol.algorithms--; \
+               i--; \
+               continue; \
+       }
+
 static int set_ciphersuite_list(gnutls_priority_t priority_cache)
 {
        unsigned i, j, z;
@@ -1247,16 +1256,10 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache)
                if (!vers)
                        continue;
 
-               /* if we have NULL ciphersuites, SRP or RSA-PSK enabled, remove TLS1.3+ protocol
-                * versions; they cannot be negotiated under TLS1.3. */
+               /* if we have NULL ciphersuites, SRP, or RSA-PSK enabled remove TLS1.3+
+                * protocol versions; they cannot be negotiated under TLS1.3. */
                if (have_null || have_srp || have_rsa_psk) {
-                       if (vers->tls13_sem) {
-                               for (j=i+1;j<priority_cache->protocol.algorithms;j++)
-                                       priority_cache->protocol.priority[j-1] = priority_cache->protocol.priority[j];
-                               priority_cache->protocol.algorithms--;
-                               i--;
-                               continue;
-                       }
+                       REMOVE_TLS13_IN_LOOP(vers, i);
                }
 
                if (vers->transport == GNUTLS_STREAM) { /* TLS */
@@ -1395,8 +1398,15 @@ static int set_ciphersuite_list(gnutls_priority_t priority_cache)
                return gnutls_assert_val(GNUTLS_E_NO_PRIORITIES_WERE_SET);
 
        /* when TLS 1.3 is available we must have groups set */
-       if (unlikely(!have_psk && tlsmax && tlsmax->id >= GNUTLS_TLS1_3 && priority_cache->groups.size == 0))
-               return gnutls_assert_val(GNUTLS_E_NO_PRIORITIES_WERE_SET);
+       if (unlikely(!have_psk && tlsmax && tlsmax->id >= GNUTLS_TLS1_3 && priority_cache->groups.size == 0)) {
+               for (i = 0; i < priority_cache->protocol.algorithms; i++) {
+                       vers = version_to_entry(priority_cache->protocol.priority[i]);
+                       if (!vers)
+                               continue;
+
+                       REMOVE_TLS13_IN_LOOP(vers, i);
+               }
+       }
 
        return 0;
 }

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c0ecfed3f2e4f1c0e9af2f7b0c60e4987a3a2096..295cfcd35451ac09b42ec5e335720d2828f360f3 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -61,6 +61,7 @@ EXTRA_DIST = suppressions.valgrind eagain-common.h cert-common.h test-chains.h \
        ocsp-tests/certs/server_good.key ocsp-tests/certs/server_bad.key ocsp-tests/certs/server_good.template \
        ocsp-tests/certs/server_bad.template ocsp-tests/certs/ocsp-staple-unrelated.der ocsp-tests/suppressions.valgrind \
        data/listings-DTLS1.0 data/listings-SSL3.0 data/listings-TLS1.0 data/listings-TLS1.1 \
+       data/listings-legacy1 data/listings-legacy2 \
        data/listings-SSL3.0-TLS1.1 p11-kit-trust-data/Example_Root_CA.p11-kit server-kx-neg-common.c \
        p11-kit-trust-data/Example_Root_CA.pem data/test1.cat data/test2.cat \
        data/test1.cat.data data/test2.cat.data data/test1.cat.out data/test2.cat.out \

diff --git a/tests/cipher-listings.sh b/tests/cipher-listings.sh
index 094ae5f38f9e94648fdacb3e11c479de15c6481c..b8f3a602e3fe5f2de2e72c5fcbb42214f2faf9ab 100755 (executable)
--- a/tests/cipher-listings.sh
+++ b/tests/cipher-listings.sh
@@ -81,6 +81,11 @@ check TLS1.0 "NORMAL:-VERS-ALL:+VERS-TLS1.0"
 check TLS1.1 "NORMAL:-VERS-ALL:+VERS-TLS1.1"
 check SSL3.0-TLS1.1 "NORMAL:-VERS-ALL:+VERS-TLS1.0:+VERS-SSL3.0:+VERS-TLS1.1"
 check DTLS1.0 "NORMAL:-VERS-ALL:+VERS-DTLS1.0"
+# Priority strings prior to 3.6.x did not require the +GROUP option; here we
+# test whether these work as expected.
+check legacy1 "NONE:+VERS-TLS-ALL:+MAC-ALL:+RSA:+AES-128-GCM:+SIGN-ALL:+COMP-NULL"
+check legacy2 "NONE:+VERS-TLS-ALL:+MAC-ALL:+RSA:+CAMELLIA-256-GCM:+SIGN-ALL:+COMP-NULL"
+
 
 rm -f ${TMPFILE}
 rm -f ${TMPFILE2}

diff --git a/tests/data/listings-legacy1 b/tests/data/listings-legacy1
new file mode 100644 (file)
index 0000000..549ca73
--- /dev/null
+++ b/tests/data/listings-legacy1
@@ -0,0 +1,4 @@
+Cipher suites for NONE:+VERS-TLS-ALL:+MAC-ALL:+RSA:+AES-128-GCM:+SIGN-ALL:+COMP-NULL
+TLS_RSA_AES_128_GCM_SHA256                             0x00, 0x9c      TLS1.2
+
+Protocols: VERS-TLS1.2, VERS-TLS1.1, VERS-TLS1.0

diff --git a/tests/data/listings-legacy2 b/tests/data/listings-legacy2
new file mode 100644 (file)
index 0000000..35ce346
--- /dev/null
+++ b/tests/data/listings-legacy2
@@ -0,0 +1,4 @@
+Cipher suites for NONE:+VERS-TLS-ALL:+MAC-ALL:+RSA:+CAMELLIA-256-GCM:+SIGN-ALL:+COMP-NULL
+TLS_RSA_CAMELLIA_256_GCM_SHA384                        0xc0, 0x7b      TLS1.2
+
+Protocols: VERS-TLS1.2, VERS-TLS1.1, VERS-TLS1.0