[ 
https://issues.apache.org/jira/browse/NIFI-7049?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Arpad Boda updated NIFI-7049:
-----------------------------
    Description: 
In case NiFi test are executed on a machine without knows_hosts file, it's 
going to fail. 

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 something stupid.
-Either the documentation or the behaviour should be fixed to make them aligned 
(you are the security guy to tell which one is right :wink: )
-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.

  was:
Just pasting my private message that summarised this error:

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 something stupid.
-Either the documentation or the behaviour should be fixed to make them aligned 
(you are the security guy to tell which one is right :wink: )
-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.


> SFTP processors shouldn't silently try to access known hosts file on the 
> system
> -------------------------------------------------------------------------------
>
>                 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
>            Priority: Major
>
> In case NiFi test are executed on a machine without knows_hosts file, it's 
> going to fail. 
> 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 something stupid.
> -Either the documentation or the behaviour should be fixed to make them 
> aligned (you are the security guy to tell which one is right :wink: )
> -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)

Reply via email to