On Thu, 22 Feb 2024 15:23:13 GMT, Jaikiran Pai <[email protected]> wrote:
>> Can I please get a review of this change which proposes to fix >> https://bugs.openjdk.org/browse/JDK-8326381? >> >> As noted in the JBS issue, the implementation in `setNeedClientAuth()` and >> `setWantClientAuth()` of `com.sun.net.httpserver.HttpsParameters` wasn't >> matching the API specification. The commit in this PR fixes that issue and >> it now matches the API specification as well as what is done in >> `javax.net.ssl.SSLParameters` class. >> >> Additionally, as noted in the JBS issue, the (internal class) >> `sun.net.httpserver.SSLStreams` had a bug where it could end up resetting >> the `needClientAuth` flag on the `SSLEngine` because of the way the >> `setNeedClientAuth()` and `setWantClientAuth()` methods were being called on >> the `SSLEngine`. This too has been fixed in this PR. >> >> A new jtreg test has been introduced to reproduce the issue in the >> `HttpsParameters` class and verify this fix. > > Jaikiran Pai has updated the pull request incrementally with two additional > commits since the last revision: > > - introduce a test to verify the server honours needClientAuth and > wantClientAuth when set through HttpsParameters > - deprecate the SSL parameters related methods on HttpsParameter Hello Michael, > On. the second point there, I think it would be useful if we had a test for > this. It could be done in another PR maybe, but it would need a client/server > interaction with the "need" flag set and if no client cert available, check > for appropriate error. If cert available the client and server can both check > that it was used, through the SSLSession created. > You could use the same approach to test the "want" flag as well potentially. I did add a test to verify this semantic. However, I took a slightly different approach in the test. When the server is configured with needClientAuth, I created a client with a `SSLContext` which doesn't have any key material (and thus cannot present a valid certificate) and only has trust material (and thus can trust the server certificate). That client then initiated a HTTP request and the test verifies that such a request fail during TLS handshake (since the client doesn't present the certificate). The wantClientAuth test too does the same but in that case the request is expected to pass even when the client can't present the valid certificate. Given the context of this issue, do you think this way of testing is reasonable? ------------- PR Comment: https://git.openjdk.org/jdk/pull/17940#issuecomment-1959689620
