Fix server hostname verification 55/118955/3 1.5.5
authorKrzysztof Gajewski <krzysztof.gajewski@nokia.com>
Tue, 9 Mar 2021 10:08:21 +0000 (11:08 +0100)
committerKrzysztof Gajewski <krzysztof.gajewski@nokia.com>
Tue, 9 Mar 2021 14:52:53 +0000 (15:52 +0100)
 - make it configurable
 - some small another sonar issues resolved

Issue-ID: DCAEGEN2-2656
Signed-off-by: Krzysztof Gajewski <krzysztof.gajewski@nokia.com>
Change-Id: I3012b60dbdfdb463d5adfd790df53953fe1f027f

14 files changed:
Changelog.md
datafile-app-server/pom.xml
datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/AppConfig.java
datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/CertificateConfig.java
datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/CloudConfigParser.java
datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/http/DfcHttpsClient.java
datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/http/HttpsClientConnectionManagerUtil.java
datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/configuration/AppConfigTest.java
datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/http/DfcHttpClientTest.java
datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/http/HttpsClientConnectionManagerUtilTest.java
datafile-app-server/src/test/resources/datafile_endpoints_test.json
datafile-app-server/src/test/resources/datafile_endpoints_test_2producers.json
pom.xml
version.properties

index 72a8b53..7909780 100644 (file)
@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [1.5.5] - 09/03/2021
+### Fixed
+- make hostname verification configurable
+- small sonar fixes related to affected code
+
 ## [1.5.4] - 23/02/2021 
 ### Added
 - JWT token support for HTTP/HTTPS
index 64b6f65..9c89586 100644 (file)
@@ -26,7 +26,7 @@
     <parent>
         <groupId>org.onap.dcaegen2.collectors</groupId>
         <artifactId>datafile</artifactId>
-        <version>1.5.4-SNAPSHOT</version>
+        <version>1.5.5-SNAPSHOT</version>
     </parent>
 
     <groupId>org.onap.dcaegen2.collectors.datafile</groupId>
index b381c02..f11a85a 100644 (file)
@@ -39,7 +39,6 @@ import javax.validation.constraints.NotNull;
 
 import org.onap.dcaegen2.collectors.datafile.exceptions.DatafileTaskException;
 import org.onap.dcaegen2.collectors.datafile.http.HttpsClientConnectionManagerUtil;
-import org.onap.dcaegen2.collectors.datafile.model.logging.MappedDiagnosticContext;
 import org.onap.dcaegen2.services.sdk.rest.services.cbs.client.api.CbsClient;
 import org.onap.dcaegen2.services.sdk.rest.services.cbs.client.api.CbsClientFactory;
 import org.onap.dcaegen2.services.sdk.rest.services.cbs.client.api.CbsRequests;
@@ -94,17 +93,16 @@ public class AppConfig {
      */
     public void initialize() {
         stop();
-        Map<String, String> context = MappedDiagnosticContext.initializeTraceContext();
 
         loadConfigurationFromFile();
 
-        refreshConfigTask = createRefreshTask(context) //
+        refreshConfigTask = createRefreshTask() //
             .subscribe(e -> logger.info("Refreshed configuration data"),
                 throwable -> logger.error("Configuration refresh terminated due to exception", throwable),
                 () -> logger.error("Configuration refresh terminated"));
     }
 
-    Flux<AppConfig> createRefreshTask(Map<String, String> context) {
+    Flux<AppConfig> createRefreshTask() {
         return createCbsClientConfiguration()
             .flatMap(this::createCbsClient)
             .flatMapMany(this::periodicConfigurationUpdates) //
@@ -173,8 +171,9 @@ public class AppConfig {
         return sftpConfiguration;
     }
 
-    private <R> Mono<R> onErrorResume(Throwable trowable) {
-        logger.error("Could not refresh application configuration {}", trowable.toString());
+    private <R> Mono<R> onErrorResume(Throwable throwable) {
+        String throwableString = throwable.toString();
+        logger.error("Could not refresh application configuration {}", throwableString);
         return Mono.empty();
     }
 
@@ -234,8 +233,10 @@ public class AppConfig {
         this.publishingConfigurations = publisherConfiguration;
         this.certificateConfiguration = certificateConfig;
         this.sftpConfiguration = sftpConfig;
+
         HttpsClientConnectionManagerUtil.setupOrUpdate(certificateConfig.keyCert(), certificateConfig.keyPasswordPath(),
-                certificateConfig.trustedCa(), certificateConfig.trustedCaPasswordPath());
+            certificateConfig.trustedCa(), certificateConfig.trustedCaPasswordPath(),
+            certificateConfig.httpsHostnameVerify());
     }
 
     JsonElement getJsonElement(InputStream inputStream) {
index 1d8b614..78be36d 100644 (file)
@@ -1,6 +1,6 @@
 /*-
  * ============LICENSE_START=======================================================
- * Copyright (C) 2018 NOKIA Intellectual Property, 2019 Nordix Foundation. All rights reserved.
+ * Copyright (C) 2018-2021 NOKIA Intellectual Property, 2019 Nordix Foundation. All rights reserved.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -47,4 +47,7 @@ public abstract class CertificateConfig implements Serializable {
     @Value.Parameter
     @Value.Redacted
     public abstract String trustedCaPasswordPath();
+
+    @Value.Parameter
+    public abstract Boolean httpsHostnameVerify();
 }
index d6b8643..025166c 100644 (file)
@@ -194,6 +194,7 @@ public class CloudConfigParser {
             .keyPasswordPath(getAsString(jsonObject, "dmaap.certificateConfig.keyPasswordPath"))
             .trustedCa(getAsString(jsonObject, "dmaap.certificateConfig.trustedCa"))
             .trustedCaPasswordPath(getAsString(jsonObject, "dmaap.certificateConfig.trustedCaPasswordPath")) //
+            .httpsHostnameVerify(getAsBooleanOrDefault(jsonObject, "dmaap.certificateConfig.httpsHostnameVerify", Boolean.TRUE))
             .build();
     }
 
@@ -222,6 +223,14 @@ public class CloudConfigParser {
         return get(obj, memberName).getAsBoolean();
     }
 
+    private static @NotNull Boolean getAsBooleanOrDefault(JsonObject obj, String memberName, Boolean def) {
+        try {
+            return get(obj, memberName).getAsBoolean();
+        } catch (DatafileTaskException e) {
+            return def;
+        }
+    }
+
     private static @NotNull JsonObject getAsJson(JsonObject obj, String memberName) throws DatafileTaskException {
         return get(obj, memberName).getAsJsonObject();
     }
index c2d72f6..9bb0118 100644 (file)
@@ -36,6 +36,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLPeerUnverifiedException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.UnknownHostException;
@@ -138,7 +139,8 @@ public class DfcHttpsClient implements FileCollectClient {
                 throw new NonRetryableDatafileTaskException(HttpUtils.retryableResponse(getResponseCode(httpResponse)));
             }
             throw new DatafileTaskException(HttpUtils.nonRetryableResponse(getResponseCode(httpResponse)));
-        } catch (ConnectTimeoutException | UnknownHostException | HttpHostConnectException | SSLHandshakeException e) {
+        } catch (ConnectTimeoutException | UnknownHostException | HttpHostConnectException | SSLHandshakeException
+                | SSLPeerUnverifiedException e) {
             throw new NonRetryableDatafileTaskException(
                     "Unable to get file from xNF. No retry attempts will be done.", e);
         }
index e60ec0f..2563856 100644 (file)
@@ -18,6 +18,8 @@ package org.onap.dcaegen2.collectors.datafile.http;
 import org.apache.http.config.Registry;
 import org.apache.http.config.RegistryBuilder;
 import org.apache.http.conn.socket.ConnectionSocketFactory;
+import org.apache.http.conn.ssl.DefaultHostnameVerifier;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 import org.apache.http.ssl.SSLContextBuilder;
@@ -28,6 +30,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.core.io.FileSystemResource;
 
+import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
 import java.io.File;
 import java.io.IOException;
@@ -62,19 +65,19 @@ public class HttpsClientConnectionManagerUtil {
     }
 
     public static void setupOrUpdate(String keyCertPath, String keyCertPasswordPath, String trustedCaPath,
-            String trustedCaPasswordPath) throws DatafileTaskException {
+            String trustedCaPasswordPath, Boolean useHostnameVerifier) throws DatafileTaskException {
         synchronized (HttpsClientConnectionManagerUtil.class) {
             if (connectionManager != null) {
                 connectionManager.close();
                 connectionManager = null;
             }
-            setup(keyCertPath, keyCertPasswordPath, trustedCaPath, trustedCaPasswordPath);
+            setup(keyCertPath, keyCertPasswordPath, trustedCaPath, trustedCaPasswordPath, useHostnameVerifier);
         }
         logger.trace("HttpsConnectionManager setup or updated");
     }
 
     private static void setup(String keyCertPath, String keyCertPasswordPath, String trustedCaPath,
-          String trustedCaPasswordPath) throws DatafileTaskException {
+          String trustedCaPasswordPath, Boolean useHostnameVerifier) throws DatafileTaskException {
         try {
             SSLContextBuilder sslBuilder = SSLContexts.custom();
             sslBuilder = supplyKeyInfo(keyCertPath, keyCertPasswordPath, sslBuilder);
@@ -82,9 +85,12 @@ public class HttpsClientConnectionManagerUtil {
 
             SSLContext sslContext = sslBuilder.build();
 
+            HostnameVerifier hostnameVerifier = (Boolean.TRUE.equals(useHostnameVerifier)) ? new DefaultHostnameVerifier() :
+                    NoopHostnameVerifier.INSTANCE;
+
             SSLConnectionSocketFactory sslConnectionSocketFactory =
                 new SSLConnectionSocketFactory(sslContext, new String[] {"TLSv1.2"}, null,
-                    (hostname, session) -> true);
+                        hostnameVerifier);
 
             Registry<ConnectionSocketFactory> socketFactoryRegistry =
                 RegistryBuilder.<ConnectionSocketFactory>create().register("https", sslConnectionSocketFactory)
index bcbe7f8..839a9a1 100644 (file)
@@ -66,7 +66,7 @@ import static org.mockito.Mockito.when;
  * @author <a href="mailto:przemyslaw.wasala@nokia.com">Przemysław Wąsala</a> on 4/9/18
  * @author <a href="mailto:henrik.b.andersson@est.tech">Henrik Andersson</a>
  */
-public class AppConfigTest {
+class AppConfigTest {
 
     public static final String CHANGE_IDENTIFIER = "PM_MEAS_FILES";
 
@@ -90,6 +90,7 @@ public class AppConfigTest {
             .keyPasswordPath("/src/test/resources/dfc.jks.pass") //
             .trustedCa("/src/test/resources/cert.jks") //
             .trustedCaPasswordPath("/src/test/resources/cert.jks.pass") //
+            .httpsHostnameVerify(true)
             .build();
 
     private AppConfig appConfigUnderTest;
@@ -105,7 +106,7 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenTheConfigurationFits() throws IOException, DatafileTaskException {
+    void whenTheConfigurationFits() throws IOException, DatafileTaskException {
         // When
         doReturn(getCorrectJson()).when(appConfigUnderTest).createInputStream(any());
         appConfigUnderTest.initialize();
@@ -122,12 +123,13 @@ public class AppConfigTest {
         assertThat(publisherCfg).isEqualToComparingFieldByField(CORRECT_PUBLISHER_CONFIG);
 
         CertificateConfig certificateConfig = appConfigUnderTest.getCertificateConfiguration();
-        assertThat(certificateConfig).isNotNull();
-        assertThat(certificateConfig).isEqualToComparingFieldByField(CORRECT_CERTIFICATE_CONFIGURATION);
+        assertThat(certificateConfig)
+            .isNotNull()
+            .isEqualToComparingFieldByField(CORRECT_CERTIFICATE_CONFIGURATION);
     }
 
     @Test
-    public void whenTheConfigurationFits_twoProducers() throws IOException, DatafileTaskException {
+    void whenTheConfigurationFits_twoProducers() throws IOException, DatafileTaskException {
         // When
         doReturn(getCorrectJsonTwoProducers()).when(appConfigUnderTest).createInputStream(any());
         appConfigUnderTest.loadConfigurationFromFile();
@@ -146,7 +148,7 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenFileIsNotExist_ThrowException() throws DatafileTaskException {
+    void whenFileIsNotExist_ThrowException() throws DatafileTaskException {
         // Given
         appConfigUnderTest.setFilepath("/temp.json");
 
@@ -162,7 +164,7 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenFileIsExistsButJsonIsIncorrect() throws IOException, DatafileTaskException {
+    void whenFileIsExistsButJsonIsIncorrect() throws IOException, DatafileTaskException {
 
         // When
         doReturn(getIncorrectJson()).when(appConfigUnderTest).createInputStream(any());
@@ -177,7 +179,7 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenTheConfigurationFits_ButRootElementIsNotAJsonObject() throws IOException, DatafileTaskException {
+    void whenTheConfigurationFits_ButRootElementIsNotAJsonObject() throws IOException, DatafileTaskException {
 
         // When
         doReturn(getCorrectJson()).when(appConfigUnderTest).createInputStream(any());
@@ -195,9 +197,9 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenPeriodicConfigRefreshNoEnvironmentVariables() {
+    void whenPeriodicConfigRefreshNoEnvironmentVariables() {
         final ListAppender<ILoggingEvent> logAppender = LoggingUtils.getLogListAppender(AppConfig.class);
-        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask(context);
+        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask();
 
         StepVerifier //
             .create(task) //
@@ -208,14 +210,14 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenPeriodicConfigRefreshNoConsul() {
+    void whenPeriodicConfigRefreshNoConsul() {
         doReturn(Mono.just(cbsClientConfiguration)).when(appConfigUnderTest).createCbsClientConfiguration();
         doReturn(Mono.just(cbsClient)).when(appConfigUnderTest).createCbsClient(cbsClientConfiguration);
         Flux<JsonObject> err = Flux.error(new IOException());
         doReturn(err).when(cbsClient).updates(any(), any(), any());
 
         final ListAppender<ILoggingEvent> logAppender = LoggingUtils.getLogListAppender(AppConfig.class);
-        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask(context);
+        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask();
 
         StepVerifier //
             .create(task) //
@@ -227,14 +229,14 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenPeriodicConfigRefreshSuccess() throws JsonIOException, JsonSyntaxException, IOException {
+    void whenPeriodicConfigRefreshSuccess() throws JsonIOException, JsonSyntaxException, IOException {
         doReturn(Mono.just(cbsClientConfiguration)).when(appConfigUnderTest).createCbsClientConfiguration();
         doReturn(Mono.just(cbsClient)).when(appConfigUnderTest).createCbsClient(cbsClientConfiguration);
 
         Flux<JsonObject> json = Flux.just(getJsonRootObject());
         doReturn(json).when(cbsClient).updates(any(), any(), any());
 
-        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask(context);
+        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask();
 
         StepVerifier //
             .create(task) //
@@ -246,7 +248,7 @@ public class AppConfigTest {
     }
 
     @Test
-    public void whenPeriodicConfigRefreshSuccess2() throws JsonIOException, JsonSyntaxException, IOException {
+    void whenPeriodicConfigRefreshSuccess2() throws JsonIOException, JsonSyntaxException, IOException {
         doReturn(Mono.just(cbsClientConfiguration)).when(appConfigUnderTest).createCbsClientConfiguration();
         doReturn(Mono.just(cbsClient)).when(appConfigUnderTest).createCbsClient(cbsClientConfiguration);
 
@@ -256,7 +258,7 @@ public class AppConfigTest {
 
         doReturn(json, err).when(cbsClient).updates(any(), any(), any());
 
-        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask(context);
+        Flux<AppConfig> task = appConfigUnderTest.createRefreshTask();
 
         StepVerifier //
             .create(task) //
@@ -275,8 +277,8 @@ public class AppConfigTest {
         assertThat(messageRouterSubscribeRequest.sourceDefinition().topicUrl())
                 .isEqualTo("http://localhost:2222/events/unauthenticated.VES_NOTIFICATION_OUTPUT");
         SecurityKeys securityKeys = consumerConfiguration.getMessageRouterSubscriberConfig().securityKeys();
-        assertThat(securityKeys.keyStore().path().toString()).isEqualTo("src/test/resources/cert.jks");
-        assertThat(securityKeys.trustStore().path().toString()).isEqualTo("src/test/resources/trust.jks");
+        assertThat(securityKeys.keyStore().path().toString()).hasToString("src/test/resources/cert.jks");
+        assertThat(securityKeys.trustStore().path().toString()).hasToString("src/test/resources/trust.jks");
         assertThat(consumerConfiguration.getMessageRouterSubscriber()).isNotNull();
     }
 
index 98804a0..2b8df34 100644 (file)
@@ -99,7 +99,7 @@ class DfcHttpClientTest {
         dfcHttpClientSpy.collectFile(REMOTE_FILE, pathMock);
         dfcHttpClientSpy.close();
 
-        verify(dfcHttpClientSpy, times(1)).getServerResponse(ArgumentMatchers.eq(REMOTE_FILE));
+        verify(dfcHttpClientSpy, times(1)).getServerResponse(REMOTE_FILE);
         verify(dfcHttpClientSpy, times(1)).processDataFromServer(any(), any(), any());
         verify(dfcHttpClientSpy, times(1)).isDownloadFailed(any());
     }
index fa4473a..6328dd3 100644 (file)
@@ -15,6 +15,7 @@
  */
 package org.onap.dcaegen2.collectors.datafile.http;
 
+import org.apache.http.conn.ssl.DefaultHostnameVerifier;
 import org.junit.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -39,13 +40,15 @@ public class HttpsClientConnectionManagerUtilTest {
 
     @Test
     public void creatingManager_successfulCase() throws Exception {
-        HttpsClientConnectionManagerUtil.setupOrUpdate(KEY_PATH, KEY_PASSWORD, TRUSTED_CA_PATH, TRUSTED_CA_PASSWORD);
+        HttpsClientConnectionManagerUtil.setupOrUpdate(KEY_PATH, KEY_PASSWORD, TRUSTED_CA_PATH, TRUSTED_CA_PASSWORD, //
+                true);
         assertNotNull(HttpsClientConnectionManagerUtil.instance());
     }
 
     @Test
     public void creatingManager_improperSecretShouldThrowException() {
-        assertThrows(DatafileTaskException.class, () -> HttpsClientConnectionManagerUtil.setupOrUpdate(KEY_PATH, KEY_IMPROPER_PASSWORD, TRUSTED_CA_PATH, TRUSTED_CA_PASSWORD));
+        assertThrows(DatafileTaskException.class, () -> HttpsClientConnectionManagerUtil.setupOrUpdate(KEY_PATH, //
+                KEY_IMPROPER_PASSWORD, TRUSTED_CA_PATH, TRUSTED_CA_PASSWORD, true));
         assertThrows(DatafileTaskException.class, () -> HttpsClientConnectionManagerUtil.instance());
     }
 
index 00b2488..7ca83b0 100644 (file)
@@ -5,6 +5,7 @@
     "dmaap.certificateConfig.keyPasswordPath": "/src/test/resources/dfc.jks.pass",
     "dmaap.certificateConfig.trustedCa": "/src/test/resources/cert.jks",
     "dmaap.certificateConfig.trustedCaPasswordPath": "/src/test/resources/cert.jks.pass",
+    "dmaap.certificateConfig.httpsHostnameVerify": true,
     "dmaap.security.trustStorePath": "src/test/resources/trust.jks",
     "dmaap.security.trustStorePasswordPath": "src/test/resources/trust.pass",
     "dmaap.security.keyStorePath": "src/test/resources/cert.jks",
index 73bb744..13d09bc 100644 (file)
@@ -5,6 +5,7 @@
     "dmaap.certificateConfig.keyPasswordPath": "/src/test/resources/dfc.jks.pass",
     "dmaap.certificateConfig.trustedCa": "/src/test/resources/ftp.jks",
     "dmaap.certificateConfig.trustedCaPasswordPath": "/src/test/resources/ftp.jks.pass",
+    "dmaap.certificateConfig.httpsHostnameVerify": true,
     "dmaap.security.trustStorePath": "src/test/resources/trust.jks",
     "dmaap.security.trustStorePasswordPath": "src/test/resources/trust.pass",
     "dmaap.security.keyStorePath": "src/test/resources/cert.jks",
diff --git a/pom.xml b/pom.xml
index 246e117..08f5fbf 100644 (file)
--- a/pom.xml
+++ b/pom.xml
@@ -31,7 +31,7 @@
 
     <groupId>org.onap.dcaegen2.collectors</groupId>
     <artifactId>datafile</artifactId>
-    <version>1.5.4-SNAPSHOT</version>
+    <version>1.5.5-SNAPSHOT</version>
 
     <name>dcaegen2-collectors.datafile</name>
     <description>datafile collector</description>
index 8086968..1daaf2d 100644 (file)
@@ -1,6 +1,6 @@
 major=1\r
 minor=5\r
-patch=4\r
+patch=5\r
 base_version=${major}.${minor}.${patch}\r
 release_version=${base_version}\r
 snapshot_version=${base_version}-SNAPSHOT\r