dombizita commented on PR #10192:
URL: https://github.com/apache/ozone/pull/10192#issuecomment-4388061824
Thanks for reviewing it @octachoron and for verifying the listed points.
> There is one edge case where we may see surprises: when more than one TLS
version is enabled, and one of them is left with zero ciphers after the filter.
This can have different results, possibly depending on the selected
`SslProvider`:
>
> * A hard error despite having allowed and valid combinations in the
configuration, like with `openssl ciphers -ciphersuites TLS_AES_256_GCM_SHA384
-V ''`.
> * Falling back to defaults on the given TLS version, like with `openssl
ciphers -ciphersuites TLS_AES_256_GCM_SHA384 -V`.
> * Disabling the corresponding TLS version.
> * Something else I am not considering.
>
> Are you aware of any reason to prefer a specific one, or anything that
guarantees the outcome? (I am checking too, but maybe this has been
investigated already.)
Regarding this corner case I'm not sure if we should handle it (if yes how)
in Ozone. I tried testing this with unit tests with the help of Cursor.
```
private boolean isProtocolSupported(String protocol) throws Exception {
return Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters()
.getProtocols()).contains(protocol);
}
private boolean isCipherSupported(String cipher) throws Exception {
return Arrays.asList(SSLContext.getDefault().getSupportedSSLParameters()
.getCipherSuites()).contains(cipher);
}
@Test
public void
testMixedProtocolsWithFilteredCipherStillServesCompatibleProtocol()
throws Exception {
String tls12Cipher = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256";
assumeTrue(isProtocolSupported("TLSv1.2"));
assumeTrue(isProtocolSupported("TLSv1.3"));
assumeTrue(isCipherSupported(tls12Cipher));
HttpServer2 server = buildServer(null, tls12Cipher, "TLSv1.2,TLSv1.3");
server.start();
try {
InetSocketAddress addr = server.getConnectorAddress(0);
SSLSocketFactory tls12Factory = createSocketFactory(
new String[]{tls12Cipher}, new String[]{"TLSv1.2"});
int responseCode = connectWithFactory(tls12Factory, addr);
assertEquals(HttpURLConnection.HTTP_OK, responseCode);
} finally {
server.stop();
}
}
@Test
public void
testMixedProtocolsWithFilteredCipherRejectsProtocolWithoutCipherOnSunJsse()
throws Exception {
String tls12Cipher = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256";
assumeTrue(isProtocolSupported("TLSv1.2"));
assumeTrue(isProtocolSupported("TLSv1.3"));
assumeTrue(isCipherSupported(tls12Cipher));
assumeTrue("SunJSSE".equals(SSLContext.getDefault().getProvider().getName()),
"Provider-specific expectation: only asserted for SunJSSE");
HttpServer2 server = buildServer(null, tls12Cipher, "TLSv1.2,TLSv1.3");
server.start();
try {
InetSocketAddress addr = server.getConnectorAddress(0);
SSLSocketFactory tls13Factory =
createSocketFactory(null, new String[]{"TLSv1.3"});
assertThrows(Exception.class, () -> connectWithFactory(tls13Factory,
addr));
} finally {
server.stop();
}
}
```
These are passing on this branch, so in the second case, when we only have
1.2 supported cipher and try to connect with 1.3 it'll throw an exception.
That's the first point in your list and I think it makes sense. Not a 100% sure
I understood your case, please let me know if I missed something.
The `SupportedCipherSuiteFilter` javadoc says "This class will filter all
requested ciphers out that are not supported by the current SSLEngine.", which
I think should be enough in Ozone.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
