On Fri, 5 Jul 2024 08:58:03 GMT, Prajwal Kumaraswamy <pkumarasw...@openjdk.org>
wrote:
> The client identity checks when "HTTPS" endpoint identification algorithm is
> set on SSL server throws "java.security.cert.CertificateException: No subject
> alternative names present" when client certificate's SubjectAltName extension
> does not match its IP address
>
> Since the server has no external knowledge of what the client's identity
> ought to be, HTTPS identity checks must be disabled on the server side.
> The exception message has been fixed to indicate the same.
>
> I have performed the test both on SSL Server Engine and SSL Server Socket and
> attached are logs and snapshot for reference, also I have ran the changes
> against external test suite and test runs are green.
To @seanjmullan 's question.
`SSLParameters` can be set to anything before the handshake starts, even
potentially to conflicting values like this. Only once handshaking is underway
can we determine the inconsistency and report the error.
See code for additional comments, then I can approve.
src/java.base/share/classes/sun/security/ssl/X509TrustManagerImpl.java line 431:
> 429:
> 430: if (!identifiable) {
> 431: try {
Minor formatting nits/suggestions:
if(
->
if (
Lines <= 80 chars, please.
Endpoint Identification algorithm
->
Endpoint Identification Algorithm
src/java.base/share/classes/sun/security/ssl/X509TrustManagerImpl.java line 432:
> 430: if (!identifiable) {
> 431: try {
> 432: checkIdentity(peerHost,
You might do the direct check for the `checkClientTrusted`/`HTTPS` and
immediately throw. If that doesn't fail, then do the fallback `checkIdentity`.
Doing so would save the overhead of calling `checkIdentity` only to find out
there was an error.
if (!identifiable) {
// Clients can't identify themselves via SNI/hostnames in HTTPS.
if (checkClientTrusted && "HTTPS".equalsIgnoreCase(algorithm)) {
throw...
}
checkIdentity(peerHost, trustedChain[0], algorithm, chainsToPublicCA);
}
-------------
PR Review: https://git.openjdk.org/jdk/pull/20048#pullrequestreview-2161335201
PR Review Comment: https://git.openjdk.org/jdk/pull/20048#discussion_r1667215199
PR Review Comment: https://git.openjdk.org/jdk/pull/20048#discussion_r1667216383
