-
Notifications
You must be signed in to change notification settings - Fork 122
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
WIP :- SSL certificate validation #207
base: develop
Are you sure you want to change the base?
Conversation
ashah-splunk
commented
Jan 19, 2023
- method to set SSL certificate
- README updated on how to set SSL certificate for verification
- Test case setup updated with commented code on how to set SSL certificate
- method to add SSL certificate - README updated on how to set SSL certificate - Test case updated with commented code using SSL certificate
###SSL Certificate Verification | ||
SSL Certificate validation is turned ON by default in Splunk Java SDK. Set SSL Certificate as shown below. | ||
```java | ||
HttpService.setSSLCert(<byte[] sslCert>); |
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.
This still needs the data input app to provide the cert. How do the data input app figure out which cert to provide here when communicating with the Splunk Enterprise/Cloud ?
@@ -109,9 +109,21 @@ To build the documentation for the SDK, it is being automatically generated with | |||
cd splunk | |||
mvn javadoc:javadoc | |||
|
|||
###SSL Certificate Verification |
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.
Missing space after ###
for headers
public static void preClassLoadActions() { | ||
// Bypass the certification validation here. | ||
public static void preClassLoadActions() throws IOException { | ||
// To Bypass the certification validation. | ||
HttpService.setValidateCertificates(false); |
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.
Could we have separate tests:
- For simulating the failure of an SSL cert validation
- and/or when setting to
true
, the ability to connect to the HTTPS resource with a valid SSL certificate?
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.
Hi! My name is JJ from Argentina, I just see the PR and I wanted to leave some comments. It's my first CR in this project, please feel free to ignore my comments if they don't apply. Thanks!
// For now this check is set as null. | ||
// TODO: Implementation logic for validating client certificate. | ||
} else { | ||
if(sslCert == 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.
I would add spaces between if
and (
also between )
and {
.
if(sslCert == null){ | ||
// On initialising before ssCert is set, TM and KM set to null | ||
context.init(null, null, null); | ||
}else{ |
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.
I would add spaces between }
and else
and {
tmf.init(ks); | ||
context.init(null, tmf.getTrustManagers(), null); | ||
} | ||
} else{ |
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.
I would add a space between else
and {
}else{ | ||
InputStream is = new ByteArrayInputStream(sslCert); | ||
CertificateFactory cf = CertificateFactory.getInstance("X.509"); | ||
X509Certificate cert = (X509Certificate)cf.generateCertificate(is); |
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.
Do we need this cast to X509Certificate
? It looks like the method ks.setCertificateEntry("cert", cert);
supports a Certificate
in the signature.