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

Respond 502 when target SSL untrusted #309

Merged

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Oct 23, 2020

Respond with status code 502 and message "Target SSL Untrusted" when attempting a target connection fails for what appears to be an SSL trust issue

Fixes #307

This can be tested by running smoketest.sh. Normally, the truststore directory should contain the vertx-fib-demo.cer copied from that project's repo, and this allows ContainerJFR to connect to the demo application when running in the smoketest. By removing the .cer from the truststore (ex. simply mv truststore/vertx-fib-demo.cer .) and re-running sh smoketest.sh an SSL trust failure can be easily triggered. Before this patch the response is a standard HTTP 404. After this patch the response is a 502 with the custom message "Target SSL Untrusted". This can be seen in the web-client notifications as well as with standard tooling such as curl.

@andrewazores andrewazores requested a review from vic-ma October 23, 2020 17:45
@andrewazores andrewazores force-pushed the target-ssl-untrusted-response branch from 8fe2746 to 5428bf8 Compare October 23, 2020 18:26
@vic-ma
Copy link
Contributor

vic-ma commented Oct 23, 2020

The HTTP_API.md should be updated to mention the possible 502.

I don't seem to have anything in my truststore/ either before or after running smoketest.sh, and I've tested on different forks. Is there something I need to do to get the certificate in there? Generally I have the disable-ssl and disable-jmx-auth env vars set to true, but I also tried disabling them for this.

@andrewazores
Copy link
Member Author

Yea, the certificate(s) aren't committed to the repo or automatically added. Just clone https://github.com/andrewazores/vertx-fib-demo and copy its certificate (src/main/extras/app/resources/vertx-fib-demo.cer) into truststore manually.

@andrewazores
Copy link
Member Author

I added documentation for the new possible 502.

Looking over that document again I noticed there are a lot of places that name 427 as a possible response code, but I'm not sure it's actually valid in all the cases documented - in particular, handlers which operate on archived recordings and claim to be able to respond with a 427. The implementation might suggest this is possible (unfortunately), but in practice a 427 would only occur when a target connection attempt is made and then fails due to missing/wrong credentials - and when operating on the archives, no target connection attempt should be made (there isn't even a targetId supplied with the request, so where would the connection be made to?). Could you take a look back over that document and see if I'm reading it correctly, and clean up those instances if so?

Respond with status code 502 and message "Target SSL Untrusted" when attempting a target connection fails for what appears to be an SSL trust issue

Fixes cryostatio#307
@andrewazores andrewazores force-pushed the target-ssl-untrusted-response branch from 4c728cd to c21877c Compare October 26, 2020 17:33
@vic-ma
Copy link
Contributor

vic-ma commented Oct 26, 2020

Where can I find the credentials to authenticate into vertx-fib-demo? I see that it's smoketest:smoketest for the cjfr JVM, but I can't find where the username and password are being set for the demo app.

Looking over that document again I noticed there are a lot of places that name 427 as a possible response code, but I'm not sure it's actually valid in all the cases documented

Yes, that seems to be the case. I'll look over it and submit a fix after.

@andrewazores
Copy link
Member Author

The credentials for vertx-fib-demo are admin:adminpass123 - it's a very secure sample application :-)

https://github.com/andrewazores/vertx-fib-demo/blob/master/src/main/extras/app/resources/jmxremote.password

There is also a user:pass account which can be used, but it doesn't have sufficient JMX permissions to really do anything. Still, it can be useful for testing what happens when a JMX connection succeeds but the operation fails.

@vic-ma
Copy link
Contributor

vic-ma commented Oct 26, 2020

Tested and everything seems good. Just one last thing for the HTTP API doc.

In TargetRecordingUploadPostHandler, there was already a 502 response code:

502 - Container JFR received an invalid response from the Grafana datasource after sending the upload request. The body is an error message.
502 - JMX connection failed. This is generally because the target application has SSL enabled over JMX, but ContainerJFR does not trust the certificate.

So the two possible reasons should be combined. The way I did this in some 500 responses was just to add an "or" at the beginning of the second reason. So, something like ". . . Or the JMX connection failed. This is generally . . ." would work I think.

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.

When target certificate is not accepted, requests fail with 404
2 participants