octachoron commented on PR #10192:
URL: https://github.com/apache/ozone/pull/10192#issuecomment-4397096384
Yes, @dombizita, thank you, this definitely helps confirm the behavior in
the corner case. As long as an empty (sub-)list doesn't result in a fallback to
defaults, we are good, and this points to that. In addition, we checked this
kind of test out together as well, with both `OPENSSL` and `JDK` as the
SslProvider:
```
@Test
public void testNoCiphersForVersionAfterFilter() throws Exception {
Server server = null;
ManagedChannel channel = null;
try {
String[] configuredCiphers = {
"TLS_FAKE_CIPHER_SUITE",
"TLS_AES_256_GCM_SHA384"
};
server = setupServer(new String[]{"TLSv1.3", "TLSv1.2"},
configuredCiphers);
server.start();
channel = setupClient(server.getPort(), new String[]{"TLSv1.2"}, new
String[]{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"});
XceiverClientProtocolServiceStub asyncStub =
XceiverClientProtocolServiceGrpc.newStub(channel);
ContainerCommandResponseProto response = sendRequest(asyncStub);
assertEquals(SUCCESS, response.getResult());
} finally {
shutdown(channel, server);
}
}
```
This fails with both providers (and `OPENSSL_REFCNT` is not supported), but
passes if the server also has `TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256`
enabled. A debugging session revealed that the fake suite is correctly
interpreted as a non-1.3 one, further validating the test. We did not find out
whether the failure of this test was caused by a mismatch between supported
versions (third scenario), or an error because no ciphers were enabled for a
version that was (first scenario), and the errors were different across
providers, but we consistently got errors on the client side. So we know the
server isn't falling back to defaults, and that behavior will be consistent for
users, even if it is the filter that results in zero ciphers left for a
version. I then tried 1.2 and 1.3 clients (and both providers) with a server
cipher list that becomes completely empty after filtering, and it showed the
same behavior.
It would probably be nice to have official test coverage for these
assumptions (just in case the underlying libraries change), but these should
give us reasonable confidence that we don't inadvertently enable ciphers, or
create confusion.
In this case, there should be no need to handle this corner case
specifically in Ozone. Thank you again for the investigation efforts and
patience.
--
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]
