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

Provide TrustManager to OkHttpClient #300

Merged
merged 1 commit into from Mar 8, 2017

Conversation

josejulio
Copy link
Member

OkHttpClient was trying to guess [1] the TrustManager when using a secure connection. The guessing was failing and this exception was being raised:

-11:15:43,034 ERROR [org.hawkular.agent.monitor.service.MonitorService] (Hawkular WildFly Agent Startup Thread) HAWKMONITOR010054: Agent encountered errors during start up and will be stopped.: java.lang.IllegalStateException: Unable to extract the trust manager on okhttp3.internal.platform.Platform@14a0e0c6, sslSocketFactory is class org.jboss.as.domain.management.security.WrapperSSLContext$WrapperSpi$WrapperSSLSocketFactory
	at okhttp3.OkHttpClient$Builder.sslSocketFactory(OkHttpClient.java:599)
	at org.hawkular.agent.monitor.util.BaseHttpClientGenerator.<init>(BaseHttpClientGenerator.java:199)
	at org.hawkular.agent.monitor.storage.HttpClientBuilder.<init>(HttpClientBuilder.java:52)
	at org.hawkular.agent.monitor.service.MonitorService.startMonitorService(MonitorService.java:630)
	at org.hawkular.agent.monitor.service.MonitorService.startMonitorService(MonitorService.java:550)
	at org.hawkular.agent.monitor.service.MonitorService$1CustomPropertyChangeListener$1.run(MonitorService.java:508)
	at java.lang.Thread.run(Thread.java:745)

One workaround would be to downgrade OkHttp to 3.0.1 [2] but the solution is (as done on this PR) to pass the X509TrustManager to the OkHttpClient.

[1] https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/internal/platform/Platform.java#L92-L95
[2] http://stackoverflow.com/questions/37277679/using-okhttpclient-on-wildfly-causing-an-exception#comment62087487_37277679

@jmazzitelli
Copy link
Contributor

I haven't looked at this change deeply, but remember each managed server (remote DMR or remote JMX) can be given a security-realm when it needs to talk securely to external servers - https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent/src/main/resources/schema/hawkular-agent-monitor-subsystem.xsd#L328

This still works, right? :)

@josejulio
Copy link
Member Author

Is OkHttp used to do that?
I'm loading the trustmanagers for every used realm but I don't recall any code that does that. I'll double check that.

I tested this with hawkular-services https with its local agent and a remote eap6 agent talking over https to hawkular-services.

@josejulio
Copy link
Member Author

It should work. I didn't touch any of that.
Tried to test yesterday (secure hawkular services connecting to a remote-dmr secure eap7) but keep getting Caused by: java.security.SignatureException: Signature does not match.
I suppose I missed something. I'll try again.

@jmazzitelli
Copy link
Contributor

@josejulio sounds like we should probably update one of our itests so that the agent can go through a secure remote-dmr. if its possible to do that, we should add an itest like that - or at least change one of the itests so it goes through a remote secure endpoint rather than an unsecured one

@jmazzitelli
Copy link
Contributor

I can confirm that this works on EAP 7.1 talking to Hawkular Server, remote DMR and remote JMX endpoints all over HTTPS with security realms defined.

However, EAP 6.4 does not work due to a partially implemented class in an EAP 6.4 library. From my email to hawkular-dev I sent today:

This is a EAP 6.4 method that OKHttp is calling when making an HTTP request requiring SSL - I'll give you the summary - its a one-line auto-generated stub method that "return null;" 

https://github.com/wildfly/wildfly-core/blame/de6b17d4d342e98871c0e95f7e6faa9006383768/domain-management/src/main/java/org/jboss/as/domain/management/security/WrapperSSLContext.java#L124-L126

I stepped into this code via a debugger and the line number and behavior (returning null always) matches up with that code.

Needless to say, this causes a NullPointerException later on in the OKHttp library and thus cannot talk to the Hawkular Server over HTTPS.

There is no much we can do about that. This PR can be merged as it is because it isn't making the agent on EAP 6.4 any more broken than it already was.

@jmazzitelli
Copy link
Contributor

jmazzitelli commented Mar 7, 2017

BTW: I take that back. This can't be merged yet until we fix the installer so it creates the proper security realm that we now need. @josejulio knows what this means :)

This has to be fixed:

https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent-installer/src/main/java/org/hawkular/wildfly/agent/installer/AgentInstaller.java#L388-L397

Need to generate truststore subsection under authentication now:

<security-realm name="ServerRealm">
    <authentication>
      <truststore path="hawkular.keystore"
                  relative-to="jboss.server.config.dir"
                  keystore-password="hawkular" />
    </authentication>
</security-realm>

Note that we should deprecate the KEY_PASSWORD related stuff since that isn't needed.

@josejulio
Copy link
Member Author

josejulio commented Mar 7, 2017

Note that we should deprecate the KEY_PASSWORD related stuff since that isn't needed.

Suppose you mean and the KEY_ALIAS

@josejulio
Copy link
Member Author

I haven't tested this yet. Might need to update tests if any of these used the KEY_PASSWORD or KEY_ALIAS.

@josejulio
Copy link
Member Author

Just tested and it works, see the generated security-realm

<security-realm name="HawkularRealm">
    <authentication>
        <truststore keystore-password="hawkular" path="hawkular.keystore" relative-to="jboss.server.config.dir"/>
    </authentication>
</security-realm>

Only issue is that i had to specify the --module-dist option, else i would get:

Mar 07, 2017 5:05:45 PM org.hawkular.wildfly.agent.installer.AgentInstaller downloadModuleZip
INFO: Downloading agent module extension from: https://localhost:8443/hawkular/wildfly-agent/download
Mar 07, 2017 5:05:45 PM org.hawkular.wildfly.agent.installer.AgentInstaller downloadModuleZip
WARN: Unable to download hawkular wildfly agent module extension: https://localhost:8443/hawkular/wildfly-agent/download
javax.net.ssl.SSLHandshakeException: sun.security.validator.ValidatorException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
	at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
	at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1949)
	at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:302)
	at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:296)
	at sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1509)
	at sun.security.ssl.ClientHandshaker.processMessage(ClientHandshaker.java:216)
	at sun.security.ssl.Handshaker.processLoop(Handshaker.java:979)
	at sun.security.ssl.Handshaker.process_record(Handshaker.java:914)
	at sun.security.ssl.SSLSocketImpl.readRecord(SSLSocketImpl.java:1062)
	at sun.security.ssl.SSLSocketImpl.performInitialHandshake(SSLSocketImpl.java:1375)
	at sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1403)
	at sun.security.ssl.SSLSocketImpl.startHandshake(SSLSocketImpl.java:1387)
	at sun.net.www.protocol.https.HttpsClient.afterConnect(HttpsClient.java:559)
	at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:185)
	at sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1546)
	at sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1474)
	at sun.net.www.protocol.https.HttpsURLConnectionImpl.getInputStream(HttpsURLConnectionImpl.java:254)
	at java.net.URL.openStream(URL.java:1045)
	at org.hawkular.wildfly.agent.installer.AgentInstaller.downloadModuleZip(AgentInstaller.java:542)
	at org.hawkular.wildfly.agent.installer.AgentInstaller.main(AgentInstaller.java:164)
Caused by: sun.security.validator.ValidatorException: PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
	at sun.security.validator.PKIXValidator.doBuild(PKIXValidator.java:387)
	at sun.security.validator.PKIXValidator.engineValidate(PKIXValidator.java:292)
	at sun.security.validator.Validator.validate(Validator.java:260)
	at sun.security.ssl.X509TrustManagerImpl.validate(X509TrustManagerImpl.java:324)
	at sun.security.ssl.X509TrustManagerImpl.checkTrusted(X509TrustManagerImpl.java:229)
	at sun.security.ssl.X509TrustManagerImpl.checkServerTrusted(X509TrustManagerImpl.java:124)
	at sun.security.ssl.ClientHandshaker.serverCertificate(ClientHandshaker.java:1491)
	... 15 more
Caused by: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target
	at sun.security.provider.certpath.SunCertPathBuilder.build(SunCertPathBuilder.java:141)
	at sun.security.provider.certpath.SunCertPathBuilder.engineBuild(SunCertPathBuilder.java:126)
	at java.security.cert.CertPathBuilder.build(CertPathBuilder.java:280)
	at sun.security.validator.PKIXValidator.doBuild(PKIXValidator.java:382)
	... 21 more

Mar 07, 2017 5:05:45 PM org.hawkular.wildfly.agent.installer.AgentInstaller main

@josejulio
Copy link
Member Author

I'll try to have EAP6 working with ssl.

@jmazzitelli
Copy link
Contributor

Yes, the installer by default will attempt to download the module distribution from your server-url. If the server-url requires SSL, the installer doesn't support that. But in that case, you can tell the installer another URL (one that is http, for example) using the --download-server-url option (that simply overrides --server-url for when the installer wants to download the extension zip.)

@jmazzitelli
Copy link
Contributor

Ok, there is more to do. We have to do the server-side portion that the installer uses. The installer calls this servlet to download and build the agent extension jar. We need to remove those unused key password and alias properties. Remove these and related code:

https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent-download/src/main/java/org/hawkular/component/wildflymonitor/WildFlyAgentServlet.java#L85-L86

We also need to remove them from the installer's default config file:

https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent-installer/src/main/resources/hawkular-wildfly-agent-installer.properties#L123-L124

and the test config:

https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent-installer/src/test/resources/test-installer.properties#L26-L27

@josejulio
Copy link
Member Author

Yes, the installer by default will attempt to download the module distribution from your server-url. If the server-url requires SSL, the installer doesn't support that. But in that case, you can tell the installer another URL (one that is http, for example) using the --download-server-url option (that simply overrides --server-url for when the installer wants to download the extension zip.)

Wasn't aware of the --download-server-url , thanks!

@josejulio
Copy link
Member Author

Ok, there is more to do. We have to do the server-side portion that the installer uses. The installer calls this servlet to download and build the agent extension jar. We need to remove those unused key password and alias properties. Remove these and related code:

https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent-download/src/main/java/org/hawkular/component/wildflymonitor/WildFlyAgentServlet.java#L85-L86

We also need to remove them from the installer's default config file:

https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent-installer/src/main/resources/hawkular-wildfly-agent-installer.properties#L123-L124

and the test config:

https://github.com/hawkular/hawkular-agent/blob/master/hawkular-wildfly-agent-installer/src/test/resources/test-installer.properties#L26-L27

Done

@jmazzitelli
Copy link
Contributor

just ran some quick smoke tests and all looks good.

  - When loading from keystore.
  - When loading from the realm data.
Update installer to set trustore instead of keystore
  - This means we no longer need key_alias and key_password
Added EAP6 support by using reflection.
@jmazzitelli
Copy link
Contributor

travis just timed out - that's why its red. If you look at the raw log output of the test run, mvn shows it all succeeded.

@jmazzitelli
Copy link
Contributor

I hate travis - now it failed because it can't start the itest servers due to:

Caused by: java.net.BindException: Address already in use

So screw it, this all passes on my box, so I'm merging.

@jmazzitelli jmazzitelli merged commit 1b3f887 into hawkular:master Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants