From 4ca217ecf81df34c57ac8b9f9cf3bfb0ee5af36d Mon Sep 17 00:00:00 2001 From: Boshi Lian Date: Sun, 30 Jul 2017 01:58:49 +0800 Subject: [PATCH] fix #12, map user should mapped from first auth --- .golangssh_lastpick | 2 +- ssh/agent/client.go | 38 +++++++++++++--- ssh/agent/server.go | 88 ++++++++++++++++++++++++++++++++++---- ssh/agent/server_test.go | 50 ++++++++++++++++++++++ ssh/cipher.go | 4 +- ssh/client_auth_test.go | 57 ++++++++++++++++++++++-- ssh/keys.go | 6 +++ ssh/keys_test.go | 7 +++ ssh/server.go | 23 ++++++++++ ssh/session.go | 20 +++++++++ ssh/sshpiper.go | 5 +-- ssh/terminal/util_bsd.go | 6 +-- ssh/terminal/util_linux.go | 9 ++-- ssh/test/session_test.go | 48 +++++++++++++++++++++ 14 files changed, 331 insertions(+), 32 deletions(-) diff --git a/.golangssh_lastpick b/.golangssh_lastpick index f9e197cb..c49d0c06 100644 --- a/.golangssh_lastpick +++ b/.golangssh_lastpick @@ -1 +1 @@ -adbae1b6b6fb4b02448a0fc0dbbc9ba2b95b294d +558b6879de74bc843225cde5686419267ff707ca diff --git a/ssh/agent/client.go b/ssh/agent/client.go index b4748328..898fc9e6 100644 --- a/ssh/agent/client.go +++ b/ssh/agent/client.go @@ -57,6 +57,17 @@ type Agent interface { Signers() ([]ssh.Signer, error) } +// ConstraintExtension describes an optional constraint defined by users. +type ConstraintExtension struct { + // ExtensionName consist of a UTF-8 string suffixed by the + // implementation domain following the naming scheme defined + // in Section 4.2 of [RFC4251], e.g. "foo@example.com". + ExtensionName string + // ExtensionDetails contains the actual content of the extended + // constraint. + ExtensionDetails []byte +} + // AddedKey describes an SSH key to be added to an Agent. type AddedKey struct { // PrivateKey must be a *rsa.PrivateKey, *dsa.PrivateKey or @@ -73,6 +84,9 @@ type AddedKey struct { // ConfirmBeforeUse, if true, requests that the agent confirm with the // user before each use of this key. ConfirmBeforeUse bool + // ConstraintExtensions are the experimental or private-use constraints + // defined by users. + ConstraintExtensions []ConstraintExtension } // See [PROTOCOL.agent], section 3. @@ -94,8 +108,9 @@ const ( agentAddSmartcardKeyConstrained = 26 // 3.7 Key constraint identifiers - agentConstrainLifetime = 1 - agentConstrainConfirm = 2 + agentConstrainLifetime = 1 + agentConstrainConfirm = 2 + agentConstrainExtension = 3 ) // maxAgentResponseBytes is the maximum agent reply size that is accepted. This @@ -151,6 +166,19 @@ type publicKey struct { Rest []byte `ssh:"rest"` } +// 3.7 Key constraint identifiers +type constrainLifetimeAgentMsg struct { + LifetimeSecs uint32 `sshtype:"1"` +} + +type constrainExtensionAgentMsg struct { + ExtensionName string `sshtype:"3"` + ExtensionDetails []byte + + // Rest is a field used for parsing, not part of message + Rest []byte `ssh:"rest"` +} + // Key represents a protocol 2 public key as defined in // [PROTOCOL.agent], section 2.5.2. type Key struct { @@ -542,11 +570,7 @@ func (c *client) Add(key AddedKey) error { var constraints []byte if secs := key.LifetimeSecs; secs != 0 { - constraints = append(constraints, agentConstrainLifetime) - - var secsBytes [4]byte - binary.BigEndian.PutUint32(secsBytes[:], secs) - constraints = append(constraints, secsBytes[:]...) + constraints = append(constraints, ssh.Marshal(constrainLifetimeAgentMsg{secs})...) } if key.ConfirmBeforeUse { diff --git a/ssh/agent/server.go b/ssh/agent/server.go index 68a333fa..793dd2e7 100644 --- a/ssh/agent/server.go +++ b/ssh/agent/server.go @@ -155,6 +155,44 @@ func (s *server) processRequest(data []byte) (interface{}, error) { return nil, fmt.Errorf("unknown opcode %d", data[0]) } +func parseConstraints(constraints []byte) (lifetimeSecs uint32, confirmBeforeUse bool, extensions []ConstraintExtension, err error) { + for len(constraints) != 0 { + switch constraints[0] { + case agentConstrainLifetime: + lifetimeSecs = binary.BigEndian.Uint32(constraints[1:5]) + constraints = constraints[5:] + case agentConstrainConfirm: + confirmBeforeUse = true + constraints = constraints[1:] + case agentConstrainExtension: + var msg constrainExtensionAgentMsg + if err = ssh.Unmarshal(constraints, &msg); err != nil { + return 0, false, nil, err + } + extensions = append(extensions, ConstraintExtension{ + ExtensionName: msg.ExtensionName, + ExtensionDetails: msg.ExtensionDetails, + }) + constraints = msg.Rest + default: + return 0, false, nil, fmt.Errorf("unknown constraint type: %d", constraints[0]) + } + } + return +} + +func setConstraints(key *AddedKey, constraintBytes []byte) error { + lifetimeSecs, confirmBeforeUse, constraintExtensions, err := parseConstraints(constraintBytes) + if err != nil { + return err + } + + key.LifetimeSecs = lifetimeSecs + key.ConfirmBeforeUse = confirmBeforeUse + key.ConstraintExtensions = constraintExtensions + return nil +} + func parseRSAKey(req []byte) (*AddedKey, error) { var k rsaKeyMsg if err := ssh.Unmarshal(req, &k); err != nil { @@ -173,7 +211,11 @@ func parseRSAKey(req []byte) (*AddedKey, error) { } priv.Precompute() - return &AddedKey{PrivateKey: priv, Comment: k.Comments}, nil + addedKey := &AddedKey{PrivateKey: priv, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func parseEd25519Key(req []byte) (*AddedKey, error) { @@ -182,7 +224,12 @@ func parseEd25519Key(req []byte) (*AddedKey, error) { return nil, err } priv := ed25519.PrivateKey(k.Priv) - return &AddedKey{PrivateKey: &priv, Comment: k.Comments}, nil + + addedKey := &AddedKey{PrivateKey: &priv, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func parseDSAKey(req []byte) (*AddedKey, error) { @@ -202,7 +249,11 @@ func parseDSAKey(req []byte) (*AddedKey, error) { X: k.X, } - return &AddedKey{PrivateKey: priv, Comment: k.Comments}, nil + addedKey := &AddedKey{PrivateKey: priv, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func unmarshalECDSA(curveName string, keyBytes []byte, privScalar *big.Int) (priv *ecdsa.PrivateKey, err error) { @@ -243,7 +294,12 @@ func parseEd25519Cert(req []byte) (*AddedKey, error) { if !ok { return nil, errors.New("agent: bad ED25519 certificate") } - return &AddedKey{PrivateKey: &priv, Certificate: cert, Comment: k.Comments}, nil + + addedKey := &AddedKey{PrivateKey: &priv, Certificate: cert, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func parseECDSAKey(req []byte) (*AddedKey, error) { @@ -257,7 +313,11 @@ func parseECDSAKey(req []byte) (*AddedKey, error) { return nil, err } - return &AddedKey{PrivateKey: priv, Comment: k.Comments}, nil + addedKey := &AddedKey{PrivateKey: priv, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func parseRSACert(req []byte) (*AddedKey, error) { @@ -300,7 +360,11 @@ func parseRSACert(req []byte) (*AddedKey, error) { } priv.Precompute() - return &AddedKey{PrivateKey: &priv, Certificate: cert, Comment: k.Comments}, nil + addedKey := &AddedKey{PrivateKey: &priv, Certificate: cert, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func parseDSACert(req []byte) (*AddedKey, error) { @@ -338,7 +402,11 @@ func parseDSACert(req []byte) (*AddedKey, error) { X: k.X, } - return &AddedKey{PrivateKey: priv, Certificate: cert, Comment: k.Comments}, nil + addedKey := &AddedKey{PrivateKey: priv, Certificate: cert, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func parseECDSACert(req []byte) (*AddedKey, error) { @@ -371,7 +439,11 @@ func parseECDSACert(req []byte) (*AddedKey, error) { return nil, err } - return &AddedKey{PrivateKey: priv, Certificate: cert, Comment: k.Comments}, nil + addedKey := &AddedKey{PrivateKey: priv, Certificate: cert, Comment: k.Comments} + if err := setConstraints(addedKey, k.Constraints); err != nil { + return nil, err + } + return addedKey, nil } func (s *server) insertIdentity(req []byte) error { diff --git a/ssh/agent/server_test.go b/ssh/agent/server_test.go index 6b0837d9..c34d05b3 100644 --- a/ssh/agent/server_test.go +++ b/ssh/agent/server_test.go @@ -8,6 +8,9 @@ import ( "crypto" "crypto/rand" "fmt" + pseudorand "math/rand" + "reflect" + "strings" "testing" "golang.org/x/crypto/ssh" @@ -207,3 +210,50 @@ func TestCertTypes(t *testing.T) { } } } + +func TestParseConstraints(t *testing.T) { + // Test LifetimeSecs + var msg = constrainLifetimeAgentMsg{pseudorand.Uint32()} + lifetimeSecs, _, _, err := parseConstraints(ssh.Marshal(msg)) + if err != nil { + t.Fatalf("parseConstraints: %v", err) + } + if lifetimeSecs != msg.LifetimeSecs { + t.Errorf("got lifetime %v, want %v", lifetimeSecs, msg.LifetimeSecs) + } + + // Test ConfirmBeforeUse + _, confirmBeforeUse, _, err := parseConstraints([]byte{agentConstrainConfirm}) + if err != nil { + t.Fatalf("%v", err) + } + if !confirmBeforeUse { + t.Error("got comfirmBeforeUse == false") + } + + // Test ConstraintExtensions + var data []byte + var expect []ConstraintExtension + for i := 0; i < 10; i++ { + var ext = ConstraintExtension{ + ExtensionName: fmt.Sprintf("name%d", i), + ExtensionDetails: []byte(fmt.Sprintf("details: %d", i)), + } + expect = append(expect, ext) + data = append(data, agentConstrainExtension) + data = append(data, ssh.Marshal(ext)...) + } + _, _, extensions, err := parseConstraints(data) + if err != nil { + t.Fatalf("%v", err) + } + if !reflect.DeepEqual(expect, extensions) { + t.Errorf("got extension %v, want %v", extensions, expect) + } + + // Test Unknown Constraint + _, _, _, err = parseConstraints([]byte{128}) + if err == nil || !strings.Contains(err.Error(), "unknown constraint") { + t.Errorf("unexpected error: %v", err) + } +} diff --git a/ssh/cipher.go b/ssh/cipher.go index 13484ab4..22bb30cc 100644 --- a/ssh/cipher.go +++ b/ssh/cipher.go @@ -392,7 +392,9 @@ func (c *gcmCipher) readPacket(seqNum uint32, r io.Reader) ([]byte, error) { c.incIV() padding := plain[0] - if padding < 4 || padding >= 20 { + if padding < 4 { + // padding is a byte, so it automatically satisfies + // the maximum size, which is 255. return nil, fmt.Errorf("ssh: illegal padding %d", padding) } diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index bd9f8a16..145b57a2 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -359,9 +359,8 @@ func testPermissionsPassing(withPermissions bool, t *testing.T) { PublicKeyCallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) { if conn.User() == "nopermissions" { return nil, nil - } else { - return &Permissions{}, nil } + return &Permissions{}, nil }, } serverConfig.AddHostKey(testSigners["rsa"]) @@ -510,9 +509,8 @@ func TestClientAuthMaxAuthTries(t *testing.T) { n-- if n == 0 { return "right", nil - } else { - return "wrong", nil } + return "wrong", nil }), tries), }, HostKeyCallback: InsecureIgnoreHostKey(), @@ -577,3 +575,54 @@ func TestClientAuthMaxAuthTriesPublicKey(t *testing.T) { t.Fatalf("client: got %s, want %s", err, expectedErr) } } + +// Test whether authentication errors are being properly logged if all +// authentication methods have been exhausted +func TestClientAuthErrorList(t *testing.T) { + publicKeyErr := errors.New("This is an error from PublicKeyCallback") + + clientConfig := &ClientConfig{ + Auth: []AuthMethod{ + PublicKeys(testSigners["rsa"]), + }, + HostKeyCallback: InsecureIgnoreHostKey(), + } + serverConfig := &ServerConfig{ + PublicKeyCallback: func(_ ConnMetadata, _ PublicKey) (*Permissions, error) { + return nil, publicKeyErr + }, + } + serverConfig.AddHostKey(testSigners["rsa"]) + + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + go NewClientConn(c2, "", clientConfig) + _, err = newServer(c1, serverConfig) + if err == nil { + t.Fatal("newServer: got nil, expected errors") + } + + authErrs, ok := err.(*ServerAuthError) + if !ok { + t.Fatalf("errors: got %T, want *ssh.ServerAuthError", err) + } + for i, e := range authErrs.Errors { + switch i { + case 0: + if e.Error() != "no auth passed yet" { + t.Fatalf("errors: got %v, want no auth passed yet", e.Error()) + } + case 1: + if e != publicKeyErr { + t.Fatalf("errors: got %v, want %v", e, publicKeyErr) + } + default: + t.Fatalf("errors: got %v, expected 2 errors", authErrs.Errors) + } + } +} diff --git a/ssh/keys.go b/ssh/keys.go index 4c8b1a8f..7a8756a9 100644 --- a/ssh/keys.go +++ b/ssh/keys.go @@ -802,6 +802,9 @@ func ParseRawPrivateKey(pemBytes []byte) (interface{}, error) { } } +// ParseRawPrivateKeyWithPassphrase returns a private key decrypted with +// passphrase from a PEM encoded private key. If wrong passphrase, return +// x509.IncorrectPasswordError. func ParseRawPrivateKeyWithPassphrase(pemBytes, passPhrase []byte) (interface{}, error) { block, _ := pem.Decode(pemBytes) if block == nil { @@ -814,6 +817,9 @@ func ParseRawPrivateKeyWithPassphrase(pemBytes, passPhrase []byte) (interface{}, var err error buf, err = x509.DecryptPEMBlock(block, passPhrase) if err != nil { + if err == x509.IncorrectPasswordError { + return nil, err + } return nil, fmt.Errorf("ssh: cannot decode encrypted private keys: %v", err) } } diff --git a/ssh/keys_test.go b/ssh/keys_test.go index 2bacc52a..20ab954e 100644 --- a/ssh/keys_test.go +++ b/ssh/keys_test.go @@ -11,6 +11,7 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/rsa" + "crypto/x509" "encoding/base64" "fmt" "reflect" @@ -165,6 +166,12 @@ func TestParseEncryptedPrivateKeysWithPassphrase(t *testing.T) { t.Errorf("Verify failed: %v", err) } } + + tt := testdata.PEMEncryptedKeys[0] + _, err := ParsePrivateKeyWithPassphrase(tt.PEMBytes, []byte("incorrect")) + if err != x509.IncorrectPasswordError { + t.Fatalf("got %v want IncorrectPasswordError", err) + } } func TestParseDSA(t *testing.T) { diff --git a/ssh/server.go b/ssh/server.go index 70d6077d..b6f4cc81 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -288,12 +288,30 @@ func checkSourceAddress(addr net.Addr, sourceAddrs string) error { return fmt.Errorf("ssh: remote address %v is not allowed because of source-address restriction", addr) } +// ServerAuthError implements the error interface. It appends any authentication +// errors that may occur, and is returned if all of the authentication methods +// provided by the user failed to authenticate. +type ServerAuthError struct { + // Errors contains authentication errors returned by the authentication + // callback methods. + Errors []error +} + +func (l ServerAuthError) Error() string { + var errs []string + for _, err := range l.Errors { + errs = append(errs, err.Error()) + } + return "[" + strings.Join(errs, ", ") + "]" +} + func (s *connection) serverAuthenticate(config *ServerConfig) (*Permissions, error) { sessionID := s.transport.getSessionID() var cache pubKeyCache var perms *Permissions authFailures := 0 + var authErrs []error userAuthLoop: for { @@ -312,6 +330,9 @@ userAuthLoop: var userAuthReq userAuthRequestMsg if packet, err := s.transport.readPacket(); err != nil { + if err == io.EOF { + return nil, &ServerAuthError{Errors: authErrs} + } return nil, err } else if err = Unmarshal(packet, &userAuthReq); err != nil { return nil, err @@ -448,6 +469,8 @@ userAuthLoop: authErr = fmt.Errorf("ssh: unknown method %q", userAuthReq.Method) } + authErrs = append(authErrs, authErr) + if config.AuthLogCallback != nil { config.AuthLogCallback(s, userAuthReq.Method, authErr) } diff --git a/ssh/session.go b/ssh/session.go index 17e2aa85..cc06e03f 100644 --- a/ssh/session.go +++ b/ssh/session.go @@ -231,6 +231,26 @@ func (s *Session) RequestSubsystem(subsystem string) error { return err } +// RFC 4254 Section 6.7. +type ptyWindowChangeMsg struct { + Columns uint32 + Rows uint32 + Width uint32 + Height uint32 +} + +// WindowChange informs the remote host about a terminal window dimension change to h rows and w columns. +func (s *Session) WindowChange(h, w int) error { + req := ptyWindowChangeMsg{ + Columns: uint32(w), + Rows: uint32(h), + Width: uint32(w * 8), + Height: uint32(h * 8), + } + _, err := s.ch.SendRequest("window-change", false, Marshal(&req)) + return err +} + // RFC 4254 Section 6.9. type signalMsg struct { Signal string diff --git a/ssh/sshpiper.go b/ssh/sshpiper.go index 381250db..ac37189f 100644 --- a/ssh/sshpiper.go +++ b/ssh/sshpiper.go @@ -227,7 +227,6 @@ func NewSSHPiperConn(conn net.Conn, piper *SSHPiperConfig) (pipe *SSHPiperConn, return msg, nil } - user := msg.User // pubKey MAP downKey, isQuery, sig, err := parsePublicKeyMsg(msg) if err != nil { @@ -238,7 +237,7 @@ func NewSSHPiperConn(conn net.Conn, piper *SSHPiperConfig) (pipe *SSHPiperConn, // no mapped user change it to none or error occur if err != nil || signer == nil { - return noneAuthMsg(user), nil + return noneAuthMsg(mappedUser), nil } upKey := signer.PublicKey() @@ -259,7 +258,7 @@ func NewSSHPiperConn(conn net.Conn, piper *SSHPiperConfig) (pipe *SSHPiperConn, } if !ok { - return noneAuthMsg(user), nil + return noneAuthMsg(mappedUser), nil } msg, err = p.signAgain(mappedUser, msg, signer, downKey) diff --git a/ssh/terminal/util_bsd.go b/ssh/terminal/util_bsd.go index 9c1ffd14..cb23a590 100644 --- a/ssh/terminal/util_bsd.go +++ b/ssh/terminal/util_bsd.go @@ -6,7 +6,7 @@ package terminal -import "syscall" +import "golang.org/x/sys/unix" -const ioctlReadTermios = syscall.TIOCGETA -const ioctlWriteTermios = syscall.TIOCSETA +const ioctlReadTermios = unix.TIOCGETA +const ioctlWriteTermios = unix.TIOCSETA diff --git a/ssh/terminal/util_linux.go b/ssh/terminal/util_linux.go index 5883b22d..5fadfe8a 100644 --- a/ssh/terminal/util_linux.go +++ b/ssh/terminal/util_linux.go @@ -4,8 +4,7 @@ package terminal -// These constants are declared here, rather than importing -// them from the syscall package as some syscall packages, even -// on linux, for example gccgo, do not declare them. -const ioctlReadTermios = 0x5401 // syscall.TCGETS -const ioctlWriteTermios = 0x5402 // syscall.TCSETS +import "golang.org/x/sys/unix" + +const ioctlReadTermios = unix.TCGETS +const ioctlWriteTermios = unix.TCSETS diff --git a/ssh/test/session_test.go b/ssh/test/session_test.go index fc7e4715..8238d9d9 100644 --- a/ssh/test/session_test.go +++ b/ssh/test/session_test.go @@ -276,6 +276,54 @@ func TestValidTerminalMode(t *testing.T) { } } +func TestWindowChange(t *testing.T) { + server := newServer(t) + defer server.Shutdown() + conn := server.Dial(clientConfig()) + defer conn.Close() + + session, err := conn.NewSession() + if err != nil { + t.Fatalf("session failed: %v", err) + } + defer session.Close() + + stdout, err := session.StdoutPipe() + if err != nil { + t.Fatalf("unable to acquire stdout pipe: %s", err) + } + + stdin, err := session.StdinPipe() + if err != nil { + t.Fatalf("unable to acquire stdin pipe: %s", err) + } + + tm := ssh.TerminalModes{ssh.ECHO: 0} + if err = session.RequestPty("xterm", 80, 40, tm); err != nil { + t.Fatalf("req-pty failed: %s", err) + } + + if err := session.WindowChange(100, 100); err != nil { + t.Fatalf("window-change failed: %s", err) + } + + err = session.Shell() + if err != nil { + t.Fatalf("session failed: %s", err) + } + + stdin.Write([]byte("stty size && exit\n")) + + var buf bytes.Buffer + if _, err := io.Copy(&buf, stdout); err != nil { + t.Fatalf("reading failed: %s", err) + } + + if sttyOutput := buf.String(); !strings.Contains(sttyOutput, "100 100") { + t.Fatalf("terminal WindowChange failure: expected \"100 100\" stty output, got %s", sttyOutput) + } +} + func TestCiphers(t *testing.T) { var config ssh.Config config.SetDefaults()