Signature algorithm selection should match the behaviour of OpenSSH

[mvegter]

Closes #454

[norrisjeremy]

I'm sorry, but I don't approve this.

We don't enable RSA/SHA1 by default (since it is insecure) and if you have a server that requires it (because it is buggy and breaks when RSA/SHA2 type authentication is attempted), then you should make sure to either remove the RSA/SHA2 algorithms or order RSA/SHA1 with a higher priority than RSA/SHA2 in the PubkeyAcceptedAlgorithms config (especially considering you already have to modify the PubkeyAcceptedAlgorithms config to even enable RSA/SHA1 in the first place).

Alternatively, you could create your own UserAuthPublicKey class that implements the behavior you need and then direct JSch to use it instead of it's built-in implementation by overriding the userauth.publickey config.

[mvegter]

We don't enable RSA/SHA1 by default (since it is insecure)

This doesn't enable RSA/SHA1, it only reorders the PubKey in the same manner as OpenSSH when configured.

[norrisjeremy]

We don't enable RSA/SHA1 by default (since it is insecure)

This doesn't enable RSA/SHA1, it only reorders the PubKey in the same manner as OpenSSH when configured.

Yes, I fully realize that. And that is one of the primary reasons I object this in the first place, since the user already has to explicitly configure PubkeyAcceptedAlgorithms to turn on RSA/SHA1.
Since they already have to modify PubkeyAcceptedAlgorithms, they can simply ensure that they either remove the RSA/SHA2 algorithms, or order the RSA/SHA1 algorithm with a higher priority so that it is attempted before RSA/SHA2.

[norrisjeremy]

Also:

• Even if we decide to accept this change, it would need to be disabled by default (we don't want to have a configuration option that is enabled by default that actively makes JSch less secure, by having it prefer the insecure RSA/SHA1 algorithm).
• The unit/integration tests seem to fail with this change incorporated, so either something is incorrect with it, or the unit/integration tests require updates.

[mvegter]

I opened the MR for easier discussion on a potential solution not as a definitive version, hence the draft state. Regarding the failing test, it actually highlights the solution as there is no server-sig-algos so we could/should use ssh-rsa first (which succeeds).

If you feel strongly about not matching the OpenSSH behaviour then that's unfortunate, as it's hard to reason why the command line succeeds but the application doesn't. Appending ssh-rsa as last resort option would work with OpenSSH but requires first position in Jsch. If we can work towards a solution that's off by default that would work for me as well

[norrisjeremy]

Hi @mvegter,

I do feel pretty strongly about this, as it weakens the overall security posture of JSch.
I'm not sure how @mwiede feels about it however?

Also, I will point out a paragraph from RFC 8332 that I believe you missed:

   When authenticating with an RSA key against a server that does not
   implement the "server-sig-algs" extension, clients MAY default to an
   "ssh-rsa" signature to avoid authentication penalties.  When the new
   rsa-sha2-* algorithms have been sufficiently widely adopted to
   warrant disabling "ssh-rsa", clients MAY default to one of the new
   algorithms.

I would argue that since it has been well over 5 years since RFC 8332 has been published, it has been sufficient period of time for vendors to implement RSA/SHA2 and fully deprecate RSA/SHA1 and clients shouldn't be forced to implement a workaround like this anymore.

Thanks,
Jeremy

[mvegter]

Hey @norrisjeremy ,

I'm fully aware of that section, I based this change on the latest implementation of OpenSSH (9.5) that still has this code : https://github.com/openssh/openssh-portable/blob/V_9_5/sshconnect2.c#L1169-L1180 from OpenSSH (7.8). Which in behaviour matches with the manual attempts I did using a OpenSSH_8.0 cli.

[norrisjeremy]

Hi @mvegter,

That still does not change my opinion: if a user absolutely requires the use of the RSA/SHA1 algorithm (which we do not enable by default in the first place), then the burden must be on them to direct JSch to use it, namely by either explicitly providing it a higher priority (i.e. placing it earlier in the PubkeyAcceptedAlgorithms config string), or by omitting other algorithms that they do not wish JSch to use (i.e. leaving out the more secure RSA/SHA2 algorithms from the PubkeyAcceptedAlgorithms config string).

We do not want JSch to make the decision for them to weaken their security, by magically reordering the RSA/SHA1 algorithm to have a higher priority than the RSA/SHA2 algorithms.

Thanks,
Jeremy

[norrisjeremy]

Hi @mvegter,

Also, now that it is finally public knowledge, another reason I oppose this change is because a malicious attacker could delete an EXT_INFO message from a server containing the server-sig-algs extension, which could then coerce JSch into actively using a weaker algorithm (RSA/SHA1) if we implemented this change. See Terrapin Attack for more details.

Thanks,
Jeremy