-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Support insecure ca root validation. #32
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,16 +114,22 @@ public static ApiClient fromConfig(Reader input) { | |
| ex.printStackTrace(); | ||
| } | ||
|
|
||
| // It's silly to have to do it in this order, but each SSL setup | ||
| // consumes the CA cert, so if we do this before the client certs | ||
| // are injected the cert input stream is exhausted and things get | ||
| // grumpy' | ||
| String caCert = config.getCertificateAuthorityData(); | ||
| String caCertFile = config.getCertificateAuthorityFile(); | ||
| try { | ||
| client.setSslCaCert(SSLUtils.getInputStreamFromDataOrFile(caCert, caCertFile)); | ||
| } catch (FileNotFoundException e) { | ||
| e.printStackTrace(); | ||
| if (config.verifySSL()) { | ||
| // It's silly to have to do it in this order, but each SSL setup | ||
| // consumes the CA cert, so if we do this before the client certs | ||
| // are injected the cert input stream is exhausted and things get | ||
| // grumpy' | ||
| String caCert = config.getCertificateAuthorityData(); | ||
| String caCertFile = config.getCertificateAuthorityFile(); | ||
| if (caCert != null || caCertFile != null) { | ||
| try { | ||
| client.setSslCaCert(SSLUtils.getInputStreamFromDataOrFile(caCert, caCertFile)); | ||
| } catch (FileNotFoundException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| } | ||
| } else { | ||
| client.setVerifyingSsl(false); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we also set this to true in the previous block. just to be bullet proof (e.g. somebody loading config with verifySSL false and then again with it set to true.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This call is static and always creates both a new |
||
| } | ||
|
|
||
| String token = config.getAccessToken(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we fail if config.verifySSL() is true and both of these two are null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because you could be using a legit well-known CA issued cert, in which case you don't have to supply your own CA root.