author | Alberto Bertogli
<albertito@blitiri.com.ar> 2016-10-09 12:18:19 UTC |
committer | Alberto Bertogli
<albertito@blitiri.com.ar> 2016-10-09 23:51:05 UTC |
parent | 220b5d20ffc7d78620228943a7c83dab31c2c37a |
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") } }