x/crypto/ssh: export supported algorithms

[drakkan]

Currently KEXs, MACs and ciphers are private, some of them are defined as constants and others as simple strings, for example take a look at the supported ciphers list

// supportedCiphers lists ciphers we support but might not recommend.
var supportedCiphers = []string{
	"aes128-ctr", "aes192-ctr", "aes256-ctr",
	"aes128-gcm@openssh.com", gcm256CipherID,
	chacha20Poly1305ID,
	"arcfour256", "arcfour128", "arcfour",
	aes128cbcID,
	tripledescbcID,
}

I propose defining all supported algorithms as constants and exporting them for better discoverability.
We should also export the list of supported ciphers, KEXs, MACs, host key, public key algorithms and so on, so an application using the library can simply check if an algorithm is supported.

cc @golang/security

[FiloSottile]

I definitely agree the supported algorithms should be somewhere on the godoc, either in the constants block or in the package description, but I am mildly opposed to exposing a list of "supported" algorithms as a slice or the like. First, it encourages enabling all supported algorithms by making it easy, and we have a lot of bad supported algorithms; second, "supported" is a complex state that can depend on when and how they are used. See #46232.

Can we see the list of proposed constants?

[drakkan]

Here is the list of proposed constants

// Supported ciphers. Ciphers based on AES CBC and RC4 are no longer considered
// secure.
const (
	CipherAlgoAES128GCM        = "aes128-gcm@openssh.com"
	CipherAlgoAES256GCM        = "aes256-gcm@openssh.com"
	CipherAlgoChacha20Poly1305 = "chacha20-poly1305@openssh.com"
	CipherAlgoAES128CTR        = "aes128-ctr"
	CipherAlgoAES192CTR        = "aes192-ctr"
	CipherAlgoAES256CTR        = "aes256-ctr"
	CipherAlgoAES128CBC        = "aes128-cbc"
	CipherAlgoTripleDESCBC     = "3des-cbc"
	CipherAlgoRC4              = "arcfour"
	CipherAlgoRC4128           = "arcfour128"
	CipherAlgoRC4256           = "arcfour256"
)

// Supported key exchanges algorithms. SHA-1 based KEXs are no longer considered
// secure.
const (
	KEXAlgoDH1SHA1    = "diffie-hellman-group1-sha1"
	KEXAlgoDH14SHA1   = "diffie-hellman-group14-sha1"
	KEXAlgoDH14SHA256 = "diffie-hellman-group14-sha256"
	KEXAlgoECDH256    = "ecdh-sha2-nistp256"
	KEXAlgoECDH384    = "ecdh-sha2-nistp384"
	KEXAlgoECDH521    = "ecdh-sha2-nistp521"
	// This KEX enables both curve25519-sha256 and curve25519-sha256@libssh.org
	KEXAlgoCurve25519SHA256 = "curve25519-sha256"
	// For the following kex only the client half contains a production ready
	// implementation. The server half only consists of a minimal implementation
	// to satisfy the automated tests and it is not accepted.
	KEXAlgoDHGEXSHA1   = "diffie-hellman-group-exchange-sha1"
	KEXAlgoDHGEXSHA256 = "diffie-hellman-group-exchange-sha256"
)

// Supported message authentication code (MAC) algorithms. SHA-1 based MACs are
// no longer considered secure.
const (
	MACAlgoHMACSHA256ETM = "hmac-sha2-256-etm@openssh.com"
	MACAlgoHMACSHA512ETM = "hmac-sha2-512-etm@openssh.com"
	MACAlgoHMACSHA256    = "hmac-sha2-256"
	MACAlgoHMACSHA512    = "hmac-sha2-512"
	MACAlgoHMACSHA1      = "hmac-sha1"
	MACAlgoHMACSHA196    = "hmac-sha1-96"
)

[FiloSottile]

Do we need both KEXAlgoCurve25519SHA256LibSSH and KEXAlgoCurve25519SHA256? Can we just have KEXAlgoCurve25519SHA256 instead and always negotiate both if enabled, assuming the wire protocol is the same or equivalent?

[drakkan]

Do we need both KEXAlgoCurve25519SHA256LibSSH and KEXAlgoCurve25519SHA256? Can we just have KEXAlgoCurve25519SHA256 instead and always negotiate both if enabled, assuming the wire protocol is the same or equivalent?

curve25519-sha256 is just the new name for curve25519-sha256@libssh.org, they already share the same implementation

https://github.com/golang/crypto/blob/d08e19beaccde615f2f1458b1b0df8fe75e20c8a/ssh/kex.go#L436

kexAlgoMap[kexAlgoCurve25519SHA256] = &curve25519sha256{}
kexAlgoMap[kexAlgoCurve25519SHA256LibSSH] = &curve25519sha256{}

we can expose only KEXAlgoCurve25519SHA256 and document that it will also enable the other variant.
I'll update the proposed constants. Thank you

[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

[hanwen]

First, it encourages enabling all supported algorithms by making it easy, and we have a lot of bad supported algorithms

We could counter this by selecting a sensible default configuration for both client and server.

If library users decide to override this default, we should provide them information to let them make informed choices.

Maybe the TLS package is a good example: see https://pkg.go.dev/crypto/tls#CipherSuites and https://pkg.go.dev/crypto/tls#InsecureCipherSuites. We could do something like

type Algorithms {
  KEX []string
  Cipher []string
  MAC []string
  PublicKey []string
  PublicKeyAlgo []string
}

// Algorithms implemented, excluding ones with security problems.
func SupportedAlgorithms() Algorithms

// Algorithms implemented with security problems.
func InsecureAlgorithms() Algorithms

second, "supported" is a complex state that can depend on when and how they are used. See #46232.

if we have a clear distinction between secure/insecure algorithms, we could move the server implementation of 'client-only' algorithms out of the test files.

[rsc]

Any more thoughts on this? What is the proposal we've converged on?

[drakkan]

I think we agree on exporting all supported algorithms as constants but we are not yet sure if exporting or not a list of supported algorithms.

Internally we have a list of supported and preferred algorithms and use the preferred ones if no algorithm is explicitly configured.

The weird thing for me the first time I used x/crypto as a user was that I had to look at the code to figure out which algorithms are supported and preferred.
I would favour exporting also the lists of algorithms I would do only a minor change to the @hanwen proposal

type Algorithms {
  KEX []string
  Cipher []string
  MAC []string
  PublicKey []string
  PublicKeyAlgo []string
}

// SupportedAlgorithms returns algorithms we support but might not recommend.
func SupportedAlgorithms() Algorithms

// PreferredAlgorithms returns the default preference for algorithms.
func PreferredAlgorithms() Algorithms

We could also change the preferred algorithms so that sha-1 based algorithms are disabled by default