Jib is not sending client certificate regardless of the javax.net.ssl.keyStore property.

[jacksierkstra]

Environment:

• Jib version: 2.4.0
• Build tool: Maven
• OS: Windows 10

Description of the issue:
The issue has been filed before, but in googleapis/google-http-java-client#904 (comment). The problem is that whenever you add the jvm property: javax.net.ssl.keyStore, it is not picked up by the eventual HttpClient.

I did some research and it seems that org.apache.http.conn.ssl.SSLConnectionSocketFactory#getSocketFactory() is always called whenever the plugin is constructing a HttpClient. If org.apache.http.conn.ssl.SSLConnectionSocketFactory#getSystemSocketFactory() would be called, the property: javax.net.ssl.keyStore would be taken into consideration and therefore mutual SSL authentication would be possible.

The problem is that org.apache.http.ssl.SSLContexts#createDefault() is always constructing an SSLContext without a keymanager and/or trustmanager and that makes mutual SSL impossible.

Expected behavior:
Whenever I set the property: javax.net.ssl.keyStore, I expect that the keystore would be set on the SSLContext.

Steps to reproduce:

1. Create a project with Jib Maven plugin.
2. Create a custom keystore.
3. Run mvn -Djavax.net.debug=ssl,handshake -Djavax.net.ssl.keyStore=/path/to/keystore -Djavax.net.ssl.keyStorePassword=changeit jib:build

[loosebazooka]

I'm not familiar with keystore stuff. Perhaps @chanseokoh can take a look when's back from vacation.

[jacksierkstra]

Ok, no problem. To give you a little bit extra information: when com.google.cloud.tools.jib.http.FailoverHttpClient is constructed, and secure transport is being initiated, it uses this method:

  private static HttpTransport getSecureHttpTransport() {
    // Do not use NetHttpTransport. It does not process response errors properly.
    // See https://github.com/google/google-http-java-client/issues/39
    //
    // A new ApacheHttpTransport needs to be created for each connection because otherwise HTTP
    // connection persistence causes the connection to throw NoHttpResponseException.
    return new ApacheHttpTransport();
  }

When new ApacheHttpTransport() is being called, it constructs a new default http client. The creation of that client looks as follows:

  public static HttpClientBuilder newDefaultHttpClientBuilder() {

    return HttpClientBuilder.create()
            .useSystemProperties()
            .setSSLSocketFactory(SSLConnectionSocketFactory.getSocketFactory()) // <--- this is the problem
            .setMaxConnTotal(200)
            .setMaxConnPerRoute(20)
            .setConnectionTimeToLive(-1, TimeUnit.MILLISECONDS)
            .setRoutePlanner(new SystemDefaultRoutePlanner(ProxySelector.getDefault()))
            .disableRedirectHandling()
            .disableAutomaticRetries();
  }

org.apache.http.conn.ssl.SSLConnectionSocketFactory has two static methods that can be called, one of which is getSocketFactory() and the other one is getSystemSocketFactory().

The problem with using getSocketFactory()

The method looks like this:

 public static SSLConnectionSocketFactory getSocketFactory() throws SSLInitializationException {
        return new SSLConnectionSocketFactory(SSLContexts.createDefault(), getDefaultHostnameVerifier());
    }

Whenever this SSLConnectionSocketFactory is constructed, the first parameter, which is the SSLContext is created by this method:

public static SSLContext createDefault() throws SSLInitializationException {
        try {
            final SSLContext sslContext = SSLContext.getInstance(SSLContextBuilder.TLS);
            sslContext.init(null, null, null);
            return sslContext;
        } catch (final NoSuchAlgorithmException ex) {
            throw new SSLInitializationException(ex.getMessage(), ex);
        } catch (final KeyManagementException ex) {
            throw new SSLInitializationException(ex.getMessage(), ex);
        }
    }

The first parameter of sslContext.init(null, null, null); is KeyManager[] and the second one is TrustManager[]. As you can see, these are always null and therefore, no keyStore and/or trustStore is taken into account.' So whenever you are setting the java parameter by -Djavax.net.ssl.keyStore=/path/to/keystore, the keystore that you provide will be disregarded.

The solution by using getSystemSocketFactory()

The method looks as follows:

    public static SSLConnectionSocketFactory getSystemSocketFactory() throws SSLInitializationException {
        return new SSLConnectionSocketFactory(
            (javax.net.ssl.SSLSocketFactory) javax.net.ssl.SSLSocketFactory.getDefault(),
            split(System.getProperty("https.protocols")),
            split(System.getProperty("https.cipherSuites")),
            getDefaultHostnameVerifier());
    }

As you can see, it creates a new SSLConnectionFactory instance, but it takes javax.net.ssl.SSLSocketFactory.getDefault(). Whenever you are setting the java parameter by -Djavax.net.ssl.keyStore=/path/to/keystore, the keystore that you provide will be used.

[chanseokoh]

@jacksierkstra thanks for the very detailed information. I do remember the context, but your comment saved me a lot of time.

We can certainly overcome this limitation on the Jib side; I will merge #2592 soon. However, I still have the general question why Google HTTP Client doesn't use the system socket factory by default (I assume it has a reason) and whether it should switch the default, as I don't think I have the expertise to answer these questions. What's your thoughts?

[chanseokoh]

That is, should Google HTTP Client fix googleapis/google-http-java-client#904 on their side as well?

[jacksierkstra]

@jacksierkstra thanks for the very detailed information. I do remember the context, but your comment saved me a lot of time.

We can certainly overcome this limitation on the Jib side; I will merge #2592 soon. However, I still have the general question why Google HTTP Client doesn't use the system socket factory by default (I assume it has a reason) and whether it should switch the default, as I don't think I have the expertise to answer these questions. What's your thoughts?

I was reading this exact issue that you are referring to as well and I am not really sure as to why they are using the SSLContexts.createDefault() as a default.

My guess is that they want to make sure that the SSLContext would not be altered somehow by anything on the system. Suppose someone replaces the keystore/truststore from your system with a fabricated one, that could mean some problems in terms of security. But again, this is just guessing from my side.

[jacksierkstra]

That is, should Google HTTP Client fix googleapis/google-http-java-client#904 on their side as well?

I am not sure if there is anything to be fixed on their side. They provide both ApacheHttpTransport() and ApacheHttpTransport(HttpClient httpClient) so I think it is up to the library/application that is using it to decide whether it is acceptable or not.

[chanseokoh]

That's true. However, I know the default Google HTTP Client honors the other system property javax.net.ssl.trustStore, so from the user perspective, this seems inconsistent apparently. In any case, thanks for filing the PR.

[jacksierkstra]

My pleasure :).

[chanseokoh]

Fixed by #2592.

[chanseokoh]

@jacksierkstra we released Jib Gradle 2.5.0 and Jib Maven 2.5.2 with the fix. Thanks for your contribution! Let us know if it still doesn't work.

[jacksierkstra]

Thank you for the notification, I will let you know if something is not working.

[chanseokoh]

Just in case, we've patched Jib Maven (patches unrelated to this issue), so the latest is 2.5.2. (OTOH, the latest for Jib Gradle is 2.5.0.)

[chanseokoh]

They fixed googleapis/google-http-java-client#904. I don't know how they fixed it, but we might be able to remove what we did on our end (#2592) after upgrading the Google HTTP Client?