/ban query support (#286)

For #285 

Turns out there were some bugs in Set, and I was using it incorrectly too.

The query syntax is a little awkward but couldn't find a nicer easy to parse format that worked with quoted string values.

auth.go

@@ -1,8 +1,11 @@
 package sshchat
 
 import (
+	"encoding/csv"
 	"errors"
+	"fmt"
 	"net"
+	"strings"
 	"time"
 
 	"github.com/shazow/ssh-chat/set"
@@ -40,19 +43,21 @@ func newAuthAddr(addr net.Addr) string {
 
 // Auth stores lookups for bans, whitelists, and ops. It implements the sshd.Auth interface.
 type Auth struct {
-	bannedAddr *set.Set
-	banned     *set.Set
-	whitelist  *set.Set
-	ops        *set.Set
+	bannedAddr   *set.Set
+	bannedClient *set.Set
+	banned       *set.Set
+	whitelist    *set.Set
+	ops          *set.Set
 }
 
 // NewAuth creates a new empty Auth.
 func NewAuth() *Auth {
 	return &Auth{
-		bannedAddr: set.New(),
-		banned:     set.New(),
-		whitelist:  set.New(),
-		ops:        set.New(),
+		bannedAddr:   set.New(),
+		bannedClient: set.New(),
+		banned:       set.New(),
+		whitelist:    set.New(),
+		ops:          set.New(),
 	}
 }
 
@@ -62,28 +67,34 @@ func (a *Auth) AllowAnonymous() bool {
 }
 
 // Check determines if a pubkey fingerprint is permitted.
-func (a *Auth) Check(addr net.Addr, key ssh.PublicKey) (bool, error) {
+func (a *Auth) Check(addr net.Addr, key ssh.PublicKey, clientVersion string) error {
 	authkey := newAuthKey(key)
 
 	if a.whitelist.Len() != 0 {
 		// Only check whitelist if there is something in it, otherwise it's disabled.
 		whitelisted := a.whitelist.In(authkey)
 		if !whitelisted {
-			return false, ErrNotWhitelisted
+			return ErrNotWhitelisted
 		}
-		return true, nil
+		return nil
 	}
 
-	banned := a.banned.In(authkey)
+	var banned bool
+	if authkey != "" {
+		banned = a.banned.In(authkey)
+	}
 	if !banned {
 		banned = a.bannedAddr.In(newAuthAddr(addr))
 	}
+	if !banned {
+		banned = a.bannedClient.In(clientVersion)
+	}
 	// Ops can bypass bans, just in case we ban ourselves.
 	if banned && !a.IsOp(key) {
-		return false, ErrBanned
+		return ErrBanned
 	}
 
-	return true, nil
+	return nil
 }
 
 // Op sets a public key as a known operator.
@@ -93,9 +104,9 @@ func (a *Auth) Op(key ssh.PublicKey, d time.Duration) {
 	}
 	authItem := newAuthItem(key)
 	if d != 0 {
-		a.ops.Add(set.Expire(authItem, d))
+		a.ops.Set(set.Expire(authItem, d))
 	} else {
-		a.ops.Add(authItem)
+		a.ops.Set(authItem)
 	}
 	logger.Debugf("Added to ops: %q (for %s)", authItem.Key(), d)
 }
@@ -116,9 +127,9 @@ func (a *Auth) Whitelist(key ssh.PublicKey, d time.Duration) {
 	}
 	authItem := newAuthItem(key)
 	if d != 0 {
-		a.whitelist.Add(set.Expire(authItem, d))
+		a.whitelist.Set(set.Expire(authItem, d))
 	} else {
-		a.whitelist.Add(authItem)
+		a.whitelist.Set(authItem)
 	}
 	logger.Debugf("Added to whitelist: %q (for %s)", authItem.Key(), d)
 }
@@ -133,33 +144,97 @@ func (a *Auth) Ban(key ssh.PublicKey, d time.Duration) {
 
 // BanFingerprint will set a public key fingerprint as banned.
 func (a *Auth) BanFingerprint(authkey string, d time.Duration) {
+	// FIXME: This is a case insensitive key, which isn't great...
 	authItem := set.StringItem(authkey)
 	if d != 0 {
-		a.banned.Add(set.Expire(authItem, d))
+		a.banned.Set(set.Expire(authItem, d))
 	} else {
-		a.banned.Add(authItem)
+		a.banned.Set(authItem)
 	}
 	logger.Debugf("Added to banned: %q (for %s)", authItem.Key(), d)
 }
 
-func (a *Auth) Banned() []string {
-	r := []string{}
-	iterGet := func(key string, _ set.Item) error {
-		r = append(r, key)
-		return nil
+// BanClient will set client version as banned. Useful for misbehaving bots.
+func (a *Auth) BanClient(client string, d time.Duration) {
+	item := set.StringItem(client)
+	if d != 0 {
+		a.bannedClient.Set(set.Expire(item, d))
+	} else {
+		a.bannedClient.Set(item)
 	}
-	a.banned.Each(iterGet)
-	a.bannedAddr.Each(iterGet)
-	return r
+	logger.Debugf("Added to banned: %q (for %s)", item.Key(), d)
+}
+
+// Banned returns the list of banned keys.
+func (a *Auth) Banned() (ip []string, fingerprint []string, client []string) {
+	a.banned.Each(func(key string, _ set.Item) error {
+		fingerprint = append(fingerprint, key)
+		return nil
+	})
+	a.bannedAddr.Each(func(key string, _ set.Item) error {
+		ip = append(ip, key)
+		return nil
+	})
+	a.bannedClient.Each(func(key string, _ set.Item) error {
+		client = append(client, key)
+		return nil
+	})
+	return
 }
 
 // Ban will set an IP address as banned.
 func (a *Auth) BanAddr(addr net.Addr, d time.Duration) {
 	authItem := set.StringItem(newAuthAddr(addr))
 	if d != 0 {
-		a.bannedAddr.Add(set.Expire(authItem, d))
+		a.bannedAddr.Set(set.Expire(authItem, d))
 	} else {
-		a.bannedAddr.Add(authItem)
+		a.bannedAddr.Set(authItem)
 	}
 	logger.Debugf("Added to bannedAddr: %q (for %s)", authItem.Key(), d)
 }
+
+// BanQuery takes space-separated key="value" pairs to ban, including ip, fingerprint, client.
+// Fields without an = will be treated as a duration, applied to the next field.
+// For example: 5s client=foo 10min ip=1.1.1.1
+// Will ban client foo for 5 seconds, and ip 1.1.1.1 for 10min.
+func (a *Auth) BanQuery(q string) error {
+	r := csv.NewReader(strings.NewReader(q))
+	r.Comma = ' '
+	fields, err := r.Read()
+	if err != nil {
+		return err
+	}
+
+	var d time.Duration
+	if last := fields[len(fields)-1]; !strings.Contains(last, "=") {
+		d, err = time.ParseDuration(last)
+		if err != nil {
+			return err
+		}
+		fields = fields[:len(fields)-1]
+	}
+	for _, field := range fields {
+		parts := strings.SplitN(field, "=", 2)
+		if len(parts) != 2 {
+			return fmt.Errorf("invalid query: %q", q)
+		}
+		key, value := parts[0], parts[1]
+		switch key {
+		case "client":
+			a.BanClient(value, d)
+		case "fingerprint":
+			// TODO: Add a validity check?
+			a.BanFingerprint(value, d)
+		case "ip":
+			ip := net.ParseIP(value)
+			if ip.String() == "" {
+				return fmt.Errorf("invalid ip value: %q", ip)
+			}
+			a.BanAddr(&net.TCPAddr{IP: ip}, d)
+		default:
+			return fmt.Errorf("unknown query field: %q", field)
+		}
+	}
+
+	return nil
+}

auth_test.go

@@ -28,8 +28,8 @@ func TestAuthWhitelist(t *testing.T) {
 	}
 
 	auth := NewAuth()
-	ok, err := auth.Check(nil, key)
-	if !ok || err != nil {
+	err = auth.Check(nil, key, "")
+	if err != nil {
 		t.Error("Failed to permit in default state:", err)
 	}
 
@@ -44,8 +44,8 @@ func TestAuthWhitelist(t *testing.T) {
 		t.Error("Clone key does not match.")
 	}
 
-	ok, err = auth.Check(nil, keyClone)
-	if !ok || err != nil {
+	err = auth.Check(nil, keyClone, "")
+	if err != nil {
 		t.Error("Failed to permit whitelisted:", err)
 	}
 
@@ -54,9 +54,8 @@ func TestAuthWhitelist(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	ok, err = auth.Check(nil, key2)
-	if ok || err == nil {
+	err = auth.Check(nil, key2, "")
+	if err == nil {
 		t.Error("Failed to restrict not whitelisted:", err)
 	}
-
 }

chat/command.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/shazow/ssh-chat/chat/message"
+	"github.com/shazow/ssh-chat/internal/sanitize"
 	"github.com/shazow/ssh-chat/set"
 )
 
@@ -155,7 +156,7 @@ func InitCommands(c *Commands) {
 			}
 
 			oldID := member.ID()
-			newID := SanitizeName(args[0])
+			newID := sanitize.Name(args[0])
 			if newID == oldID {
 				return errors.New("new name is the same as the original")
 			}

go.mod

@@ -31,8 +31,8 @@ require (
 	github.com/zmb3/gogetdoc v0.0.0-20181208215853-c5ca8f4d4936 // indirect
 	golang.org/x/arch v0.0.0-20181203225421-5a4828bb7045 // indirect
 	golang.org/x/crypto v0.0.0-20180904163835-0709b304e793
-	golang.org/x/lint v0.0.0-20181212231659-93c0bb5c8393 // indirect
-	golang.org/x/tools v0.0.0-20181214171254-3c39ce7b6105 // indirect
+	golang.org/x/lint v0.0.0-20181217174547-8f45f776aaf1 // indirect
+	golang.org/x/tools v0.0.0-20181221235234-d00ac6d27372 // indirect
 	gopkg.in/alecthomas/kingpin.v3-unstable v3.0.0-20180810215634-df19058c872c // indirect
 	gopkg.in/yaml.v2 v2.2.2 // indirect
 	honnef.co/go/tools v0.0.0-20180920025451-e3ad64cb4ed3 // indirect

go.sum

@@ -77,6 +77,8 @@ golang.org/x/crypto v0.0.0-20180904163835-0709b304e793 h1:u+LnwYTOOW7Ukr/fppxEb1
 golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
 golang.org/x/lint v0.0.0-20181212231659-93c0bb5c8393 h1:dGRlBktj39730qkqD0/XX5lfeyP6d8Mcn0W0VmIAwnU=
 golang.org/x/lint v0.0.0-20181212231659-93c0bb5c8393/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
+golang.org/x/lint v0.0.0-20181217174547-8f45f776aaf1 h1:rJm0LuqUjoDhSk2zO9ISMSToQxGz7Os2jRiOL8AWu4c=
+golang.org/x/lint v0.0.0-20181217174547-8f45f776aaf1/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
 golang.org/x/sys v0.0.0-20180117170059-2c42eef0765b h1:mxo/dXmtEd5rXc/ZzMKg0qDhMT+51+LvV65S9dP6nh4=
 golang.org/x/sys v0.0.0-20180117170059-2c42eef0765b/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33 h1:I6FyU15t786LL7oL/hn43zqTuEGr4PN7F4XJ1p4E3Y8=
@@ -87,6 +89,8 @@ golang.org/x/tools v0.0.0-20181130195746-895048a75ecf/go.mod h1:n7NCudcB/nEzxVGm
 golang.org/x/tools v0.0.0-20181207195948-8634b1ecd393/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/tools v0.0.0-20181214171254-3c39ce7b6105 h1:kFsnkWrmuEx8NF7fFPXVUvSHzRcmD/9TevF5wNmXizs=
 golang.org/x/tools v0.0.0-20181214171254-3c39ce7b6105/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/tools v0.0.0-20181221235234-d00ac6d27372 h1:zWPUEY/PjVHT+zO3L8OfkjrtIjf55joTxn/RQP/AjOI=
+golang.org/x/tools v0.0.0-20181221235234-d00ac6d27372/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 gopkg.in/alecthomas/kingpin.v3-unstable v3.0.0-20180810215634-df19058c872c h1:vTxShRUnK60yd8DZU+f95p1zSLj814+5CuEh7NjF2/Y=
 gopkg.in/alecthomas/kingpin.v3-unstable v3.0.0-20180810215634-df19058c872c/go.mod h1:3HH7i1SgMqlzxCcBmUHW657sD4Kvv9sC3HpL3YukzwA=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

host.go

@@ -418,8 +418,8 @@ func (h *Host) InitCommands(c *chat.Commands) {
 	c.Add(chat.Command{
 		Op:         true,
 		Prefix:     "/ban",
-		PrefixHelp: "USER [DURATION]",
-		Help:       "Ban USER from the server.",
+		PrefixHelp: "QUERY [DURATION]",
+		Help:       "Ban from the server. QUERY can be a username to ban the fingerprint and ip, or quoted \"key=value\" pairs with keys like ip, fingerprint, client.",
 		Handler: func(room *chat.Room, msg message.CommandMsg) error {
 			// TODO: Would be nice to specify what to ban. Key? Ip? etc.
 			if !room.IsOp(msg.From()) {
@@ -431,8 +431,13 @@ func (h *Host) InitCommands(c *chat.Commands) {
 				return errors.New("must specify user")
 			}
 
-			target, ok := h.GetUser(args[0])
+			query := args[0]
+			target, ok := h.GetUser(query)
 			if !ok {
+				query = strings.Join(args, " ")
+				if strings.Contains(query, "=") {
+					return h.auth.BanQuery(query)
+				}
 				return errors.New("user not found")
 			}
 
@@ -464,12 +469,18 @@ func (h *Host) InitCommands(c *chat.Commands) {
 				return errors.New("must be op")
 			}
 
-			banned := h.auth.Banned()
+			bannedIPs, bannedFingerprints, bannedClients := h.auth.Banned()
 
 			buf := bytes.Buffer{}
-			fmt.Fprintf(&buf, "Banned:\n")
-			for _, key := range banned {
-				fmt.Fprintf(&buf, "   %s\n", key)
+			fmt.Fprintf(&buf, "Banned:")
+			for _, key := range bannedIPs {
+				fmt.Fprintf(&buf, "\n   \"ip=%s\"", key)
+			}
+			for _, key := range bannedFingerprints {
+				fmt.Fprintf(&buf, "\n   \"fingerprint=%s\"", key)
+			}
+			for _, key := range bannedClients {
+				fmt.Fprintf(&buf, "\n   \"client=%s\"", key)
 			}
 
 			room.Send(message.NewSystemMsg(buf.String(), msg.From()))

identity.go

@@ -4,8 +4,8 @@ import (
 	"net"
 	"time"
 
-	"github.com/shazow/ssh-chat/chat"
 	"github.com/shazow/ssh-chat/chat/message"
+	"github.com/shazow/ssh-chat/internal/sanitize"
 	"github.com/shazow/ssh-chat/sshd"
 )
 
@@ -20,7 +20,7 @@ type Identity struct {
 func NewIdentity(conn sshd.Connection) *Identity {
 	return &Identity{
 		Connection: conn,
-		id:         chat.SanitizeName(conn.Name()),
+		id:         sanitize.Name(conn.Name()),
 		created:    time.Now(),
 	}
 }
@@ -49,7 +49,7 @@ func (i Identity) Whois() string {
 	}
 	return "name: " + i.Name() + message.Newline +
 		" > fingerprint: " + fingerprint + message.Newline +
-		" > client: " + chat.SanitizeData(string(i.ClientVersion())) + message.Newline +
+		" > client: " + sanitize.Data(string(i.ClientVersion()), 64) + message.Newline +
 		" > joined: " + humanSince(time.Since(i.created)) + " ago"
 }
 
@@ -63,6 +63,6 @@ func (i Identity) WhoisAdmin() string {
 	return "name: " + i.Name() + message.Newline +
 		" > ip: " + ip + message.Newline +
 		" > fingerprint: " + fingerprint + message.Newline +
-		" > client: " + chat.SanitizeData(string(i.ClientVersion())) + message.Newline +
+		" > client: " + sanitize.Data(string(i.ClientVersion()), 64) + message.Newline +
 		" > joined: " + humanSince(time.Since(i.created)) + " ago"
 }

internal/sanitize/sanitize.go

@@ -1,4 +1,4 @@
-package chat
+package sanitize
 
 import "regexp"
 
@@ -6,8 +6,8 @@ var reStripName = regexp.MustCompile("[^\\w.-]")
 
 const maxLength = 16
 
-// SanitizeName returns a name with only allowed characters and a reasonable length
-func SanitizeName(s string) string {
+// Name returns a name with only allowed characters and a reasonable length
+func Name(s string) string {
 	s = reStripName.ReplaceAllString(s, "")
 	nameLength := maxLength
 	if len(s) <= maxLength {
@@ -19,7 +19,10 @@ func SanitizeName(s string) string {
 
 var reStripData = regexp.MustCompile("[^[:ascii:]]|[[:cntrl:]]")
 
-// SanitizeData returns a string with only allowed characters for client-provided metadata inputs.
-func SanitizeData(s string) string {
+// Data returns a string with only allowed characters for client-provided metadata inputs.
+func Data(s string, maxlen int) string {
+	if len(s) > maxlen {
+		s = s[:maxlen]
+	}
 	return reStripData.ReplaceAllString(s, "")
 }

set/set.go

@@ -55,6 +55,7 @@ func (s *Set) In(key string) bool {
 	s.RUnlock()
 	if ok && item.Value() == nil {
 		s.cleanup(key)
+		ok = false
 	}
 	return ok
 }
@@ -105,6 +106,19 @@ func (s *Set) Add(item Item) error {
 	return nil
 }
 
+// Set item to this set, even if it already exists.
+func (s *Set) Set(item Item) error {
+	if item.Value() == nil {
+		return ErrNil
+	}
+	key := s.normalize(item.Key())
+
+	s.Lock()
+	defer s.Unlock()
+	s.lookup[key] = item
+	return nil
+}
+
 // Remove item from this set.
 func (s *Set) Remove(key string) error {
 	key = s.normalize(key)

sshd/auth.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"net"
 
+	"github.com/shazow/ssh-chat/internal/sanitize"
 	"golang.org/x/crypto/ssh"
 )
 
@@ -13,8 +14,8 @@ import (
 type Auth interface {
 	// Whether to allow connections without a public key.
 	AllowAnonymous() bool
-	// Given address and public key, return if the connection should be permitted.
-	Check(net.Addr, ssh.PublicKey) (bool, error)
+	// Given address and public key and client agent string, returns nil if the connection should be allowed.
+	Check(net.Addr, ssh.PublicKey, string) error
 }
 
 // MakeAuth makes an ssh.ServerConfig which performs authentication against an Auth implementation.
@@ -23,8 +24,8 @@ func MakeAuth(auth Auth) *ssh.ServerConfig {
 		NoClientAuth: false,
 		// Auth-related things should be constant-time to avoid timing attacks.
 		PublicKeyCallback: func(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) {
-			ok, err := auth.Check(conn.RemoteAddr(), key)
-			if !ok {
+			err := auth.Check(conn.RemoteAddr(), key, sanitize.Data(string(conn.ClientVersion()), 64))
+			if err != nil {
 				return nil, err
 			}
 			perm := &ssh.Permissions{Extensions: map[string]string{
@@ -36,7 +37,7 @@ func MakeAuth(auth Auth) *ssh.ServerConfig {
 			if !auth.AllowAnonymous() {
 				return nil, errors.New("public key authentication required")
 			}
-			_, err := auth.Check(conn.RemoteAddr(), nil)
+			err := auth.Check(conn.RemoteAddr(), nil, sanitize.Data(string(conn.ClientVersion()), 64))
 			return nil, err
 		},
 	}

sshd/client_test.go

@@ -15,8 +15,8 @@ type RejectAuth struct{}
 func (a RejectAuth) AllowAnonymous() bool {
 	return false
 }
-func (a RejectAuth) Check(net.Addr, ssh.PublicKey) (bool, error) {
-	return false, errRejectAuth
+func (a RejectAuth) Check(net.Addr, ssh.PublicKey, string) error {
+	return errRejectAuth
 }
 
 func TestClientReject(t *testing.T) {