HTTP-119 use default certs when no security configured for https

davidradl · davidradl · commit ce521674ec10 · 2024-10-14T16:59:09.000+01:00
Signed-off-by: davidradl &lt;david_radley@uk.ibm.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Added
 
+-   Added support for built in JVM certificates if no security is configured. 
 -   Added support for OIDC Bearer tokens.
 
 ## [0.15.0] - 2024-07-30
diff --git a/README.md b/README.md
@@ -392,18 +392,19 @@ An example of such a mask would be `3XX, 4XX, 5XX`. In this case, all 300s, 400s
    Many status codes can be defined in one value, where each code should be separated with comma, for example:
   `401, 402, 403`. In this example, codes 401, 402 and 403 would not be interpreted as error codes.
 
-## TLS and mTLS support
-Both Http Sink and Lookup Source connectors supports Https communication using TLS 1.2 and mTLS.
+## TLS (more secure replacement for SSL) and mTLS support 
+Both Http Sink and Lookup Source connectors support Https communication using TLS 1.2 and mTLS.
 To enable Https communication simply use `https` protocol in endpoint's URL.
-If certificate used by HTTP server is self-signed, or it is signed byt not globally recognize CA
-you would have to add this certificate to connector's keystore as trusted certificate.
-In order to do so, use `gid.connector.http.security.cert.server` connector property,
-which value is a path to the certificate. You can also use your organization's CA Root certificate.
-You can specify many certificate, separating each path with `,`.
-
-You can also configure connector to use mTLS. For this simply use `gid.connector.http.security.cert.client`
-and `gid.connector.http.security.key.client` connector properties to specify path to certificate and
-private key that should be used by connector. Key MUST be in `PCKS8` format. Both PEM and DER keys are
+To specify certificate(s) to be used by the server, use `gid.connector.http.security.cert.server` connector property;
+the value is a comma separated list of paths to certificate(s), for example you can use your organization's CA
+Root certificate, or a self-signed certificate. 
+
+Note that if there are no security properties for a `https` url then, the JVMs default certificates are
+used - allowing use of globally recognized CAs without the need for configuration.
+
+You can also configure the connector to use mTLS. For this simply use `gid.connector.http.security.cert.client`
+and `gid.connector.http.security.key.client` connector properties to specify paths to the certificate and 
+private key. The key MUST be in `PCKS8` format. Both PEM and DER keys are
 allowed.
 
 All properties can be set via Sink's builder `.setProperty(...)` method or through Sink and Source table DDL.
@@ -415,7 +416,7 @@ To enable this option use `gid.connector.http.security.cert.server.allowSelfSign
 ## Basic Authentication
 The connector supports Basic Authentication using a HTTP `Authorization` header.
 The header value can be set via properties, similarly as for other headers. The connector converts the passed value to Base64 and uses it for the request.
-If the used value starts with the prefix `Basic `, or `gid.connector.http.source.lookup.use-raw-authorization-header`
+If the used value starts with the prefix `Basic`, or `gid.connector.http.source.lookup.use-raw-authorization-header`
 is set to `'true'`, it will be used as header value as is, without any extra modification.
 
 ## OIDC Bearer Authentication
@@ -452,13 +453,13 @@ be requested if the current time is later than the cached token expiry time minu
 | lookup.max-retries                                            | optional | The max retry times if the lookup failed; default is 3. See the following <a href="#lookup-cache">Lookup Cache</a> section for more details.                                                                                                                                                                                                                      |
 | gid.connector.http.lookup.error.code                          | optional | List of HTTP status codes that should be treated as errors by HTTP Source, separated with comma.                                                                                                                                                                                                                                                                  |
 | gid.connector.http.lookup.error.code.exclude                  | optional | List of HTTP status codes that should be excluded from the `gid.connector.http.lookup.error.code` list, separated with comma.                                                                                                                                                                                                                                     |
-| gid.connector.http.security.cert.server                       | optional | Path to trusted HTTP server certificate that should be add to connectors key store. More than one path can be specified using `,` as path delimiter.                                                                                                                                                                                                              |
+| gid.connector.http.security.cert.server                       | optional | Comma separated paths to trusted HTTP server certificates that should be add to connectors trust store.                                                                                                                                                                                          |
 | gid.connector.http.security.cert.client                       | optional | Path to trusted certificate that should be used by connector's HTTP client for mTLS communication.                                                                                                                                                                                                                                                                |
 | gid.connector.http.security.key.client                        | optional | Path to trusted private key that should be used by connector's HTTP client for mTLS communication.                                                                                                                                                                                                                                                                |
 | gid.connector.http.security.cert.server.allowSelfSigned       | optional | Accept untrusted certificates for TLS communication.                                                                                                                                                                                                                                                                                                              |
-| gid.connector.http.security.oidc.token.request                | optional | OIDC `Token Request` body in `application/x-www-form-urlencoded` encoding                                                                                                                                                          |
-| gid.connector.http.security.oidc.token.endpoint.url           | optional | OIDC `Token Endpoint` url, to which the token request will be issued                                                                                                                                                               |
-| gid.connector.http.security.oidc.token.expiry.reduction       | optional | OIDC tokens will be requested if the current time is later than the cached token expiry time minus this value.                                                                                                                     |
+| gid.connector.http.security.oidc.token.request                | optional | OIDC `Token Request` body in `application/x-www-form-urlencoded` encoding                                                                                                                                                                                                                                                                                         |
+| gid.connector.http.security.oidc.token.endpoint.url           | optional | OIDC `Token Endpoint` url, to which the token request will be issued                                                                                                                                                                                                                                                                                              |
+| gid.connector.http.security.oidc.token.expiry.reduction       | optional | OIDC tokens will be requested if the current time is later than the cached token expiry time minus this value.                                                                                                                                                                                                                                                    |
 | gid.connector.http.source.lookup.request.timeout              | optional | Sets HTTP request timeout in seconds. If not specified, the default value of 30 seconds will be used.                                                                                                                                                                                                                                                             |
 | gid.connector.http.source.lookup.request.thread-pool.size     | optional | Sets the size of pool thread for HTTP lookup request processing. Increasing this value would mean that more concurrent requests can be processed in the same time. If not specified, the default value of 8 threads will be used.                                                                                                                                 |
 | gid.connector.http.source.lookup.response.thread-pool.size    | optional | Sets the size of pool thread for HTTP lookup response processing. Increasing this value would mean that more concurrent requests can be processed in the same time. If not specified, the default value of 4 threads will be used.                                                                                                                                |
diff --git a/src/main/java/com/getindata/connectors/http/internal/security/SecurityContext.java b/src/main/java/com/getindata/connectors/http/internal/security/SecurityContext.java
@@ -168,7 +168,7 @@ public void addCertToTrustStore(String certPath) {
     }
 
     /**
-     * Add certificate and private key that should be used by anny Http Connector instance that uses
+     * Add certificate and private key that should be used by any Http Connector instance that uses
      * this {@link SSLContext} instance. Certificate and key are added only to this Context's
      * {@link KeyStore} and not for entire JVM.
      * @param publicKeyPath path to public key/certificate used for mTLS.
diff --git a/src/main/java/com/getindata/connectors/http/internal/utils/JavaNetHttpClientFactory.java b/src/main/java/com/getindata/connectors/http/internal/utils/JavaNetHttpClientFactory.java
@@ -2,6 +2,7 @@
 
 import java.net.http.HttpClient;
 import java.net.http.HttpClient.Redirect;
+import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Properties;
@@ -24,8 +25,8 @@
 public class JavaNetHttpClientFactory {
 
     /**
-     * Creates Java's {@link HttpClient} instance that will be using default, JVM shared {@link
-     * java.util.concurrent.ForkJoinPool} for async calls.
+     * Creates Java's {@link HttpClient} instance that will be using default, JVM shared
+     * {@link java.util.concurrent.ForkJoinPool} for async calls.
      *
      * @param properties properties used to build {@link SSLContext}
      * @return new {@link HttpClient} instance.
@@ -73,21 +74,39 @@ public static HttpClient createClient(Properties properties, Executor executor)
      * @return new {@link SSLContext} instance.
      */
     private static SSLContext getSslContext(Properties properties) {
-        SecurityContext securityContext = createSecurityContext(properties);
+
+        String keyStorePath =
+            properties.getProperty(HttpConnectorConfigConstants.KEY_STORE_PATH, "");
 
         boolean selfSignedCert = Boolean.parseBoolean(
             properties.getProperty(HttpConnectorConfigConstants.ALLOW_SELF_SIGNED, "false"));
 
         String[] serverTrustedCerts = properties
-            .getProperty(HttpConnectorConfigConstants.SERVER_TRUSTED_CERT, "")
-            .split(HttpConnectorConfigConstants.PROP_DELIM);
+             .getProperty(HttpConnectorConfigConstants.SERVER_TRUSTED_CERT, "")
+                .split(HttpConnectorConfigConstants.PROP_DELIM);
 
         String clientCert = properties
             .getProperty(HttpConnectorConfigConstants.CLIENT_CERT, "");
 
         String clientPrivateKey = properties
             .getProperty(HttpConnectorConfigConstants.CLIENT_PRIVATE_KEY, "");
 
+        if (StringUtils.isNullOrWhitespaceOnly(keyStorePath)
+            && !selfSignedCert
+            // checking the property in this way so that serverTrustedCerts is not left and null
+            // or empty, which causes the http client to error.
+            && (properties.getProperty(HttpConnectorConfigConstants.SERVER_TRUSTED_CERT) == null)
+            && StringUtils.isNullOrWhitespaceOnly(clientCert)
+            && StringUtils.isNullOrWhitespaceOnly(clientPrivateKey)) {
+            try {
+                return SSLContext.getDefault();
+            } catch (NoSuchAlgorithmException e) {
+                throw new RuntimeException(e);
+            }
+        }
+
+        SecurityContext securityContext = createSecurityContext(properties);
+
         for (String cert : serverTrustedCerts) {
             if (!StringUtils.isNullOrWhitespaceOnly(cert)) {
                 securityContext.addCertToTrustStore(cert);
diff --git a/src/test/java/com/getindata/connectors/http/internal/table/lookup/querycreators/ElasticSearchLiteQueryCreatorTest.java b/src/test/java/com/getindata/connectors/http/internal/table/lookup/querycreators/ElasticSearchLiteQueryCreatorTest.java
@@ -18,6 +18,24 @@
 
 public class ElasticSearchLiteQueryCreatorTest {
 
+    @Test
+    public void testWithEmptyLookup() {
+
+        // GIVEN
+        LookupRow lookupRow = new LookupRow();
+        lookupRow.setLookupPhysicalRowDataType(DataTypes.STRING());
+
+        GenericRowData lookupDataRow = GenericRowData.of(StringData.fromString("val1"));
+
+        // WHEN
+        var queryCreator = new ElasticSearchLiteQueryCreator(lookupRow);
+        var createdQuery = queryCreator.createLookupQuery(lookupDataRow);
+
+        // THEN
+        assertThat(createdQuery.getLookupQuery()).isEqualTo("");
+        assertThat(createdQuery.getBodyBasedUrlQueryParameters()).isEmpty();
+    }
+
     @Test
     public void testQueryCreationForSingleQueryStringParam() {
 
diff --git a/src/test/java/com/getindata/connectors/http/internal/table/lookup/querycreators/GenericJsonQueryCreatorTest.java b/src/test/java/com/getindata/connectors/http/internal/table/lookup/querycreators/GenericJsonQueryCreatorTest.java
@@ -31,7 +31,6 @@ class GenericJsonQueryCreatorTest {
 
     @BeforeEach
     public void setUp() {
-
         DataType lookupPhysicalDataType = row(List.of(
                 DataTypes.FIELD("id", DataTypes.INT()),
                 DataTypes.FIELD("uuid", DataTypes.STRING())
@@ -52,6 +51,23 @@ public void shouldSerializeToJson() {
         row.setField(0, 11);
         row.setField(1, StringData.fromString("myUuid"));
 
+        this.jsonQueryCreator.createLookupQuery(row);
+        LookupQueryInfo lookupQuery = this.jsonQueryCreator.createLookupQuery(row);
+        assertThat(lookupQuery.getBodyBasedUrlQueryParameters().isEmpty());
+        assertThat(lookupQuery.getLookupQuery()).isEqualTo("{\"id\":11,\"uuid\":\"myUuid\"}");
+    }
+
+    @Test
+    public void shouldSerializeToJsonTwice() {
+        GenericRowData row = new GenericRowData(2);
+        row.setField(0, 11);
+        row.setField(1, StringData.fromString("myUuid"));
+
+        this.jsonQueryCreator.createLookupQuery(row);
+
+        // Call createLookupQuery two times
+        // to check that serialization schema is not opened Two times.
+        this.jsonQueryCreator.createLookupQuery(row);
         LookupQueryInfo lookupQuery = this.jsonQueryCreator.createLookupQuery(row);
         assertThat(lookupQuery.getBodyBasedUrlQueryParameters().isEmpty());
         assertThat(lookupQuery.getLookupQuery()).isEqualTo("{\"id\":11,\"uuid\":\"myUuid\"}");
diff --git a/src/test/java/com/getindata/connectors/http/internal/table/lookup/querycreators/QueryFormatAwareConfigurationTest.java b/src/test/java/com/getindata/connectors/http/internal/table/lookup/querycreators/QueryFormatAwareConfigurationTest.java
@@ -0,0 +1,38 @@
+package com.getindata.connectors.http.internal.table.lookup.querycreators;
+
+import java.util.Collections;
+import java.util.Optional;
+
+import org.apache.flink.configuration.ConfigOption;
+import org.apache.flink.configuration.ConfigOptions;
+import org.apache.flink.configuration.Configuration;
+import org.junit.jupiter.api.Test;
+import static org.assertj.core.api.Assertions.assertThat;
+
+class QueryFormatAwareConfigurationTest {
+
+    private static final ConfigOption<String> configOption = ConfigOptions.key("key")
+            .stringType()
+            .noDefaultValue();
+
+    @Test
+    public void testWithDot() {
+        QueryFormatAwareConfiguration queryConfig = new QueryFormatAwareConfiguration(
+                "prefix.", Configuration.fromMap(Collections.singletonMap("prefix.key", "val"))
+        );
+
+        Optional<String> optional = queryConfig.getOptional(configOption);
+        assertThat(optional.get()).isEqualTo("val");
+    }
+
+    @Test
+    public void testWithoutDot() {
+        QueryFormatAwareConfiguration queryConfig = new QueryFormatAwareConfiguration(
+                "prefix", Configuration.fromMap(Collections.singletonMap("prefix.key", "val"))
+        );
+
+        Optional<String> optional = queryConfig.getOptional(configOption);
+        assertThat(optional.get()).isEqualTo("val");
+    }
+
+}

Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@ public void addCertToTrustStore(String certPath) {`
`168`	`168`	`}`
`169`	`169`
`170`	`170`	`/**`
`171`		`- * Add certificate and private key that should be used by anny Http Connector instance that uses`
	`171`	`+ * Add certificate and private key that should be used by any Http Connector instance that uses`
`172`	`172`	`* this {@link SSLContext} instance. Certificate and key are added only to this Context's`
`173`	`173`	`* {@link KeyStore} and not for entire JVM.`
`174`	`174`	`* @param publicKeyPath path to public key/certificate used for mTLS.`