JDK-8262186 : Call X509KeyManager.chooseClientAlias once for all key types
  • Type: Bug
  • Component: security-libs
  • Sub-Component: javax.net.ssl
  • Affected Version: 8,11,15,17
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2021-02-17
  • Updated: 2023-11-21
  • Resolved: 2021-08-31
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 17 JDK 18
17.0.10-oracleFixed 18 b13Fixed
Related Reports
CSR :  
Duplicate :  
Duplicate :  
Relates :  
Relates :  
Sub Tasks
JDK-8273156 :  
Description
ADDITIONAL SYSTEM INFORMATION :
Not OS-specific

A DESCRIPTION OF THE PROBLEM :
e are creating a HTTPS connection to a server using client authentication. While connecting, the client explicitly chooses to present one specific entry in the client keystore by implementing X509KeyManager's chooseClientAlias callback method. However, the way this method is called by SSL code has changed over time:
- In jdk 15.0.2 and TLS 1.2, the method is called several times, presenting one key type per call only. 
- In jdk 15.0.2 and TLS 1.1, the method is called only once, and all possible key types are presented in that single call.
- In jdk 1.8.0_192 and TLS 1.2, the method is called only once, and all possible key types are presented in that single call.
- In jdk 1.8.0_192 and TLS 1.1, the method is called only once, and all possible key types are presented in that single call.

The javadoc of the method [1] says:
String[] keyType - the key algorithm type name(s), ordered with the most-preferred key type first.

The parameter type and description kind of suggests, though it does not explicitly say so, that all possible key types are passed to this method when it is called, which was the case in the past, and is also still the case for TLS 1.1.

I was at least very surprised by the above change, and would not think that anybody reading the javadoc would expect such a call semantics.
Is ist a bug? I do not know. But in my opinion, it is certainly an unwanted situation. So I see possible ways to improve this:
- only call the method once with all possible key types, as it was in the past
- change the javadoc to indicate that the method might be called several times during a single key resolution, and that not all possible key types may be presented in a single call.

In our application, it would be more convenient to get all possible key types in a singe call, as then one can treat a mismatch as an error and notify the user accordingly. If the method is called several times, key type mismatch cannot be treated as error, as the next call could be the one with the matching key type.

REGRESSION : Last worked in version 8

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1 ) From the supplied source code, extract the two files HttpsServer.java and HttpsClient.java. 
2 ) Create a JKS keystore with a RSA private key (can be self-siged) and a JKS truststore trusting the key (I'd attach my truststore and keystore, but there is no upload field available here :-( ). Use 'changeit' as password. Put them in the classpath as localhost.jks and truststore.jks.
3) Run HttpsServer.java, which will start a simple Https Server on port 9061
4) Run HttpsClient.java, which will connect to the Https Server using client authentication and print the calls to the chooseClientAlias method.

Changing to TLS v1.1 can be done by changing line 49 in the client to 
    SSLContext sslContext = SSLContext.getInstance("TLSv1.1");

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
For the earlier call semantics, the output of the client would be
testSSL start
Opening client connection...
Initializing client key store...
Initializing client trust store...
Initializing client ssl context...
Creating client ssl socket factory...
client connecting...
passed key types : [RSA, DSA, EC]
Successfully received Data

ACTUAL -
For the changed call semantics, the output would be
testSSL start
Opening client connection...
Initializing client key store...
Initializing client trust store...
Initializing client ssl context...
Creating client ssl socket factory...
client connecting...
passed key types : [EC]
passed key types : [RSA]
Successfully received Data

---------- BEGIN SOURCE ----------
---------- HttpsClient.java
import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.net.Socket;
import java.net.URL;
import java.security.KeyStore;
import java.security.Principal;
import java.security.PrivateKey;
import java.security.cert.X509Certificate;
import java.util.Arrays;

import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.KeyManager;
import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManagerFactory;
import javax.net.ssl.X509KeyManager;

public class HttpsClient
{
  private static final KeyStore keyStore;
  private static final KeyStore trustStore;
  private static final String STORE_PASSWORD = "changeit";

  static
  {
    keyStore = readKeystore("localhost.jks");
    trustStore = readKeystore("truststore.jks");
  }

  public static void main(String[] argv) throws Exception
  {
    System.out.println("testSSL start");
    // System.setProperty("javax.net.debug", "all");
    String serverUrl = "https://localhost:" + HttpsServer.PORT + "/hello";

    System.out.println("Opening client connection...");
    HttpsURLConnection conn = (HttpsURLConnection) new URL(serverUrl).openConnection();

    System.out.println("Initializing client key store...");
    KeyManagerFactory kmf = KeyManagerFactory.getInstance("SunX509");
    kmf.init(keyStore, STORE_PASSWORD.toCharArray());

    System.out.println("Initializing client trust store...");
    TrustManagerFactory tm = TrustManagerFactory.getInstance("SunX509");
    tm.init(trustStore);

    SSLContext sslContext = SSLContext.getInstance("SSL");
    System.out.println("Initializing client ssl context...");
    sslContext.init(Arrays.stream(kmf.getKeyManagers())
            .map(k -> {if (k instanceof X509KeyManager)
                          {return new KeyManagerDelegate((X509KeyManager) k); }
                        else
                          {return k; }})
            .toArray(KeyManager[]::new),
        tm.getTrustManagers(),
        null);

    System.out.println("Creating client ssl socket factory...");
    SSLSocketFactory sslsocketfactory = sslContext.getSocketFactory();
    conn.setSSLSocketFactory(sslsocketfactory);
    System.out.println("client connecting...");
    conn.connect();

    try (BufferedReader bufferedreader = new BufferedReader(new InputStreamReader(conn.getInputStream())))
    {
      String result = bufferedreader.readLine();
      if (!"This is the response".equals(result))
      {
        throw new RuntimeException("Test failed, result is " + result);
      }
      else
      {
        System.out.println("Successfully received Data");
      }
    }
  }

  public static class KeyManagerDelegate implements X509KeyManager
  {
    private X509KeyManager delegate;

    public KeyManagerDelegate(X509KeyManager delegate)
    {
      this.delegate = delegate;
    }

    @Override
    public String[] getClientAliases(String s, Principal[] principals)
    {
      return delegate.getClientAliases(s, principals);
    }

    @Override
    public String chooseClientAlias(String[] strings, Principal[] principals, Socket socket)
    {
      System.out.println("passed key types : " + Arrays.toString(strings));
      return delegate.chooseClientAlias(strings, principals, socket);
    }

    @Override
    public String[] getServerAliases(String s, Principal[] principals)
    {
      return delegate.getServerAliases(s, principals);
    }

    @Override
    public String chooseServerAlias(String s, Principal[] principals, Socket socket)
    {
      return delegate.chooseServerAlias(s, principals, socket);
    }

    @Override
    public X509Certificate[] getCertificateChain(String s)
    {
      return delegate.getCertificateChain(s);
    }

    @Override
    public PrivateKey getPrivateKey(String s)
    {
      return delegate.getPrivateKey(s);
    }
  }

  private static KeyStore readKeystore(String name)
  {
    try (InputStream resource = HttpsClient.class.getClassLoader().getResourceAsStream(name))
    {
      KeyStore keystore = KeyStore.getInstance("JKS");
      keystore.load(resource, "changeit".toCharArray());
      return keystore;
    }
    catch (Exception e)
    {
      throw new RuntimeException(e);
    }
  }
}
----- HttpsServer.java
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.InetSocketAddress;
import java.nio.charset.StandardCharsets;
import java.security.KeyStore;

import javax.net.ssl.KeyManagerFactory;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.TrustManagerFactory;

import com.sun.net.httpserver.HttpExchange;
import com.sun.net.httpserver.HttpsConfigurator;

public class HttpsServer
{
  private static final KeyStore keyStore;
  private static final KeyStore trustStore;
  private static final String STORE_PASSWORD = "changeit";
  public static int PORT = 9061;

  static
  {
    keyStore = readKeystore(getResourceAsStream("localhost.jks"));
    trustStore = readKeystore(getResourceAsStream("truststore.jks"));
  }

  public static void main(String[] argv) throws Exception
  {
    System.out.println("starting SSL server...");
    System.setProperty("javax.net.debug", "all");
    com.sun.net.httpserver.HttpsServer httpsServer = null;
    try
    {
      httpsServer = createHttpsServer();
      Thread.sleep(3600000L);
    }
    finally
    {
      System.out.println("stopping SSL Server");
      if (httpsServer != null)
      {
        httpsServer.stop(0);
      }
    }
  }

  private static com.sun.net.httpserver.HttpsServer createHttpsServer() throws Exception
  {
    InetSocketAddress address = new InetSocketAddress(PORT);

    com.sun.net.httpserver.HttpsServer httpsServer = com.sun.net.httpserver.HttpsServer.create(address, 0);

    httpsServer.setHttpsConfigurator(new MyHttpsConfigurator());
    httpsServer.setExecutor(null);
    httpsServer.start();
    System.out.println("Server started.");


    httpsServer.createContext("/hello", new MyHttpHandler());
    System.out.println("hello context created.");
    return httpsServer;
  }

  private static class MyHttpsConfigurator extends HttpsConfigurator
  {

    public MyHttpsConfigurator() throws Exception
    {
      super(createSslContext());
    }

    private static SSLContext createSslContext() throws Exception
    {
      System.out.println("getting instance of sslContext....");
      SSLContext sslContext = SSLContext.getInstance("TLS");

      System.out.println("Initializing server Keystore....");

      KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("SunX509");
      keyManagerFactory.init(keyStore, STORE_PASSWORD.toCharArray());

      System.out.println("Initializing server Truststore....");
      TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("SunX509");
      trustManagerFactory.init(trustStore);

      System.out.println("Initializing sslContext....");
      sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null);

      return sslContext;
    }

    @Override
    public void configure(com.sun.net.httpserver.HttpsParameters params)
    {
      try
      {
        SSLContext c = SSLContext.getDefault();
        SSLEngine engine = c.createSSLEngine();
        params.setCipherSuites(engine.getEnabledCipherSuites());
        params.setProtocols(engine.getEnabledProtocols());

        SSLParameters defaultSSLParameters = c.getDefaultSSLParameters();
        defaultSSLParameters.setNeedClientAuth(true);
        params.setNeedClientAuth(true);
        params.setSSLParameters(defaultSSLParameters);
      }
      catch (Exception e)
      {
        throw new RuntimeException(e);
      }
    }
  }

  private static class MyHttpHandler implements com.sun.net.httpserver.HttpHandler
  {
    @Override
    public void handle(HttpExchange t) throws IOException
    {
      System.out.println("handler was called");
      String response = "This is the response";
      t.getResponseHeaders().add("Content-Type", "text/plain; charset=utf-8");
      t.sendResponseHeaders(200, response.length());
      OutputStream os = t.getResponseBody();
      os.write(response.getBytes(StandardCharsets.UTF_8));
      os.close();
    }
  }

  private static InputStream getResourceAsStream(String s)
  {
    return HttpsServer.class.getClassLoader().getResourceAsStream(s);
  }

  private static KeyStore readKeystore(InputStream resource)
  {
    try
    {
      KeyStore keystore = KeyStore.getInstance("JKS");
      keystore.load(resource, "changeit".toCharArray());
      return keystore;
    }
    catch (Exception e)
    {
      throw new RuntimeException(e);
    }
  }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
ignore that the method is called several times and always return null if there is no match

FREQUENCY : always



Comments
Fix request [17u] I backport this for parity with 17.0.10-oracle. Typical risk of changing security files. But I think we should go along, also the change brings the code to the lovel of a former status of head. I had to resolve. Now the code looks like head after pushing the same list of changes to the files concerned. Test passes and fails without the fix. SAP nightly testing passed. A CSR might be needed. Investigating why this is missing for 17.0.10-oracle. Update: no CSR has been added for the Oracle 17u backport. I think this is not really needed as the CSR for explicitly states that it is not a Spec change. I would even consider this as a bug. So I am asking for approval now to get it fixed in the 17.0.10 timeframe.
20-11-2023

A pull request was submitted for review. URL: https://git.openjdk.org/jdk17u-dev/pull/1885 Date: 2023-10-16 13:24:43 +0000
16-10-2023

Requested the submitter verify the fix by downloading the latest version of JDK 18 from https://jdk.java.net/18/
23-09-2021

Changeset: 3d657eb0 Author: Weijun Wang <weijun@openjdk.org> Date: 2021-08-31 20:07:02 +0000 URL: https://git.openjdk.java.net/jdk/commit/3d657eb0a626e33995af5d5ddf12b26d06317962
31-08-2021

The observations on Windows 10: JDK 1.8.0_261-ea-b04: Passed. JDK 1.8.0_261-b05: Failed, the method is called several times. JDK 11.0.6: Failed. JDK 15: Failed. JDK 17: Failed.
23-02-2021

Requested the truststore and keystore used in the reproducer from the submitter.
22-02-2021