RFR: 8326381: com.sun.net.httpserver.HttpsParameters and SSLStreams incorrectly handle needClientAuth and wantClientAuth [v6]

[Jaikiran Pai]

On Mon, 26 Feb 2024 11:36:23 GMT, Jaikiran Pai <jpai at openjdk.org> 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:
> 
>  - John's review - set need/wantClientAuth to false and expect both need/wantClientAuth to be false
>  - assert that client auth was indeed initiated by server during TLS handshake

test/jdk/com/sun/net/httpserver/HttpsParametersClientAuthTest.java line 236:

> 234:                         Throwable cause = ioe;
> 235:                         while (cause != null) {
> 236:                             // either of SocketException or SSLHandshakeException are OK

I have this on my TODO list and I plan to come back to this shortly after I am done with the CSR text. I was expecting this to be one specific type of exception but I'm seeing 2 different types depending on how I launch the test. Plus there's an open PR in security-libs area which is proposing some changes in the alert/exception raised from the TLS handshake layer when the client doesn't send certificates to the server. I need to review those changes to see what kind of impact if any that change will have here.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/17940#discussion_r1502482513