From c5370d2cc696bb18a6ddc151cee09673f06e8497 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Wed, 28 Jun 2023 18:03:43 +0200 Subject: [PATCH 01/10] ssh: check the declared public key algo against decoded one This check will ensure we don't accept e.g. ssh-rsa-cert-v01@openssh.com algorithm with ssh-rsa public key type. The algorithm and public key type must be consistent: both must be certificate algorithms, or neither. Change-Id: I1d75074fb4d6db3a8796408e98ddffe577a96ab1 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/506836 TryBot-Result: Gopher Robot Reviewed-by: Dmitri Shuralyov Reviewed-by: Cherry Mui Auto-Submit: Filippo Valsorda Reviewed-by: Filippo Valsorda Run-TryBot: Filippo Valsorda --- ssh/client_auth_test.go | 91 +++++++++++++++++++++++++++++++++++++++++ ssh/server.go | 11 ++++- 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index 70558a9a77..0d2d8d255c 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -1076,3 +1076,94 @@ func TestCompatibleAlgoAndSignatures(t *testing.T) { } } } + +// configurablePublicKeyCallback is a public key callback that allows to +// configure the signature algorithm and format. This way we can emulate the +// behavior of buggy clients. +type configurablePublicKeyCallback struct { + signer AlgorithmSigner + signatureAlgo string + signatureFormat string +} + +func (cb configurablePublicKeyCallback) method() string { + return "publickey" +} + +func (cb configurablePublicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader, extensions map[string][]byte) (authResult, []string, error) { + pub := cb.signer.PublicKey() + + ok, err := validateKey(pub, cb.signatureAlgo, user, c) + if err != nil { + return authFailure, nil, err + } + if !ok { + return authFailure, nil, fmt.Errorf("invalid public key") + } + + pubKey := pub.Marshal() + data := buildDataSignedForAuth(session, userAuthRequestMsg{ + User: user, + Service: serviceSSH, + Method: cb.method(), + }, cb.signatureAlgo, pubKey) + sign, err := cb.signer.SignWithAlgorithm(rand, data, underlyingAlgo(cb.signatureFormat)) + if err != nil { + return authFailure, nil, err + } + + s := Marshal(sign) + sig := make([]byte, stringLength(len(s))) + marshalString(sig, s) + msg := publickeyAuthMsg{ + User: user, + Service: serviceSSH, + Method: cb.method(), + HasSig: true, + Algoname: cb.signatureAlgo, + PubKey: pubKey, + Sig: sig, + } + p := Marshal(&msg) + if err := c.writePacket(p); err != nil { + return authFailure, nil, err + } + var success authResult + success, methods, err := handleAuthResponse(c) + if err != nil { + return authFailure, nil, err + } + if success == authSuccess || !containsMethod(methods, cb.method()) { + return success, methods, err + } + + return authFailure, methods, nil +} + +func TestPublicKeyAndAlgoCompatibility(t *testing.T) { + cert := &Certificate{ + Key: testPublicKeys["rsa"], + ValidBefore: CertTimeInfinity, + CertType: UserCert, + } + cert.SignCert(rand.Reader, testSigners["ecdsa"]) + certSigner, err := NewCertSigner(cert, testSigners["rsa"]) + if err != nil { + t.Fatalf("NewCertSigner: %v", err) + } + + clientConfig := &ClientConfig{ + User: "user", + HostKeyCallback: InsecureIgnoreHostKey(), + Auth: []AuthMethod{ + configurablePublicKeyCallback{ + signer: certSigner.(AlgorithmSigner), + signatureAlgo: KeyAlgoRSASHA256, + signatureFormat: KeyAlgoRSASHA256, + }, + }, + } + if err := tryAuth(t, clientConfig); err == nil { + t.Error("cert login passed with incompatible public key type and algorithm") + } +} diff --git a/ssh/server.go b/ssh/server.go index b21322affa..727c71b9c7 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -576,7 +576,16 @@ userAuthLoop: if !ok || len(payload) > 0 { return nil, parseError(msgUserAuthRequest) } - + // Ensure the declared public key algo is compatible with the + // decoded one. This check will ensure we don't accept e.g. + // ssh-rsa-cert-v01@openssh.com algorithm with ssh-rsa public + // key type. The algorithm and public key type must be + // consistent: both must be certificate algorithms, or neither. + if !contains(algorithmsForKeyFormat(pubKey.Type()), algo) { + authErr = fmt.Errorf("ssh: public key type %q not compatible with selected algorithm %q", + pubKey.Type(), algo) + break + } // Ensure the public key algo and signature algo // are supported. Compare the private key // algorithm name that corresponds to algo with From d359caa4a39d59a440003b37a6cc7ace3871fd4a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Fri, 7 Feb 2020 20:24:11 -0800 Subject: [PATCH 02/10] ssh: support for marshaling keys using the OpenSSH format This adds methods to marshal private keys, encrypted and unencrypted to the OpenSSH format. Fixes golang/go#37132 Change-Id: I1a95301f789ce04858e6b147748c6e8b7700384b Reviewed-on: https://go-review.googlesource.com/c/crypto/+/218620 Run-TryBot: Roland Shoemaker TryBot-Result: Gopher Robot LUCI-TryBot-Result: Go LUCI Reviewed-by: Cherry Mui Reviewed-by: Roland Shoemaker Auto-Submit: Roland Shoemaker --- ssh/keys.go | 286 +++++++++++++++++++++++++++++++++++++++-------- ssh/keys_test.go | 68 +++++++++++ 2 files changed, 308 insertions(+), 46 deletions(-) diff --git a/ssh/keys.go b/ssh/keys.go index dac8ee7244..1bf28d87f6 100644 --- a/ssh/keys.go +++ b/ssh/keys.go @@ -13,11 +13,13 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/md5" + "crypto/rand" "crypto/rsa" "crypto/sha256" "crypto/x509" "encoding/asn1" "encoding/base64" + "encoding/binary" "encoding/hex" "encoding/pem" "errors" @@ -295,6 +297,18 @@ func MarshalAuthorizedKey(key PublicKey) []byte { return b.Bytes() } +// MarshalPrivateKey returns a PEM block with the private key serialized in the +// OpenSSH format. +func MarshalPrivateKey(key crypto.PrivateKey, comment string) (*pem.Block, error) { + return marshalOpenSSHPrivateKey(key, comment, unencryptedOpenSSHMarshaler) +} + +// MarshalPrivateKeyWithPassphrase returns a PEM block holding the encrypted +// private key serialized in the OpenSSH format. +func MarshalPrivateKeyWithPassphrase(key crypto.PrivateKey, comment string, passphrase []byte) (*pem.Block, error) { + return marshalOpenSSHPrivateKey(key, comment, passphraseProtectedOpenSSHMarshaler(passphrase)) +} + // PublicKey represents a public key using an unspecified algorithm. // // Some PublicKeys provided by this package also implement CryptoPublicKey. @@ -1241,28 +1255,106 @@ func passphraseProtectedOpenSSHKey(passphrase []byte) openSSHDecryptFunc { } } +func unencryptedOpenSSHMarshaler(privKeyBlock []byte) ([]byte, string, string, string, error) { + key := generateOpenSSHPadding(privKeyBlock, 8) + return key, "none", "none", "", nil +} + +func passphraseProtectedOpenSSHMarshaler(passphrase []byte) openSSHEncryptFunc { + return func(privKeyBlock []byte) ([]byte, string, string, string, error) { + salt := make([]byte, 16) + if _, err := rand.Read(salt); err != nil { + return nil, "", "", "", err + } + + opts := struct { + Salt []byte + Rounds uint32 + }{salt, 16} + + // Derive key to encrypt the private key block. + k, err := bcrypt_pbkdf.Key(passphrase, salt, int(opts.Rounds), 32+aes.BlockSize) + if err != nil { + return nil, "", "", "", err + } + + // Add padding matching the block size of AES. + keyBlock := generateOpenSSHPadding(privKeyBlock, aes.BlockSize) + + // Encrypt the private key using the derived secret. + + dst := make([]byte, len(keyBlock)) + key, iv := k[:32], k[32:] + block, err := aes.NewCipher(key) + if err != nil { + return nil, "", "", "", err + } + + stream := cipher.NewCTR(block, iv) + stream.XORKeyStream(dst, keyBlock) + + return dst, "aes256-ctr", "bcrypt", string(Marshal(opts)), nil + } +} + +const privateKeyAuthMagic = "openssh-key-v1\x00" + type openSSHDecryptFunc func(CipherName, KdfName, KdfOpts string, PrivKeyBlock []byte) ([]byte, error) +type openSSHEncryptFunc func(PrivKeyBlock []byte) (ProtectedKeyBlock []byte, cipherName, kdfName, kdfOptions string, err error) + +type openSSHEncryptedPrivateKey struct { + CipherName string + KdfName string + KdfOpts string + NumKeys uint32 + PubKey []byte + PrivKeyBlock []byte +} + +type openSSHPrivateKey struct { + Check1 uint32 + Check2 uint32 + Keytype string + Rest []byte `ssh:"rest"` +} + +type openSSHRSAPrivateKey struct { + N *big.Int + E *big.Int + D *big.Int + Iqmp *big.Int + P *big.Int + Q *big.Int + Comment string + Pad []byte `ssh:"rest"` +} + +type openSSHEd25519PrivateKey struct { + Pub []byte + Priv []byte + Comment string + Pad []byte `ssh:"rest"` +} + +type openSSHECDSAPrivateKey struct { + Curve string + Pub []byte + D *big.Int + Comment string + Pad []byte `ssh:"rest"` +} // parseOpenSSHPrivateKey parses an OpenSSH private key, using the decrypt // function to unwrap the encrypted portion. unencryptedOpenSSHKey can be used // as the decrypt function to parse an unencrypted private key. See // https://github.com/openssh/openssh-portable/blob/master/PROTOCOL.key. func parseOpenSSHPrivateKey(key []byte, decrypt openSSHDecryptFunc) (crypto.PrivateKey, error) { - const magic = "openssh-key-v1\x00" - if len(key) < len(magic) || string(key[:len(magic)]) != magic { + if len(key) < len(privateKeyAuthMagic) || string(key[:len(privateKeyAuthMagic)]) != privateKeyAuthMagic { return nil, errors.New("ssh: invalid openssh private key format") } - remaining := key[len(magic):] - - var w struct { - CipherName string - KdfName string - KdfOpts string - NumKeys uint32 - PubKey []byte - PrivKeyBlock []byte - } + remaining := key[len(privateKeyAuthMagic):] + var w openSSHEncryptedPrivateKey if err := Unmarshal(remaining, &w); err != nil { return nil, err } @@ -1284,13 +1376,7 @@ func parseOpenSSHPrivateKey(key []byte, decrypt openSSHDecryptFunc) (crypto.Priv return nil, err } - pk1 := struct { - Check1 uint32 - Check2 uint32 - Keytype string - Rest []byte `ssh:"rest"` - }{} - + var pk1 openSSHPrivateKey if err := Unmarshal(privKeyBlock, &pk1); err != nil || pk1.Check1 != pk1.Check2 { if w.CipherName != "none" { return nil, x509.IncorrectPasswordError @@ -1300,18 +1386,7 @@ func parseOpenSSHPrivateKey(key []byte, decrypt openSSHDecryptFunc) (crypto.Priv switch pk1.Keytype { case KeyAlgoRSA: - // https://github.com/openssh/openssh-portable/blob/master/sshkey.c#L2760-L2773 - key := struct { - N *big.Int - E *big.Int - D *big.Int - Iqmp *big.Int - P *big.Int - Q *big.Int - Comment string - Pad []byte `ssh:"rest"` - }{} - + var key openSSHRSAPrivateKey if err := Unmarshal(pk1.Rest, &key); err != nil { return nil, err } @@ -1337,13 +1412,7 @@ func parseOpenSSHPrivateKey(key []byte, decrypt openSSHDecryptFunc) (crypto.Priv return pk, nil case KeyAlgoED25519: - key := struct { - Pub []byte - Priv []byte - Comment string - Pad []byte `ssh:"rest"` - }{} - + var key openSSHEd25519PrivateKey if err := Unmarshal(pk1.Rest, &key); err != nil { return nil, err } @@ -1360,14 +1429,7 @@ func parseOpenSSHPrivateKey(key []byte, decrypt openSSHDecryptFunc) (crypto.Priv copy(pk, key.Priv) return &pk, nil case KeyAlgoECDSA256, KeyAlgoECDSA384, KeyAlgoECDSA521: - key := struct { - Curve string - Pub []byte - D *big.Int - Comment string - Pad []byte `ssh:"rest"` - }{} - + var key openSSHECDSAPrivateKey if err := Unmarshal(pk1.Rest, &key); err != nil { return nil, err } @@ -1415,6 +1477,131 @@ func parseOpenSSHPrivateKey(key []byte, decrypt openSSHDecryptFunc) (crypto.Priv } } +func marshalOpenSSHPrivateKey(key crypto.PrivateKey, comment string, encrypt openSSHEncryptFunc) (*pem.Block, error) { + var w openSSHEncryptedPrivateKey + var pk1 openSSHPrivateKey + + // Random check bytes. + var check uint32 + if err := binary.Read(rand.Reader, binary.BigEndian, &check); err != nil { + return nil, err + } + + pk1.Check1 = check + pk1.Check2 = check + w.NumKeys = 1 + + // Use a []byte directly on ed25519 keys. + if k, ok := key.(*ed25519.PrivateKey); ok { + key = *k + } + + switch k := key.(type) { + case *rsa.PrivateKey: + E := new(big.Int).SetInt64(int64(k.PublicKey.E)) + // Marshal public key: + // E and N are in reversed order in the public and private key. + pubKey := struct { + KeyType string + E *big.Int + N *big.Int + }{ + KeyAlgoRSA, + E, k.PublicKey.N, + } + w.PubKey = Marshal(pubKey) + + // Marshal private key. + key := openSSHRSAPrivateKey{ + N: k.PublicKey.N, + E: E, + D: k.D, + Iqmp: k.Precomputed.Qinv, + P: k.Primes[0], + Q: k.Primes[1], + Comment: comment, + } + pk1.Keytype = KeyAlgoRSA + pk1.Rest = Marshal(key) + case ed25519.PrivateKey: + pub := make([]byte, ed25519.PublicKeySize) + priv := make([]byte, ed25519.PrivateKeySize) + copy(pub, k[32:]) + copy(priv, k) + + // Marshal public key. + pubKey := struct { + KeyType string + Pub []byte + }{ + KeyAlgoED25519, pub, + } + w.PubKey = Marshal(pubKey) + + // Marshal private key. + key := openSSHEd25519PrivateKey{ + Pub: pub, + Priv: priv, + Comment: comment, + } + pk1.Keytype = KeyAlgoED25519 + pk1.Rest = Marshal(key) + case *ecdsa.PrivateKey: + var curve, keyType string + switch name := k.Curve.Params().Name; name { + case "P-256": + curve = "nistp256" + keyType = KeyAlgoECDSA256 + case "P-384": + curve = "nistp384" + keyType = KeyAlgoECDSA384 + case "P-521": + curve = "nistp521" + keyType = KeyAlgoECDSA521 + default: + return nil, errors.New("ssh: unhandled elliptic curve " + name) + } + + pub := elliptic.Marshal(k.Curve, k.PublicKey.X, k.PublicKey.Y) + + // Marshal public key. + pubKey := struct { + KeyType string + Curve string + Pub []byte + }{ + keyType, curve, pub, + } + w.PubKey = Marshal(pubKey) + + // Marshal private key. + key := openSSHECDSAPrivateKey{ + Curve: curve, + Pub: pub, + D: k.D, + Comment: comment, + } + pk1.Keytype = keyType + pk1.Rest = Marshal(key) + default: + return nil, fmt.Errorf("ssh: unsupported key type %T", k) + } + + var err error + // Add padding and encrypt the key if necessary. + w.PrivKeyBlock, w.CipherName, w.KdfName, w.KdfOpts, err = encrypt(Marshal(pk1)) + if err != nil { + return nil, err + } + + b := Marshal(w) + block := &pem.Block{ + Type: "OPENSSH PRIVATE KEY", + Bytes: append([]byte(privateKeyAuthMagic), b...), + } + return block, nil +} + func checkOpenSSHKeyPadding(pad []byte) error { for i, b := range pad { if int(b) != i+1 { @@ -1424,6 +1611,13 @@ func checkOpenSSHKeyPadding(pad []byte) error { return nil } +func generateOpenSSHPadding(block []byte, blockSize int) []byte { + for i, l := 0, len(block); (l+i)%blockSize != 0; i++ { + block = append(block, byte(i+1)) + } + return block +} + // FingerprintLegacyMD5 returns the user presentation of the key's // fingerprint as described by RFC 4716 section 4. func FingerprintLegacyMD5(pubKey PublicKey) string { diff --git a/ssh/keys_test.go b/ssh/keys_test.go index 334ef74532..a8e216ede8 100644 --- a/ssh/keys_test.go +++ b/ssh/keys_test.go @@ -281,6 +281,74 @@ func TestMarshalParsePublicKey(t *testing.T) { } } +func TestMarshalPrivateKey(t *testing.T) { + tests := []struct { + name string + }{ + {"rsa-openssh-format"}, + {"ed25519"}, + {"p256-openssh-format"}, + {"p384-openssh-format"}, + {"p521-openssh-format"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expected, ok := testPrivateKeys[tt.name] + if !ok { + t.Fatalf("cannot find key %s", tt.name) + } + + block, err := MarshalPrivateKey(expected, "test@golang.org") + if err != nil { + t.Fatalf("cannot marshal %s: %v", tt.name, err) + } + + key, err := ParseRawPrivateKey(pem.EncodeToMemory(block)) + if err != nil { + t.Fatalf("cannot parse %s: %v", tt.name, err) + } + + if !reflect.DeepEqual(expected, key) { + t.Errorf("unexpected marshaled key %s", tt.name) + } + }) + } +} + +func TestMarshalPrivateKeyWithPassphrase(t *testing.T) { + tests := []struct { + name string + }{ + {"rsa-openssh-format"}, + {"ed25519"}, + {"p256-openssh-format"}, + {"p384-openssh-format"}, + {"p521-openssh-format"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expected, ok := testPrivateKeys[tt.name] + if !ok { + t.Fatalf("cannot find key %s", tt.name) + } + + block, err := MarshalPrivateKeyWithPassphrase(expected, "test@golang.org", []byte("test-passphrase")) + if err != nil { + t.Fatalf("cannot marshal %s: %v", tt.name, err) + } + + key, err := ParseRawPrivateKeyWithPassphrase(pem.EncodeToMemory(block), []byte("test-passphrase")) + if err != nil { + t.Fatalf("cannot parse %s: %v", tt.name, err) + } + + if !reflect.DeepEqual(expected, key) { + t.Errorf("unexpected marshaled key %s", tt.name) + } + }) + } +} + type testAuthResult struct { pubKey PublicKey options []string From e90f1e17ee2ffe351a8295e8ae8b66afda2969c6 Mon Sep 17 00:00:00 2001 From: Daniel Mangum Date: Wed, 6 Sep 2023 19:20:46 +0000 Subject: [PATCH 03/10] cryptobyte: add uint48 methods Adds uint48 methods for cryptobyte.Builder and cryptobyte.String. Supporting 48-bit unsigned integers is useful for working with protocols that use them for sequence numbers, such as DTLS. Fixes golang/go#61275 Change-Id: Ibe49422d37644b9212b28b123dc5e01850f7b05b GitHub-Last-Rev: 11b388c240109c8f4ac23880645c901ce6d2f093 GitHub-Pull-Request: golang/crypto#265 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/508675 Reviewed-by: Roland Shoemaker Reviewed-by: Cherry Mui Reviewed-by: qiulaidongfeng <2645477756@qq.com> Run-TryBot: Roland Shoemaker Auto-Submit: Roland Shoemaker TryBot-Result: Gopher Robot --- cryptobyte/builder.go | 5 +++++ cryptobyte/cryptobyte_test.go | 21 +++++++++++++++++++++ cryptobyte/string.go | 11 +++++++++++ 3 files changed, 37 insertions(+) diff --git a/cryptobyte/builder.go b/cryptobyte/builder.go index c05ac7d16d..cf254f5f1e 100644 --- a/cryptobyte/builder.go +++ b/cryptobyte/builder.go @@ -95,6 +95,11 @@ func (b *Builder) AddUint32(v uint32) { b.add(byte(v>>24), byte(v>>16), byte(v>>8), byte(v)) } +// AddUint48 appends a big-endian, 48-bit value to the byte string. +func (b *Builder) AddUint48(v uint64) { + b.add(byte(v>>40), byte(v>>32), byte(v>>24), byte(v>>16), byte(v>>8), byte(v)) +} + // AddUint64 appends a big-endian, 64-bit value to the byte string. func (b *Builder) AddUint64(v uint64) { b.add(byte(v>>56), byte(v>>48), byte(v>>40), byte(v>>32), byte(v>>24), byte(v>>16), byte(v>>8), byte(v)) diff --git a/cryptobyte/cryptobyte_test.go b/cryptobyte/cryptobyte_test.go index dc90e81fa9..9b55d52e32 100644 --- a/cryptobyte/cryptobyte_test.go +++ b/cryptobyte/cryptobyte_test.go @@ -179,6 +179,27 @@ func TestUint32(t *testing.T) { } } +func TestUint48(t *testing.T) { + var b Builder + var u uint64 = 0xfefcff3cfdfc + b.AddUint48(u) + if err := builderBytesEq(&b, 254, 252, 255, 60, 253, 252); err != nil { + t.Error(err) + } + + var s String = b.BytesOrPanic() + var v uint64 + if !s.ReadUint48(&v) { + t.Error("ReadUint48() = false, want true") + } + if v != u { + t.Errorf("v = %x, want %x", v, u) + } + if len(s) != 0 { + t.Errorf("len(s) = %d, want 0", len(s)) + } +} + func TestUint64(t *testing.T) { var b Builder b.AddUint64(0xf2fefefcff3cfdfc) diff --git a/cryptobyte/string.go b/cryptobyte/string.go index 0531a3d6f1..10692a8a31 100644 --- a/cryptobyte/string.go +++ b/cryptobyte/string.go @@ -81,6 +81,17 @@ func (s *String) ReadUint32(out *uint32) bool { return true } +// ReadUint48 decodes a big-endian, 48-bit value into out and advances over it. +// It reports whether the read was successful. +func (s *String) ReadUint48(out *uint64) bool { + v := s.read(6) + if v == nil { + return false + } + *out = uint64(v[0])<<40 | uint64(v[1])<<32 | uint64(v[2])<<24 | uint64(v[3])<<16 | uint64(v[4])<<8 | uint64(v[5]) + return true +} + // ReadUint64 decodes a big-endian, 64-bit value into out and advances over it. // It reports whether the read was successful. func (s *String) ReadUint64(out *uint64) bool { From 3f0842a46434ea6f56bf6e684c2b83d90e9cff07 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Sat, 9 Sep 2023 18:06:46 -0700 Subject: [PATCH 04/10] sha3: have ShakeHash extend hash.Hash Package sha3 recommends the SHAKE functions for new uses, but this is currently somewhat inconvenient because ShakeHash does not implement hash.Hash. This is understandable, as SHAKE supports arbitrary-length outputs whereas hash.Hash only supports fixed-length outputs. But there's a natural fixed-length output to provide: the minimum output that still provides SHAKE's full-strength generic security. While here, tweak Sum so that its temporary buffer can be stack allocated. Also, tweak the panic message in Write so that the error text is more readily understandable to Go programmers without needing to be familiar with crypto jargon, and add a similar check in Sum. Change-Id: Icf037d3990a71de5630f8825606614443f8c5245 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/526937 LUCI-TryBot-Result: Go LUCI Reviewed-by: Adam Langley Auto-Submit: Matthew Dempsky --- sha3/sha3.go | 14 +++++++++----- sha3/sha3_s390x.go | 10 ++++++---- sha3/shake.go | 29 ++++++++++++++--------------- 3 files changed, 29 insertions(+), 24 deletions(-) diff --git a/sha3/sha3.go b/sha3/sha3.go index fa182beb40..4884d172a4 100644 --- a/sha3/sha3.go +++ b/sha3/sha3.go @@ -121,11 +121,11 @@ func (d *state) padAndPermute(dsbyte byte) { copyOut(d, d.buf) } -// Write absorbs more data into the hash's state. It produces an error -// if more data is written to the ShakeHash after writing +// Write absorbs more data into the hash's state. It panics if any +// output has already been read. func (d *state) Write(p []byte) (written int, err error) { if d.state != spongeAbsorbing { - panic("sha3: write to sponge after read") + panic("sha3: Write after Read") } if d.buf == nil { d.buf = d.storage.asBytes()[:0] @@ -182,12 +182,16 @@ func (d *state) Read(out []byte) (n int, err error) { } // Sum applies padding to the hash state and then squeezes out the desired -// number of output bytes. +// number of output bytes. It panics if any output has already been read. func (d *state) Sum(in []byte) []byte { + if d.state != spongeAbsorbing { + panic("sha3: Sum after Read") + } + // Make a copy of the original hash so that caller can keep writing // and summing. dup := d.clone() - hash := make([]byte, dup.outputLen) + hash := make([]byte, dup.outputLen, 64) // explicit cap to allow stack allocation dup.Read(hash) return append(in, hash...) } diff --git a/sha3/sha3_s390x.go b/sha3/sha3_s390x.go index 63a3edb4ce..ec26f147ff 100644 --- a/sha3/sha3_s390x.go +++ b/sha3/sha3_s390x.go @@ -49,7 +49,7 @@ type asmState struct { buf []byte // care must be taken to ensure cap(buf) is a multiple of rate rate int // equivalent to block size storage [3072]byte // underlying storage for buf - outputLen int // output length if fixed, 0 if not + outputLen int // output length for full security function code // KIMD/KLMD function code state spongeDirection // whether the sponge is absorbing or squeezing } @@ -72,8 +72,10 @@ func newAsmState(function code) *asmState { s.outputLen = 64 case shake_128: s.rate = 168 + s.outputLen = 32 case shake_256: s.rate = 136 + s.outputLen = 64 default: panic("sha3: unrecognized function code") } @@ -108,7 +110,7 @@ func (s *asmState) resetBuf() { // It never returns an error. func (s *asmState) Write(b []byte) (int, error) { if s.state != spongeAbsorbing { - panic("sha3: write to sponge after read") + panic("sha3: Write after Read") } length := len(b) for len(b) > 0 { @@ -192,8 +194,8 @@ func (s *asmState) Read(out []byte) (n int, err error) { // Sum appends the current hash to b and returns the resulting slice. // It does not change the underlying hash state. func (s *asmState) Sum(b []byte) []byte { - if s.outputLen == 0 { - panic("sha3: cannot call Sum on SHAKE functions") + if s.state != spongeAbsorbing { + panic("sha3: Sum after Read") } // Copy the state to preserve the original. diff --git a/sha3/shake.go b/sha3/shake.go index d7be2954ab..bb69984027 100644 --- a/sha3/shake.go +++ b/sha3/shake.go @@ -17,26 +17,25 @@ package sha3 import ( "encoding/binary" + "hash" "io" ) -// ShakeHash defines the interface to hash functions that -// support arbitrary-length output. +// ShakeHash defines the interface to hash functions that support +// arbitrary-length output. When used as a plain [hash.Hash], it +// produces minimum-length outputs that provide full-strength generic +// security. type ShakeHash interface { - // Write absorbs more data into the hash's state. It panics if input is - // written to it after output has been read from it. - io.Writer + hash.Hash // Read reads more output from the hash; reading affects the hash's // state. (ShakeHash.Read is thus very different from Hash.Sum) - // It never returns an error. + // It never returns an error, but subsequent calls to Write or Sum + // will panic. io.Reader // Clone returns a copy of the ShakeHash in its current state. Clone() ShakeHash - - // Reset resets the ShakeHash to its initial state. - Reset() } // cSHAKE specific context @@ -81,8 +80,8 @@ func leftEncode(value uint64) []byte { return b[i-1:] } -func newCShake(N, S []byte, rate int, dsbyte byte) ShakeHash { - c := cshakeState{state: &state{rate: rate, dsbyte: dsbyte}} +func newCShake(N, S []byte, rate, outputLen int, dsbyte byte) ShakeHash { + c := cshakeState{state: &state{rate: rate, outputLen: outputLen, dsbyte: dsbyte}} // leftEncode returns max 9 bytes c.initBlock = make([]byte, 0, 9*2+len(N)+len(S)) @@ -119,7 +118,7 @@ func NewShake128() ShakeHash { if h := newShake128Asm(); h != nil { return h } - return &state{rate: rate128, dsbyte: dsbyteShake} + return &state{rate: rate128, outputLen: 32, dsbyte: dsbyteShake} } // NewShake256 creates a new SHAKE256 variable-output-length ShakeHash. @@ -129,7 +128,7 @@ func NewShake256() ShakeHash { if h := newShake256Asm(); h != nil { return h } - return &state{rate: rate256, dsbyte: dsbyteShake} + return &state{rate: rate256, outputLen: 64, dsbyte: dsbyteShake} } // NewCShake128 creates a new instance of cSHAKE128 variable-output-length ShakeHash, @@ -142,7 +141,7 @@ func NewCShake128(N, S []byte) ShakeHash { if len(N) == 0 && len(S) == 0 { return NewShake128() } - return newCShake(N, S, rate128, dsbyteCShake) + return newCShake(N, S, rate128, 32, dsbyteCShake) } // NewCShake256 creates a new instance of cSHAKE256 variable-output-length ShakeHash, @@ -155,7 +154,7 @@ func NewCShake256(N, S []byte) ShakeHash { if len(N) == 0 && len(S) == 0 { return NewShake256() } - return newCShake(N, S, rate256, dsbyteCShake) + return newCShake(N, S, rate256, 64, dsbyteCShake) } // ShakeSum128 writes an arbitrary-length digest of data into hash. From 28c53ff63c09fc7df7793600caa30989bc69e194 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 8 Jul 2023 15:39:11 +0200 Subject: [PATCH 05/10] ssh: add MultiAlgorithmSigner MultiAlgorithmSigner allows to restrict client-side, server-side and certificate signing algorithms. Fixes golang/go#52132 Fixes golang/go#36261 Change-Id: I295092f1bba647327aaaf294f110e9157d294159 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/508398 Reviewed-by: Filippo Valsorda Run-TryBot: Filippo Valsorda Reviewed-by: Ian Lance Taylor Auto-Submit: Filippo Valsorda TryBot-Result: Gopher Robot Reviewed-by: Matthew Dempsky --- ssh/certs.go | 38 +++++++++++---- ssh/certs_test.go | 100 +++++++++++++++++++++++++++++++++++++--- ssh/client_auth.go | 96 ++++++++++++++++++++++++++------------ ssh/client_auth_test.go | 69 ++++++++++++++++++++++++++- ssh/example_test.go | 68 ++++++++++++++++++++++++++- ssh/handshake.go | 34 +++++++++----- ssh/handshake_test.go | 90 ++++++++++++++++++++++++++++++++++++ ssh/keys.go | 88 ++++++++++++++++++++++++++++++++--- ssh/keys_test.go | 37 +++++++++++++-- 9 files changed, 554 insertions(+), 66 deletions(-) diff --git a/ssh/certs.go b/ssh/certs.go index fc04d03e19..27d0e14aa9 100644 --- a/ssh/certs.go +++ b/ssh/certs.go @@ -16,8 +16,9 @@ import ( // Certificate algorithm names from [PROTOCOL.certkeys]. These values can appear // in Certificate.Type, PublicKey.Type, and ClientConfig.HostKeyAlgorithms. -// Unlike key algorithm names, these are not passed to AlgorithmSigner and don't -// appear in the Signature.Format field. +// Unlike key algorithm names, these are not passed to AlgorithmSigner nor +// returned by MultiAlgorithmSigner and don't appear in the Signature.Format +// field. const ( CertAlgoRSAv01 = "ssh-rsa-cert-v01@openssh.com" CertAlgoDSAv01 = "ssh-dss-cert-v01@openssh.com" @@ -255,10 +256,17 @@ func NewCertSigner(cert *Certificate, signer Signer) (Signer, error) { return nil, errors.New("ssh: signer and cert have different public key") } - if algorithmSigner, ok := signer.(AlgorithmSigner); ok { + switch s := signer.(type) { + case MultiAlgorithmSigner: + return &multiAlgorithmSigner{ + AlgorithmSigner: &algorithmOpenSSHCertSigner{ + &openSSHCertSigner{cert, signer}, s}, + supportedAlgorithms: s.Algorithms(), + }, nil + case AlgorithmSigner: return &algorithmOpenSSHCertSigner{ - &openSSHCertSigner{cert, signer}, algorithmSigner}, nil - } else { + &openSSHCertSigner{cert, signer}, s}, nil + default: return &openSSHCertSigner{cert, signer}, nil } } @@ -432,7 +440,9 @@ func (c *CertChecker) CheckCert(principal string, cert *Certificate) error { } // SignCert signs the certificate with an authority, setting the Nonce, -// SignatureKey, and Signature fields. +// SignatureKey, and Signature fields. If the authority implements the +// MultiAlgorithmSigner interface the first algorithm in the list is used. This +// is useful if you want to sign with a specific algorithm. func (c *Certificate) SignCert(rand io.Reader, authority Signer) error { c.Nonce = make([]byte, 32) if _, err := io.ReadFull(rand, c.Nonce); err != nil { @@ -440,8 +450,20 @@ func (c *Certificate) SignCert(rand io.Reader, authority Signer) error { } c.SignatureKey = authority.PublicKey() - // Default to KeyAlgoRSASHA512 for ssh-rsa signers. - if v, ok := authority.(AlgorithmSigner); ok && v.PublicKey().Type() == KeyAlgoRSA { + if v, ok := authority.(MultiAlgorithmSigner); ok { + if len(v.Algorithms()) == 0 { + return errors.New("the provided authority has no signature algorithm") + } + // Use the first algorithm in the list. + sig, err := v.SignWithAlgorithm(rand, c.bytesForSigning(), v.Algorithms()[0]) + if err != nil { + return err + } + c.Signature = sig + return nil + } else if v, ok := authority.(AlgorithmSigner); ok && v.PublicKey().Type() == KeyAlgoRSA { + // Default to KeyAlgoRSASHA512 for ssh-rsa signers. + // TODO: consider using KeyAlgoRSASHA256 as default. sig, err := v.SignWithAlgorithm(rand, c.bytesForSigning(), KeyAlgoRSASHA512) if err != nil { return err diff --git a/ssh/certs_test.go b/ssh/certs_test.go index e6004835ea..97b7486d0c 100644 --- a/ssh/certs_test.go +++ b/ssh/certs_test.go @@ -187,10 +187,30 @@ func TestHostKeyCert(t *testing.T) { } for _, test := range []struct { - addr string - succeed bool + addr string + succeed bool + certSignerAlgorithms []string // Empty means no algorithm restrictions. + clientHostKeyAlgorithms []string }{ {addr: "hostname:22", succeed: true}, + { + addr: "hostname:22", + succeed: true, + certSignerAlgorithms: []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}, + clientHostKeyAlgorithms: []string{CertAlgoRSASHA512v01}, + }, + { + addr: "hostname:22", + succeed: false, + certSignerAlgorithms: []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}, + clientHostKeyAlgorithms: []string{CertAlgoRSAv01}, + }, + { + addr: "hostname:22", + succeed: false, + certSignerAlgorithms: []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}, + clientHostKeyAlgorithms: []string{KeyAlgoRSASHA512}, // Not a certificate algorithm. + }, {addr: "otherhost:22", succeed: false}, // The certificate is valid for 'otherhost' as hostname, but we only recognize the authority of the signer for the address 'hostname:22' {addr: "lasthost:22", succeed: false}, } { @@ -207,14 +227,24 @@ func TestHostKeyCert(t *testing.T) { conf := ServerConfig{ NoClientAuth: true, } - conf.AddHostKey(certSigner) + if len(test.certSignerAlgorithms) > 0 { + mas, err := NewSignerWithAlgorithms(certSigner.(AlgorithmSigner), test.certSignerAlgorithms) + if err != nil { + errc <- err + return + } + conf.AddHostKey(mas) + } else { + conf.AddHostKey(certSigner) + } _, _, _, err := NewServerConn(c1, &conf) errc <- err }() config := &ClientConfig{ - User: "user", - HostKeyCallback: checker.CheckHostKey, + User: "user", + HostKeyCallback: checker.CheckHostKey, + HostKeyAlgorithms: test.clientHostKeyAlgorithms, } _, _, _, err = NewClientConn(c2, test.addr, config) @@ -242,6 +272,20 @@ func (s *legacyRSASigner) Sign(rand io.Reader, data []byte) (*Signature, error) } func TestCertTypes(t *testing.T) { + algorithmSigner, ok := testSigners["rsa"].(AlgorithmSigner) + if !ok { + t.Fatal("rsa test signer does not implement the AlgorithmSigner interface") + } + multiAlgoSignerSHA256, err := NewSignerWithAlgorithms(algorithmSigner, []string{KeyAlgoRSASHA256}) + if err != nil { + t.Fatalf("unable to create multi algorithm signer SHA256: %v", err) + } + // Algorithms are in order of preference, we expect rsa-sha2-512 to be used. + multiAlgoSignerSHA512, err := NewSignerWithAlgorithms(algorithmSigner, []string{KeyAlgoRSASHA512, KeyAlgoRSASHA256}) + if err != nil { + t.Fatalf("unable to create multi algorithm signer SHA512: %v", err) + } + var testVars = []struct { name string signer Signer @@ -251,8 +295,10 @@ func TestCertTypes(t *testing.T) { {CertAlgoECDSA384v01, testSigners["ecdsap384"], ""}, {CertAlgoECDSA521v01, testSigners["ecdsap521"], ""}, {CertAlgoED25519v01, testSigners["ed25519"], ""}, - {CertAlgoRSAv01, testSigners["rsa"], KeyAlgoRSASHA512}, + {CertAlgoRSAv01, testSigners["rsa"], KeyAlgoRSASHA256}, {"legacyRSASigner", &legacyRSASigner{testSigners["rsa"]}, KeyAlgoRSA}, + {"multiAlgoRSASignerSHA256", multiAlgoSignerSHA256, KeyAlgoRSASHA256}, + {"multiAlgoRSASignerSHA512", multiAlgoSignerSHA512, KeyAlgoRSASHA512}, {CertAlgoDSAv01, testSigners["dsa"], ""}, } @@ -318,3 +364,45 @@ func TestCertTypes(t *testing.T) { }) } } + +func TestCertSignWithMultiAlgorithmSigner(t *testing.T) { + type testcase struct { + sigAlgo string + algoritms []string + } + cases := []testcase{ + { + sigAlgo: KeyAlgoRSA, + algoritms: []string{KeyAlgoRSA, KeyAlgoRSASHA512}, + }, + { + sigAlgo: KeyAlgoRSASHA256, + algoritms: []string{KeyAlgoRSASHA256, KeyAlgoRSA, KeyAlgoRSASHA512}, + }, + { + sigAlgo: KeyAlgoRSASHA512, + algoritms: []string{KeyAlgoRSASHA512, KeyAlgoRSASHA256}, + }, + } + + cert := &Certificate{ + Key: testPublicKeys["rsa"], + ValidBefore: CertTimeInfinity, + CertType: UserCert, + } + + for _, c := range cases { + t.Run(c.sigAlgo, func(t *testing.T) { + signer, err := NewSignerWithAlgorithms(testSigners["rsa"].(AlgorithmSigner), c.algoritms) + if err != nil { + t.Fatalf("NewSignerWithAlgorithms error: %v", err) + } + if err := cert.SignCert(rand.Reader, signer); err != nil { + t.Fatalf("SignCert error: %v", err) + } + if cert.Signature.Format != c.sigAlgo { + t.Fatalf("got signature format %q, want %q", cert.Signature.Format, c.sigAlgo) + } + }) + } +} diff --git a/ssh/client_auth.go b/ssh/client_auth.go index 409b5ea1d4..5c3bc25723 100644 --- a/ssh/client_auth.go +++ b/ssh/client_auth.go @@ -71,7 +71,9 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error { for auth := AuthMethod(new(noneAuth)); auth != nil; { ok, methods, err := auth.auth(sessionID, config.User, c.transport, config.Rand, extensions) if err != nil { - return err + // We return the error later if there is no other method left to + // try. + ok = authFailure } if ok == authSuccess { // success @@ -101,6 +103,12 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error { } } } + + if auth == nil && err != nil { + // We have an error and there are no other authentication methods to + // try, so we return it. + return err + } } return fmt.Errorf("ssh: unable to authenticate, attempted methods %v, no supported methods remain", tried) } @@ -217,21 +225,45 @@ func (cb publicKeyCallback) method() string { return "publickey" } -func pickSignatureAlgorithm(signer Signer, extensions map[string][]byte) (as AlgorithmSigner, algo string) { +func pickSignatureAlgorithm(signer Signer, extensions map[string][]byte) (MultiAlgorithmSigner, string, error) { + var as MultiAlgorithmSigner keyFormat := signer.PublicKey().Type() - // Like in sendKexInit, if the public key implements AlgorithmSigner we - // assume it supports all algorithms, otherwise only the key format one. - as, ok := signer.(AlgorithmSigner) - if !ok { - return algorithmSignerWrapper{signer}, keyFormat + // If the signer implements MultiAlgorithmSigner we use the algorithms it + // support, if it implements AlgorithmSigner we assume it supports all + // algorithms, otherwise only the key format one. + switch s := signer.(type) { + case MultiAlgorithmSigner: + as = s + case AlgorithmSigner: + as = &multiAlgorithmSigner{ + AlgorithmSigner: s, + supportedAlgorithms: algorithmsForKeyFormat(underlyingAlgo(keyFormat)), + } + default: + as = &multiAlgorithmSigner{ + AlgorithmSigner: algorithmSignerWrapper{signer}, + supportedAlgorithms: []string{underlyingAlgo(keyFormat)}, + } + } + + getFallbackAlgo := func() (string, error) { + // Fallback to use if there is no "server-sig-algs" extension or a + // common algorithm cannot be found. We use the public key format if the + // MultiAlgorithmSigner supports it, otherwise we return an error. + if !contains(as.Algorithms(), underlyingAlgo(keyFormat)) { + return "", fmt.Errorf("ssh: no common public key signature algorithm, server only supports %q for key type %q, signer only supports %v", + underlyingAlgo(keyFormat), keyFormat, as.Algorithms()) + } + return keyFormat, nil } extPayload, ok := extensions["server-sig-algs"] if !ok { - // If there is no "server-sig-algs" extension, fall back to the key - // format algorithm. - return as, keyFormat + // If there is no "server-sig-algs" extension use the fallback + // algorithm. + algo, err := getFallbackAlgo() + return as, algo, err } // The server-sig-algs extension only carries underlying signature @@ -245,15 +277,22 @@ func pickSignatureAlgorithm(signer Signer, extensions map[string][]byte) (as Alg } } - keyAlgos := algorithmsForKeyFormat(keyFormat) + // Filter algorithms based on those supported by MultiAlgorithmSigner. + var keyAlgos []string + for _, algo := range algorithmsForKeyFormat(keyFormat) { + if contains(as.Algorithms(), underlyingAlgo(algo)) { + keyAlgos = append(keyAlgos, algo) + } + } + algo, err := findCommon("public key signature algorithm", keyAlgos, serverAlgos) if err != nil { - // If there is no overlap, try the key anyway with the key format - // algorithm, to support servers that fail to list all supported - // algorithms. - return as, keyFormat + // If there is no overlap, return the fallback algorithm to support + // servers that fail to list all supported algorithms. + algo, err := getFallbackAlgo() + return as, algo, err } - return as, algo + return as, algo, nil } func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader, extensions map[string][]byte) (authResult, []string, error) { @@ -267,10 +306,17 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand return authFailure, nil, err } var methods []string + var errSigAlgo error for _, signer := range signers { pub := signer.PublicKey() - as, algo := pickSignatureAlgorithm(signer, extensions) - + as, algo, err := pickSignatureAlgorithm(signer, extensions) + if err != nil && errSigAlgo == nil { + // If we cannot negotiate a signature algorithm store the first + // error so we can return it to provide a more meaningful message if + // no other signers work. + errSigAlgo = err + continue + } ok, err := validateKey(pub, algo, user, c) if err != nil { return authFailure, nil, err @@ -317,22 +363,12 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand // contain the "publickey" method, do not attempt to authenticate with any // other keys. According to RFC 4252 Section 7, the latter can occur when // additional authentication methods are required. - if success == authSuccess || !containsMethod(methods, cb.method()) { + if success == authSuccess || !contains(methods, cb.method()) { return success, methods, err } } - return authFailure, methods, nil -} - -func containsMethod(methods []string, method string) bool { - for _, m := range methods { - if m == method { - return true - } - } - - return false + return authFailure, methods, errSigAlgo } // validateKey validates the key provided is acceptable to the server. diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index 0d2d8d255c..16d4113389 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -1077,6 +1077,73 @@ func TestCompatibleAlgoAndSignatures(t *testing.T) { } } +func TestPickSignatureAlgorithm(t *testing.T) { + type testcase struct { + name string + extensions map[string][]byte + } + cases := []testcase{ + { + name: "server with empty server-sig-algs", + extensions: map[string][]byte{ + "server-sig-algs": []byte(``), + }, + }, + { + name: "server with no server-sig-algs", + extensions: nil, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + signer, ok := testSigners["rsa"].(MultiAlgorithmSigner) + if !ok { + t.Fatalf("rsa test signer does not implement the MultiAlgorithmSigner interface") + } + // The signer supports the public key algorithm which is then returned. + _, algo, err := pickSignatureAlgorithm(signer, c.extensions) + if err != nil { + t.Fatalf("got %v, want no error", err) + } + if algo != signer.PublicKey().Type() { + t.Fatalf("got algo %q, want %q", algo, signer.PublicKey().Type()) + } + // Test a signer that uses a certificate algorithm as the public key + // type. + cert := &Certificate{ + CertType: UserCert, + Key: signer.PublicKey(), + } + cert.SignCert(rand.Reader, signer) + + certSigner, err := NewCertSigner(cert, signer) + if err != nil { + t.Fatalf("error generating cert signer: %v", err) + } + // The signer supports the public key algorithm and the + // public key format is a certificate type so the cerificate + // algorithm matching the key format must be returned + _, algo, err = pickSignatureAlgorithm(certSigner, c.extensions) + if err != nil { + t.Fatalf("got %v, want no error", err) + } + if algo != certSigner.PublicKey().Type() { + t.Fatalf("got algo %q, want %q", algo, certSigner.PublicKey().Type()) + } + signer, err = NewSignerWithAlgorithms(signer.(AlgorithmSigner), []string{KeyAlgoRSASHA512, KeyAlgoRSASHA256}) + if err != nil { + t.Fatalf("unable to create signer with algorithms: %v", err) + } + // The signer does not support the public key algorithm so an error + // is returned. + _, _, err = pickSignatureAlgorithm(signer, c.extensions) + if err == nil { + t.Fatal("got no error, no common public key signature algorithm error expected") + } + }) + } +} + // configurablePublicKeyCallback is a public key callback that allows to // configure the signature algorithm and format. This way we can emulate the // behavior of buggy clients. @@ -1133,7 +1200,7 @@ func (cb configurablePublicKeyCallback) auth(session []byte, user string, c pack if err != nil { return authFailure, nil, err } - if success == authSuccess || !containsMethod(methods, cb.method()) { + if success == authSuccess || !contains(methods, cb.method()) { return success, methods, err } diff --git a/ssh/example_test.go b/ssh/example_test.go index bee679603b..0a6b0767c9 100644 --- a/ssh/example_test.go +++ b/ssh/example_test.go @@ -7,6 +7,8 @@ package ssh_test import ( "bufio" "bytes" + "crypto/rand" + "crypto/rsa" "fmt" "log" "net" @@ -75,7 +77,6 @@ func ExampleNewServerConn() { if err != nil { log.Fatal("Failed to parse private key: ", err) } - config.AddHostKey(private) // Once a ServerConfig has been configured, connections can be @@ -139,6 +140,36 @@ func ExampleNewServerConn() { } } +func ExampleServerConfig_AddHostKey() { + // Minimal ServerConfig supporting only password authentication. + config := &ssh.ServerConfig{ + PasswordCallback: func(c ssh.ConnMetadata, pass []byte) (*ssh.Permissions, error) { + // Should use constant-time compare (or better, salt+hash) in + // a production setting. + if c.User() == "testuser" && string(pass) == "tiger" { + return nil, nil + } + return nil, fmt.Errorf("password rejected for %q", c.User()) + }, + } + + privateBytes, err := os.ReadFile("id_rsa") + if err != nil { + log.Fatal("Failed to load private key: ", err) + } + + private, err := ssh.ParsePrivateKey(privateBytes) + if err != nil { + log.Fatal("Failed to parse private key: ", err) + } + // Restrict host key algorithms to disable ssh-rsa. + signer, err := ssh.NewSignerWithAlgorithms(private.(ssh.AlgorithmSigner), []string{ssh.KeyAlgoRSASHA256, ssh.KeyAlgoRSASHA512}) + if err != nil { + log.Fatal("Failed to create private key with restricted algorithms: ", err) + } + config.AddHostKey(signer) +} + func ExampleClientConfig_HostKeyCallback() { // Every client must provide a host key check. Here is a // simple-minded parse of OpenSSH's known_hosts file @@ -318,3 +349,38 @@ func ExampleSession_RequestPty() { log.Fatal("failed to start shell: ", err) } } + +func ExampleCertificate_SignCert() { + // Sign a certificate with a specific algorithm. + privateKey, err := rsa.GenerateKey(rand.Reader, 3072) + if err != nil { + log.Fatal("unable to generate RSA key: ", err) + } + publicKey, err := ssh.NewPublicKey(&privateKey.PublicKey) + if err != nil { + log.Fatal("unable to get RSA public key: ", err) + } + caKey, err := rsa.GenerateKey(rand.Reader, 3072) + if err != nil { + log.Fatal("unable to generate CA key: ", err) + } + signer, err := ssh.NewSignerFromKey(caKey) + if err != nil { + log.Fatal("unable to generate signer from key: ", err) + } + mas, err := ssh.NewSignerWithAlgorithms(signer.(ssh.AlgorithmSigner), []string{ssh.KeyAlgoRSASHA256}) + if err != nil { + log.Fatal("unable to create signer with algoritms: ", err) + } + certificate := ssh.Certificate{ + Key: publicKey, + CertType: ssh.UserCert, + } + if err := certificate.SignCert(rand.Reader, mas); err != nil { + log.Fatal("unable to sign certificate: ", err) + } + // Save the public key to a file and check that rsa-sha-256 is used for + // signing: + // ssh-keygen -L -f + fmt.Println(string(ssh.MarshalAuthorizedKey(&certificate))) +} diff --git a/ssh/handshake.go b/ssh/handshake.go index 07a1843e0a..b95b112dd8 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -461,19 +461,24 @@ func (t *handshakeTransport) sendKexInit() error { isServer := len(t.hostKeys) > 0 if isServer { for _, k := range t.hostKeys { - // If k is an AlgorithmSigner, presume it supports all signature algorithms - // associated with the key format. (Ideally AlgorithmSigner would have a - // method to advertise supported algorithms, but it doesn't. This means that - // adding support for a new algorithm is a breaking change, as we will - // immediately negotiate it even if existing implementations don't support - // it. If that ever happens, we'll have to figure something out.) - // If k is not an AlgorithmSigner, we can only assume it only supports the - // algorithms that matches the key format. (This means that Sign can't pick - // a different default.) + // If k is a MultiAlgorithmSigner, we restrict the signature + // algorithms. If k is a AlgorithmSigner, presume it supports all + // signature algorithms associated with the key format. If k is not + // an AlgorithmSigner, we can only assume it only supports the + // algorithms that matches the key format. (This means that Sign + // can't pick a different default). keyFormat := k.PublicKey().Type() - if _, ok := k.(AlgorithmSigner); ok { + + switch s := k.(type) { + case MultiAlgorithmSigner: + for _, algo := range algorithmsForKeyFormat(keyFormat) { + if contains(s.Algorithms(), underlyingAlgo(algo)) { + msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, algo) + } + } + case AlgorithmSigner: msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, algorithmsForKeyFormat(keyFormat)...) - } else { + default: msg.ServerHostKeyAlgos = append(msg.ServerHostKeyAlgos, keyFormat) } } @@ -685,9 +690,16 @@ func (a algorithmSignerWrapper) SignWithAlgorithm(rand io.Reader, data []byte, a func pickHostKey(hostKeys []Signer, algo string) AlgorithmSigner { for _, k := range hostKeys { + if s, ok := k.(MultiAlgorithmSigner); ok { + if !contains(s.Algorithms(), underlyingAlgo(algo)) { + continue + } + } + if algo == k.PublicKey().Type() { return algorithmSignerWrapper{k} } + k, ok := k.(AlgorithmSigner) if !ok { continue diff --git a/ssh/handshake_test.go b/ssh/handshake_test.go index 879143a6b5..65afc2059e 100644 --- a/ssh/handshake_test.go +++ b/ssh/handshake_test.go @@ -620,3 +620,93 @@ func TestNoSHA2Support(t *testing.T) { t.Fatal(err) } } + +func TestMultiAlgoSignerHandshake(t *testing.T) { + algorithmSigner, ok := testSigners["rsa"].(AlgorithmSigner) + if !ok { + t.Fatal("rsa test signer does not implement the AlgorithmSigner interface") + } + multiAlgoSigner, err := NewSignerWithAlgorithms(algorithmSigner, []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}) + if err != nil { + t.Fatalf("unable to create multi algorithm signer: %v", err) + } + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + serverConf := &ServerConfig{ + PasswordCallback: func(conn ConnMetadata, password []byte) (*Permissions, error) { + return &Permissions{}, nil + }, + } + serverConf.AddHostKey(multiAlgoSigner) + go NewServerConn(c1, serverConf) + + clientConf := &ClientConfig{ + User: "test", + Auth: []AuthMethod{Password("testpw")}, + HostKeyCallback: FixedHostKey(testSigners["rsa"].PublicKey()), + HostKeyAlgorithms: []string{KeyAlgoRSASHA512}, + } + + if _, _, _, err := NewClientConn(c2, "", clientConf); err != nil { + t.Fatal(err) + } +} + +func TestMultiAlgoSignerNoCommonHostKeyAlgo(t *testing.T) { + algorithmSigner, ok := testSigners["rsa"].(AlgorithmSigner) + if !ok { + t.Fatal("rsa test signer does not implement the AlgorithmSigner interface") + } + multiAlgoSigner, err := NewSignerWithAlgorithms(algorithmSigner, []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}) + if err != nil { + t.Fatalf("unable to create multi algorithm signer: %v", err) + } + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + // ssh-rsa is disabled server side + serverConf := &ServerConfig{ + PasswordCallback: func(conn ConnMetadata, password []byte) (*Permissions, error) { + return &Permissions{}, nil + }, + } + serverConf.AddHostKey(multiAlgoSigner) + go NewServerConn(c1, serverConf) + + // the client only supports ssh-rsa + clientConf := &ClientConfig{ + User: "test", + Auth: []AuthMethod{Password("testpw")}, + HostKeyCallback: FixedHostKey(testSigners["rsa"].PublicKey()), + HostKeyAlgorithms: []string{KeyAlgoRSA}, + } + + _, _, _, err = NewClientConn(c2, "", clientConf) + if err == nil { + t.Fatal("succeeded connecting with no common hostkey algorithm") + } +} + +func TestPickIncompatibleHostKeyAlgo(t *testing.T) { + algorithmSigner, ok := testSigners["rsa"].(AlgorithmSigner) + if !ok { + t.Fatal("rsa test signer does not implement the AlgorithmSigner interface") + } + multiAlgoSigner, err := NewSignerWithAlgorithms(algorithmSigner, []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}) + if err != nil { + t.Fatalf("unable to create multi algorithm signer: %v", err) + } + signer := pickHostKey([]Signer{multiAlgoSigner}, KeyAlgoRSA) + if signer != nil { + t.Fatal("incompatible signer returned") + } +} diff --git a/ssh/keys.go b/ssh/keys.go index 1bf28d87f6..bcaae50c72 100644 --- a/ssh/keys.go +++ b/ssh/keys.go @@ -335,7 +335,7 @@ type CryptoPublicKey interface { // A Signer can create signatures that verify against a public key. // -// Some Signers provided by this package also implement AlgorithmSigner. +// Some Signers provided by this package also implement MultiAlgorithmSigner. type Signer interface { // PublicKey returns the associated PublicKey. PublicKey() PublicKey @@ -350,9 +350,9 @@ type Signer interface { // An AlgorithmSigner is a Signer that also supports specifying an algorithm to // use for signing. // -// An AlgorithmSigner can't advertise the algorithms it supports, so it should -// be prepared to be invoked with every algorithm supported by the public key -// format. +// An AlgorithmSigner can't advertise the algorithms it supports, unless it also +// implements MultiAlgorithmSigner, so it should be prepared to be invoked with +// every algorithm supported by the public key format. type AlgorithmSigner interface { Signer @@ -363,6 +363,75 @@ type AlgorithmSigner interface { SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error) } +// MultiAlgorithmSigner is an AlgorithmSigner that also reports the algorithms +// supported by that signer. +type MultiAlgorithmSigner interface { + AlgorithmSigner + + // Algorithms returns the available algorithms in preference order. The list + // must not be empty, and it must not include certificate types. + Algorithms() []string +} + +// NewSignerWithAlgorithms returns a signer restricted to the specified +// algorithms. The algorithms must be set in preference order. The list must not +// be empty, and it must not include certificate types. An error is returned if +// the specified algorithms are incompatible with the public key type. +func NewSignerWithAlgorithms(signer AlgorithmSigner, algorithms []string) (MultiAlgorithmSigner, error) { + if len(algorithms) == 0 { + return nil, errors.New("ssh: please specify at least one valid signing algorithm") + } + var signerAlgos []string + supportedAlgos := algorithmsForKeyFormat(underlyingAlgo(signer.PublicKey().Type())) + if s, ok := signer.(*multiAlgorithmSigner); ok { + signerAlgos = s.Algorithms() + } else { + signerAlgos = supportedAlgos + } + + for _, algo := range algorithms { + if !contains(supportedAlgos, algo) { + return nil, fmt.Errorf("ssh: algorithm %q is not supported for key type %q", + algo, signer.PublicKey().Type()) + } + if !contains(signerAlgos, algo) { + return nil, fmt.Errorf("ssh: algorithm %q is restricted for the provided signer", algo) + } + } + return &multiAlgorithmSigner{ + AlgorithmSigner: signer, + supportedAlgorithms: algorithms, + }, nil +} + +type multiAlgorithmSigner struct { + AlgorithmSigner + supportedAlgorithms []string +} + +func (s *multiAlgorithmSigner) Algorithms() []string { + return s.supportedAlgorithms +} + +func (s *multiAlgorithmSigner) isAlgorithmSupported(algorithm string) bool { + if algorithm == "" { + algorithm = underlyingAlgo(s.PublicKey().Type()) + } + for _, algo := range s.supportedAlgorithms { + if algorithm == algo { + return true + } + } + return false +} + +func (s *multiAlgorithmSigner) SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error) { + if !s.isAlgorithmSupported(algorithm) { + return nil, fmt.Errorf("ssh: algorithm %q is not supported: %v", algorithm, s.supportedAlgorithms) + } + return s.AlgorithmSigner.SignWithAlgorithm(rand, data, algorithm) +} + type rsaPublicKey rsa.PublicKey func (r *rsaPublicKey) Type() string { @@ -526,6 +595,10 @@ func (k *dsaPrivateKey) Sign(rand io.Reader, data []byte) (*Signature, error) { return k.SignWithAlgorithm(rand, data, k.PublicKey().Type()) } +func (k *dsaPrivateKey) Algorithms() []string { + return []string{k.PublicKey().Type()} +} + func (k *dsaPrivateKey) SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error) { if algorithm != "" && algorithm != k.PublicKey().Type() { return nil, fmt.Errorf("ssh: unsupported signature algorithm %s", algorithm) @@ -975,13 +1048,16 @@ func (s *wrappedSigner) Sign(rand io.Reader, data []byte) (*Signature, error) { return s.SignWithAlgorithm(rand, data, s.pubKey.Type()) } +func (s *wrappedSigner) Algorithms() []string { + return algorithmsForKeyFormat(s.pubKey.Type()) +} + func (s *wrappedSigner) SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error) { if algorithm == "" { algorithm = s.pubKey.Type() } - supportedAlgos := algorithmsForKeyFormat(s.pubKey.Type()) - if !contains(supportedAlgos, algorithm) { + if !contains(s.Algorithms(), algorithm) { return nil, fmt.Errorf("ssh: unsupported signature algorithm %q for key format %q", algorithm, s.pubKey.Type()) } diff --git a/ssh/keys_test.go b/ssh/keys_test.go index a8e216ede8..365d93db99 100644 --- a/ssh/keys_test.go +++ b/ssh/keys_test.go @@ -111,9 +111,9 @@ func TestKeySignVerify(t *testing.T) { } func TestKeySignWithAlgorithmVerify(t *testing.T) { - for _, priv := range testSigners { - if algorithmSigner, ok := priv.(AlgorithmSigner); !ok { - t.Errorf("Signers constructed by ssh package should always implement the AlgorithmSigner interface: %T", priv) + for k, priv := range testSigners { + if algorithmSigner, ok := priv.(MultiAlgorithmSigner); !ok { + t.Errorf("Signers %q constructed by ssh package should always implement the MultiAlgorithmSigner interface: %T", k, priv) } else { pub := priv.PublicKey() data := []byte("sign me") @@ -684,3 +684,34 @@ func TestSKKeys(t *testing.T) { } } } + +func TestNewSignerWithAlgos(t *testing.T) { + algorithSigner, ok := testSigners["rsa"].(AlgorithmSigner) + if !ok { + t.Fatal("rsa test signer does not implement the AlgorithmSigner interface") + } + _, err := NewSignerWithAlgorithms(algorithSigner, nil) + if err == nil { + t.Error("signer with algos created with no algorithms") + } + + _, err = NewSignerWithAlgorithms(algorithSigner, []string{KeyAlgoED25519}) + if err == nil { + t.Error("signer with algos created with invalid algorithms") + } + + _, err = NewSignerWithAlgorithms(algorithSigner, []string{CertAlgoRSASHA256v01}) + if err == nil { + t.Error("signer with algos created with certificate algorithms") + } + + mas, err := NewSignerWithAlgorithms(algorithSigner, []string{KeyAlgoRSASHA256, KeyAlgoRSASHA512}) + if err != nil { + t.Errorf("unable to create signer with valid algorithms: %v", err) + } + + _, err = NewSignerWithAlgorithms(mas, []string{KeyAlgoRSA}) + if err == nil { + t.Error("signer with algos created with restricted algorithms") + } +} From a1aeb9b34eb6b8f469bbd66b9cd1c9d905cb3714 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Tue, 5 Sep 2023 19:47:13 +0200 Subject: [PATCH 06/10] ssh: add test cases for compatibility with old (buggy) clients Improved test cases for CL 506835. Change-Id: If4a98ae4a7b39d2e59b203d10080b71283e1a80e Reviewed-on: https://go-review.googlesource.com/c/crypto/+/525735 TryBot-Result: Gopher Robot Reviewed-by: Matthew Dempsky Run-TryBot: Filippo Valsorda Reviewed-by: Ian Lance Taylor Reviewed-by: Filippo Valsorda Auto-Submit: Filippo Valsorda --- ssh/client_auth_test.go | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index 16d4113389..bf0aa1fe23 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -1234,3 +1234,51 @@ func TestPublicKeyAndAlgoCompatibility(t *testing.T) { t.Error("cert login passed with incompatible public key type and algorithm") } } + +func TestClientAuthGPGAgentCompat(t *testing.T) { + clientConfig := &ClientConfig{ + User: "testuser", + HostKeyCallback: InsecureIgnoreHostKey(), + Auth: []AuthMethod{ + // algorithm rsa-sha2-512 and signature format ssh-rsa. + configurablePublicKeyCallback{ + signer: testSigners["rsa"].(AlgorithmSigner), + signatureAlgo: KeyAlgoRSASHA512, + signatureFormat: KeyAlgoRSA, + }, + }, + } + if err := tryAuth(t, clientConfig); err != nil { + t.Fatalf("unable to dial remote side: %s", err) + } +} + +func TestCertAuthOpenSSHCompat(t *testing.T) { + cert := &Certificate{ + Key: testPublicKeys["rsa"], + ValidBefore: CertTimeInfinity, + CertType: UserCert, + } + cert.SignCert(rand.Reader, testSigners["ecdsa"]) + certSigner, err := NewCertSigner(cert, testSigners["rsa"]) + if err != nil { + t.Fatalf("NewCertSigner: %v", err) + } + + clientConfig := &ClientConfig{ + User: "user", + HostKeyCallback: InsecureIgnoreHostKey(), + Auth: []AuthMethod{ + // algorithm ssh-rsa-cert-v01@openssh.com and signature format + // rsa-sha2-256. + configurablePublicKeyCallback{ + signer: certSigner.(AlgorithmSigner), + signatureAlgo: CertAlgoRSAv01, + signatureFormat: KeyAlgoRSASHA256, + }, + }, + } + if err := tryAuth(t, clientConfig); err != nil { + t.Fatalf("unable to dial remote side: %s", err) + } +} From b665ba6f6f9abc2ab124f8ec2cb44de0e44ce077 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 25 Sep 2023 17:15:31 +0000 Subject: [PATCH 07/10] all: use crypto/ed25519 instead of golang.org/x/crypto/ed25519 This is a follow-up to CL 317169, which dropped go1.12 compatibility, and made the golang.org/x/crypto/ed25519 package an alias / wrapper for crypto/ed25519 in stdlib. This patch updates uses within this repository to use stdlib instead of depending on the wrapper. With this patch applied, the only remaining use of the wrapper is in ed25519_test, which appears to be in place to verify compatibility of the wrapper itself. Change-Id: I0195396102a75ae20bdd82ca8ab59855c0eb5cea GitHub-Last-Rev: 24dbec563cbd84bc47bdc7736b0245fc83dd3353 GitHub-Pull-Request: golang/crypto#238 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/448238 Reviewed-by: Bryan Mills Run-TryBot: Nicola Murino Reviewed-by: Nicola Murino TryBot-Result: Gopher Robot Reviewed-by: Joedian Reid Reviewed-by: Roland Shoemaker Reviewed-by: Than McIntosh --- internal/wycheproof/eddsa_test.go | 3 +-- nacl/sign/sign.go | 2 +- ssh/agent/client.go | 2 +- ssh/agent/server.go | 2 +- ssh/keys.go | 2 +- ssh/keys_test.go | 2 +- 6 files changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/wycheproof/eddsa_test.go b/internal/wycheproof/eddsa_test.go index 0a7fbb7e0e..118dfc043f 100644 --- a/internal/wycheproof/eddsa_test.go +++ b/internal/wycheproof/eddsa_test.go @@ -8,9 +8,8 @@ package wycheproof import ( + "crypto/ed25519" "testing" - - "golang.org/x/crypto/ed25519" ) func TestEddsa(t *testing.T) { diff --git a/nacl/sign/sign.go b/nacl/sign/sign.go index 8a6acdcc09..109c08bb95 100644 --- a/nacl/sign/sign.go +++ b/nacl/sign/sign.go @@ -21,9 +21,9 @@ package sign import ( + "crypto/ed25519" "io" - "golang.org/x/crypto/ed25519" "golang.org/x/crypto/internal/alias" ) diff --git a/ssh/agent/client.go b/ssh/agent/client.go index c3e112a939..9f09aae7dd 100644 --- a/ssh/agent/client.go +++ b/ssh/agent/client.go @@ -16,6 +16,7 @@ import ( "bytes" "crypto/dsa" "crypto/ecdsa" + "crypto/ed25519" "crypto/elliptic" "crypto/rsa" "encoding/base64" @@ -26,7 +27,6 @@ import ( "math/big" "sync" - "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh" ) diff --git a/ssh/agent/server.go b/ssh/agent/server.go index 9a769de03d..dd2e0a3e71 100644 --- a/ssh/agent/server.go +++ b/ssh/agent/server.go @@ -7,6 +7,7 @@ package agent import ( "crypto/dsa" "crypto/ecdsa" + "crypto/ed25519" "crypto/elliptic" "crypto/rsa" "encoding/binary" @@ -16,7 +17,6 @@ import ( "log" "math/big" - "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh" ) diff --git a/ssh/keys.go b/ssh/keys.go index bcaae50c72..ef1bad731b 100644 --- a/ssh/keys.go +++ b/ssh/keys.go @@ -11,6 +11,7 @@ import ( "crypto/cipher" "crypto/dsa" "crypto/ecdsa" + "crypto/ed25519" "crypto/elliptic" "crypto/md5" "crypto/rand" @@ -28,7 +29,6 @@ import ( "math/big" "strings" - "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh/internal/bcrypt_pbkdf" ) diff --git a/ssh/keys_test.go b/ssh/keys_test.go index 365d93db99..76d2338f43 100644 --- a/ssh/keys_test.go +++ b/ssh/keys_test.go @@ -8,6 +8,7 @@ import ( "bytes" "crypto/dsa" "crypto/ecdsa" + "crypto/ed25519" "crypto/elliptic" "crypto/rand" "crypto/rsa" @@ -21,7 +22,6 @@ import ( "strings" "testing" - "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh/testdata" ) From ec07f4e35b9ef2fcfd790dd9437ef46bd64b3a4b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 6 Nov 2022 16:20:46 +0000 Subject: [PATCH 08/10] chacha20: drop Go 1.10 compatibility for arm64 Other packages already dropped compatibility with go < 1.12, so it should be safe to remove it for this package as well. Change-Id: Ib1424763e3aa94d0187a667ebee058100136f53b GitHub-Last-Rev: 51df9690a5f37ba50d5ae5e84cf31b78fb6c5cd8 GitHub-Pull-Request: golang/crypto#241 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/448241 Reviewed-by: Roland Shoemaker Auto-Submit: Filippo Valsorda Reviewed-by: Dmitri Shuralyov Reviewed-by: Than McIntosh TryBot-Result: Gopher Robot Run-TryBot: Roland Shoemaker Reviewed-by: Filippo Valsorda Reviewed-by: Joedian Reid --- chacha20/chacha_arm64.go | 4 ++-- chacha20/chacha_arm64.s | 4 ++-- chacha20/chacha_noasm.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/chacha20/chacha_arm64.go b/chacha20/chacha_arm64.go index 94c71ac1ac..5dfacbb983 100644 --- a/chacha20/chacha_arm64.go +++ b/chacha20/chacha_arm64.go @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.11 && gc && !purego -// +build go1.11,gc,!purego +//go:build gc && !purego +// +build gc,!purego package chacha20 diff --git a/chacha20/chacha_arm64.s b/chacha20/chacha_arm64.s index 63cae9e6f0..f1f66230d1 100644 --- a/chacha20/chacha_arm64.s +++ b/chacha20/chacha_arm64.s @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.11 && gc && !purego -// +build go1.11,gc,!purego +//go:build gc && !purego +// +build gc,!purego #include "textflag.h" diff --git a/chacha20/chacha_noasm.go b/chacha20/chacha_noasm.go index 025b49897e..02ff3d05e9 100644 --- a/chacha20/chacha_noasm.go +++ b/chacha20/chacha_noasm.go @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build (!arm64 && !s390x && !ppc64le) || (arm64 && !go1.11) || !gc || purego -// +build !arm64,!s390x,!ppc64le arm64,!go1.11 !gc purego +//go:build (!arm64 && !s390x && !ppc64le) || !gc || purego +// +build !arm64,!s390x,!ppc64le !gc purego package chacha20 From 833695f0a57b3037385dc9c0073bc88773cae6f3 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Thu, 31 Aug 2023 14:41:07 +0200 Subject: [PATCH 09/10] ssh: add server side support for ping@openssh.com protocol extension Fixes golang/go#62390 Change-Id: Ie4dc577fb55b45a0c26a9e2dc5903af2bd382e00 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/524775 TryBot-Result: Gopher Robot Reviewed-by: Matthew Dempsky Reviewed-by: Than McIntosh Run-TryBot: Nicola Murino Reviewed-by: Filippo Valsorda --- ssh/doc.go | 1 + ssh/handshake.go | 10 +++++++--- ssh/messages.go | 14 ++++++++++++++ ssh/mux.go | 6 ++++++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/ssh/doc.go b/ssh/doc.go index f6bff60dc7..edbe63340d 100644 --- a/ssh/doc.go +++ b/ssh/doc.go @@ -13,6 +13,7 @@ others. References: + [PROTOCOL]: https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL?rev=HEAD [PROTOCOL.certkeys]: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD [SSH-PARAMETERS]: http://www.iana.org/assignments/ssh-parameters/ssh-parameters.xml#ssh-parameters-1 diff --git a/ssh/handshake.go b/ssh/handshake.go index b95b112dd8..70a7369ff9 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -647,16 +647,20 @@ func (t *handshakeTransport) enterKeyExchange(otherInitPacket []byte) error { // On the server side, after the first SSH_MSG_NEWKEYS, send a SSH_MSG_EXT_INFO // message with the server-sig-algs extension if the client supports it. See - // RFC 8308, Sections 2.4 and 3.1. + // RFC 8308, Sections 2.4 and 3.1, and [PROTOCOL], Section 1.9. if !isClient && firstKeyExchange && contains(clientInit.KexAlgos, "ext-info-c") { extInfo := &extInfoMsg{ - NumExtensions: 1, - Payload: make([]byte, 0, 4+15+4+len(supportedPubKeyAuthAlgosList)), + NumExtensions: 2, + Payload: make([]byte, 0, 4+15+4+len(supportedPubKeyAuthAlgosList)+4+16+4+1), } extInfo.Payload = appendInt(extInfo.Payload, len("server-sig-algs")) extInfo.Payload = append(extInfo.Payload, "server-sig-algs"...) extInfo.Payload = appendInt(extInfo.Payload, len(supportedPubKeyAuthAlgosList)) extInfo.Payload = append(extInfo.Payload, supportedPubKeyAuthAlgosList...) + extInfo.Payload = appendInt(extInfo.Payload, len("ping@openssh.com")) + extInfo.Payload = append(extInfo.Payload, "ping@openssh.com"...) + extInfo.Payload = appendInt(extInfo.Payload, 1) + extInfo.Payload = append(extInfo.Payload, "0"...) if err := t.conn.writePacket(Marshal(extInfo)); err != nil { return err } diff --git a/ssh/messages.go b/ssh/messages.go index 922032d952..b55f860564 100644 --- a/ssh/messages.go +++ b/ssh/messages.go @@ -349,6 +349,20 @@ type userAuthGSSAPIError struct { LanguageTag string } +// Transport layer OpenSSH extension. See [PROTOCOL], section 1.9 +const msgPing = 192 + +type pingMsg struct { + Data string `sshtype:"192"` +} + +// Transport layer OpenSSH extension. See [PROTOCOL], section 1.9 +const msgPong = 193 + +type pongMsg struct { + Data string `sshtype:"193"` +} + // typeTags returns the possible type bytes for the given reflect.Type, which // should be a struct. The possible values are separated by a '|' character. func typeTags(structType reflect.Type) (tags []byte) { diff --git a/ssh/mux.go b/ssh/mux.go index 9654c01869..d2d24c635d 100644 --- a/ssh/mux.go +++ b/ssh/mux.go @@ -231,6 +231,12 @@ func (m *mux) onePacket() error { return m.handleChannelOpen(packet) case msgGlobalRequest, msgRequestSuccess, msgRequestFailure: return m.handleGlobalPacket(packet) + case msgPing: + var msg pingMsg + if err := Unmarshal(packet, &msg); err != nil { + return fmt.Errorf("failed to unmarshal ping@openssh.com message: %w", err) + } + return m.sendMessage(pongMsg(msg)) } // assume a channel packet. From e3cc52e598e302f8c613a645bb7231264d8ec995 Mon Sep 17 00:00:00 2001 From: Gopher Robot Date: Thu, 5 Oct 2023 15:04:52 +0000 Subject: [PATCH 10/10] go.mod: update golang.org/x dependencies Update golang.org/x dependencies to their latest tagged versions. Change-Id: Ib80d50bdd762d1ba04f9267aeddc17272ef8cd66 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/532976 LUCI-TryBot-Result: Go LUCI Reviewed-by: Carlos Amedee Reviewed-by: Dmitri Shuralyov Auto-Submit: Gopher Robot --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index a25e214230..97cbcb6929 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +4,8 @@ go 1.17 require ( golang.org/x/net v0.10.0 // tagx:ignore - golang.org/x/sys v0.12.0 - golang.org/x/term v0.12.0 + golang.org/x/sys v0.13.0 + golang.org/x/term v0.13.0 ) require golang.org/x/text v0.13.0 // indirect diff --git a/go.sum b/go.sum index e2faf11c7a..f46a99b380 100644 --- a/go.sum +++ b/go.sum @@ -19,14 +19,14 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.12.0 h1:CM0HF96J0hcLAwsHPJZjfdNzs0gftsLfgKt57wWHJ0o= -golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.13.0 h1:Af8nKPmuFypiUBjVoU9V20FiaFXOcuZI21p0ycVYYGE= +golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= -golang.org/x/term v0.12.0 h1:/ZfYdc3zq+q02Rv9vGqTeSItdzZTSNDmfTi0mBAuidU= -golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= +golang.org/x/term v0.13.0 h1:bb+I9cTfFazGW51MZqBVmZy7+JEJMouUHTUSKVQLBek= +golang.org/x/term v0.13.0/go.mod h1:LTmsnFJwVN6bCy1rVCoS+qHT1HhALEFxKncY3WNNh4U= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=