This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 0839aed24e Fully align order output with OpenSSL
0839aed24e is described below
commit 0839aed24e56c233fa1168052841a299696226af
Author: Mark Thomas <[email protected]>
AuthorDate: Fri Jun 12 17:05:23 2026 +0100
Fully align order output with OpenSSL
---
.../ciphers/OpenSSLCipherConfigurationParser.java | 99 +++++++++++++++-------
.../TestOpenSSLCipherConfigurationParser.java | 27 +++---
.../TestOpenSSLCipherConfigurationParserOnly.java | 17 ++++
webapps/docs/changelog.xml | 4 +
4 files changed, 107 insertions(+), 40 deletions(-)
diff --git
a/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
b/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
index 70f846e124..a48d52148d 100644
---
a/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
+++
b/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
@@ -526,16 +526,16 @@ public class OpenSSLCipherConfigurationParser {
}
static void moveToEnd(final LinkedHashSet<Cipher> ciphers, final
Collection<Cipher> toBeMovedCiphers) {
- List<Cipher> movedCiphers = new ArrayList<>(toBeMovedCiphers);
- movedCiphers.retainAll(ciphers);
+ List<Cipher> movedCiphers = new ArrayList<>(ciphers);
+ movedCiphers.retainAll(toBeMovedCiphers);
movedCiphers.forEach(ciphers::remove);
ciphers.addAll(movedCiphers);
}
static void moveToStart(final LinkedHashSet<Cipher> ciphers, final
Collection<Cipher> toBeMovedCiphers) {
- List<Cipher> movedCiphers = new ArrayList<>(toBeMovedCiphers);
+ List<Cipher> movedCiphers = new ArrayList<>(ciphers);
List<Cipher> originalCiphers = new ArrayList<>(ciphers);
- movedCiphers.retainAll(ciphers);
+ movedCiphers.retainAll(toBeMovedCiphers);
ciphers.clear();
ciphers.addAll(movedCiphers);
ciphers.addAll(originalCiphers);
@@ -569,45 +569,86 @@ public class OpenSSLCipherConfigurationParser {
}
/*
- * See
https://github.com/openssl/openssl/blob/7c96dbcdab959fef74c4caae63cdebaa354ab252/ssl/ssl_ciph.c#L1371
+ * See https://github.com/openssl/openssl/blob/master/ssl/ssl_ciph.c
+ *
+ * Most recently reviewed at afaa70c on 2026-06-12
*/
static LinkedHashSet<Cipher> defaultSort(final LinkedHashSet<Cipher>
ciphers) {
- final LinkedHashSet<Cipher> result = new
LinkedHashSet<>(ciphers.size());
- final LinkedHashSet<Cipher> ecdh = new LinkedHashSet<>(ciphers.size());
+ LinkedHashSet<Cipher> result = new LinkedHashSet<>(ciphers.size());
+
+ // Copy because we need to manipulate the order before adding
+ LinkedHashSet<Cipher> source = new LinkedHashSet<>(ciphers);
+
+ // Can't find this in the OpenSSL source but observed in test results
+ Set<Cipher> camelliaWithDSS = filterByAuthentication(source,
Collections.singleton(Authentication.DSS));
+ camelliaWithDSS = filterByEncryption(
+ camelliaWithDSS, new
LinkedHashSet<>(Arrays.asList(Encryption.CAMELLIA128, Encryption.CAMELLIA256)));
+ moveToEnd(source, camelliaWithDSS);
+
+ /*
+ * This change is made to source so it effectively applies to each
group that is subsequently added to the
+ * result.
+ */
+ LinkedHashSet<Cipher> eecdh = filterByKeyExchange(source,
Collections.singleton(KeyExchange.EECDH));
+ moveToStart(source, eecdh);
+ moveToStart(source, filterByAuthentication(eecdh,
Collections.singleton(Authentication.ECDSA)));
- /* Everything else being equal, prefer ephemeral ECDH over other key
exchange mechanisms */
- ecdh.addAll(filterByKeyExchange(ciphers,
Collections.singleton(KeyExchange.EECDH)));
+ // Now start adding ciphers to the result
- /* AES is our preferred symmetric cipher */
- Set<Encryption> aes = new HashSet<>(
- Arrays.asList(Encryption.AES128, Encryption.AES128CCM,
Encryption.AES128CCM8, Encryption.AES128GCM,
- Encryption.AES256, Encryption.AES256CCM,
Encryption.AES256CCM8, Encryption.AES256GCM));
+ // Prefer GCM over CHACHA
+ result.addAll(filterByEncryption(source,
Collections.singleton(Encryption.AES256GCM)));
+ result.addAll(filterByEncryption(source,
Collections.singleton(Encryption.AES128GCM)));
+ result.addAll(filterByEncryption(source,
Collections.singleton(Encryption.CHACHA20POLY1305)));
- /* Now arrange all ciphers by preference: */
- result.addAll(filterByEncryption(ecdh, aes));
- result.addAll(filterByEncryption(ciphers, aes));
+ // Generally prefer AES
+ result.addAll(filterByEncryption(source,
+ new HashSet<>(Arrays.asList(Encryption.AES256CCM,
Encryption.AES256CCM8, Encryption.AES256,
+ Encryption.AES128CCM, Encryption.AES128CCM8,
Encryption.AES128))));
- /* Add everything else */
- result.addAll(ecdh);
- result.addAll(ciphers);
+ // Add everything else
+ result.addAll(source);
- /* Low priority for MD5 */
+ // Move MD5 to end
moveToEnd(result, filterByMessageDigest(result,
Collections.singleton(MessageDigest.MD5)));
- /*
- * Move anonymous ciphers to the end. Usually, these will remain
disabled. (For applications that allow them,
- * they aren't too bad, but we prefer authenticated ciphers.)
- */
+ // Move anonymous ciphers to the end.
moveToEnd(result, filterByAuthentication(result,
Collections.singleton(Authentication.aNULL)));
-
- /* Move ciphers without forward secrecy to the end */
- moveToEnd(result, filterByAuthentication(result,
Collections.singleton(Authentication.ECDH)));
moveToEnd(result, filterByKeyExchange(result,
Collections.singleton(KeyExchange.RSA)));
moveToEnd(result, filterByKeyExchange(result,
Collections.singleton(KeyExchange.PSK)));
- /* RC4 is sort-of broken -- move to the end */
+ // RC4 is sort-of broken -- move to the end
moveToEnd(result, filterByEncryption(result,
Collections.singleton(Encryption.RC4)));
- return strengthSort(result);
+
+ // Sort by encryption strength
+ result = strengthSort(result);
+
+ // Partially override strength sort to prefer TLS 1.2
+ moveToStart(result, filterByProtocol(result,
Collections.singleton(Protocol.TLSv1_2)));
+
+ /*
+ * Irrespective of strength, enforce the following order:
+ * (EC)DHE + AEAD > (EC)DHE > rest of AEAD > rest.
+ * Within each group, ciphers remain sorted by strength and previous
+ * preference, i.e.,
+ * 1) ECDHE > DHE
+ * 2) GCM > CHACHA
+ * 3) AES > rest
+ * 4) TLS 1.2 > legacy
+ *
+ * Moving to start so move in reverse order.
+ */
+ Set<Cipher> ecdheAndDhe = new LinkedHashSet<>(result.size());
+ Set<Cipher> ecdheAndDheWithAead = new LinkedHashSet<>(result.size());
+
+ ecdheAndDhe.addAll(filterByKeyExchange(result, new
HashSet<>(Arrays.asList(KeyExchange.EDH, KeyExchange.EECDH))));
+ ecdheAndDheWithAead.addAll(ecdheAndDhe);
+ ecdheAndDheWithAead = filterByMessageDigest(ecdheAndDheWithAead,
Collections.singleton(MessageDigest.AEAD));
+
+ moveToStart(result, filterByMessageDigest(result,
Collections.singleton(MessageDigest.AEAD)));
+ moveToStart(result, ecdheAndDhe);
+ moveToStart(result, ecdheAndDheWithAead);
+
+ return result;
}
static Set<Cipher> filterByStrengthBits(Set<Cipher> ciphers, int
strength_bits) {
diff --git
a/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java
b/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java
index 41e81cc3e6..cd038cfe93 100644
---
a/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java
+++
b/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java
@@ -489,7 +489,7 @@ public class TestOpenSSLCipherConfigurationParser {
public void testSpecification05() throws Exception {
if (TesterOpenSSL.VERSION < 30200) {
// OpenSSL 3.2.x moved the CCM8 ciphers from high to medium
- testSpecification("HIGH:!AESCCM8:!aNULL:!eNULL");
+ testSpecification("HIGH:!AESCCM8:@STRENGTH:!aNULL:!eNULL");
} else {
testSpecification("HIGH:@STRENGTH:!aNULL:!eNULL");
}
@@ -533,16 +533,21 @@ public class TestOpenSSLCipherConfigurationParser {
+ specification + "'",
new TreeSet<>(jsseCipherListFromParser), new
TreeSet<>(jsseCipherListFromOpenSSL));
- // OpenSSL treats many ciphers as having equal preference. The order
- // returned depends on the order they are requested. The following code
- // checks that the Parser produces a cipher list that is consistent
with
- // OpenSSL's preference order by confirming that running through
OpenSSL
- // does not change the order.
- String parserOrderedExpression =
listToString(jsseCipherListFromParser, ',');
- Assert.assertEquals(
- listToString(OpenSSLCipherConfigurationParser.parseExpression(
- parserOrderedExpression), ','),
- parserOrderedExpression);
+ /*
+ * The parser generated cipher list is checked for an exact match with
the list generated by OpenSSL.
+ *
+ * This check tests for the exact order as generated by OpenSSL master
running on Linux.
+ *
+ * There was a change in the default ordering between 3.2.x and 3.3.x.
Skip the exact order check if testing
+ * against OpenSSL 3.0.x (the only currently supported version before
3.3.x).
+ *
+ * It is assumed that the cipher order does not vary by operating
system. If that assumption is wrong, it should
+ * be caught by CI and the test can be adjusted.
+ */
+ if (TesterOpenSSL.VERSION > 30299) {
+ Assert.assertEquals(specification,
listToString(jsseCipherListFromOpenSSL, ','),
+ listToString(jsseCipherListFromParser, ','));
+ }
}
diff --git
a/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParserOnly.java
b/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParserOnly.java
index 795e977bf2..d3bc8b8d81 100644
---
a/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParserOnly.java
+++
b/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParserOnly.java
@@ -83,6 +83,23 @@ public class TestOpenSSLCipherConfigurationParserOnly {
Assert.assertEquals(expected.toString(), result.toString());
}
+ @Test
+ public void testDefaultSort04() throws Exception {
+ // Reproducing a failure observed when aligning sorting with OpenSSL
master
+
+ // ECDHE should beat DHE
+ LinkedHashSet<Cipher> input = new LinkedHashSet<>();
+ input.add(Cipher.TLS_DHE_RSA_WITH_AES_256_CCM_8);
+ input.add(Cipher.TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8);
+ LinkedHashSet<Cipher> result =
OpenSSLCipherConfigurationParser.defaultSort(input);
+
+ LinkedHashSet<Cipher> expected = new LinkedHashSet<>();
+ expected.add(Cipher.TLS_ECDHE_ECDSA_WITH_AES_128_CCM_8);
+ expected.add(Cipher.TLS_DHE_RSA_WITH_AES_256_CCM_8);
+
+ Assert.assertEquals(expected.toString(), result.toString());
+ }
+
@Test
public void testRename01() throws Exception {
// EDH -> DHE
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c7c5e062f8..302c47c91e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -355,6 +355,10 @@
<bug>69988</bug>: Fix post handshake authentication for TLS 1.3. It was
broken by a breaking change in OpenSSL between 1.1.1 and 3.0.0. (markt)
</fix>
+ <fix>
+ When processing an OpenSSL cipher specification, fully align the order
+ of the resulting ciphers with the order produced by OpenSSL. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
