Skip to content

Commit

Permalink
prevent unicode-confusion in password by applying PRECIS, and usernam…
Browse files Browse the repository at this point in the history
…e/email address by applying unicode NFC normalization

an é (e with accent) can also be written as e+\u0301. the first form is NFC,
the second NFD. when logging in, we transform usernames (email addresses) to
NFC. so both forms will be accepted. if a client is using NFD, they can log
in too.

for passwords, we apply the PRECIS "opaquestring", which (despite the name)
transforms the value too: unicode spaces are replaced with ascii spaces. the
string is also normalized to NFC. PRECIS may reject confusing passwords when
you set a password.
  • Loading branch information
mjl- committed Mar 9, 2024
1 parent 8e6fe74 commit c57aeac
Show file tree
Hide file tree
Showing 99 changed files with 59,625 additions and 114 deletions.
12 changes: 6 additions & 6 deletions imapserver/append_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ func TestAppend(t *testing.T) {
tc3 := startNoSwitchboard(t)
defer tc3.close()

tc2.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", password0)
tc2.client.Select("inbox")
tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc.client.Select("inbox")
tc3.client.Login("mjl@mox.example", "testtest")
tc3.client.Login("mjl@mox.example", password0)

tc2.transactf("bad", "append") // Missing params.
tc2.transactf("bad", `append inbox`) // Missing message.
Expand All @@ -32,13 +32,13 @@ func TestAppend(t *testing.T) {
tc2.transactf("bad", "append inbox (\\Badflag) {1+}\r\nx") // Unknown flag.
tc2 = startNoSwitchboard(t)
defer tc2.close()
tc2.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", password0)
tc2.client.Select("inbox")

tc2.transactf("bad", "append inbox () \"bad time\" {1+}\r\nx") // Bad time.
tc2 = startNoSwitchboard(t)
defer tc2.close()
tc2.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", password0)
tc2.client.Select("inbox")

tc2.transactf("no", "append nobox (\\Seen) \" 1-Jan-2022 10:10:00 +0100\" {1}")
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestAppend(t *testing.T) {

tclimit := startArgs(t, false, false, true, true, "limit")
defer tclimit.close()
tclimit.client.Login("limit@mox.example", "testtest")
tclimit.client.Login("limit@mox.example", password0)
tclimit.client.Select("inbox")
// First message of 1 byte is within limits.
tclimit.transactf("ok", "append inbox (\\Seen Label1 $label2) \" 1-Jan-2022 10:10:00 +0100\" {1+}\r\nx")
Expand Down
47 changes: 37 additions & 10 deletions imapserver/authenticate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,18 @@ import (
"strings"
"testing"

"golang.org/x/text/secure/precis"

"github.com/mjl-/mox/scram"
)

func TestAuthenticateLogin(t *testing.T) {
// NFD username and PRECIS-cleaned password.
tc := start(t)
tc.client.Login("mo\u0301x@mox.example", password1)
tc.close()
}

func TestAuthenticatePlain(t *testing.T) {
tc := start(t)

Expand All @@ -28,21 +37,26 @@ func TestAuthenticatePlain(t *testing.T) {
tc.xcode("AUTHENTICATIONFAILED")
tc.transactf("no", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000test")))
tc.xcode("AUTHENTICATIONFAILED")
tc.transactf("no", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000testtesttest")))
tc.transactf("no", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000test"+password0)))
tc.xcode("AUTHENTICATIONFAILED")
tc.transactf("bad", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000")))
tc.xcode("")
tc.transactf("no", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("other\u0000mjl@mox.example\u0000testtest")))
tc.transactf("no", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("other\u0000mjl@mox.example\u0000"+password0)))
tc.xcode("AUTHORIZATIONFAILED")
tc.transactf("ok", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000testtest")))
tc.transactf("ok", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000"+password0)))
tc.close()

tc = start(t)
tc.transactf("ok", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("mjl@mox.example\u0000mjl@mox.example\u0000"+password0)))
tc.close()

// NFD username and PRECIS-cleaned password.
tc = start(t)
tc.transactf("ok", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("mjl@mox.example\u0000mjl@mox.example\u0000testtest")))
tc.transactf("ok", "authenticate plain %s", base64.StdEncoding.EncodeToString([]byte("mo\u0301x@mox.example\u0000mo\u0301x@mox.example\u0000"+password1)))
tc.close()

tc = start(t)
tc.client.AuthenticatePlain("mjl@mox.example", "testtest")
tc.client.AuthenticatePlain("mjl@mox.example", password0)
tc.close()

tc = start(t)
Expand All @@ -55,7 +69,7 @@ func TestAuthenticatePlain(t *testing.T) {

tc.cmdf("", "authenticate plain")
tc.readprefixline("+ ")
tc.writelinef("%s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000testtest")))
tc.writelinef("%s", base64.StdEncoding.EncodeToString([]byte("\u0000mjl@mox.example\u0000"+password0)))
tc.readstatus("ok")
}

Expand All @@ -77,7 +91,7 @@ func TestAuthenticateSCRAMSHA256PLUS(t *testing.T) {

func testAuthenticateSCRAM(t *testing.T, tls bool, method string, h func() hash.Hash) {
tc := startArgs(t, true, tls, true, true, "mjl")
tc.client.AuthenticateSCRAM(method, h, "mjl@mox.example", "testtest")
tc.client.AuthenticateSCRAM(method, h, "mjl@mox.example", password0)
tc.close()

auth := func(status string, serverFinalError error, username, password string) {
Expand Down Expand Up @@ -129,11 +143,15 @@ func testAuthenticateSCRAM(t *testing.T, tls bool, method string, h func() hash.
auth("no", scram.ErrInvalidProof, "mjl@mox.example", "badpass")
auth("no", scram.ErrInvalidProof, "mjl@mox.example", "")
// todo: server aborts due to invalid username. we should probably make client continue with fake determinisitically generated salt and result in error in the end.
// auth("no", nil, "other@mox.example", "testtest")
// auth("no", nil, "other@mox.example", password0)

tc.transactf("no", "authenticate bogus ")
tc.transactf("bad", "authenticate %s not base64...", method)
tc.transactf("bad", "authenticate %s %s", method, base64.StdEncoding.EncodeToString([]byte("bad data")))

// NFD username, with PRECIS-cleaned password.
auth("ok", nil, "mo\u0301x@mox.example", password1)

tc.close()
}

Expand Down Expand Up @@ -163,6 +181,10 @@ func TestAuthenticateCRAMMD5(t *testing.T) {
}

chal := xreadContinuation()
pw, err := precis.OpaqueString.String(password)
if err == nil {
password = pw
}
h := hmac.New(md5.New, []byte(password))
h.Write([]byte(chal))
resp := fmt.Sprintf("%s %x", username, h.Sum(nil))
Expand All @@ -177,9 +199,14 @@ func TestAuthenticateCRAMMD5(t *testing.T) {

auth("no", "mjl@mox.example", "badpass")
auth("no", "mjl@mox.example", "")
auth("no", "other@mox.example", "testtest")
auth("no", "other@mox.example", password0)

auth("ok", "mjl@mox.example", "testtest")
auth("ok", "mjl@mox.example", password0)

tc.close()

// NFD username, with PRECIS-cleaned password.
tc = start(t)
auth("ok", "mo\u0301x@mox.example", password1)
tc.close()
}
16 changes: 8 additions & 8 deletions imapserver/condstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func testCondstoreQresync(t *testing.T, qresync bool) {
capability = "Qresync"
}

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc.client.Enable(capability)
tc.transactf("ok", "Select inbox")
tc.xuntaggedOpt(false, imapclient.UntaggedResult{Status: imapclient.OK, RespText: imapclient.RespText{Code: "HIGHESTMODSEQ", CodeArg: imapclient.CodeHighestModSeq(1), More: "x"}})
Expand Down Expand Up @@ -101,13 +101,13 @@ func testCondstoreQresync(t *testing.T, qresync bool) {
// tc2 is a client without condstore, so no modseq responses.
tc2 := startNoSwitchboard(t)
defer tc2.close()
tc2.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", password0)
tc2.client.Select("inbox")

// tc3 is a client with condstore, so with modseq responses.
tc3 := startNoSwitchboard(t)
defer tc3.close()
tc3.client.Login("mjl@mox.example", "testtest")
tc3.client.Login("mjl@mox.example", password0)
tc3.client.Enable(capability)
tc3.client.Select("inbox")

Expand Down Expand Up @@ -357,7 +357,7 @@ func testCondstoreQresync(t *testing.T, qresync bool) {
xtc.close()
store.CheckConsistencyOnClose = true
}()
xtc.client.Login("mjl@mox.example", "testtest")
xtc.client.Login("mjl@mox.example", password0)
fn(xtc)
tagcount++
label := fmt.Sprintf("l%d", tagcount)
Expand Down Expand Up @@ -444,13 +444,13 @@ func testCondstoreQresync(t *testing.T, qresync bool) {
// tc2o is a client without condstore, so no modseq responses.
tc2o := startNoSwitchboard(t)
defer tc2o.close()
tc2o.client.Login("mjl@mox.example", "testtest")
tc2o.client.Login("mjl@mox.example", password0)
tc2o.client.Select("otherbox")

// tc3o is a client with condstore, so with modseq responses.
tc3o := startNoSwitchboard(t)
defer tc3o.close()
tc3o.client.Login("mjl@mox.example", "testtest")
tc3o.client.Login("mjl@mox.example", password0)
tc3o.client.Enable(capability)
tc3o.client.Select("otherbox")

Expand Down Expand Up @@ -529,7 +529,7 @@ func testQresync(t *testing.T, tc *testconn, clientModseq int64) {

// Vanished not allowed without first enabling qresync. ../rfc/7162:1697
xtc := startNoSwitchboard(t)
xtc.client.Login("mjl@mox.example", "testtest")
xtc.client.Login("mjl@mox.example", password0)
xtc.transactf("ok", "Select inbox (Condstore)")
xtc.transactf("bad", "Uid Fetch 1:* (Flags) (Changedsince 1 Vanished)")
// Prevent triggering the consistency checker, we still have modseq/createseq at 0.
Expand All @@ -553,7 +553,7 @@ func testQresync(t *testing.T, tc *testconn, clientModseq int64) {

// Must enable qresync explicitly before using. ../rfc/7162:1446
xtc = startNoSwitchboard(t)
xtc.client.Login("mjl@mox.example", "testtest")
xtc.client.Login("mjl@mox.example", password0)
xtc.transactf("bad", "Select inbox (Qresync 1 0)")
// Prevent triggering the consistency checker, we still have modseq/createseq at 0.
store.CheckConsistencyOnClose = false
Expand Down
6 changes: 3 additions & 3 deletions imapserver/copy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ func TestCopy(t *testing.T) {
tc2 := startNoSwitchboard(t)
defer tc2.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc.client.Select("inbox")

tc2.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", password0)
tc2.client.Select("Trash")

tc.transactf("bad", "copy") // Missing params.
Expand Down Expand Up @@ -61,7 +61,7 @@ func TestCopy(t *testing.T) {

tclimit := startArgs(t, false, false, true, true, "limit")
defer tclimit.close()
tclimit.client.Login("limit@mox.example", "testtest")
tclimit.client.Login("limit@mox.example", password0)
tclimit.client.Select("inbox")
// First message of 1 byte is within limits.
tclimit.transactf("ok", "append inbox (\\Seen Label1 $label2) \" 1-Jan-2022 10:10:00 +0100\" {1+}\r\nx")
Expand Down
4 changes: 2 additions & 2 deletions imapserver/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ func TestCreate(t *testing.T) {
tc2 := startNoSwitchboard(t)
defer tc2.close()

tc.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc2.client.Login("mjl@mox.example", password0)

tc.transactf("no", "create inbox") // Already exists and not allowed. ../rfc/9051:1913
tc.transactf("no", "create Inbox") // Idem.
Expand Down
6 changes: 3 additions & 3 deletions imapserver/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ func TestDelete(t *testing.T) {
tc3 := startNoSwitchboard(t)
defer tc3.close()

tc.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", "testtest")
tc3.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc2.client.Login("mjl@mox.example", password0)
tc3.client.Login("mjl@mox.example", password0)

tc.transactf("bad", "delete") // Missing mailbox.
tc.transactf("no", "delete inbox") // Cannot delete inbox.
Expand Down
4 changes: 2 additions & 2 deletions imapserver/expunge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ func TestExpunge(t *testing.T) {
tc2 := startNoSwitchboard(t)
defer tc2.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc.client.Select("inbox")

tc2.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", password0)
tc2.client.Select("inbox")

tc.transactf("bad", "expunge leftover") // Leftover data.
Expand Down
2 changes: 1 addition & 1 deletion imapserver/fetch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func TestFetch(t *testing.T) {
tc := start(t)
defer tc.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc.client.Enable("imap4rev2")
received, err := time.Parse(time.RFC3339, "2022-11-16T10:01:00+01:00")
tc.check(err, "parse time")
Expand Down
2 changes: 1 addition & 1 deletion imapserver/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func FuzzServer(f *testing.F) {
f.Fatalf("open account: %v", err)
}
defer acc.Close()
err = acc.SetPassword(log, "testtest")
err = acc.SetPassword(log, password0)
if err != nil {
f.Fatalf("set password: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions imapserver/idle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import (
func TestIdle(t *testing.T) {
tc1 := start(t)
defer tc1.close()
tc1.transactf("ok", "login mjl@mox.example testtest")
tc1.client.Login("mjl@mox.example", password0)

tc2 := startNoSwitchboard(t)
defer tc2.close()
tc2.transactf("ok", "login mjl@mox.example testtest")
tc2.client.Login("mjl@mox.example", password0)

tc1.transactf("ok", "select inbox")
tc2.transactf("ok", "select inbox")
Expand Down
4 changes: 2 additions & 2 deletions imapserver/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ func TestListBasic(t *testing.T) {
tc := start(t)
defer tc.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)

ulist := func(name string, flags ...string) imapclient.UntaggedList {
if len(flags) == 0 {
Expand Down Expand Up @@ -61,7 +61,7 @@ func TestListExtended(t *testing.T) {
tc := start(t)
defer tc.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)

ulist := func(name string, flags ...string) imapclient.UntaggedList {
if len(flags) == 0 {
Expand Down
2 changes: 1 addition & 1 deletion imapserver/lsub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ func TestLsub(t *testing.T) {
tc := start(t)
defer tc.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)

tc.transactf("bad", "lsub") // Missing params.
tc.transactf("bad", `lsub ""`) // Missing param.
Expand Down
6 changes: 3 additions & 3 deletions imapserver/move_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ func TestMove(t *testing.T) {
tc3 := startNoSwitchboard(t)
defer tc3.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc.client.Select("inbox")

tc2.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", password0)
tc2.client.Select("Trash")

tc3.client.Login("mjl@mox.example", "testtest")
tc3.client.Login("mjl@mox.example", password0)
tc3.client.Select("inbox")

tc.transactf("bad", "move") // Missing params.
Expand Down
4 changes: 2 additions & 2 deletions imapserver/rename_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ func TestRename(t *testing.T) {
tc2 := startNoSwitchboard(t)
defer tc2.close()

tc.client.Login("mjl@mox.example", "testtest")
tc2.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc2.client.Login("mjl@mox.example", password0)

tc.transactf("bad", "rename") // Missing parameters.
tc.transactf("bad", "rename x") // Missing destination.
Expand Down
2 changes: 1 addition & 1 deletion imapserver/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (tc *testconn) xesearch(exp imapclient.UntaggedEsearch) {
func TestSearch(t *testing.T) {
tc := start(t)
defer tc.close()
tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)
tc.client.Select("inbox")

// Add 5 and delete first 4 messages. So UIDs start at 5.
Expand Down
2 changes: 1 addition & 1 deletion imapserver/selectexamine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func testSelectExamine(t *testing.T, examine bool) {
tc := start(t)
defer tc.close()

tc.client.Login("mjl@mox.example", "testtest")
tc.client.Login("mjl@mox.example", password0)

cmd := "select"
okcode := "READ-WRITE"
Expand Down

0 comments on commit c57aeac

Please sign in to comment.