You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/10/10 21:49:38 UTC
[GitHub] [drill] cgivre opened a new pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
cgivre opened a new pull request #2331:
URL: https://github.com/apache/drill/pull/2331
# [DRILL-8008](https://issues.apache.org/jira/browse/DRILL-8008): Add Config Option to HTTP Plugin to Skip SSL Validation
## Description
In the current implementation, Drill validates all SSL certificates when querying REST APIs. In some circumstances, such as a corporate network, or for testing, a user might want to disable this functionality. This PR adds a config option to the HTTP plugin to disable SSL validation.
## Documentation
Update `README` with the following:
#### verifySSLCert
Default is `true`, but when set to false, Drill will trust all SSL certificates. Useful for debugging or on internal corporate networks using self-signed certificates or
private certificate authorities.
## Testing
Manually tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2331:
URL: https://github.com/apache/drill/pull/2331#issuecomment-941237683
@vvysotskyi Thanks for your quick review. I addressed your comments. Anything else?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2331:
URL: https://github.com/apache/drill/pull/2331#issuecomment-941293287
@cgivre, looks good, but please fix the checkstyle failure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2331:
URL: https://github.com/apache/drill/pull/2331#issuecomment-941320244
> @cgivre, looks good, but please fix the checkstyle failure.
Oops... Sorry about that... Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2331:
URL: https://github.com/apache/drill/pull/2331#issuecomment-941237683
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2331:
URL: https://github.com/apache/drill/pull/2331#discussion_r727345517
##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpApiConfig.java
##########
@@ -102,6 +102,8 @@
private final int xmlDataLevel;
@JsonProperty
private final boolean errorOn400;
+ @JsonProperty
Review comment:
Done
##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java
##########
@@ -116,6 +125,28 @@ private OkHttpClient setupHttpClient() {
builder.writeTimeout(timeout, TimeUnit.SECONDS);
builder.readTimeout(timeout, TimeUnit.SECONDS);
+ // Code to skip SSL Certificate validation
+ // Sourced from https://stackoverflow.com/questions/60110848/how-to-disable-ssl-verification
+ if (! scanDefn.tableSpec().connectionConfig().verifySSLCert()) {
+ try {
+ TrustManager[] trustAllCerts = getAllTrustingTrustManager();
+ SSLContext sslContext = SSLContext.getInstance("SSL");
+ sslContext.init(null, trustAllCerts, new java.security.SecureRandom());
+ SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
+
+
+ builder.sslSocketFactory(sslSocketFactory, (X509TrustManager) trustAllCerts[0]);
+ builder.hostnameVerifier(new HostnameVerifier() {
+ @Override
+ public boolean verify(String hostname, SSLSession session) {
+ return true;
+ }
+ });
Review comment:
Done! Much cleaner.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2331:
URL: https://github.com/apache/drill/pull/2331#discussion_r727366808
##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java
##########
@@ -116,6 +125,28 @@ private OkHttpClient setupHttpClient() {
builder.writeTimeout(timeout, TimeUnit.SECONDS);
builder.readTimeout(timeout, TimeUnit.SECONDS);
+ // Code to skip SSL Certificate validation
+ // Sourced from https://stackoverflow.com/questions/60110848/how-to-disable-ssl-verification
+ if (! scanDefn.tableSpec().connectionConfig().verifySSLCert()) {
+ try {
+ TrustManager[] trustAllCerts = getAllTrustingTrustManager();
+ SSLContext sslContext = SSLContext.getInstance("SSL");
+ sslContext.init(null, trustAllCerts, new java.security.SecureRandom());
+ SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
+
+
+ builder.sslSocketFactory(sslSocketFactory, (X509TrustManager) trustAllCerts[0]);
+ builder.hostnameVerifier(new HostnameVerifier() {
+ @Override
+ public boolean verify(String hostname, SSLSession session) {
+ return true;
+ }
+ });
Review comment:
Done! Much cleaner.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre commented on a change in pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2331:
URL: https://github.com/apache/drill/pull/2331#discussion_r727345517
##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpApiConfig.java
##########
@@ -102,6 +102,8 @@
private final int xmlDataLevel;
@JsonProperty
private final boolean errorOn400;
+ @JsonProperty
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre merged pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2331:
URL: https://github.com/apache/drill/pull/2331
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on a change in pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2331:
URL: https://github.com/apache/drill/pull/2331#discussion_r726523723
##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java
##########
@@ -116,6 +125,28 @@ private OkHttpClient setupHttpClient() {
builder.writeTimeout(timeout, TimeUnit.SECONDS);
builder.readTimeout(timeout, TimeUnit.SECONDS);
+ // Code to skip SSL Certificate validation
+ // Sourced from https://stackoverflow.com/questions/60110848/how-to-disable-ssl-verification
+ if (! scanDefn.tableSpec().connectionConfig().verifySSLCert()) {
+ try {
+ TrustManager[] trustAllCerts = getAllTrustingTrustManager();
+ SSLContext sslContext = SSLContext.getInstance("SSL");
+ sslContext.init(null, trustAllCerts, new java.security.SecureRandom());
+ SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory();
+
+
+ builder.sslSocketFactory(sslSocketFactory, (X509TrustManager) trustAllCerts[0]);
+ builder.hostnameVerifier(new HostnameVerifier() {
+ @Override
+ public boolean verify(String hostname, SSLSession session) {
+ return true;
+ }
+ });
Review comment:
Can we use lambda here instead of declaring an anonymous class?
##########
File path: contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/HttpApiConfig.java
##########
@@ -102,6 +102,8 @@
private final int xmlDataLevel;
@JsonProperty
private final boolean errorOn400;
+ @JsonProperty
Review comment:
Please add `@JsonInclude` annotation, since it wouldn't be possible to disable this property.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] vvysotskyi commented on pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on pull request #2331:
URL: https://github.com/apache/drill/pull/2331#issuecomment-941293287
@cgivre, looks good, but please fix the checkstyle failure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [drill] cgivre merged pull request #2331: DRILL-8008: Add Config Option to HTTP Plugin to Skip SSL Validation
Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2331:
URL: https://github.com/apache/drill/pull/2331
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org