git » chasquid » commit ad25706

Normalize local usernames using PRECIS

author Alberto Bertogli
2016-10-09 12:18:19 UTC
committer Alberto Bertogli
2016-10-09 23:51:05 UTC
parent 220b5d20ffc7d78620228943a7c83dab31c2c37a

Normalize local usernames using PRECIS

This patch implements local username normalization using PRECIS
(https://tools.ietf.org/html/rfc7564,
https://tools.ietf.org/html/rfc7613)

It makes chasquid accept local email and authentication regardless of
the case. It covers both userdb and aliases.

Note that non-local usernames remain untouched.

chasquid.go +10 -2
cmd/chasquid-util/chasquid-util.go +7 -1
internal/aliases/aliases.go +5 -0
internal/auth/auth.go +12 -6
internal/normalize/normalize.go +31 -0
internal/normalize/normalize_test.go +64 -0
internal/userdb/userdb.go +4 -28
internal/userdb/userdb_test.go +15 -6

diff --git a/chasquid.go b/chasquid.go
index c12ff2f..e7b39ad 100644
--- a/chasquid.go
+++ b/chasquid.go
@@ -25,6 +25,7 @@ import (
 	"blitiri.com.ar/go/chasquid/internal/config"
 	"blitiri.com.ar/go/chasquid/internal/courier"
 	"blitiri.com.ar/go/chasquid/internal/envelope"
+	"blitiri.com.ar/go/chasquid/internal/normalize"
 	"blitiri.com.ar/go/chasquid/internal/queue"
 	"blitiri.com.ar/go/chasquid/internal/set"
 	"blitiri.com.ar/go/chasquid/internal/spf"
@@ -738,8 +739,15 @@ func (c *Conn) RCPT(params string) (code int, msg string) {
 		return 503, "relay not allowed"
 	}
 
-	if localDst && !c.userExists(addr) {
-		return 550, "recipient unknown, please check the address for typos"
+	if localDst {
+		addr, err = normalize.Addr(addr)
+		if err != nil {
+			return 550, "recipient invalid, please check the address for typos"
+		}
+
+		if !c.userExists(addr) {
+			return 550, "recipient unknown, please check the address for typos"
+		}
 	}
 
 	c.rcptTo = append(c.rcptTo, addr)
diff --git a/cmd/chasquid-util/chasquid-util.go b/cmd/chasquid-util/chasquid-util.go
index 50df5b1..eca4aaf 100644
--- a/cmd/chasquid-util/chasquid-util.go
+++ b/cmd/chasquid-util/chasquid-util.go
@@ -11,6 +11,7 @@ import (
 
 	"blitiri.com.ar/go/chasquid/internal/aliases"
 	"blitiri.com.ar/go/chasquid/internal/config"
+	"blitiri.com.ar/go/chasquid/internal/normalize"
 	"blitiri.com.ar/go/chasquid/internal/userdb"
 
 	"github.com/docopt/docopt-go"
@@ -77,9 +78,14 @@ func AddUser() {
 		}
 	}
 
+	user, err := normalize.User(args["<username>"].(string))
+	if err != nil {
+		Fatalf("Error normalizing user: %v", err)
+	}
+
 	password := getPassword()
 
-	err = db.AddUser(args["<username>"].(string), password)
+	err = db.AddUser(user, password)
 	if err != nil {
 		Fatalf("Error adding user: %v", err)
 	}
diff --git a/internal/aliases/aliases.go b/internal/aliases/aliases.go
index db05402..d5ead18 100644
--- a/internal/aliases/aliases.go
+++ b/internal/aliases/aliases.go
@@ -19,6 +19,8 @@
 // is a tradeoff between flexibility and keeping the file format easy to edit
 // for people.
 //
+// User names will be normalized internally to lower-case.
+//
 // Usually there will be one database per domain, and there's no need to
 // include the "@" in the user (in this case, "@" will be forbidden).
 //
@@ -59,6 +61,7 @@ import (
 	"sync"
 
 	"blitiri.com.ar/go/chasquid/internal/envelope"
+	"blitiri.com.ar/go/chasquid/internal/normalize"
 )
 
 // Recipient represents a single recipient, after resolving aliases.
@@ -176,6 +179,7 @@ func (v *Resolver) cleanIfLocal(addr string) string {
 
 	user = removeAllAfter(user, v.SuffixSep)
 	user = removeChars(user, v.DropChars)
+	user, _ = normalize.User(user)
 	return user + "@" + domain
 }
 
@@ -277,6 +281,7 @@ func parseFile(domain, path string) (map[string][]Recipient, error) {
 		}
 
 		addr = addr + "@" + domain
+		addr, _ = normalize.Addr(addr)
 
 		if rawalias[0] == '|' {
 			cmd := strings.TrimSpace(rawalias[1:])
diff --git a/internal/auth/auth.go b/internal/auth/auth.go
index d9fcb26..2c7caec 100644
--- a/internal/auth/auth.go
+++ b/internal/auth/auth.go
@@ -7,8 +7,10 @@ import (
 	"math/rand"
 	"strings"
 	"time"
-	"unicode/utf8"
 
+	"golang.org/x/net/idna"
+
+	"blitiri.com.ar/go/chasquid/internal/normalize"
 	"blitiri.com.ar/go/chasquid/internal/userdb"
 )
 
@@ -73,11 +75,15 @@ func DecodeResponse(response string) (user, domain, passwd string, err error) {
 	user = idsp[0]
 	domain = idsp[1]
 
-	// TODO: Quedamos aca. Validar dominio no (solo) como utf8, sino ver que
-	// no contenga ni "/" ni "..". Podemos usar golang.org/x/net/idna para
-	// convertirlo a unicode primero, o al reves. No se que queremos.
-	if !utf8.ValidString(user) || !utf8.ValidString(domain) {
-		err = fmt.Errorf("User/domain is not valid UTF-8")
+	// Normalize the user and domain. This is so users can write the username
+	// in their own style and still can log in.  For the domain, we use IDNA
+	// to turn it to utf8 which is what we use internally.
+	user, err = normalize.User(user)
+	if err != nil {
+		return
+	}
+	domain, err = idna.ToUnicode(domain)
+	if err != nil {
 		return
 	}
 
diff --git a/internal/normalize/normalize.go b/internal/normalize/normalize.go
new file mode 100644
index 0000000..1dce214
--- /dev/null
+++ b/internal/normalize/normalize.go
@@ -0,0 +1,31 @@
+// Package normalize contains functions to normalize usernames and addresses.
+package normalize
+
+import (
+	"blitiri.com.ar/go/chasquid/internal/envelope"
+	"golang.org/x/text/secure/precis"
+)
+
+// User normalices an username using PRECIS.
+// On error, it will also return the original username to simplify callers.
+func User(user string) (string, error) {
+	norm, err := precis.UsernameCaseMapped.String(user)
+	if err != nil {
+		return user, err
+	}
+
+	return norm, nil
+}
+
+// Name normalices an email address using PRECIS.
+// On error, it will also return the original address to simplify callers.
+func Addr(addr string) (string, error) {
+	user, domain := envelope.Split(addr)
+
+	user, err := User(user)
+	if err != nil {
+		return addr, err
+	}
+
+	return user + "@" + domain, nil
+}
diff --git a/internal/normalize/normalize_test.go b/internal/normalize/normalize_test.go
new file mode 100644
index 0000000..96f0300
--- /dev/null
+++ b/internal/normalize/normalize_test.go
@@ -0,0 +1,64 @@
+package normalize
+
+import "testing"
+
+func TestUser(t *testing.T) {
+	valid := []struct{ user, norm string }{
+		{"ÑAndÚ", "ñandú"},
+		{"Pingüino", "pingüino"},
+	}
+	for _, c := range valid {
+		nu, err := User(c.user)
+		if nu != c.norm {
+			t.Errorf("%q normalized to %q, expected %q", c.user, nu, c.norm)
+		}
+		if err != nil {
+			t.Errorf("%q error: %v", c.user, err)
+		}
+
+	}
+
+	invalid := []string{
+		"á é", "a\te", "x ", "x\xa0y", "x\x85y", "x\vy", "x\fy", "x\ry",
+		"henry\u2163", "\u265a", "\u00b9",
+	}
+	for _, u := range invalid {
+		nu, err := User(u)
+		if err == nil {
+			t.Errorf("expected User(%+q) to fail, but did not", u)
+		}
+		if nu != u {
+			t.Errorf("%+q failed norm, but returned %+q", u, nu)
+		}
+	}
+}
+
+func TestAddr(t *testing.T) {
+	valid := []struct{ user, norm string }{
+		{"ÑAndÚ@pampa", "ñandú@pampa"},
+		{"Pingüino@patagonia", "pingüino@patagonia"},
+	}
+	for _, c := range valid {
+		nu, err := Addr(c.user)
+		if nu != c.norm {
+			t.Errorf("%q normalized to %q, expected %q", c.user, nu, c.norm)
+		}
+		if err != nil {
+			t.Errorf("%q error: %v", c.user, err)
+		}
+
+	}
+
+	invalid := []string{
+		"á é@i", "henry\u2163@throne",
+	}
+	for _, u := range invalid {
+		nu, err := Addr(u)
+		if err == nil {
+			t.Errorf("expected Addr(%+q) to fail, but did not", u)
+		}
+		if nu != u {
+			t.Errorf("%+q failed norm, but returned %+q", u, nu)
+		}
+	}
+}
diff --git a/internal/userdb/userdb.go b/internal/userdb/userdb.go
index 9089a1b..182cc72 100644
--- a/internal/userdb/userdb.go
+++ b/internal/userdb/userdb.go
@@ -37,12 +37,11 @@ import (
 	"crypto/rand"
 	"errors"
 	"fmt"
-	"strings"
 	"sync"
-	"unicode/utf8"
 
 	"golang.org/x/crypto/scrypt"
 
+	"blitiri.com.ar/go/chasquid/internal/normalize"
 	"blitiri.com.ar/go/chasquid/internal/protoio"
 )
 
@@ -55,7 +54,7 @@ type DB struct {
 }
 
 var (
-	ErrInvalidUsername = errors.New("username contains invalid characters")
+	ErrInvalidUsername = errors.New("invalid username")
 )
 
 func New(fname string) *DB {
@@ -107,15 +106,6 @@ func (db *DB) Write() error {
 	return protoio.WriteTextMessage(db.fname, db.db, 0660)
 }
 
-// Does this user exist in the database?
-func (db *DB) Exists(user string) bool {
-	db.mu.RLock()
-	_, ok := db.db.Users[user]
-	db.mu.RUnlock()
-
-	return ok
-}
-
 // Is this password valid for the user?
 func (db *DB) Authenticate(name, plainPassword string) bool {
 	db.mu.RLock()
@@ -142,23 +132,10 @@ func (p *Password) PasswordMatches(plain string) bool {
 	}
 }
 
-// Check if the given user name is valid.
-// User names have to be UTF-8, and must not have some particular characters,
-// including whitespace.
-func ValidUsername(name string) bool {
-	return utf8.ValidString(name) &&
-		!strings.ContainsAny(name, illegalUsernameChars)
-}
-
-// Illegal characters. Only whitespace for now, to prevent/minimize the
-// chances of parsing issues.
-// TODO: do we want to stop other characters, specifically about email? Or
-// keep this generic and handle the mail-specific filtering in chasquid?
-const illegalUsernameChars = "\t\n\v\f\r \xa0\x85"
-
 // Add a user to the database. If the user is already present, override it.
+// Note we enforce that the name has been normalized previously.
 func (db *DB) AddUser(name, plainPassword string) error {
-	if !ValidUsername(name) {
+	if norm, err := normalize.User(name); err != nil || name != norm {
 		return ErrInvalidUsername
 	}
 
@@ -195,7 +172,6 @@ func (db *DB) AddUser(name, plainPassword string) error {
 // otherwise.
 func (db *DB) RemoveUser(name string) bool {
 	db.mu.Lock()
-
 	_, present := db.db.Users[name]
 	delete(db.db.Users, name)
 	db.mu.Unlock()
diff --git a/internal/userdb/userdb_test.go b/internal/userdb/userdb_test.go
index 7af4677..f132933 100644
--- a/internal/userdb/userdb_test.go
+++ b/internal/userdb/userdb_test.go
@@ -129,7 +129,7 @@ func TestWrite(t *testing.T) {
 
 	db = mustLoad(t, fname)
 	for _, name := range []string{"user1", "ñoño"} {
-		if !db.Exists(name) {
+		if !db.HasUser(name) {
 			t.Errorf("user %q not in database", name)
 		}
 		if db.db.Users[name].GetScheme() == nil {
@@ -179,8 +179,17 @@ func TestInvalidUsername(t *testing.T) {
 	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
+	// Names that are invalid.
 	names := []string{
-		" ", "  ", "a b", "ñ ñ", "a\xa0b", "a\x85b", "a\nb", "a\tb", "a\xffb"}
+		// Contain various types of spaces.
+		" ", "  ", "a b", "ñ ñ", "a\xa0b", "a\x85b", "a\nb", "a\tb", "a\xffb",
+
+		// Contain characters not allowed by PRECIS.
+		"\u00b9", "\u2163",
+
+		// Names that are not normalized, but would otherwise be valid.
+		"A", "Ñ",
+	}
 	for _, name := range names {
 		err := db.AddUser(name, "passwd")
 		if err == nil {
@@ -289,7 +298,7 @@ func TestHasUser(t *testing.T) {
 	defer removeIfSuccessful(t, fname)
 	db := mustLoad(t, fname)
 
-	if ok := db.HasUser("unknown"); ok {
+	if db.HasUser("unknown") {
 		t.Errorf("unknown user exists")
 	}
 
@@ -297,15 +306,15 @@ func TestHasUser(t *testing.T) {
 		t.Fatalf("error adding user: %v", err)
 	}
 
-	if ok := db.HasUser("unknown"); ok {
+	if db.HasUser("unknown") {
 		t.Errorf("unknown user exists")
 	}
 
-	if ok := db.HasUser("user"); !ok {
+	if !db.HasUser("user") {
 		t.Errorf("known user does not exist")
 	}
 
-	if ok := db.HasUser("user"); !ok {
+	if !db.HasUser("user") {
 		t.Errorf("known user does not exist")
 	}
 }