Add ssh client_auth false check

[ikstream]

This seems to be a very rare case, but ssh client_auth might return false if no common auth method can be established.

[4elta]

thanks for bringing this up and for providing a PR.

SSHConnection:list(username) returns ...

• a list of authentication methods on success
• false on failure

therefore, i think the parser should interpret the value false as "could not establish client authentication methods". basically the same as the ERROR case.

if "ERROR" in script_output or "false" in script_output:
   service['issues'].append(...)

the value of none is an actual "authentication method name" according to the RFC.

parsing the value false as client_authentication_method = [ 'false' ] wouldn't be correct; it wouldn't make sense, as this value would then end up in the analysis as following:

• client authentication method: false

[4elta]

could you please provide the <script id="ssh-auth-methods"> tag for references?

[ikstream]

<ports><port protocol="tcp" portid="22"><state state="open" reason="syn-ack" reason_ttl="63"/>
<service name="ssh" product="OpenSSH" version="really really old one" extrainfo="protocol 2.0" method="probed" conf="10"><cpe>cpe:/a:openbsd:openssh:really really old one</cpe></service>
<script id="ssh-auth-methods" output="&#xa;  Supported authentication methods: false">
<elem key="Supported authentication methods">false</elem>

[ikstream]

I am not sure how we should interpret the false situation. The SSH server provides kex, encryption and mac algorithms.

Section 5.1 of the RFC you linked states

The value of 'partial success' MUST be TRUE if the authentication
   request to which this is a response was successful.  It MUST be FALSE
   if the request was not successfully processed.

I don't know what caused the unsuccessful processing but I think it should be included in the output.

[4elta]

partial success (boolean) has nothing to to with authentications that can continue (string). this is something different to what (authentication) method name stands for.

whatever the cause for that behaviour (i.e. incorrectly assigning the method false to the authentication method name variable) is, i still think, that we should treat Supported authentication methods: false as what it realy is (i.e. an error): could not establish authentication method.

if "ERROR" in script_output or "Supported authentication methods: false" in script_output:
   service['issues'].append(
      Issue("client authentication method: unknown")
   )

it just wouldn't make sense to store false in the client_authentication_method array, because false isn't a valie authentication method (in contrast: none is a valid option).

[ikstream]

Switch to unknown, also still think, that the actual returned value would be better, but it reads better in the context.

[4elta]

thank you for updating this PR. unfortunately, i still don't aggree with the proposed changes.

i will try to improve my argument already given in my previous comment...

interpreting the situation authentications that can continue: false as an actual client authentication method (i.e. appending the value unknown to the client_authentication_methods) is, in my opinion, incorrect behaviour by the parser and could lead to incorrect results. because the analyzer will only raise an issue if this unknown authentication method is not recommended (i.e. via the recommendations file).

in fact, this situation (i.e. the tool's determination of the server's config: Supported authentication methods: false) is, according to the RFC, in itself incorrect behaviour (of the tool) and thus already an issue: the tool could not (successfully) determine which client authentication methods the server allows.

the parser, not the analyzer (if at all), must draw attention to this issue:

if "ERROR" in script_output or "Supported authentication methods: false" in script_output:
   service['issues'].append(
      Issue("client authentication method: unknown")
   )