Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSL Configurator fix - CertificateRequest not being made in SSL mutual authentication #24847

Merged
merged 1 commit into from Mar 9, 2024

Conversation

lisa-lthorrold
Copy link
Contributor

@lisa-lthorrold lisa-lthorrold commented Mar 8, 2024

The call setWantClientAuth(-) and setNeedClientAuth(-) have an XOR effect in https://github.com/openjdk/jdk17/blob/master/src/java.base/share/classes/javax/net/ssl/SSLParameters.java#L219 and https://github.com/openjdk/jdk/blob/27a03e0dc3e08094aebc3524f68617f7e7fb5c5d/src/java.base/share/classes/javax/net/ssl/SSLParameters.java#L218

That is, setting one will undo the effect of the other, regardless of the boolean value. In it's current form, the setNeedClientAuth value is not respected if it is true, which leads to a missing CertificateRequest in the TLS handshake. The change in this PR only sets the value if indicated to be true, and gives preference to needClientAuth in the case both are set by the user in the configuration.

Copy link
Contributor

@dmatej dmatej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was fixing that in Payara years ago, thanks! :-)

@dmatej dmatej added this to the 7.0.14 milestone Mar 8, 2024
@dmatej dmatej added the bug Something isn't working label Mar 8, 2024
@dmatej dmatej merged commit 52e6b5b into eclipse-ee4j:master Mar 9, 2024
2 checks passed
@lisa-lthorrold lisa-lthorrold deleted the ssl_parameter_fix branch March 9, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants