From 986d3313588aa5c68f1df95eac956f79cf3b2c01 Mon Sep 17 00:00:00 2001 From: Sven Blumenstein Date: Wed, 24 Aug 2016 15:36:42 +0200 Subject: [PATCH] crypto/ssh: keep user in ConnMetadata if NoClientAuth is used The current behaviour of the crypto/ssh server implementation is to remove the username from ConnMetadata if the connection is done without authentication (NoClientAuth). This appears to be a bug. This behaviour is different from other SSH server implementations like for example Paramiko (Python) which keeps the username. Additionally RFC4252 (https://www.ietf.org/rfc/rfc4252.txt) section 5 states the username has to be included in every USERAUTH message. Change-Id: I27fa50db92eb535e90fe088453faa6f2a76ee31f Reviewed-on: https://go-review.googlesource.com/27612 Reviewed-by: Han-Wen Nienhuys Run-TryBot: Han-Wen Nienhuys TryBot-Result: Gobot Gobot --- ssh/client_auth_test.go | 29 +++++++++++++++++++++++++++++ ssh/connection.go | 1 - ssh/server.go | 1 - 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/ssh/client_auth_test.go b/ssh/client_auth_test.go index 548a9b95..55833e57 100644 --- a/ssh/client_auth_test.go +++ b/ssh/client_auth_test.go @@ -441,3 +441,32 @@ func ExampleRetryableAuthMethod(t *testing.T) { t.Fatalf("unable to dial remote side: %s", err) } } + +// Test if username is received on server side when NoClientAuth is used +func TestClientAuthNone(t *testing.T) { + user := "testuser" + serverConfig := &ServerConfig{ + NoClientAuth: true, + } + serverConfig.AddHostKey(testSigners["rsa"]) + + clientConfig := &ClientConfig{ + User: user, + } + + c1, c2, err := netPipe() + if err != nil { + t.Fatalf("netPipe: %v", err) + } + defer c1.Close() + defer c2.Close() + + go NewClientConn(c2, "", clientConfig) + serverConn, err := newServer(c1, serverConfig) + if err != nil { + t.Fatal("newServer: %v", err) + } + if serverConn.User() != user { + t.Fatalf("server: got %q, want %q", serverConn.User(), user) + } +} diff --git a/ssh/connection.go b/ssh/connection.go index 979d919e..e786f2f9 100644 --- a/ssh/connection.go +++ b/ssh/connection.go @@ -23,7 +23,6 @@ func (e *OpenChannelError) Error() string { // ConnMetadata holds metadata for the connection. type ConnMetadata interface { // User returns the user ID for this connection. - // It is empty if no authentication is used. User() string // SessionID returns the sesson hash, also denoted by H. diff --git a/ssh/server.go b/ssh/server.go index e73a1c1a..37df1b30 100644 --- a/ssh/server.go +++ b/ssh/server.go @@ -284,7 +284,6 @@ userAuthLoop: switch userAuthReq.Method { case "none": if config.NoClientAuth { - s.user = "" authErr = nil } case "password":