Revert "ssh: add support for banners"

This reverts commit ed5229da99e3a6df35c756cd64b6982d19505d86.

Reason for revert: missing language tag in banner message breaks auth against other implementations.

Change-Id: I18ac5b3fe3b4693688b82ff4b0db02dab739c45b
Reviewed-on: https://go-review.googlesource.com/72381
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

ssh/client.go

@@ -9,7 +9,6 @@ import (
 	"errors"
 	"fmt"
 	"net"
-	"os"
 	"sync"
 	"time"
 )
@@ -188,10 +187,6 @@ func Dial(network, addr string, config *ClientConfig) (*Client, error) {
 // net.Conn underlying the the SSH connection.
 type HostKeyCallback func(hostname string, remote net.Addr, key PublicKey) error
 
-// BannerCallback is the function type used for treat the banner sent by
-// the server. A BannerCallback receives the message sent by the remote server.
-type BannerCallback func(message string) error
-
 // A ClientConfig structure is used to configure a Client. It must not be
 // modified after having been passed to an SSH function.
 type ClientConfig struct {
@@ -214,12 +209,6 @@ type ClientConfig struct {
 	// FixedHostKey can be used for simplistic host key checks.
 	HostKeyCallback HostKeyCallback
 
-	// BannerCallback is called during the SSH dance to display a custom
-	// server's message. The client configuration can supply this callback to
-	// handle it as wished. The function BannerDisplayStderr can be used for
-	// simplistic display on Stderr.
-	BannerCallback BannerCallback
-
 	// ClientVersion contains the version identification string that will
 	// be used for the connection. If empty, a reasonable default is used.
 	ClientVersion string
@@ -266,13 +255,3 @@ func FixedHostKey(key PublicKey) HostKeyCallback {
 	hk := &fixedHostKey{key}
 	return hk.check
 }
-
-// BannerDisplayStderr returns a function that can be used for
-// ClientConfig.BannerCallback to display banners on os.Stderr.
-func BannerDisplayStderr() BannerCallback {
-	return func(banner string) error {
-		_, err := os.Stderr.WriteString(banner)
-
-		return err
-	}
-}

ssh/client_auth.go

@@ -283,9 +283,7 @@ func confirmKeyAck(key PublicKey, c packetConn) (bool, error) {
 		}
 		switch packet[0] {
 		case msgUserAuthBanner:
-			if err := handleBannerResponse(c, packet); err != nil {
-				return false, err
-			}
+			// TODO(gpaul): add callback to present the banner to the user
 		case msgUserAuthPubKeyOk:
 			var msg userAuthPubKeyOkMsg
 			if err := Unmarshal(packet, &msg); err != nil {
@@ -327,9 +325,7 @@ func handleAuthResponse(c packetConn) (bool, []string, error) {
 
 		switch packet[0] {
 		case msgUserAuthBanner:
-			if err := handleBannerResponse(c, packet); err != nil {
-				return false, nil, err
-			}
+			// TODO: add callback to present the banner to the user
 		case msgUserAuthFailure:
 			var msg userAuthFailureMsg
 			if err := Unmarshal(packet, &msg); err != nil {
@@ -344,24 +340,6 @@ func handleAuthResponse(c packetConn) (bool, []string, error) {
 	}
 }
 
-func handleBannerResponse(c packetConn, packet []byte) error {
-	var msg userAuthBannerMsg
-	if err := Unmarshal(packet, &msg); err != nil {
-		return err
-	}
-
-	transport, ok := c.(*handshakeTransport)
-	if !ok {
-		return nil
-	}
-
-	if transport.bannerCallback != nil {
-		return transport.bannerCallback(msg.Message)
-	}
-
-	return nil
-}
-
 // KeyboardInteractiveChallenge should print questions, optionally
 // disabling echoing (e.g. for passwords), and return all the answers.
 // Challenge may be called multiple times in a single session. After
@@ -407,9 +385,7 @@ func (cb KeyboardInteractiveChallenge) auth(session []byte, user string, c packe
 		// like handleAuthResponse, but with less options.
 		switch packet[0] {
 		case msgUserAuthBanner:
-			if err := handleBannerResponse(c, packet); err != nil {
-				return false, nil, err
-			}
+			// TODO: Print banners during userauth.
 			continue
 		case msgUserAuthInfoRequest:
 			// OK

ssh/client_test.go

@@ -79,40 +79,3 @@ func TestHostKeyCheck(t *testing.T) {
 		}
 	}
 }
-func TestBannerCallback(t *testing.T) {
-	c1, c2, err := netPipe()
-	if err != nil {
-		t.Fatalf("netPipe: %v", err)
-	}
-	defer c1.Close()
-	defer c2.Close()
-
-	serverConf := &ServerConfig{
-		NoClientAuth: true,
-		BannerCallback: func(conn ConnMetadata) string {
-			return "Hello World"
-		},
-	}
-	serverConf.AddHostKey(testSigners["rsa"])
-	go NewServerConn(c1, serverConf)
-
-	var receivedBanner string
-	clientConf := ClientConfig{
-		User:            "user",
-		HostKeyCallback: InsecureIgnoreHostKey(),
-		BannerCallback: func(message string) error {
-			receivedBanner = message
-			return nil
-		},
-	}
-
-	_, _, _, err = NewClientConn(c2, "", &clientConf)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	expected := "Hello World"
-	if receivedBanner != expected {
-		t.Fatalf("got %s; want %s", receivedBanner, expected)
-	}
-}

ssh/handshake.go

@@ -78,11 +78,6 @@ type handshakeTransport struct {
 	dialAddress     string
 	remoteAddr      net.Addr
 
-	// bannerCallback is non-empty if we are the client and it has been set in
-	// ClientConfig. In that case it is called during the user authentication
-	// dance to handle a custom server's message.
-	bannerCallback BannerCallback
-
 	// Algorithms agreed in the last key exchange.
 	algorithms *algorithms
 
@@ -125,7 +120,6 @@ func newClientTransport(conn keyingTransport, clientVersion, serverVersion []byt
 	t.dialAddress = dialAddr
 	t.remoteAddr = addr
 	t.hostKeyCallback = config.HostKeyCallback
-	t.bannerCallback = config.BannerCallback
 	if config.HostKeyAlgorithms != nil {
 		t.hostKeyAlgorithms = config.HostKeyAlgorithms
 	} else {

ssh/messages.go

@@ -23,6 +23,10 @@ const (
 	msgUnimplemented = 3
 	msgDebug         = 4
 	msgNewKeys       = 21
+
+	// Standard authentication messages
+	msgUserAuthSuccess = 52
+	msgUserAuthBanner  = 53
 )
 
 // SSH messages:
@@ -133,16 +137,6 @@ type userAuthFailureMsg struct {
 	PartialSuccess bool
 }
 
-// See RFC 4252, section 5.1
-const msgUserAuthSuccess = 52
-
-// See RFC 4252, section 5.4
-const msgUserAuthBanner = 53
-
-type userAuthBannerMsg struct {
-	Message string `sshtype:"53"`
-}
-
 // See RFC 4256, section 3.2
 const msgUserAuthInfoRequest = 60
 const msgUserAuthInfoResponse = 61

ssh/server.go

@@ -95,10 +95,6 @@ type ServerConfig struct {
 	// Note that RFC 4253 section 4.2 requires that this string start with
 	// "SSH-2.0-".
 	ServerVersion string
-
-	// BannerCallback, if present, is called and the return string is sent to
-	// the client after key exchange completed but before authentication.
-	BannerCallback func(conn ConnMetadata) string
 }
 
 // AddHostKey adds a private key as a host key. If an existing host
@@ -347,19 +343,6 @@ userAuthLoop:
 		}
 
 		s.user = userAuthReq.User
-
-		if authFailures == 0 && config.BannerCallback != nil {
-			msg := config.BannerCallback(s)
-			if msg != "" {
-				bannerMsg := &userAuthBannerMsg{
-					Message: msg,
-				}
-				if err := s.transport.writePacket(Marshal(bannerMsg)); err != nil {
-					return nil, err
-				}
-			}
-		}
-
 		perms = nil
 		authErr := errors.New("no auth passed yet")