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 --- .../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 +- 6 files changed, 115 insertions(+), 39 deletions(-) create mode 100644 datafile-app-server/src/test/java/org/onap/dcaegen2/collectors/datafile/ftp/SftpClientSettingsTest.java (limited to 'datafile-app-server/src/test') 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 +} -- cgit 1.2.3-korg