diff --git a/ssh/cipher.go b/ssh/cipher.go index 0646e1e..97e5d9b 100644 --- a/ssh/cipher.go +++ b/ssh/cipher.go @@ -74,6 +74,9 @@ var DefaultCipherOrder = []string{ "arcfour256", "arcfour128", } +// cipherModes documents properties of supported ciphers. Ciphers not included +// are not supported and will not be negotiated, even if explicitly requested in +// ClientConfig.Crypto.Ciphers. var cipherModes = map[string]*cipherMode{ // Ciphers from RFC4344, which introduced many CTR-based ciphers. Algorithms // are defined in the order specified in the RFC. diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index 11c7227..d4002c6 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -276,3 +276,23 @@ func TestClientHMAC(t *testing.T) { c.Close() } } + +// issue 4285. +func TestClientUnsupportedCipher(t *testing.T) { + kc := new(keychain) + kc.keys = append(kc.keys, rsakey) + config := &ClientConfig{ + User: "testuser", + Auth: []ClientAuth{ + ClientAuthKeyring(kc), + }, + Crypto: CryptoConfig{ + Ciphers: []string{"aes128-cbc"}, // not currently supported + }, + } + c, err := Dial("tcp", newMockAuthServer(t), config) + if err == nil { + t.Errorf("expected no ciphers in common") + c.Close() + } +} diff --git a/ssh/common.go b/ssh/common.go index 9cb740d..f62b255 100644 --- a/ssh/common.go +++ b/ssh/common.go @@ -8,8 +8,8 @@ import ( "crypto/dsa" "crypto/rsa" "errors" + "fmt" "math/big" - "strconv" "sync" ) @@ -77,7 +77,7 @@ type UnexpectedMessageError struct { } func (u UnexpectedMessageError) Error() string { - return "ssh: unexpected message type " + strconv.Itoa(int(u.got)) + " (expected " + strconv.Itoa(int(u.expected)) + ")" + return fmt.Sprintf("ssh: unexpected message type %d (expected %d)", u.got, u.expected) } // ParseError results from a malformed SSH message. @@ -86,7 +86,7 @@ type ParseError struct { } func (p ParseError) Error() string { - return "ssh: parse error in message type " + strconv.Itoa(int(p.msgType)) + return fmt.Sprintf("ssh: parse error in message type %d", p.msgType) } type handshakeMagics struct { @@ -102,7 +102,18 @@ func findCommonAlgorithm(clientAlgos []string, serverAlgos []string) (commonAlgo } } } + return +} +func findCommonCipher(clientCiphers []string, serverCiphers []string) (commonCipher string, ok bool) { + for _, clientCipher := range clientCiphers { + for _, serverCipher := range serverCiphers { + // reject the cipher if we have no cipherModes definition + if clientCipher == serverCipher && cipherModes[clientCipher] != nil { + return clientCipher, true + } + } + } return } @@ -117,12 +128,12 @@ func findAgreedAlgorithms(transport *transport, clientKexInit, serverKexInit *ke return } - transport.writer.cipherAlgo, ok = findCommonAlgorithm(clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer) + transport.writer.cipherAlgo, ok = findCommonCipher(clientKexInit.CiphersClientServer, serverKexInit.CiphersClientServer) if !ok { return } - transport.reader.cipherAlgo, ok = findCommonAlgorithm(clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient) + transport.reader.cipherAlgo, ok = findCommonCipher(clientKexInit.CiphersServerClient, serverKexInit.CiphersServerClient) if !ok { return } diff --git a/ssh/test/session_test.go b/ssh/test/session_test.go index 91dea2a..532bb7b 100644 --- a/ssh/test/session_test.go +++ b/ssh/test/session_test.go @@ -20,7 +20,7 @@ import ( func TestRunCommandSuccess(t *testing.T) { server := newServer(t) defer server.Shutdown() - conn := server.Dial() + conn := server.Dial(clientConfig()) defer conn.Close() session, err := conn.NewSession() @@ -37,7 +37,7 @@ func TestRunCommandSuccess(t *testing.T) { func TestRunCommandFailed(t *testing.T) { server := newServer(t) defer server.Shutdown() - conn := server.Dial() + conn := server.Dial(clientConfig()) defer conn.Close() session, err := conn.NewSession() @@ -54,7 +54,7 @@ func TestRunCommandFailed(t *testing.T) { func TestRunCommandWeClosed(t *testing.T) { server := newServer(t) defer server.Shutdown() - conn := server.Dial() + conn := server.Dial(clientConfig()) defer conn.Close() session, err := conn.NewSession() @@ -74,7 +74,7 @@ func TestRunCommandWeClosed(t *testing.T) { func TestFuncLargeRead(t *testing.T) { server := newServer(t) defer server.Shutdown() - conn := server.Dial() + conn := server.Dial(clientConfig()) defer conn.Close() session, err := conn.NewSession() @@ -106,7 +106,7 @@ func TestFuncLargeRead(t *testing.T) { func TestInvalidTerminalMode(t *testing.T) { server := newServer(t) defer server.Shutdown() - conn := server.Dial() + conn := server.Dial(clientConfig()) defer conn.Close() session, err := conn.NewSession() @@ -123,7 +123,7 @@ func TestInvalidTerminalMode(t *testing.T) { func TestValidTerminalMode(t *testing.T) { server := newServer(t) defer server.Shutdown() - conn := server.Dial() + conn := server.Dial(clientConfig()) defer conn.Close() session, err := conn.NewSession() diff --git a/ssh/test/tcpip_test.go b/ssh/test/tcpip_test.go index 9ef4877..ee06b60 100644 --- a/ssh/test/tcpip_test.go +++ b/ssh/test/tcpip_test.go @@ -27,7 +27,7 @@ func TestTCPIPHTTPS(t *testing.T) { func doTest(t *testing.T, url string) { server := newServer(t) defer server.Shutdown() - conn := server.Dial() + conn := server.Dial(clientConfig()) defer conn.Close() tr := &http.Transport{ diff --git a/ssh/test/test_unix_test.go b/ssh/test/test_unix_test.go index 4409e84..9df54ba 100644 --- a/ssh/test/test_unix_test.go +++ b/ssh/test/test_unix_test.go @@ -9,6 +9,7 @@ package test // functional test harness for unix. import ( + "bytes" "crypto" "crypto/dsa" "crypto/rsa" @@ -40,7 +41,7 @@ Pidfile {{.Dir}}/sshd.pid KeyRegenerationInterval 3600 ServerKeyBits 768 SyslogFacility AUTH -LogLevel INFO +LogLevel DEBUG2 LoginGraceTime 120 PermitRootLogin no StrictModes no @@ -146,29 +147,13 @@ type server struct { cleanup func() // executed during Shutdown configfile string cmd *exec.Cmd + output bytes.Buffer // holds stderr from sshd process } -func (s *server) Dial() *ssh.ClientConn { - s.cmd = exec.Command("sshd", "-f", s.configfile, "-i") - stdin, err := s.cmd.StdinPipe() - if err != nil { - s.t.Fatal(err) - } - stdout, err := s.cmd.StdoutPipe() - if err != nil { - s.t.Fatal(err) - } - s.cmd.Stderr = os.Stderr - err = s.cmd.Start() - if err != nil { - s.Shutdown() - s.t.Fatal(err) - } - +func clientConfig() *ssh.ClientConfig { user, err := user.Current() if err != nil { - s.Shutdown() - s.t.Fatal(err) + panic(err) } kc := new(keychain) kc.keys = append(kc.keys, rsakey) @@ -178,8 +163,29 @@ func (s *server) Dial() *ssh.ClientConn { ssh.ClientAuthKeyring(kc), }, } + return config +} + +func (s *server) Dial(config *ssh.ClientConfig) *ssh.ClientConn { + s.cmd = exec.Command("sshd", "-f", s.configfile, "-i") + stdin, err := s.cmd.StdinPipe() + if err != nil { + s.t.Fatal(err) + } + stdout, err := s.cmd.StdoutPipe() + if err != nil { + s.t.Fatal(err) + } + s.cmd.Stderr = os.Stderr // &s.output + err = s.cmd.Start() + if err != nil { + s.t.FailNow() + s.Shutdown() + s.t.Fatal(err) + } conn, err := ssh.Client(&client{stdin, stdout}, config) if err != nil { + s.t.FailNow() s.Shutdown() s.t.Fatal(err) } @@ -190,6 +196,10 @@ func (s *server) Shutdown() { if err := s.cmd.Process.Kill(); err != nil { s.t.Error(err) } + if s.t.Failed() { + // log any output from sshd process + s.t.Log(s.output.String()) + } s.cleanup() }