From 5135fde49e1268873e688d14f541b8ff673bae22 Mon Sep 17 00:00:00 2001 From: Jan Malkiewicz Date: Wed, 15 Jul 2020 15:28:41 +0200 Subject: Add sftp strict host key checking to DFC. Issue-ID: DCAEGEN2-2219 Signed-off-by: Jan Malkiewicz Change-Id: Iadf6c6bd743c42ebb3bf9ad8ac443fc0f3f58063 --- datafile-app-server/config/application.yaml | 2 +- .../dpo/blueprints/k8s-datafile.yaml | 2 +- .../dpo/spec/datafile-component-spec.json | 11 ++++ datafile-app-server/dpo/tosca_models/schema.yaml | 2 + datafile-app-server/dpo/tosca_models/template.yaml | 1 + datafile-app-server/pom.xml | 2 +- datafile-app-server/src/main/docker/Dockerfile | 18 ++++-- .../datafile/configuration/AppConfig.java | 37 +++++++----- .../datafile/configuration/CloudConfigParser.java | 42 +++++++++++-- .../datafile/configuration/SftpConfig.java | 42 +++++++++++++ .../collectors/datafile/ftp/FtpsClient.java | 6 +- .../collectors/datafile/ftp/SftpClient.java | 38 +++++++++--- .../datafile/ftp/SftpClientSettings.java | 63 ++++++++++++++++++++ .../collectors/datafile/model/FileData.java | 10 ++-- .../collectors/datafile/tasks/FileCollector.java | 5 +- .../collectors/datafile/ftp/FtpsClientTest.java | 4 +- .../datafile/ftp/SftpClientSettingsTest.java | 68 ++++++++++++++++++++++ .../collectors/datafile/ftp/SftpClientTest.java | 54 +++++++++-------- .../collectors/datafile/model/FileDataTest.java | 24 ++++---- .../test/resources/datafile_endpoints_test.json | 1 + .../datafile_endpoints_test_2producers.json | 3 +- pom.xml | 6 +- 22 files changed, 353 insertions(+), 88 deletions(-) create mode 100644 datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/SftpConfig.java create mode 100644 datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettings.java create mode 100644 datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettingsTest.java diff --git a/datafile-app-server/config/application.yaml b/datafile-app-server/config/application.yaml index bae8025e..69771e20 100644 --- a/datafile-app-server/config/application.yaml +++ b/datafile-app-server/config/application.yaml @@ -23,4 +23,4 @@ logging: org.onap.dcaegen2.collectors.datafile: WARN file: /var/log/ONAP/application.log app: - filepath: src/test/resources/datafile_endpoints_test.json + filepath: config/datafile_endpoints_test.json diff --git a/datafile-app-server/dpo/blueprints/k8s-datafile.yaml b/datafile-app-server/dpo/blueprints/k8s-datafile.yaml index 5a0b0bb6..a38d5e3b 100644 --- a/datafile-app-server/dpo/blueprints/k8s-datafile.yaml +++ b/datafile-app-server/dpo/blueprints/k8s-datafile.yaml @@ -73,7 +73,6 @@ node_templates: PM_MEAS_FILES: dmaap_info: <> type: data_router - streams_subscribes: {} dmaap.ftpesConfig.keyCert: /opt/app/datafile/config/cert.jks dmaap.ftpesConfig.keyPasswordPath: /opt/app/datafile/config/jks.pass dmaap.ftpesConfig.trustedCa: /opt/app/datafile/config/trust.jks @@ -83,6 +82,7 @@ node_templates: dmaap.security.keyStorePath: /opt/app/datafile/etc/cert/key.p12 dmaap.security.trustStorePasswordPath: /opt/app/datafile/etc/cert/trust.pass dmaap.security.trustStorePath: /opt/app/datafile/etc/cert/trust.jks + sftp.security.strictHostKeyChecking: true streams_subscribes: dmaap_subscriber: dmaap_info: diff --git a/datafile-app-server/dpo/spec/datafile-component-spec.json b/datafile-app-server/dpo/spec/datafile-component-spec.json index 6047a7c0..e7843283 100644 --- a/datafile-app-server/dpo/spec/datafile-component-spec.json +++ b/datafile-app-server/dpo/spec/datafile-component-spec.json @@ -140,6 +140,17 @@ "policy_editable": false, "type": "string", "required": true + }, + { + "name": "sftp.security.strictHostKeyChecking", + "value": true, + "description": "", + "designer_editable": true, + "sourced_at_deployment": false, + "policy_editable": false, + "type": "string", + "required": true } + ] } diff --git a/datafile-app-server/dpo/tosca_models/schema.yaml b/datafile-app-server/dpo/tosca_models/schema.yaml index 474af7ac..f5eca0e5 100644 --- a/datafile-app-server/dpo/tosca_models/schema.yaml +++ b/datafile-app-server/dpo/tosca_models/schema.yaml @@ -528,6 +528,8 @@ node_types: type: string streams_subscribes: type: string + sftp.security.strictHostKeyChecking: + type: boolean requirements: - stream_subscribe_0: capability: dcae.capabilities.dmmap.topic diff --git a/datafile-app-server/dpo/tosca_models/template.yaml b/datafile-app-server/dpo/tosca_models/template.yaml index a1fdadb7..246f4a45 100644 --- a/datafile-app-server/dpo/tosca_models/template.yaml +++ b/datafile-app-server/dpo/tosca_models/template.yaml @@ -31,6 +31,7 @@ topology_template: security.keyStorePath: /opt/app/datafile/etc/cert/cert.jks security.trustStorePasswordPath: /opt/app/datafile/etc/cert/trust.pass security.trustStorePath: /opt/app/datafile/etc/cert/trust.jks + sftp.security.strictHostKeyChecking: true service_name: datafile streams_subscribes: '{''dmaap_subscriber'': {''dmmap_info'': {''topic_url'': ''http://message-router.onap.svc.cluster.local:3904/events/unauthenticated.VES_NOTIFICATION_OUTPUT/OpenDcae-c12/C12''}}}' requirements: diff --git a/datafile-app-server/pom.xml b/datafile-app-server/pom.xml index fdec5992..a5767fb2 100644 --- a/datafile-app-server/pom.xml +++ b/datafile-app-server/pom.xml @@ -25,7 +25,7 @@ org.onap.dcaegen2.collectors datafile - 1.4.0-SNAPSHOT + 1.4.1-SNAPSHOT org.onap.dcaegen2.collectors.datafile diff --git a/datafile-app-server/src/main/docker/Dockerfile b/datafile-app-server/src/main/docker/Dockerfile index 0763e2bf..313c8d07 100755 --- a/datafile-app-server/src/main/docker/Dockerfile +++ b/datafile-app-server/src/main/docker/Dockerfile @@ -1,6 +1,7 @@ # # ============LICENSE_START======================================================= # Copyright (C) 2019 Nordix Foundation. +# Copyright (C) 2020 Nokia. # ================================================================================ # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -19,19 +20,26 @@ # FROM openjdk:11.0.6-jre-slim +EXPOSE 8100 8433 + +RUN apt-get update && apt-get install -y \ + vim \ + curl + WORKDIR /opt/app/datafile RUN mkdir -p /var/log/ONAP RUN mkdir -p /opt/app/datafile/etc/cert/ -ADD /target/datafile-app-server.jar /opt/app/datafile/ -ADD /config/application.yaml /opt/app/datafile/config/ - -EXPOSE 8100 8433 - RUN groupadd -r onap && useradd -ms /bin/bash datafile -g onap RUN chown -R datafile:onap /var/log/ONAP + +ADD /config/application.yaml /opt/app/datafile/config/ RUN chmod -R 777 /opt/app/datafile/config/ USER datafile +RUN mkdir -p /home/datafile/.ssh + +ADD /target/datafile-app-server.jar /opt/app/datafile/ + ENTRYPOINT ["/usr/local/openjdk-11/bin/java", "-jar", "/opt/app/datafile/datafile-app-server.jar"] diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/AppConfig.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/AppConfig.java index 21c51566..c257ceed 100644 --- a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/AppConfig.java +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/AppConfig.java @@ -68,11 +68,13 @@ import reactor.core.publisher.Mono; @EnableConfigurationProperties @ConfigurationProperties("app") public class AppConfig { + private static final Logger logger = LoggerFactory.getLogger(AppConfig.class); private ConsumerConfiguration dmaapConsumerConfiguration; private Map publishingConfigurations; private FtpesConfig ftpesConfiguration; + private SftpConfig sftpConfiguration; @Value("#{systemEnvironment}") Properties systemEnvironment; private Disposable refreshConfigTask = null; @@ -90,6 +92,7 @@ public class AppConfig { public void initialize() { stop(); Map context = MappedDiagnosticContext.initializeTraceContext(); + loadConfigurationFromFile(); refreshConfigTask = createRefreshTask(context) // @@ -131,7 +134,6 @@ public class AppConfig { * Checks if there is a configuration for the given feed. * * @param changeIdentifier the change identifier the feed is configured to belong to. - * * @return true if a feed is configured for the given change identifier, false if not. */ public synchronized boolean isFeedConfigured(String changeIdentifier) { @@ -143,7 +145,6 @@ public class AppConfig { * * @param changeIdentifier the change identifier the feed is configured to belong to. * @return the PublisherConfiguration for the feed belonging to the given change identifier. - * * @throws DatafileTaskException if no configuration has been loaded or the configuration is missing for the given * change identifier. */ @@ -165,6 +166,10 @@ public class AppConfig { return ftpesConfiguration; } + public synchronized SftpConfig getSftpConfiguration() { + return sftpConfiguration; + } + private Mono onErrorResume(Throwable trowable) { logger.error("Could not refresh application configuration {}", trowable.toString()); return Mono.empty(); @@ -178,27 +183,25 @@ public class AppConfig { return CbsClientFactory.createCbsClient(env); } - /** - * Parse configuration. - * - * @param jsonObject the DFC service's configuration - * @return this which is updated if successful - */ - private AppConfig parseCloudConfig(JsonObject jsonObject) { + private AppConfig parseCloudConfig(JsonObject configurationObject) { try { - CloudConfigParser parser = new CloudConfigParser(jsonObject); + CloudConfigParser parser = new CloudConfigParser(configurationObject, systemEnvironment); setConfiguration(parser.getDmaapConsumerConfig(), parser.getDmaapPublisherConfigurations(), - parser.getFtpesConfig()); - + parser.getFtpesConfig(), parser.getSftpConfig()); + logConfig(); } catch (DatafileTaskException e) { logger.error("Could not parse configuration {}", e.toString(), e); } return this; } - /** - * Reads the configuration from file. - */ + private void logConfig() { + logger.debug("Read and parsed sFTP configuration: [{}]", sftpConfiguration); + logger.debug("Read and parsed FTPes configuration: [{}]", ftpesConfiguration); + logger.debug("Read and parsed DMaaP configuration: [{}]", dmaapConsumerConfiguration); + logger.debug("Read and parsed Publish configuration: [{}]", publishingConfigurations); + } + void loadConfigurationFromFile() { GsonBuilder gsonBuilder = new GsonBuilder(); ServiceLoader.load(TypeAdapterFactory.class).forEach(gsonBuilder::registerTypeAdapterFactory); @@ -217,10 +220,12 @@ public class AppConfig { } private synchronized void setConfiguration(@NotNull ConsumerConfiguration consumerConfiguration, - @NotNull Map publisherConfiguration, @NotNull FtpesConfig ftpesConfig) { + @NotNull Map publisherConfiguration, @NotNull FtpesConfig ftpesConfig, + @NotNull SftpConfig sftpConfig) { this.dmaapConsumerConfiguration = consumerConfiguration; this.publishingConfigurations = publisherConfiguration; this.ftpesConfiguration = ftpesConfig; + this.sftpConfiguration = sftpConfig; } JsonElement getJsonElement(JsonParser parser, InputStream inputStream) { diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/CloudConfigParser.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/CloudConfigParser.java index 23197025..a86a32b8 100644 --- a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/CloudConfigParser.java +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/CloudConfigParser.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.Properties; import java.util.Set; import javax.validation.constraints.NotNull; @@ -38,6 +39,7 @@ import org.onap.dcaegen2.collectors.datafile.exceptions.DatafileTaskException; * @author Henrik Andersson */ public class CloudConfigParser { + private static final String DMAAP_SECURITY_TRUST_STORE_PATH = "dmaap.security.trustStorePath"; private static final String DMAAP_SECURITY_TRUST_STORE_PASS_PATH = "dmaap.security.trustStorePasswordPath"; private static final String DMAAP_SECURITY_KEY_STORE_PATH = "dmaap.security.keyStorePath"; @@ -45,19 +47,23 @@ public class CloudConfigParser { private static final String DMAAP_SECURITY_ENABLE_DMAAP_CERT_AUTH = "dmaap.security.enableDmaapCertAuth"; private static final String CONFIG = "config"; + private static final String KNOWN_HOSTS_FILE_PATH_ENV_PROPERTY = "KNOWN_HOSTS_FILE_PATH"; + private static final String CBS_PROPERTY_SFTP_SECURITY_STRICT_HOST_KEY_CHECKING = + "sftp.security.strictHostKeyChecking"; + + private final Properties systemEnvironment; + private final JsonObject jsonObject; - public CloudConfigParser(JsonObject jsonObject) { + public CloudConfigParser(JsonObject jsonObject, Properties systemEnvironment) { this.jsonObject = jsonObject.getAsJsonObject(CONFIG); - + this.systemEnvironment = systemEnvironment; } /** * Get the publisher configurations. * - * @return a map with change identifier as key and the connected publisher configuration as - * value. - * + * @return a map with change identifier as key and the connected publisher configuration as value. * @throws DatafileTaskException if a member of the configuration is missing. */ public @NotNull Map getDmaapPublisherConfigurations() throws DatafileTaskException { @@ -113,6 +119,19 @@ public class CloudConfigParser { .build(); } + /** + * Get the sFTP configuration. + * + * @return the sFTP configuration. + * @throws DatafileTaskException if a member of the configuration is missing. + */ + public @NotNull SftpConfig getSftpConfig() throws DatafileTaskException { + String filePath = determineKnownHostsFilePath(); + return new ImmutableSftpConfig.Builder() // + .strictHostKeyChecking(getAsBoolean(jsonObject, CBS_PROPERTY_SFTP_SECURITY_STRICT_HOST_KEY_CHECKING)) + .knownHostsFilePath(filePath).build(); + } + /** * Get the security configuration for communication with the xNF. * @@ -128,6 +147,15 @@ public class CloudConfigParser { .build(); } + private String determineKnownHostsFilePath() { + String filePath = ""; + if (systemEnvironment != null) { + filePath = + systemEnvironment.getProperty(KNOWN_HOSTS_FILE_PATH_ENV_PROPERTY, "/home/datafile/.ssh/known_hosts"); + } + return filePath; + } + private static @NotNull JsonElement get(JsonObject obj, String memberName) throws DatafileTaskException { JsonElement elem = obj.get(memberName); if (elem == null) { @@ -140,6 +168,10 @@ public class CloudConfigParser { return get(obj, memberName).getAsString(); } + private static @NotNull Boolean getAsBoolean(JsonObject obj, String memberName) throws DatafileTaskException { + return get(obj, memberName).getAsBoolean(); + } + private static @NotNull JsonObject getAsJson(JsonObject obj, String memberName) throws DatafileTaskException { return get(obj, memberName).getAsJsonObject(); } diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/SftpConfig.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/SftpConfig.java new file mode 100644 index 00000000..6acc2d56 --- /dev/null +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/configuration/SftpConfig.java @@ -0,0 +1,42 @@ +/*- + * ============LICENSE_START======================================================= + * Copyright (C) 2020 NOKIA Intellectual Property. 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.dcaegen2.collectors.datafile.configuration; + +import java.io.Serializable; + +import org.immutables.gson.Gson; +import org.immutables.value.Value; +import org.springframework.stereotype.Component; + +@Component +@Value.Style(builder = "new") +@Value.Immutable +@Gson.TypeAdapters +public abstract class SftpConfig implements Serializable { + + private static final long serialVersionUID = 1L; + + @Value.Parameter + public abstract Boolean strictHostKeyChecking(); + + @Value.Parameter + public abstract String knownHostsFilePath(); +} diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java index f7121efc..fea578ba 100644 --- a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClient.java @@ -31,10 +31,12 @@ import java.security.NoSuchAlgorithmException; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; import java.util.Optional; + import javax.net.ssl.KeyManager; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; + import org.apache.commons.net.ftp.FTP; import org.apache.commons.net.ftp.FTPReply; import org.apache.commons.net.ftp.FTPSClient; @@ -222,8 +224,8 @@ public class FtpsClient implements FileCollectClient { } } - private KeyManager createKeyManager(Path keyCertPath, String keyCertPassword) - throws IOException, KeyStoreException, NoSuchAlgorithmException, CertificateException, UnrecoverableKeyException { + private KeyManager createKeyManager(Path keyCertPath, String keyCertPassword) throws IOException, KeyStoreException, + NoSuchAlgorithmException, CertificateException, UnrecoverableKeyException { logger.trace("Creating key manager from file: {}", keyCertPath); try (InputStream fis = createInputStream(keyCertPath)) { KeyStore keyStore = KeyStore.getInstance("JKS"); diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java index da8361ff..d1685203 100644 --- a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClient.java @@ -1,6 +1,6 @@ /*- * ============LICENSE_START====================================================================== - * Copyright (C) 2018-2019 Nordix Foundation. All rights reserved. + * Copyright (C) 2018-2019 Nordix Foundation, 2020 Nokia. 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. You may obtain a copy of the License at @@ -26,6 +26,7 @@ import com.jcraft.jsch.SftpException; import java.nio.file.Path; import java.util.Optional; +import org.jetbrains.annotations.NotNull; import org.onap.dcaegen2.collectors.datafile.exceptions.DatafileTaskException; import org.onap.dcaegen2.collectors.datafile.exceptions.NonRetryableDatafileTaskException; import org.slf4j.Logger; @@ -35,19 +36,22 @@ import org.slf4j.LoggerFactory; * Gets file from xNF with SFTP protocol. * * @author Martin Yan - * */ public class SftpClient implements FileCollectClient { + private static final Logger logger = LoggerFactory.getLogger(SftpClient.class); private static final int SFTP_DEFAULT_PORT = 22; + private static final String STRICT_HOST_KEY_CHECKING = "StrictHostKeyChecking"; private final FileServerData fileServerData; protected Session session = null; protected ChannelSftp sftpChannel = null; + private final SftpClientSettings settings; - public SftpClient(FileServerData fileServerData) { + public SftpClient(FileServerData fileServerData, SftpClientSettings sftpConfig) { this.fileServerData = fileServerData; + this.settings = sftpConfig; } @Override @@ -56,7 +60,7 @@ public class SftpClient implements FileCollectClient { try { sftpChannel.get(remoteFile, localFile.toString()); - logger.trace("File {} Download Successfull from xNF", localFile.getFileName()); + logger.trace("File {} Download successful from xNF", localFile.getFileName()); } catch (SftpException e) { boolean retry = e.id != ChannelSftp.SSH_FX_NO_SUCH_FILE && e.id != ChannelSftp.SSH_FX_PERMISSION_DENIED && e.id != ChannelSftp.SSH_FX_OP_UNSUPPORTED; @@ -101,29 +105,47 @@ public class SftpClient implements FileCollectClient { } } } + JSch createJsch() { + return new JSch(); + } private int getPort(Optional port) { return port.isPresent() ? port.get() : SFTP_DEFAULT_PORT; } private Session setUpSession(FileServerData fileServerData) throws JSchException { + boolean useStrictHostChecking = this.settings.shouldUseStrictHostChecking(); + JSch jsch = createJschClient(useStrictHostChecking); + return createJshSession(jsch, fileServerData, useStrictHostChecking); + } + + private JSch createJschClient(boolean useStrictHostChecking) throws JSchException { JSch jsch = createJsch(); + if (useStrictHostChecking) { + jsch.setKnownHosts(this.settings.getKnownHostsFilePath()); + } + return jsch; + } + private Session createJshSession(JSch jsch, FileServerData fileServerData, boolean useStrictHostKeyChecking) + throws JSchException { Session newSession = jsch.getSession(fileServerData.userId(), fileServerData.serverAddress(), getPort(fileServerData.port())); - newSession.setConfig("StrictHostKeyChecking", "no"); + newSession.setConfig(STRICT_HOST_KEY_CHECKING, toYesNo(useStrictHostKeyChecking)); newSession.setPassword(fileServerData.password()); newSession.connect(); return newSession; } + @NotNull + private String toYesNo(boolean useStrictHostKeyChecking) { + return useStrictHostKeyChecking ? "yes" : "no"; + } + private ChannelSftp getChannel(Session session) throws JSchException { Channel channel = session.openChannel("sftp"); channel.connect(); return (ChannelSftp) channel; } - protected JSch createJsch() { - return new JSch(); - } } diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettings.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettings.java new file mode 100644 index 00000000..8cab4327 --- /dev/null +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettings.java @@ -0,0 +1,63 @@ +/*- + * ============LICENSE_START====================================================================== + * Copyright (C) 2020 Nokia. 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. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied. See the License for the specific language governing permissions + * and limitations under the License. + * ============LICENSE_END======================================================================== + */ + +package org.onap.dcaegen2.collectors.datafile.ftp; + +import java.io.File; +import org.onap.dcaegen2.collectors.datafile.configuration.SftpConfig; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SftpClientSettings { + + private static final Logger logger = LoggerFactory.getLogger(SftpClientSettings.class); + + private final SftpConfig sftpConfig; + + public SftpClientSettings(SftpConfig sftpConfig) { + this.sftpConfig = sftpConfig; + } + + public boolean shouldUseStrictHostChecking() { + boolean strictHostKeyChecking = false; + if (isStrictHostKeyCheckingEnabled()) { + File file = new File(getKnownHostsFilePath()); + strictHostKeyChecking = file.isFile(); + logUsageOfStrictHostCheckingFlag(strictHostKeyChecking, file.getAbsolutePath()); + } else { + logger.info("StrictHostKeyChecking will be disabled."); + } + return strictHostKeyChecking; + } + + public String getKnownHostsFilePath() { + return this.sftpConfig.knownHostsFilePath(); + } + + private boolean isStrictHostKeyCheckingEnabled() { + return Boolean.TRUE.equals(this.sftpConfig.strictHostKeyChecking()); + } + + private void logUsageOfStrictHostCheckingFlag(boolean strictHostKeyChecking, String filePath) { + if (strictHostKeyChecking) { + logger.info("StrictHostKeyChecking will be enabled with KNOW_HOSTS_FILE_PATH [{}].", filePath); + } else { + logger.warn( + "StrictHostKeyChecking is enabled but environment variable KNOW_HOSTS_FILE_PATH is not set or points to not existing file [{}] --> falling back to StrictHostKeyChecking='no'.", + filePath); + } + } +} diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/model/FileData.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/model/FileData.java index 4805cb47..8721f61e 100644 --- a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/model/FileData.java +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/model/FileData.java @@ -130,12 +130,12 @@ public abstract class FileData { String[] userAndPassword = userInfoString.split(":"); if (userAndPassword.length == 2) { return Optional.of(userAndPassword); - }else if(userAndPassword.length == 1)//if just user + } else if (userAndPassword.length == 1)// if just user { - String[] tab = new String[2]; - tab[0] = userAndPassword[0]; - tab[1] = "";//add empty password - return Optional.of(tab); + String[] tab = new String[2]; + tab[0] = userAndPassword[0]; + tab[1] = "";// add empty password + return Optional.of(tab); } } return Optional.empty(); diff --git a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/tasks/FileCollector.java b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/tasks/FileCollector.java index 3e292975..e9c4aceb 100644 --- a/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/tasks/FileCollector.java +++ b/datafile-app-server/src/main/java/org/onap/dcaegen2/collectors/datafile/tasks/FileCollector.java @@ -29,6 +29,7 @@ import org.onap.dcaegen2.collectors.datafile.exceptions.NonRetryableDatafileTask import org.onap.dcaegen2.collectors.datafile.ftp.FileCollectClient; import org.onap.dcaegen2.collectors.datafile.ftp.FtpsClient; import org.onap.dcaegen2.collectors.datafile.ftp.SftpClient; +import org.onap.dcaegen2.collectors.datafile.ftp.SftpClientSettings; import org.onap.dcaegen2.collectors.datafile.model.Counters; import org.onap.dcaegen2.collectors.datafile.model.FileData; import org.onap.dcaegen2.collectors.datafile.model.FilePublishInformation; @@ -67,7 +68,6 @@ public class FileCollector { * @param numRetries the number of retries if the publishing fails * @param firstBackoff the time to delay the first retry * @param contextMap context for logging. - * * @return the data needed to publish the file. */ public Mono collectFile(FileData fileData, long numRetries, Duration firstBackoff, @@ -154,7 +154,8 @@ public class FileCollector { } protected SftpClient createSftpClient(FileData fileData) { - return new SftpClient(fileData.fileServerData()); + return new SftpClient(fileData.fileServerData(), + new SftpClientSettings(datafileAppConfig.getSftpConfiguration())); } protected FtpsClient createFtpsClient(FileData fileData) { diff --git a/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java index 11a428bc..f64cf433 100644 --- a/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java +++ b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/FtpsClientTest.java @@ -75,8 +75,8 @@ public class FtpsClientTest { @BeforeEach protected void setUp() throws Exception { - clientUnderTestSpy = spy(new FtpsClient(createFileServerData(), Paths.get(FTP_KEY_PATH), FTP_KEY_PASSWORD, TRUSTED_CA_PATH, - TRUSTED_CA_PASSWORD)); + clientUnderTestSpy = spy(new FtpsClient(createFileServerData(), Paths.get(FTP_KEY_PATH), FTP_KEY_PASSWORD, + TRUSTED_CA_PATH, TRUSTED_CA_PASSWORD)); clientUnderTestSpy.realFtpsClient = ftpsClientMock; } diff --git a/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettingsTest.java b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettingsTest.java new file mode 100644 index 00000000..8c17790e --- /dev/null +++ b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettingsTest.java @@ -0,0 +1,68 @@ +/*- + * ============LICENSE_START====================================================================== + * Copyright (C) 2018-2019 Nordix Foundation. All rights reserved. + * Copyright (C) 2020 Nokia. 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. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied. See the License for the specific language governing permissions + * and limitations under the License. + * ============LICENSE_END======================================================================== + */ + +package org.onap.dcaegen2.collectors.datafile.ftp; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.onap.dcaegen2.collectors.datafile.configuration.ImmutableSftpConfig; +import org.onap.dcaegen2.collectors.datafile.configuration.SftpConfig; + +public class SftpClientSettingsTest { + + @Test + public void shouldUseFtpStrictHostChecking(@TempDir Path tempDir) throws Exception { + File knowHostsFile = new File(tempDir.toFile(), "known_hosts"); + knowHostsFile.createNewFile(); + + SftpConfig config = createSampleSftpConfigWithStrictHostChecking(knowHostsFile.getAbsolutePath()); + SftpClientSettings sftpClient = new SftpClientSettings(config); + + assertThat(sftpClient.shouldUseStrictHostChecking()).isTrue(); + } + + @Test + public void shouldNotUseFtpStrictHostChecking_whenFileDoesNotExist() { + SftpConfig config = createSampleSftpConfigWithStrictHostChecking("unknown_file"); + SftpClientSettings sftpClient = new SftpClientSettings(config); + + sftpClient.shouldUseStrictHostChecking(); + assertThat(sftpClient.shouldUseStrictHostChecking()).isFalse(); + } + + @Test + public void shouldNotUseFtpStrictHostChecking_whenExplicitlySwitchedOff() { + SftpClientSettings sftpClient = new SftpClientSettings(createSampleSftpConfigNoStrictHostChecking()); + sftpClient.shouldUseStrictHostChecking(); + assertThat(sftpClient.shouldUseStrictHostChecking()).isFalse(); + } + + private SftpConfig createSampleSftpConfigNoStrictHostChecking() { + return new ImmutableSftpConfig.Builder() // + .strictHostKeyChecking(false).knownHostsFilePath("N/A").build(); + } + + private SftpConfig createSampleSftpConfigWithStrictHostChecking(String pathToKnownHostsFile) { + return new ImmutableSftpConfig.Builder() // + .strictHostKeyChecking(true).knownHostsFilePath(pathToKnownHostsFile).build(); + } +} diff --git a/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientTest.java b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientTest.java index 1c58650d..d50bfc8d 100644 --- a/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientTest.java +++ b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientTest.java @@ -1,6 +1,6 @@ /*- * ============LICENSE_START====================================================================== - * Copyright (C) 2018-2019 Nordix Foundation. All rights reserved. + * Copyright (C) 2020 Nokia. 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. You may obtain a copy of the License at @@ -34,7 +34,6 @@ import com.jcraft.jsch.JSchException; import com.jcraft.jsch.Session; import com.jcraft.jsch.SftpException; -import java.io.IOException; import java.nio.file.Paths; import java.util.Optional; @@ -42,11 +41,14 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.onap.dcaegen2.collectors.datafile.configuration.ImmutableSftpConfig; +import org.onap.dcaegen2.collectors.datafile.configuration.SftpConfig; import org.onap.dcaegen2.collectors.datafile.exceptions.DatafileTaskException; import org.onap.dcaegen2.collectors.datafile.exceptions.NonRetryableDatafileTaskException; @ExtendWith(MockitoExtension.class) public class SftpClientTest { + private static final String HOST = "127.0.0.1"; private static final int SFTP_PORT = 1021; private static final String USERNAME = "bob"; @@ -62,8 +64,7 @@ public class SftpClientTest { private ChannelSftp channelMock; @Test - public void openWithPort_success() - throws DatafileTaskException, IOException, JSchException, SftpException, Exception { + public void openWithPort_success() throws Exception { FileServerData expectedFileServerData = ImmutableFileServerData.builder() // .serverAddress(HOST) // .userId(USERNAME) // @@ -71,7 +72,7 @@ public class SftpClientTest { .port(SFTP_PORT) // .build(); - SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData)); + SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData, createSampleSftpClientSettings())); doReturn(jschMock).when(sftpClientSpy).createJsch(); when(jschMock.getSession(anyString(), anyString(), anyInt())).thenReturn(sessionMock); @@ -91,8 +92,7 @@ public class SftpClientTest { } @Test - public void openWithoutPort_success() - throws DatafileTaskException, IOException, JSchException, SftpException, Exception { + public void openWithoutPort_success() throws Exception { FileServerData expectedFileServerData = ImmutableFileServerData.builder() // .serverAddress(HOST) // .userId(USERNAME) // @@ -100,7 +100,7 @@ public class SftpClientTest { .port(Optional.empty()) // .build(); - SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData)); + SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData, createSampleSftpClientSettings())); doReturn(jschMock).when(sftpClientSpy).createJsch(); when(jschMock.getSession(anyString(), anyString(), anyInt())).thenReturn(sessionMock); @@ -112,8 +112,7 @@ public class SftpClientTest { } @Test - public void open_throwsExceptionWithRetry() - throws DatafileTaskException, IOException, JSchException, SftpException, Exception { + public void open_throwsExceptionWithRetry() throws Exception { FileServerData expectedFileServerData = ImmutableFileServerData.builder() // .serverAddress(HOST) // .userId(USERNAME) // @@ -121,7 +120,7 @@ public class SftpClientTest { .port(SFTP_PORT) // .build(); - SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData)); + SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData, createSampleSftpClientSettings())); doReturn(jschMock).when(sftpClientSpy).createJsch(); when(jschMock.getSession(anyString(), anyString(), anyInt())).thenThrow(new JSchException("Failed")); @@ -131,8 +130,7 @@ public class SftpClientTest { } @Test - public void openAuthFail_throwsExceptionWithoutRetry() - throws DatafileTaskException, IOException, JSchException, SftpException, Exception { + public void openAuthFail_throwsExceptionWithoutRetry() throws Exception { FileServerData expectedFileServerData = ImmutableFileServerData.builder() // .serverAddress(HOST) // .userId(USERNAME) // @@ -140,7 +138,7 @@ public class SftpClientTest { .port(SFTP_PORT) // .build(); - SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData)); + SftpClient sftpClientSpy = spy(new SftpClient(expectedFileServerData, createSampleSftpClientSettings())); doReturn(jschMock).when(sftpClientSpy).createJsch(); when(jschMock.getSession(anyString(), anyString(), anyInt())).thenThrow(new JSchException("Auth fail")); @@ -154,14 +152,14 @@ public class SftpClientTest { @SuppressWarnings("resource") @Test - public void collectFile_succes() throws DatafileTaskException, SftpException { + public void collectFile_success() throws DatafileTaskException, SftpException { FileServerData expectedFileServerData = ImmutableFileServerData.builder() // .serverAddress(HOST) // .userId(USERNAME) // .password(PASSWORD) // .port(SFTP_PORT) // .build(); - SftpClient sftpClient = new SftpClient(expectedFileServerData); + SftpClient sftpClient = new SftpClient(expectedFileServerData, createSampleSftpClientSettings()); sftpClient.sftpChannel = channelMock; @@ -172,8 +170,7 @@ public class SftpClientTest { } @Test - public void collectFile_throwsExceptionWithRetry() - throws IOException, JSchException, SftpException, DatafileTaskException { + public void collectFile_throwsExceptionWithRetry() throws SftpException { FileServerData expectedFileServerData = ImmutableFileServerData.builder() // .serverAddress(HOST) // .userId(USERNAME) // @@ -181,7 +178,7 @@ public class SftpClientTest { .port(SFTP_PORT) // .build(); - try (SftpClient sftpClient = new SftpClient(expectedFileServerData)) { + try (SftpClient sftpClient = new SftpClient(expectedFileServerData, createSampleSftpClientSettings())) { sftpClient.sftpChannel = channelMock; doThrow(new SftpException(ChannelSftp.SSH_FX_BAD_MESSAGE, "Failed")).when(channelMock).get(anyString(), anyString()); @@ -194,8 +191,7 @@ public class SftpClientTest { } @Test - public void collectFileFileMissing_throwsExceptionWithoutRetry() - throws IOException, JSchException, SftpException, DatafileTaskException { + public void collectFileFileMissing_throwsExceptionWithoutRetry() throws SftpException { FileServerData expectedFileServerData = ImmutableFileServerData.builder() // .serverAddress(HOST) // .userId(USERNAME) // @@ -203,7 +199,7 @@ public class SftpClientTest { .port(SFTP_PORT) // .build(); - try (SftpClient sftpClient = new SftpClient(expectedFileServerData)) { + try (SftpClient sftpClient = new SftpClient(expectedFileServerData, createSampleSftpClientSettings())) { sftpClient.sftpChannel = channelMock; doThrow(new SftpException(ChannelSftp.SSH_FX_NO_SUCH_FILE, "Failed")).when(channelMock).get(anyString(), anyString()); @@ -217,8 +213,8 @@ public class SftpClientTest { } @Test - public void close_succes() throws DatafileTaskException, SftpException { - SftpClient sftpClient = new SftpClient(null); + public void close_success() { + SftpClient sftpClient = new SftpClient(null, createSampleSftpClientSettings()); sftpClient.session = sessionMock; sftpClient.sftpChannel = channelMock; @@ -231,4 +227,14 @@ public class SftpClientTest { verify(channelMock).exit();; verifyNoMoreInteractions(channelMock); } + + private SftpClientSettings createSampleSftpClientSettings() { + return new SftpClientSettings(createSampleSftpConfigNoStrictHostChecking()); + } + + private SftpConfig createSampleSftpConfigNoStrictHostChecking() { + return new ImmutableSftpConfig.Builder() // + .strictHostKeyChecking(false).knownHostsFilePath("N/A").build(); + } + } diff --git a/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/model/FileDataTest.java b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/model/FileDataTest.java index 10a84e76..e4b528a9 100644 --- a/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/model/FileDataTest.java +++ b/datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/model/FileDataTest.java @@ -38,7 +38,7 @@ public class FileDataTest { private static final String LOCATION_WITH_USER = FTPES_SCHEME + USER + ":" + PWD + "@" + SERVER_ADDRESS + ":" + PORT_22 + REMOTE_FILE_LOCATION; private static final String LOCATION_WITH_USER_NO_PASSWORD = - FTPES_SCHEME + USER + "@" + SERVER_ADDRESS + ":" + PORT_22 + REMOTE_FILE_LOCATION; + FTPES_SCHEME + USER + "@" + SERVER_ADDRESS + ":" + PORT_22 + REMOTE_FILE_LOCATION; private static final String LOCATION_WITHOUT_USER = FTPES_SCHEME + SERVER_ADDRESS + ":" + PORT_22 + REMOTE_FILE_LOCATION; @@ -67,17 +67,17 @@ public class FileDataTest { .build(); } - private FileData properFileDataWithUserNoPassword() { - return ImmutableFileData.builder() // - .name("name") // - .location(LOCATION_WITH_USER_NO_PASSWORD) // - .compression("comp") // - .fileFormatType("type") // - .fileFormatVersion("version") // - .scheme(Scheme.FTPS) // - .messageMetaData(messageMetaData()) // - .build(); - } + private FileData properFileDataWithUserNoPassword() { + return ImmutableFileData.builder() // + .name("name") // + .location(LOCATION_WITH_USER_NO_PASSWORD) // + .compression("comp") // + .fileFormatType("type") // + .fileFormatVersion("version") // + .scheme(Scheme.FTPS) // + .messageMetaData(messageMetaData()) // + .build(); + } private FileData properFileDataWithoutUser() { return ImmutableFileData.builder() // diff --git a/datafile-app-server/src/test/resources/datafile_endpoints_test.json b/datafile-app-server/src/test/resources/datafile_endpoints_test.json index 58f4eb89..62119e61 100644 --- a/datafile-app-server/src/test/resources/datafile_endpoints_test.json +++ b/datafile-app-server/src/test/resources/datafile_endpoints_test.json @@ -10,6 +10,7 @@ "dmaap.security.keyStorePath": "keyStorePath", "dmaap.security.keyStorePasswordPath": "keyStorePasswordPath", "dmaap.security.enableDmaapCertAuth": "true", + "sftp.security.strictHostKeyChecking": "false", "streams_publishes": { "PM_MEAS_FILES": { "type": "data_router", diff --git a/datafile-app-server/src/test/resources/datafile_endpoints_test_2producers.json b/datafile-app-server/src/test/resources/datafile_endpoints_test_2producers.json index 40c28dde..480a6f79 100644 --- a/datafile-app-server/src/test/resources/datafile_endpoints_test_2producers.json +++ b/datafile-app-server/src/test/resources/datafile_endpoints_test_2producers.json @@ -10,6 +10,7 @@ "dmaap.security.keyStorePath": "keyStorePath", "dmaap.security.keyStorePasswordPath": "keyStorePasswordPath", "dmaap.security.enableDmaapCertAuth": "true", + "sftp.security.strictHostKeyChecking": "false", "streams_publishes": { "PM_MEAS_FILES": { "type": "data_router", @@ -54,4 +55,4 @@ } } } -} \ No newline at end of file +} diff --git a/pom.xml b/pom.xml index 407f8afb..47f1fcce 100644 --- a/pom.xml +++ b/pom.xml @@ -30,7 +30,7 @@ org.onap.dcaegen2.collectors datafile - 1.4.0-SNAPSHOT + 1.4.1-SNAPSHOT dcaegen2-collectors.datafile datafile collector @@ -44,7 +44,7 @@ - 11 + 8 1.1.6 4.1.4 3.6 @@ -60,7 +60,7 @@ true - 5.1.0 + 5.5.2 5.3.2 1.1.0 2.23.4 -- cgit 1.2.3-korg