Add /allowlist command (#399)

* move loading whitelist+ops from file to auth and save the loaded files fro reloading

* add /whitelist command with lots of open questions

* add test for /whitelist

* gofmt

* use the same auth (the tests don't seem to care, but htis is more right)

* mutex whitelistMode and remove some deferred TODOs

* s/whitelist/allowlist/ (user-facing); move helper functions outside the handler function

* check for ops in Auth.CheckPublicKey and move /allowlist handling to helper functions

* possibly fix the test timeout in HostNameCollision

* Revert "possibly fix the test timeout in HostNameCollision" (didn't work)

This reverts commit 664dbb0.

* managed to reproduce the timeout after updating, hopefully it's the same one

* remove some unimportant TODOs; add a message when reverify kicks people; add a reverify test

* add client connection with key; add test for /allowlist import AGE

* hopefully make test less racy

* s/whitelist/allowlist/

* fix crash on specifying exactly one more -v flag than the max level

* use a key loader function to move file reading out of auth

* add loader to allowlist test

* minor message changes

* add --whitelist with a warning; update tests for messages

* apparently, we have another prefix

* check names directly on the User objects in TestHostNameCollision

* not allowlisted -> not allowed

* small message change

* update test

Author: mikitsu

auth.go

@@ -8,16 +8,21 @@ import (
 	"fmt"
 	"net"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/shazow/ssh-chat/set"
 	"github.com/shazow/ssh-chat/sshd"
 	"golang.org/x/crypto/ssh"
 )
 
-// ErrNotWhitelisted Is the error returned when a key is checked that is not whitelisted,
-// when whitelisting is enabled.
-var ErrNotWhitelisted = errors.New("not whitelisted")
+// KeyLoader loads public keys, e.g. from an authorized_keys file.
+// It must return a nil slice on error.
+type KeyLoader func() ([]ssh.PublicKey, error)
+
+// ErrNotAllowed Is the error returned when a key is checked that is not allowlisted,
+// when allowlisting is enabled.
+var ErrNotAllowed = errors.New("not allowed")
 
 // ErrBanned is the error returned when a client is banned.
 var ErrBanned = errors.New("banned")
@@ -47,15 +52,20 @@ func newAuthAddr(addr net.Addr) string {
 	return host
 }
 
-// Auth stores lookups for bans, whitelists, and ops. It implements the sshd.Auth interface.
-// If the contained passphrase is not empty, it complements a whitelist.
+// Auth stores lookups for bans, allowlists, and ops. It implements the sshd.Auth interface.
+// If the contained passphrase is not empty, it complements a allowlist.
 type Auth struct {
 	passphraseHash []byte
 	bannedAddr     *set.Set
 	bannedClient   *set.Set
 	banned         *set.Set
-	whitelist      *set.Set
+	allowlist      *set.Set
 	ops            *set.Set
+
+	settingsMu      sync.RWMutex
+	allowlistMode   bool
+	opLoader        KeyLoader
+	allowlistLoader KeyLoader
 }
 
 // NewAuth creates a new empty Auth.
@@ -64,11 +74,23 @@ func NewAuth() *Auth {
 		bannedAddr:   set.New(),
 		bannedClient: set.New(),
 		banned:       set.New(),
-		whitelist:    set.New(),
+		allowlist:    set.New(),
 		ops:          set.New(),
 	}
 }
 
+func (a *Auth) AllowlistMode() bool {
+	a.settingsMu.RLock()
+	defer a.settingsMu.RUnlock()
+	return a.allowlistMode
+}
+
+func (a *Auth) SetAllowlistMode(value bool) {
+	a.settingsMu.Lock()
+	defer a.settingsMu.Unlock()
+	a.allowlistMode = value
+}
+
 // SetPassphrase enables passphrase authentication with the given passphrase.
 // If an empty passphrase is given, disable passphrase authentication.
 func (a *Auth) SetPassphrase(passphrase string) {
@@ -82,7 +104,7 @@ func (a *Auth) SetPassphrase(passphrase string) {
 
 // AllowAnonymous determines if anonymous users are permitted.
 func (a *Auth) AllowAnonymous() bool {
-	return a.whitelist.Len() == 0 && a.passphraseHash == nil
+	return !a.AllowlistMode() && a.passphraseHash == nil
 }
 
 // AcceptPassphrase determines if passphrase authentication is accepted.
@@ -115,11 +137,11 @@ func (a *Auth) CheckBans(addr net.Addr, key ssh.PublicKey, clientVersion string)
 // CheckPubkey determines if a pubkey fingerprint is permitted.
 func (a *Auth) CheckPublicKey(key ssh.PublicKey) error {
 	authkey := newAuthKey(key)
-	whitelisted := a.whitelist.In(authkey)
-	if a.AllowAnonymous() || whitelisted {
+	allowlisted := a.allowlist.In(authkey)
+	if a.AllowAnonymous() || allowlisted || a.IsOp(key) {
 		return nil
 	} else {
-		return ErrNotWhitelisted
+		return ErrNotAllowed
 	}
 }
 
@@ -151,25 +173,68 @@ func (a *Auth) Op(key ssh.PublicKey, d time.Duration) {
 
 // IsOp checks if a public key is an op.
 func (a *Auth) IsOp(key ssh.PublicKey) bool {
-	if key == nil {
-		return false
-	}
 	authkey := newAuthKey(key)
 	return a.ops.In(authkey)
 }
 
-// Whitelist will set a public key as a whitelisted user.
-func (a *Auth) Whitelist(key ssh.PublicKey, d time.Duration) {
+// LoadOps sets the public keys form loader to operators and saves the loader for later use
+func (a *Auth) LoadOps(loader KeyLoader) error {
+	a.settingsMu.Lock()
+	a.opLoader = loader
+	a.settingsMu.Unlock()
+	return a.ReloadOps()
+}
+
+// ReloadOps sets the public keys from a loader saved in the last call to operators
+func (a *Auth) ReloadOps() error {
+	a.settingsMu.RLock()
+	defer a.settingsMu.RUnlock()
+	return addFromLoader(a.opLoader, a.Op)
+}
+
+// Allowlist will set a public key as a allowlisted user.
+func (a *Auth) Allowlist(key ssh.PublicKey, d time.Duration) {
 	if key == nil {
 		return
 	}
+	var err error
 	authItem := newAuthItem(key)
 	if d != 0 {
-		a.whitelist.Set(set.Expire(authItem, d))
+		err = a.allowlist.Set(set.Expire(authItem, d))
 	} else {
-		a.whitelist.Set(authItem)
+		err = a.allowlist.Set(authItem)
+	}
+	if err == nil {
+		logger.Debugf("Added to allowlist: %q (for %s)", authItem.Key(), d)
+	} else {
+		logger.Errorf("Error adding %q to allowlist for %s: %s", authItem.Key(), d, err)
+	}
+}
+
+// LoadAllowlist adds the public keys from the loader to the allowlist and saves the loader for later use
+func (a *Auth) LoadAllowlist(loader KeyLoader) error {
+	a.settingsMu.Lock()
+	a.allowlistLoader = loader
+	a.settingsMu.Unlock()
+	return a.ReloadAllowlist()
+}
+
+//  LoadAllowlist adds the public keys from a loader saved in a previous call to the allowlist
+func (a *Auth) ReloadAllowlist() error {
+	a.settingsMu.RLock()
+	defer a.settingsMu.RUnlock()
+	return addFromLoader(a.allowlistLoader, a.Allowlist)
+}
+
+func addFromLoader(loader KeyLoader, adder func(ssh.PublicKey, time.Duration)) error {
+	if loader == nil {
+		return nil
+	}
+	keys, err := loader()
+	for _, key := range keys {
+		adder(key, 0)
 	}
-	logger.Debugf("Added to whitelist: %q (for %s)", authItem.Key(), d)
+	return err
 }
 
 // Ban will set a public key as banned.

auth_test.go

@@ -21,7 +21,7 @@ func ClonePublicKey(key ssh.PublicKey) (ssh.PublicKey, error) {
 	return ssh.ParsePublicKey(key.Marshal())
 }
 
-func TestAuthWhitelist(t *testing.T) {
+func TestAuthAllowlist(t *testing.T) {
 	key, err := NewRandomPublicKey(512)
 	if err != nil {
 		t.Fatal(err)
@@ -33,7 +33,8 @@ func TestAuthWhitelist(t *testing.T) {
 		t.Error("Failed to permit in default state:", err)
 	}
 
-	auth.Whitelist(key, 0)
+	auth.Allowlist(key, 0)
+	auth.SetAllowlistMode(true)
 
 	keyClone, err := ClonePublicKey(key)
 	if err != nil {
@@ -46,7 +47,7 @@ func TestAuthWhitelist(t *testing.T) {
 
 	err = auth.CheckPublicKey(keyClone)
 	if err != nil {
-		t.Error("Failed to permit whitelisted:", err)
+		t.Error("Failed to permit allowlisted:", err)
 	}
 
 	key2, err := NewRandomPublicKey(512)
@@ -56,7 +57,7 @@ func TestAuthWhitelist(t *testing.T) {
 
 	err = auth.CheckPublicKey(key2)
 	if err == nil {
-		t.Error("Failed to restrict not whitelisted:", err)
+		t.Error("Failed to restrict not allowlisted:", err)
 	}
 }
 

cmd/ssh-chat/cmd.go

@@ -36,8 +36,9 @@ type Options struct {
 	Pprof      int      `long:"pprof" description:"Enable pprof http server for profiling."`
 	Verbose    []bool   `short:"v" long:"verbose" description:"Show verbose logging."`
 	Version    bool     `long:"version" description:"Print version and exit."`
-	Whitelist  string   `long:"whitelist" description:"Optional file of public keys who are allowed to connect."`
-	Passphrase string   `long:"unsafe-passphrase" description:"Require an interactive passphrase to connect. Whitelist feature is more secure."`
+	Allowlist  string   `long:"allowlist" description:"Optional file of public keys who are allowed to connect."`
+	Whitelist  string   `long:"whitelist" dexcription:"Old name for allowlist option"`
+	Passphrase string   `long:"unsafe-passphrase" description:"Require an interactive passphrase to connect. Allowlist feature is more secure."`
 }
 
 const extraHelp = `There are hidden options and easter eggs in ssh-chat. The source code is a good
@@ -87,7 +88,7 @@ func main() {
 
 	// Figure out the log level
 	numVerbose := len(options.Verbose)
-	if numVerbose > len(logLevels) {
+	if numVerbose >= len(logLevels) {
 		numVerbose = len(logLevels) - 1
 	}
 
@@ -141,35 +142,20 @@ func main() {
 		auth.SetPassphrase(options.Passphrase)
 	}
 
-	err = fromFile(options.Admin, func(line []byte) error {
-		key, _, _, _, err := ssh.ParseAuthorizedKey(line)
-		if err != nil {
-			if err.Error() == "ssh: no key found" {
-				return nil // Skip line
-			}
-			return err
-		}
-		auth.Op(key, 0)
-		return nil
-	})
+	err = auth.LoadOps(loaderFromFile(options.Admin, logger))
 	if err != nil {
 		fail(5, "Failed to load admins: %v\n", err)
 	}
 
-	err = fromFile(options.Whitelist, func(line []byte) error {
-		key, _, _, _, err := ssh.ParseAuthorizedKey(line)
-		if err != nil {
-			if err.Error() == "ssh: no key found" {
-				return nil // Skip line
-			}
-			return err
-		}
-		auth.Whitelist(key, 0)
-		return nil
-	})
+	if options.Allowlist == "" && options.Whitelist != "" {
+		fmt.Println("--whitelist was renamed to --allowlist.")
+		options.Allowlist = options.Whitelist
+	}
+	err = auth.LoadAllowlist(loaderFromFile(options.Allowlist, logger))
 	if err != nil {
-		fail(6, "Failed to load whitelist: %v\n", err)
+		fail(6, "Failed to load allowlist: %v\n", err)
 	}
+	auth.SetAllowlistMode(options.Allowlist != "")
 
 	if options.Motd != "" {
 		host.GetMOTD = func() (string, error) {
@@ -210,24 +196,32 @@ func main() {
 	fmt.Fprintln(os.Stderr, "Interrupt signal detected, shutting down.")
 }
 
-func fromFile(path string, handler func(line []byte) error) error {
+func loaderFromFile(path string, logger *golog.Logger) sshchat.KeyLoader {
 	if path == "" {
-		// Skip
 		return nil
 	}
-
-	file, err := os.Open(path)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	scanner := bufio.NewScanner(file)
-	for scanner.Scan() {
-		err := handler(scanner.Bytes())
+	return func() ([]ssh.PublicKey, error) {
+		file, err := os.Open(path)
 		if err != nil {
-			return err
+			return nil, err
+		}
+		defer file.Close()
+
+		var keys []ssh.PublicKey
+		scanner := bufio.NewScanner(file)
+		for scanner.Scan() {
+			key, _, _, _, err := ssh.ParseAuthorizedKey(scanner.Bytes())
+			if err != nil {
+				if err.Error() == "ssh: no key found" {
+					continue // Skip line
+				}
+				return nil, err
+			}
+			keys = append(keys, key)
+		}
+		if keys == nil {
+			logger.Warning("file", path, "contained no keys")
 		}
+		return keys, nil
 	}
-	return nil
 }