Move password authentication handling into sshd/auth (fixes #394).

Password authentication is now completely handeled in Auth. The normal
keyboard-interactive handler checks if passwords are supported and asks
for them, removing the need to override the callbacks.

Brute force throttling is removed; I'd like to base it on IP address
banning, which requires changes to the checks.

I'm not sure, but I think timing attacks against the password are fixed:
- The hashing of the real password happens only at startup.
- The hashing of a provided password is something an attacker can do
themselves; It doesn't leak anything about the real password.
- The hash comparison is constant-time.

auth.go

@@ -1,6 +1,8 @@
 package sshchat
 
 import (
+	"crypto/sha256"
+	"crypto/subtle"
 	"encoding/csv"
 	"errors"
 	"fmt"
@@ -43,7 +45,9 @@ func newAuthAddr(addr net.Addr) string {
 }
 
 // Auth stores lookups for bans, whitelists, and ops. It implements the sshd.Auth interface.
+// If the contained password is not empty, it complements a whitelist.
 type Auth struct {
+	passwordHash []byte
 	bannedAddr   *set.Set
 	bannedClient *set.Set
 	banned       *set.Set
@@ -51,7 +55,7 @@ type Auth struct {
 	ops          *set.Set
 }
 
-// NewAuth creates a new empty Auth.
+// NewAuth creates a Auth, optionally with a password.
 func NewAuth() *Auth {
 	return &Auth{
 		bannedAddr:   set.New(),
@@ -62,17 +66,33 @@ func NewAuth() *Auth {
 	}
 }
 
+// SetPassword anables password authentication with the given password.
+// If an emty password is geiven, disable password authentication.
+func (a *Auth) SetPassword(password string) {
+	if password == "" {
+		a.passwordHash = nil
+	} else {
+		hashArray := sha256.Sum256([]byte(password))
+		a.passwordHash = hashArray[:]
+	}
+}
+
 // AllowAnonymous determines if anonymous users are permitted.
 func (a *Auth) AllowAnonymous() bool {
-	return a.whitelist.Len() == 0
+	return a.whitelist.Len() == 0 && a.passwordHash == nil
+}
+
+// AcceptPassword determines if password authentication is accepted.
+func (a *Auth) AcceptPassword() bool {
+	return a.passwordHash != nil
 }
 
 // Check determines if a pubkey fingerprint is permitted.
 func (a *Auth) Check(addr net.Addr, key ssh.PublicKey, clientVersion string) error {
 	authkey := newAuthKey(key)
 
-	if a.whitelist.Len() != 0 {
+	if !a.AllowAnonymous() {
-		// Only check whitelist if there is something in it, otherwise it's disabled.
+		// Only check whitelist if we don't allow everyone to connect.
 		whitelisted := a.whitelist.In(authkey)
 		if !whitelisted {
 			return ErrNotWhitelisted
@@ -98,6 +118,18 @@ func (a *Auth) Check(addr net.Addr, key ssh.PublicKey, clientVersion string) err
 	return nil
 }
 
+// CheckPassword determines if a password is permitted.
+func (a *Auth) CheckPassword(password string) error {
+	if !a.AcceptPassword() {
+		return errors.New("passwords not accepted") // this should never happen
+	}
+	passedPasswordhash := sha256.Sum256([]byte(password))
+	if subtle.ConstantTimeCompare(passedPasswordhash[:], a.passwordHash) == 0 {
+		return errors.New("incorrect password")
+	}
+	return nil
+}
+
 // Op sets a public key as a known operator.
 func (a *Auth) Op(key ssh.PublicKey, d time.Duration) {
 	if key == nil {

auth_test.go

@@ -59,3 +59,37 @@ func TestAuthWhitelist(t *testing.T) {
 		t.Error("Failed to restrict not whitelisted:", err)
 	}
 }
+
+func TestAuthPasswords(t *testing.T) {
+	auth := NewAuth()
+
+	if auth.AcceptPassword() {
+		t.Error("Doesn't known it won't accept passwords.")
+	}
+	auth.SetPassword("")
+	if auth.AcceptPassword() {
+		t.Error("Doesn't known it won't accept passwords.")
+	}
+
+	err := auth.CheckPassword("Pa$$w0rd")
+	if err == nil {
+		t.Error("Failed to deny without password:", err)
+	}
+
+	auth.SetPassword("Pa$$w0rd")
+
+	err = auth.CheckPassword("Pa$$w0rd")
+	if err != nil {
+		t.Error("Failed to allow vaild password:", err)
+	}
+
+	err = auth.CheckPassword("something else")
+	if err == nil {
+		t.Error("Failed to restrict wrong password:", err)
+	}
+
+	auth.SetPassword("")
+	if auth.AcceptPassword() {
+		t.Error("Didn't clear password.")
+	}
+}

cmd/ssh-chat/cmd.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -10,7 +9,6 @@ import (
 	"os/signal"
 	"os/user"
 	"strings"
-	"time"
 
 	"github.com/alexcesaro/log"
 	"github.com/alexcesaro/log/golog"
@@ -123,44 +121,6 @@ func main() {
 	config.ServerVersion = "SSH-2.0-Go ssh-chat"
 	// FIXME: Should we be using config.NoClientAuth = true by default?
 
-	if options.Passphrase != "" {
-		if options.Whitelist != "" {
-			logger.Warning("Passphrase is disabled while whitelist is enabled.")
-		}
-		if config.KeyboardInteractiveCallback != nil {
-			fail(1, "Passphrase authentication conflicts with existing KeyboardInteractive setup.") // This should not happen
-		}
-
-		// We use KeyboardInteractiveCallback instead of PasswordCallback to
-		// avoid preventing the client from including a pubkey in the user
-		// identification.
-		config.KeyboardInteractiveCallback = func(conn ssh.ConnMetadata, challenge ssh.KeyboardInteractiveChallenge) (*ssh.Permissions, error) {
-			answers, err := challenge("", "", []string{"Passphrase required to connect: "}, []bool{true})
-			if err != nil {
-				return nil, err
-			}
-			if len(answers) == 1 && answers[0] == options.Passphrase {
-				// Success
-				return nil, nil
-			}
-			// It's not gonna do much but may as well throttle brute force attempts a little
-			time.Sleep(2 * time.Second)
-
-			return nil, errors.New("incorrect passphrase")
-		}
-
-		// We also need to override the PublicKeyCallback to prevent rando pubkeys from bypassing
-		cb := config.PublicKeyCallback
-		config.PublicKeyCallback = func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) {
-			perms, err := cb(conn, key)
-			if err == nil {
-				err = errors.New("passphrase authentication required")
-			}
-			return perms, err
-		}
-
-	}
-
 	s, err := sshd.ListenSSH(options.Bind, config)
 	if err != nil {
 		fail(4, "Failed to listen on socket: %v\n", err)
@@ -174,6 +134,10 @@ func main() {
 	host.SetTheme(message.Themes[0])
 	host.Version = Version
 
+	if options.Passphrase != "" {
+		auth.SetPassword(options.Passphrase)
+	}
+
 	err = fromFile(options.Admin, func(line []byte) error {
 		key, _, _, _, err := ssh.ParseAuthorizedKey(line)
 		if err != nil {

sshd/auth.go

@@ -14,8 +14,12 @@ import (
 type Auth interface {
 	// Whether to allow connections without a public key.
 	AllowAnonymous() bool
+	// If password authtication is accepted
+	AcceptPassword() bool
 	// Given address and public key and client agent string, returns nil if the connection should be allowed.
 	Check(net.Addr, ssh.PublicKey, string) error
+	// Given a password, returns nil if the connection should be allowed
+	CheckPassword(string) error
 }
 
 // MakeAuth makes an ssh.ServerConfig which performs authentication against an Auth implementation.
@@ -34,11 +38,26 @@ func MakeAuth(auth Auth) *ssh.ServerConfig {
 			}}
 			return perm, nil
 		},
+
+		// We use KeyboardInteractiveCallback instead of PasswordCallback to
+		// avoid preventing the client from including a pubkey in the user
+		// identification.
 		KeyboardInteractiveCallback: func(conn ssh.ConnMetadata, challenge ssh.KeyboardInteractiveChallenge) (*ssh.Permissions, error) {
-			if !auth.AllowAnonymous() {
+			var err error
-				return nil, errors.New("public key authentication required")
+			if auth.AcceptPassword() {
+				var answers []string
+				answers, err = challenge("", "", []string{"Passphrase required to connect: "}, []bool{true})
+				if err == nil {
+					if len(answers) != 1 {
+						err = errors.New("didn't get password")
+					} else {
+						err = auth.CheckPassword(answers[0])
+						// TODO: some kind of brute force throttling here?
+					}
+				}
+			} else {
+				err = auth.Check(conn.RemoteAddr(), nil, sanitize.Data(string(conn.ClientVersion()), 64))
 			}
-			err := auth.Check(conn.RemoteAddr(), nil, sanitize.Data(string(conn.ClientVersion()), 64))
 			return nil, err
 		},
 	}

sshd/client_test.go

@@ -15,9 +15,15 @@ type RejectAuth struct{}
 func (a RejectAuth) AllowAnonymous() bool {
 	return false
 }
+func (a RejectAuth) AcceptPassword() bool {
+	return false
+}
 func (a RejectAuth) Check(net.Addr, ssh.PublicKey, string) error {
 	return errRejectAuth
 }
+func (a RejectAuth) CheckPassword(string) error {
+	return errRejectAuth
+}
 
 func TestClientReject(t *testing.T) {
 	signer, err := NewRandomSigner(512)