[jira] [Commented] (NIFI-7049) SFTP processors shouldn't silently try to access known hosts file of the user

["Dayakar Morem (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/NIFI-7049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17022000#comment-17022000 ] 

Dayakar Morem commented on NIFI-7049:
-------------------------------------

[https://github.com/hierynomus/sshj/blob/master/src/main/java/net/schmizz/sshj/SSHClient.java#L621] , here first they are checking .ssh directory under "user.home", if this directory not present then throwing IOException. If ".ssh" directory is present under user.home but "known_hosts" or "known_hosts2 files" are not present under this directory then it is not throwing any exception. The exception will come if "Strict Host Key Checking" property is enabled for the processor as there are no known hosts.

 

To fix this issue, I think we no need to load default known_host file(s) from user.home directory or we no need to throw exception even if .ssh directory is not present when "Strict Host Key Checking" is disabled. Any other thoughts...

> SFTP processors shouldn't silently try to access known hosts file of the user
> -----------------------------------------------------------------------------
>
>                 Key: NIFI-7049
>                 URL: https://issues.apache.org/jira/browse/NIFI-7049
>             Project: Apache NiFi
>          Issue Type: Bug
>          Components: Extensions
>    Affects Versions: 1.10.0
>            Reporter: Arpad Boda
>            Assignee: Dayakar Morem
>            Priority: Major
>
> In case NiFi test are executed on a machine without knows_hosts file, it's going to fail:
> {code}
> [INFO] Running org.apache.nifi.processors.standard.TestGetSFTP
> [ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.374 s <<< FAILURE! - in org.apache.nifi.processors.standard.TestGetSFTP
> [ERROR] testGetSFTPFileBasicRead(org.apache.nifi.processors.standard.TestGetSFTP)  Time elapsed: 0.132 s  <<< FAILURE!
> java.lang.AssertionError: expected:<4> but was:<0>
> 	at org.apache.nifi.processors.standard.TestGetSFTP.testGetSFTPFileBasicRead(TestGetSFTP.java:88)
> [ERROR] testGetSFTPIgnoreDottedFiles(org.apache.nifi.processors.standard.TestGetSFTP)  Time elapsed: 0.013 s  <<< FAILURE!
> java.lang.AssertionError: expected:<2> but was:<0>
> 	at org.apache.nifi.processors.standard.TestGetSFTP.testGetSFTPIgnoreDottedFiles(TestGetSFTP.java:110)
> {code}
> Just pasting my private message that summarised this error previously:
> https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/util/SFTPTransfer.java#L556
> So the problem is that host key file is not a mandatory, but  in case it’s not provided, we call load on the 3rd party lib without arguments:
> https://github.com/hierynomus/sshj/blob/master/src/main/java/net/schmizz/sshj/SSHClient.java#L621
> Which tries to load keys from the default location, but this is far from what we state in our documentation:
> {code}Host Key File            If supplied, the given file will be used as the Host Key; otherwise, no use host key file will be used {code}
> So there are multiple issues here:
> -Even though the ssh connection fails, somewhere the IO exception is swallowed. Didn’t reproduce to check the logs, but I would expect exceptions to be thrown in the testcase and these being talkative about the error. My gut feeling says that we do the same in case the user specifies a host key file, but it’s somehow not accessible.
> -Strict host check on/off might not be enough to cover all the scenarios as there are three: host 1# known and key matches, 2# host not known and we either trust or not, 3# host known, but there is a mismatch (probably man in the middle). I think this property should be improved at least in documentation point of view as currently only the code tells what do we do in 2#. Which depends on whether the file exists or not, so most probably something unintended.
> -Either the documentation or the behaviour should be fixed to make them aligned 
> -The testcase should either use a predefined key or have host key checking completely off. According to what we see above, not sure about the latter being nicely supported.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)