x/crypto/ssh: allow to configure public key authentication algorithms on the server side

[drakkan]

Currently supportedPubKeyAuthAlgos is hard-coded and contains all supported public key authentication algorithms.

Algorithms and signature formats not included in that list are not accepted by our server implementation.

I propose to make it configurable to allow to disable the weaker algorithms, for example ssh-dss or ssh-rsa.

A simple implementation would be to add them to the ServerConfig struct:

// ServerConfig holds server specific configuration data.
type ServerConfig struct {
	// Config contains configuration shared between client and server.
	Config
 
  // PublicKeyAuthAlgorithms specifies the supported client public key
  // authentication algorithms. Note that this should not include certificate
  // types since those use the underlying algorithm. This list is sent to the
  // client if it supports the server-sig-algs extension. Order is irrelevant.
  // If unspecified then a default set of algorithms is used.
  PublicKeyAuthAlgorithms []string
  ....
}

This is consistent with the way we currently allow to customize KeyExchanges, Ciphers and MACs.

An alternative to consider is to add a callback to the ServerConfig struct, for example

// ServerConfig holds server specific configuration data.
type ServerConfig struct {
   ...
   
   PublicKeyAuthAlgorithmsCallback func(ConnMetadata) []string
   ...
}

this would allow for more flexibility, for example allowing algorithms based on the client version, but if we are to go this route we should probably allow the same for KeyExchanges, Ciphers and MACs as well and it is probably better to think about something more generic using a single callback for all the customizable algorithms.

[ianlancetaylor]

CC @golang/security

[FiloSottile]

For #56561. Not a fan of yet another knob, but I don't have a better idea and LGTM.

[gopherbot]

Change https://go.dev/cl/510775 mentions this issue: ssh: allow to configure public key auth algorithms on the server side

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

Are people generally in favor of this? Have all remaining concerns been addressed?

[hanwen]

In short, I agree with Filippo: not pretty, but OK to add.

I propose to make it configurable to allow to disable the weaker algorithms, for example ssh-dss or ssh-rsa.

This is only about disabling RSA signatures on SHA1 digests? (You can disable ssh-dss by rejecting any keys in the PublicKeyCallback).

If I could redesign this from scratch, I'd pass the algorithm as an extra parameter to ServerConfig.PublicKeyCallback, then we wouldn't need this extra knob. The extra knob does have a slight performance advantage, because it also avoids an extra roundtrip where the client offers a public key only to have it be rejected.

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group