diff --git a/ssh/agent/client.go b/ssh/agent/client.go index b909471cc0..3cfe723d29 100644 --- a/ssh/agent/client.go +++ b/ssh/agent/client.go @@ -25,7 +25,6 @@ import ( "math/big" "sync" - "crypto" "golang.org/x/crypto/ed25519" "golang.org/x/crypto/ssh" ) @@ -771,19 +770,26 @@ func (s *agentKeyringSigner) Sign(rand io.Reader, data []byte) (*ssh.Signature, return s.agent.Sign(s.pub, data) } -func (s *agentKeyringSigner) SignWithOpts(rand io.Reader, data []byte, opts crypto.SignerOpts) (*ssh.Signature, error) { +func (s *agentKeyringSigner) SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*ssh.Signature, error) { + if algorithm == "" || algorithm == s.pub.Type() { + return s.Sign(rand, data) + } + var flags SignatureFlags - if opts != nil { - switch opts.HashFunc() { - case crypto.SHA256: - flags = SignatureFlagRsaSha256 - case crypto.SHA512: - flags = SignatureFlagRsaSha512 - } + switch algorithm { + case ssh.KeyAlgoRSASHA256: + flags = SignatureFlagRsaSha256 + case ssh.KeyAlgoRSASHA512: + flags = SignatureFlagRsaSha512 + default: + return nil, fmt.Errorf("agent: unsupported algorithm %q", algorithm) } + return s.agent.SignWithFlags(s.pub, data, flags) } +var _ ssh.AlgorithmSigner = &agentKeyringSigner{} + // Calls an extension method. It is up to the agent implementation as to whether or not // any particular extension is supported and may always return an error. Because the // type of the response is up to the implementation, this returns the bytes of the diff --git a/ssh/client_auth.go b/ssh/client_auth.go index affc01da23..a962a679a5 100644 --- a/ssh/client_auth.go +++ b/ssh/client_auth.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "strings" ) type authResult int @@ -29,6 +30,33 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error { if err != nil { return err } + // The server may choose to send a SSH_MSG_EXT_INFO at this point (if we + // advertised willingness to receive one, which we always do) or not. See + // RFC 8308, Section 2.4. + extensions := make(map[string][]byte) + if len(packet) > 0 && packet[0] == msgExtInfo { + var extInfo extInfoMsg + if err := Unmarshal(packet, &extInfo); err != nil { + return err + } + payload := extInfo.Payload + for i := uint32(0); i < extInfo.NumExtensions; i++ { + name, rest, ok := parseString(payload) + if !ok { + return parseError(msgExtInfo) + } + value, rest, ok := parseString(rest) + if !ok { + return parseError(msgExtInfo) + } + extensions[string(name)] = value + payload = rest + } + packet, err = c.transport.readPacket() + if err != nil { + return err + } + } var serviceAccept serviceAcceptMsg if err := Unmarshal(packet, &serviceAccept); err != nil { return err @@ -41,7 +69,7 @@ func (c *connection) clientAuthenticate(config *ClientConfig) error { sessionID := c.transport.getSessionID() for auth := AuthMethod(new(noneAuth)); auth != nil; { - ok, methods, err := auth.auth(sessionID, config.User, c.transport, config.Rand) + ok, methods, err := auth.auth(sessionID, config.User, c.transport, config.Rand, extensions) if err != nil { return err } @@ -93,7 +121,7 @@ type AuthMethod interface { // If authentication is not successful, a []string of alternative // method names is returned. If the slice is nil, it will be ignored // and the previous set of possible methods will be reused. - auth(session []byte, user string, p packetConn, rand io.Reader) (authResult, []string, error) + auth(session []byte, user string, p packetConn, rand io.Reader, extensions map[string][]byte) (authResult, []string, error) // method returns the RFC 4252 method name. method() string @@ -102,7 +130,7 @@ type AuthMethod interface { // "none" authentication, RFC 4252 section 5.2. type noneAuth int -func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) { +func (n *noneAuth) auth(session []byte, user string, c packetConn, rand io.Reader, _ map[string][]byte) (authResult, []string, error) { if err := c.writePacket(Marshal(&userAuthRequestMsg{ User: user, Service: serviceSSH, @@ -122,7 +150,7 @@ func (n *noneAuth) method() string { // a function call, e.g. by prompting the user. type passwordCallback func() (password string, err error) -func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) { +func (cb passwordCallback) auth(session []byte, user string, c packetConn, rand io.Reader, _ map[string][]byte) (authResult, []string, error) { type passwordAuthMsg struct { User string `sshtype:"50"` Service string @@ -189,7 +217,36 @@ func (cb publicKeyCallback) method() string { return "publickey" } -func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) { +func pickSignatureAlgorithm(signer Signer, extensions map[string][]byte) (as AlgorithmSigner, algo string) { + 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 + } + + 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 + } + + serverAlgos := strings.Split(string(extPayload), ",") + keyAlgos := algorithmsForKeyFormat(keyFormat) + 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 + } + return as, algo +} + +func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand io.Reader, extensions map[string][]byte) (authResult, []string, error) { // Authentication is performed by sending an enquiry to test if a key is // acceptable to the remote. If the key is acceptable, the client will // attempt to authenticate with the valid key. If not the client will repeat @@ -201,7 +258,10 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand } var methods []string for _, signer := range signers { - ok, err := validateKey(signer.PublicKey(), user, c) + pub := signer.PublicKey() + as, algo := pickSignatureAlgorithm(signer, extensions) + + ok, err := validateKey(pub, algo, user, c) if err != nil { return authFailure, nil, err } @@ -209,13 +269,13 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand continue } - pub := signer.PublicKey() pubKey := pub.Marshal() - sign, err := signer.Sign(rand, buildDataSignedForAuth(session, userAuthRequestMsg{ + data := buildDataSignedForAuth(session, userAuthRequestMsg{ User: user, Service: serviceSSH, Method: cb.method(), - }, []byte(pub.Type()), pubKey)) + }, algo, pubKey) + sign, err := as.SignWithAlgorithm(rand, data, underlyingAlgo(algo)) if err != nil { return authFailure, nil, err } @@ -229,7 +289,7 @@ func (cb publicKeyCallback) auth(session []byte, user string, c packetConn, rand Service: serviceSSH, Method: cb.method(), HasSig: true, - Algoname: pub.Type(), + Algoname: algo, PubKey: pubKey, Sig: sig, } @@ -266,26 +326,25 @@ func containsMethod(methods []string, method string) bool { } // validateKey validates the key provided is acceptable to the server. -func validateKey(key PublicKey, user string, c packetConn) (bool, error) { +func validateKey(key PublicKey, algo string, user string, c packetConn) (bool, error) { pubKey := key.Marshal() msg := publickeyAuthMsg{ User: user, Service: serviceSSH, Method: "publickey", HasSig: false, - Algoname: key.Type(), + Algoname: algo, PubKey: pubKey, } if err := c.writePacket(Marshal(&msg)); err != nil { return false, err } - return confirmKeyAck(key, c) + return confirmKeyAck(key, algo, c) } -func confirmKeyAck(key PublicKey, c packetConn) (bool, error) { +func confirmKeyAck(key PublicKey, algo string, c packetConn) (bool, error) { pubKey := key.Marshal() - algoname := key.Type() for { packet, err := c.readPacket() @@ -302,14 +361,14 @@ func confirmKeyAck(key PublicKey, c packetConn) (bool, error) { if err := Unmarshal(packet, &msg); err != nil { return false, err } - if msg.Algo != algoname || !bytes.Equal(msg.PubKey, pubKey) { + if msg.Algo != algo || !bytes.Equal(msg.PubKey, pubKey) { return false, nil } return true, nil case msgUserAuthFailure: return false, nil default: - return false, unexpectedMessageError(msgUserAuthSuccess, packet[0]) + return false, unexpectedMessageError(msgUserAuthPubKeyOk, packet[0]) } } } @@ -330,6 +389,7 @@ func PublicKeysCallback(getSigners func() (signers []Signer, err error)) AuthMet // along with a list of remaining authentication methods to try next and // an error if an unexpected response was received. func handleAuthResponse(c packetConn) (authResult, []string, error) { + gotMsgExtInfo := false for { packet, err := c.readPacket() if err != nil { @@ -341,6 +401,12 @@ func handleAuthResponse(c packetConn) (authResult, []string, error) { if err := handleBannerResponse(c, packet); err != nil { return authFailure, nil, err } + case msgExtInfo: + // Ignore post-authentication RFC 8308 extensions, once. + if gotMsgExtInfo { + return authFailure, nil, unexpectedMessageError(msgUserAuthSuccess, packet[0]) + } + gotMsgExtInfo = true case msgUserAuthFailure: var msg userAuthFailureMsg if err := Unmarshal(packet, &msg); err != nil { @@ -395,7 +461,7 @@ func (cb KeyboardInteractiveChallenge) method() string { return "keyboard-interactive" } -func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) { +func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packetConn, rand io.Reader, _ map[string][]byte) (authResult, []string, error) { type initiateMsg struct { User string `sshtype:"50"` Service string @@ -412,6 +478,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe return authFailure, nil, err } + gotMsgExtInfo := false for { packet, err := c.readPacket() if err != nil { @@ -425,6 +492,13 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe return authFailure, nil, err } continue + case msgExtInfo: + // Ignore post-authentication RFC 8308 extensions, once. + if gotMsgExtInfo { + return authFailure, nil, unexpectedMessageError(msgUserAuthInfoRequest, packet[0]) + } + gotMsgExtInfo = true + continue case msgUserAuthInfoRequest: // OK case msgUserAuthFailure: @@ -497,9 +571,9 @@ type retryableAuthMethod struct { maxTries int } -func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader) (ok authResult, methods []string, err error) { +func (r *retryableAuthMethod) auth(session []byte, user string, c packetConn, rand io.Reader, extensions map[string][]byte) (ok authResult, methods []string, err error) { for i := 0; r.maxTries <= 0 || i < r.maxTries; i++ { - ok, methods, err = r.authMethod.auth(session, user, c, rand) + ok, methods, err = r.authMethod.auth(session, user, c, rand, extensions) if ok != authFailure || err != nil { // either success, partial success or error terminate return ok, methods, err } @@ -542,7 +616,7 @@ type gssAPIWithMICCallback struct { target string } -func (g *gssAPIWithMICCallback) auth(session []byte, user string, c packetConn, rand io.Reader) (authResult, []string, error) { +func (g *gssAPIWithMICCallback) auth(session []byte, user string, c packetConn, rand io.Reader, _ map[string][]byte) (authResult, []string, error) { m := &userAuthRequestMsg{ User: user, Service: serviceSSH, diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index f73079b10d..a6bedbfcaa 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -105,11 +105,63 @@ func tryAuthBothSides(t *testing.T, config *ClientConfig, gssAPIWithMICConfig *G return err, serverAuthErrors } +type loggingAlgorithmSigner struct { + used []string + AlgorithmSigner +} + +func (l *loggingAlgorithmSigner) Sign(rand io.Reader, data []byte) (*Signature, error) { + l.used = append(l.used, "[Sign]") + return l.AlgorithmSigner.Sign(rand, data) +} + +func (l *loggingAlgorithmSigner) SignWithAlgorithm(rand io.Reader, data []byte, algorithm string) (*Signature, error) { + l.used = append(l.used, algorithm) + return l.AlgorithmSigner.SignWithAlgorithm(rand, data, algorithm) +} + func TestClientAuthPublicKey(t *testing.T) { + signer := &loggingAlgorithmSigner{AlgorithmSigner: testSigners["rsa"].(AlgorithmSigner)} config := &ClientConfig{ User: "testuser", Auth: []AuthMethod{ - PublicKeys(testSigners["rsa"]), + PublicKeys(signer), + }, + HostKeyCallback: InsecureIgnoreHostKey(), + } + if err := tryAuth(t, config); err != nil { + t.Fatalf("unable to dial remote side: %s", err) + } + // Once the server implements the server-sig-algs extension, this will turn + // into KeyAlgoRSASHA256. + if len(signer.used) != 1 || signer.used[0] != KeyAlgoRSA { + t.Errorf("unexpected Sign/SignWithAlgorithm calls: %q", signer.used) + } +} + +// TestClientAuthNoSHA2 tests a ssh-rsa Signer that doesn't implement AlgorithmSigner. +func TestClientAuthNoSHA2(t *testing.T) { + config := &ClientConfig{ + User: "testuser", + Auth: []AuthMethod{ + PublicKeys(&legacyRSASigner{testSigners["rsa"]}), + }, + HostKeyCallback: InsecureIgnoreHostKey(), + } + if err := tryAuth(t, config); err != nil { + t.Fatalf("unable to dial remote side: %s", err) + } +} + +// TestClientAuthThirdKey checks that the third configured can succeed. If we +// were to do three attempts for each key (rsa-sha2-256, rsa-sha2-512, ssh-rsa), +// we'd hit the six maximum attempts before reaching it. +func TestClientAuthThirdKey(t *testing.T) { + config := &ClientConfig{ + User: "testuser", + Auth: []AuthMethod{ + PublicKeys(testSigners["rsa-openssh-format"], + testSigners["rsa-openssh-format"], testSigners["rsa"]), }, HostKeyCallback: InsecureIgnoreHostKey(), } diff --git a/ssh/common.go b/ssh/common.go index d6d9bf9642..2a47a61ded 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -297,8 +297,9 @@ func (c *Config) SetDefaults() { } // buildDataSignedForAuth returns the data that is signed in order to prove -// possession of a private key. See RFC 4252, section 7. -func buildDataSignedForAuth(sessionID []byte, req userAuthRequestMsg, algo, pubKey []byte) []byte { +// possession of a private key. See RFC 4252, section 7. algo is the advertised +// algorithm, and may be a certificate type. +func buildDataSignedForAuth(sessionID []byte, req userAuthRequestMsg, algo string, pubKey []byte) []byte { data := struct { Session []byte Type byte @@ -306,7 +307,7 @@ func buildDataSignedForAuth(sessionID []byte, req userAuthRequestMsg, algo, pubK Service string Method string Sign bool - Algo []byte + Algo string PubKey []byte }{ sessionID, diff --git a/ssh/handshake.go b/ssh/handshake.go index 4bceb331d5..f815cdb4c9 100644 --- a/ssh/handshake.go +++ b/ssh/handshake.go @@ -476,6 +476,13 @@ func (t *handshakeTransport) sendKexInit() error { } } else { msg.ServerHostKeyAlgos = t.hostKeyAlgorithms + + // As a client we opt in to receiving SSH_MSG_EXT_INFO so we know what + // algorithms the server supports for public key authentication. See RFC + // 8303, Section 2.1. + msg.KexAlgos = make([]string, 0, len(t.config.KeyExchanges)+1) + msg.KexAlgos = append(msg.KexAlgos, t.config.KeyExchanges...) + msg.KexAlgos = append(msg.KexAlgos, "ext-info-c") } packet := Marshal(msg) diff --git a/ssh/messages.go b/ssh/messages.go index 62f9330413..19bc67c464 100644 --- a/ssh/messages.go +++ b/ssh/messages.go @@ -141,6 +141,14 @@ type serviceAcceptMsg struct { Service string `sshtype:"6"` } +// See RFC 8308, section 2.3 +const msgExtInfo = 7 + +type extInfoMsg struct { + NumExtensions uint32 `sshtype:"7"` + Payload []byte `ssh:"rest"` +} + // See RFC 4252, section 5. const msgUserAuthRequest = 50 @@ -782,6 +790,8 @@ func decode(packet []byte) (interface{}, error) { msg = new(serviceRequestMsg) case msgServiceAccept: msg = new(serviceAcceptMsg) + case msgExtInfo: + msg = new(extInfoMsg) case msgKexInit: msg = new(kexInitMsg) case msgKexDHInit: @@ -843,6 +853,7 @@ var packetTypeNames = map[byte]string{ msgDisconnect: "disconnectMsg", msgServiceRequest: "serviceRequestMsg", msgServiceAccept: "serviceAcceptMsg", + msgExtInfo: "extInfoMsg", msgKexInit: "kexInitMsg", msgKexDHInit: "kexDHInitMsg", msgKexDHReply: "kexDHReplyMsg", diff --git a/ssh/server.go b/ssh/server.go index 9bb714c115..70045bdfd8 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -554,6 +554,7 @@ userAuthLoop: if !ok || len(payload) > 0 { return nil, parseError(msgUserAuthRequest) } + // Ensure the public key algo and signature algo // are supported. Compare the private key // algorithm name that corresponds to algo with @@ -563,7 +564,12 @@ userAuthLoop: authErr = fmt.Errorf("ssh: algorithm %q not accepted", sig.Format) break } - signedData := buildDataSignedForAuth(sessionID, userAuthReq, algoBytes, pubKeyData) + if underlyingAlgo(algo) != sig.Format { + authErr = fmt.Errorf("ssh: signature %q not compatible with selected algorithm %q", sig.Format, algo) + break + } + + signedData := buildDataSignedForAuth(sessionID, userAuthReq, algo, pubKeyData) if err := pubKey.Verify(signedData, sig); err != nil { return nil, err