From d84c96eca585490bc1f9ca42cbcf7e364c6a766c Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Mon, 1 Jan 2024 13:15:25 +0100 Subject: [PATCH] imapserver: allow creating mailboxes with characters &#*%, and encode mailbox names in imap with imaputf7 when needed the imapserver started with imap4rev2-only and utf8=only. to prevent potential issues with imaputf7, which makes "&" special, we refused any mailbox with an "&" in the name. we already tried decoding utf7, falling back to using a mailbox name verbatim. that behaviour wasn't great. we now treat the enabled extensions IMAP4rev2 and/or UTF8=ACCEPT as indication whether mailbox names are in imaputf7. if they are, the encoding must be correct. we now also send mailbox names in imaputf7 when imap4rev2/utf8=accept isn't enabled. and we now allow "*" and "%" (wildcard characters for matching) in mailbox names. not ideal for IMAP LIST with patterns, but not enough reason to refuse them in mailbox names. people that migrate may run into this, possibly as blocker. we also allow "#" in mailbox names, but not as first character, to prevent potential clashes with IMAP namespaces in the future. based on report from Damian Poddebniak using https://github.com/duesee/imap-flow and issue #110, thanks for reporting! --- imapserver/create_test.go | 13 ++++--- imapserver/list.go | 2 +- imapserver/parse.go | 41 +++++++++++---------- imapserver/server.go | 36 ++++++++++--------- imapserver/utf7.go | 75 +++++++++++++++++++++++++++++++++++---- imapserver/utf7_test.go | 18 +++++++++- store/account.go | 16 ++++++--- 7 files changed, 150 insertions(+), 51 deletions(-) diff --git a/imapserver/create_test.go b/imapserver/create_test.go index 39837e1..23aa01b 100644 --- a/imapserver/create_test.go +++ b/imapserver/create_test.go @@ -67,8 +67,13 @@ func TestCreate(t *testing.T) { tc.transactf("bad", `create "\x9f"`) tc.transactf("bad", `create "\u2028"`) tc.transactf("bad", `create "\u2029"`) - tc.transactf("no", `create "%%"`) - tc.transactf("no", `create "*"`) - tc.transactf("no", `create "#"`) - tc.transactf("no", `create "&"`) + tc.transactf("ok", `create "%%"`) + tc.transactf("ok", `create "*"`) + tc.transactf("no", `create "#"`) // Leading hash not allowed. + tc.transactf("ok", `create "test#"`) + + // UTF-7 checks are only for IMAP4 before rev2 and without UTF8=ACCEPT. + tc.transactf("ok", `create "&"`) // Interpreted as UTF-8, no UTF-7. + tc2.transactf("bad", `create "&"`) // Bad UTF-7. + tc2.transactf("ok", `create "&Jjo-"`) // ☺, valid UTF-7. } diff --git a/imapserver/list.go b/imapserver/list.go index bddaca9..b8535ed 100644 --- a/imapserver/list.go +++ b/imapserver/list.go @@ -212,7 +212,7 @@ func (c *conn) cmdList(tag, cmd string, p *parser) { if extended != nil { extStr = " " + extended.pack(c) } - line := fmt.Sprintf(`* LIST %s "/" %s%s`, flags.pack(c), astring(name).pack(c), extStr) + line := fmt.Sprintf(`* LIST %s "/" %s%s`, flags.pack(c), astring(c.encodeMailbox(name)).pack(c), extStr) responseLines = append(responseLines, line) if retStatusAttrs != nil && info.mailbox != nil { diff --git a/imapserver/parse.go b/imapserver/parse.go index 8008326..323d4fe 100644 --- a/imapserver/parse.go +++ b/imapserver/parse.go @@ -7,8 +7,6 @@ import ( "strconv" "strings" "time" - - "golang.org/x/exp/slog" ) var ( @@ -392,30 +390,37 @@ func (p *parser) xatom() string { return p.xtakechars(atomChar, "atom") } +func (p *parser) xdecodeMailbox(s string) string { + // UTF-7 is deprecated for IMAP4rev2-only clients, and not used with UTF8=ACCEPT. + // The future should be without UTF-7, we don't encode/decode it with modern + // clients. Most clients are IMAP4rev1, we need to handle UTF-7. + // ../rfc/3501:964 ../rfc/9051:7885 + // Thunderbird will enable UTF8=ACCEPT and send "&" unencoded. ../rfc/9051:7953 + if p.conn.utf8strings() { + return s + } + ns, err := utf7decode(s) + if err != nil { + p.xerrorf("decoding utf7 mailbox name: %v", err) + } + return ns +} + func (p *parser) xmailbox() string { s := p.xastring() - // UTF-7 is deprecated in IMAP4rev2. IMAP4rev1 does not fully forbid - // UTF-8 returned in mailbox names. We'll do our best by attempting to - // decode utf-7. But if that doesn't work, we'll just use the original - // string. - // ../rfc/3501:964 - if !p.conn.enabled[capIMAP4rev2] { - ns, err := utf7decode(s) - if err != nil { - p.conn.log.Infox("decoding utf7 or mailbox name", err, slog.String("name", s)) - } else { - s = ns - } - } - return s + return p.xdecodeMailbox(s) } // ../rfc/9051:6605 func (p *parser) xlistMailbox() string { + var s string if p.hasPrefix(`"`) || p.hasPrefix("{") { - return p.xstring() + s = p.xstring() + } else { + s = p.xtakechars(atomChar+listWildcards+respSpecials, "list-char") } - return p.xtakechars(atomChar+listWildcards+respSpecials, "list-char") + // Presumably UTF-7 encoding applies to mailbox patterns too. + return p.xdecodeMailbox(s) } // ../rfc/9051:6707 ../rfc/9051:6848 ../rfc/5258:1095 ../rfc/5258:1169 ../rfc/5258:1196 diff --git a/imapserver/server.go b/imapserver/server.go index b0eca2a..4583a13 100644 --- a/imapserver/server.go +++ b/imapserver/server.go @@ -13,12 +13,9 @@ LIST-EXTENDED, SPECIAL-USE, MOVE, UTF8=ONLY. We take a liberty with UTF8=ONLY. We are supposed to wait for ENABLE of UTF8=ACCEPT or IMAP4rev2 before we respond with quoted strings that contain -non-ASCII UTF-8. But we will unconditionally accept UTF-8 at the moment. See +non-ASCII UTF-8. Until that's enabled, we do use UTF-7 for mailbox names. See ../rfc/6855:251 -We always respond with utf8 mailbox names. We do parse utf7 (only in IMAP4rev1, -not in IMAP4rev2). ../rfc/3501:964 - - We never execute multiple commands at the same time for a connection. We expect a client to open multiple connections instead. ../rfc/9051:1110 - Do not write output on a connection with an account lock held. Writing can block, a slow client could block account operations. - When handling commands that modify the selected mailbox, always check that the mailbox is not opened readonly. And always revalidate the selected mailbox, another session may have deleted the mailbox. @@ -161,7 +158,7 @@ var authFailDelay = time.Second // After authentication failure. // TLS. The client should not be selecting PLUS variants on non-TLS connections, // instead opting to do the bare SCRAM variant without indicating the server claims // to support the PLUS variant (skipping the server downgrade detection check). -const serverCapabilities = "IMAP4rev2 IMAP4rev1 ENABLE LITERAL+ IDLE SASL-IR BINARY UNSELECT UIDPLUS ESEARCH SEARCHRES MOVE UTF8=ONLY LIST-EXTENDED SPECIAL-USE LIST-STATUS AUTH=SCRAM-SHA-256-PLUS AUTH=SCRAM-SHA-256 AUTH=SCRAM-SHA-1-PLUS AUTH=SCRAM-SHA-1 AUTH=CRAM-MD5 ID APPENDLIMIT=9223372036854775807 CONDSTORE QRESYNC" +const serverCapabilities = "IMAP4rev2 IMAP4rev1 ENABLE LITERAL+ IDLE SASL-IR BINARY UNSELECT UIDPLUS ESEARCH SEARCHRES MOVE UTF8=ACCEPT LIST-EXTENDED SPECIAL-USE LIST-STATUS AUTH=SCRAM-SHA-256-PLUS AUTH=SCRAM-SHA-256 AUTH=SCRAM-SHA-1-PLUS AUTH=SCRAM-SHA-1 AUTH=CRAM-MD5 ID APPENDLIMIT=9223372036854775807 CONDSTORE QRESYNC" type conn struct { cid int64 @@ -387,6 +384,13 @@ func (c *conn) utf8strings() bool { return c.enabled[capIMAP4rev2] || c.enabled[capUTF8Accept] } +func (c *conn) encodeMailbox(s string) string { + if c.utf8strings() { + return s + } + return utf7encode(s) +} + func (c *conn) xdbwrite(fn func(tx *bstore.Tx)) { err := c.account.DB.Write(context.TODO(), func(tx *bstore.Tx) error { fn(tx) @@ -1311,19 +1315,19 @@ func (c *conn) applyChanges(changes []store.Change, initial bool) { // unrecognized \NonExistent and interpret this as a newly created mailbox, while // the goal was to remove it... if c.enabled[capIMAP4rev2] { - c.bwritelinef(`* LIST (\NonExistent) "/" %s`, astring(ch.Name).pack(c)) + c.bwritelinef(`* LIST (\NonExistent) "/" %s`, astring(c.encodeMailbox(ch.Name)).pack(c)) } case store.ChangeAddMailbox: - c.bwritelinef(`* LIST (%s) "/" %s`, strings.Join(ch.Flags, " "), astring(ch.Mailbox.Name).pack(c)) + c.bwritelinef(`* LIST (%s) "/" %s`, strings.Join(ch.Flags, " "), astring(c.encodeMailbox(ch.Mailbox.Name)).pack(c)) case store.ChangeRenameMailbox: // OLDNAME only with IMAP4rev2 or NOTIFY ../rfc/9051:2726 ../rfc/5465:628 var oldname string if c.enabled[capIMAP4rev2] { - oldname = fmt.Sprintf(` ("OLDNAME" (%s))`, string0(ch.OldName).pack(c)) + oldname = fmt.Sprintf(` ("OLDNAME" (%s))`, string0(c.encodeMailbox(ch.OldName)).pack(c)) } - c.bwritelinef(`* LIST (%s) "/" %s%s`, strings.Join(ch.Flags, " "), astring(ch.NewName).pack(c), oldname) + c.bwritelinef(`* LIST (%s) "/" %s%s`, strings.Join(ch.Flags, " "), astring(c.encodeMailbox(ch.NewName)).pack(c), oldname) case store.ChangeAddSubscription: - c.bwritelinef(`* LIST (%s) "/" %s`, strings.Join(append([]string{`\Subscribed`}, ch.Flags...), " "), astring(ch.Name).pack(c)) + c.bwritelinef(`* LIST (%s) "/" %s`, strings.Join(append([]string{`\Subscribed`}, ch.Flags...), " "), astring(c.encodeMailbox(ch.Name)).pack(c)) default: panic(fmt.Sprintf("internal error, missing case for %#v", change)) } @@ -2041,7 +2045,7 @@ func (c *conn) cmdSelectExamine(isselect bool, tag, cmd string, p *parser) { } c.bwritelinef(`* OK [UIDVALIDITY %d] x`, mb.UIDValidity) c.bwritelinef(`* OK [UIDNEXT %d] x`, mb.UIDNext) - c.bwritelinef(`* LIST () "/" %s`, astring(mb.Name).pack(c)) + c.bwritelinef(`* LIST () "/" %s`, astring(c.encodeMailbox(mb.Name)).pack(c)) if c.enabled[capCondstore] { // ../rfc/7162:417 // ../rfc/7162-eid5055 ../rfc/7162:484 ../rfc/7162:1167 @@ -2239,9 +2243,9 @@ func (c *conn) cmdCreate(tag, cmd string, p *parser) { var oldname string // OLDNAME only with IMAP4rev2 or NOTIFY ../rfc/9051:2726 ../rfc/5465:628 if c.enabled[capIMAP4rev2] && n == name && name != origName && !(name == "Inbox" || strings.HasPrefix(name, "Inbox/")) { - oldname = fmt.Sprintf(` ("OLDNAME" (%s))`, string0(origName).pack(c)) + oldname = fmt.Sprintf(` ("OLDNAME" (%s))`, string0(c.encodeMailbox(origName)).pack(c)) } - c.bwritelinef(`* LIST (\Subscribed) "/" %s%s`, astring(n).pack(c), oldname) + c.bwritelinef(`* LIST (\Subscribed) "/" %s%s`, astring(c.encodeMailbox(n)).pack(c), oldname) } c.ok(tag, cmd) } @@ -2526,7 +2530,7 @@ func (c *conn) cmdLsub(tag, cmd string, p *parser) { continue } have[name] = true - line := fmt.Sprintf(`* LSUB () "/" %s`, astring(name).pack(c)) + line := fmt.Sprintf(`* LSUB () "/" %s`, astring(c.encodeMailbox(name)).pack(c)) lines = append(lines, line) } @@ -2541,7 +2545,7 @@ func (c *conn) cmdLsub(tag, cmd string, p *parser) { if have[mb.Name] || !subscribedKids[mb.Name] || !re.MatchString(mb.Name) { return nil } - line := fmt.Sprintf(`* LSUB (\NoSelect) "/" %s`, astring(mb.Name).pack(c)) + line := fmt.Sprintf(`* LSUB (\NoSelect) "/" %s`, astring(c.encodeMailbox(mb.Name)).pack(c)) lines = append(lines, line) return nil }) @@ -2639,7 +2643,7 @@ func (c *conn) xstatusLine(tx *bstore.Tx, mb store.Mailbox, attrs []string) stri xsyntaxErrorf("unknown attribute %q", a) } } - return fmt.Sprintf("* STATUS %s (%s)", astring(mb.Name).pack(c), strings.Join(status, " ")) + return fmt.Sprintf("* STATUS %s (%s)", astring(c.encodeMailbox(mb.Name)).pack(c), strings.Join(status, " ")) } func flaglist(fl store.Flags, keywords []string) listspace { diff --git a/imapserver/utf7.go b/imapserver/utf7.go index b8236d9..ace5e5c 100644 --- a/imapserver/utf7.go +++ b/imapserver/utf7.go @@ -1,11 +1,17 @@ package imapserver import ( + "bytes" "encoding/base64" "errors" "fmt" + "unicode/utf16" ) +// IMAP4rev1 uses a modified version of UTF-7. +// ../rfc/3501:1050 +// ../rfc/2152:69 + const utf7chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+," var utf7encoding = base64.NewEncoding(utf7chars).WithPadding(base64.NoPadding) @@ -16,6 +22,7 @@ var ( errUTF7OddSized = errors.New("utf7: odd-sized data") errUTF7UnneededShift = errors.New("utf7: unneeded shift") errUTF7UnfinishedShift = errors.New("utf7: unfinished shift") + errUTF7BadSurrogate = errors.New("utf7: bad utf16 surrogates") ) func utf7decode(s string) (string, error) { @@ -60,20 +67,36 @@ func utf7decode(s string) (string, error) { x := make([]rune, len(buf)/2) j := 0 + trymerge := false for i := 0; i < len(buf); i += 2 { x[j] = rune(buf[i])<<8 | rune(buf[i+1]) + if trymerge { + s0 := utf16.IsSurrogate(x[j-1]) + s1 := utf16.IsSurrogate(x[j]) + if s0 && s1 { + c := utf16.DecodeRune(x[j-1], x[j]) + if c == 0xfffd { + return "", fmt.Errorf("%w: decoding %x %x", errUTF7BadSurrogate, x[j-1], x[j]) + } + x[j-1] = c + trymerge = false + continue + } else if s0 != s1 { + return "", fmt.Errorf("%w: not both surrogate: %x %x", errUTF7BadSurrogate, x[j-1], x[j]) + } + } j++ + trymerge = true } + x = x[:j] - need := false for _, c := range x { if c < 0x20 || c > 0x7e || c == '&' { - need = true + r += string(c) + } else { + // ../rfc/3501:1057 + return "", errUTF7UnneededShift } - r += string(c) - } - if !need { - return "", errUTF7UnneededShift } } if shifted { @@ -81,3 +104,43 @@ func utf7decode(s string) (string, error) { } return r, nil } + +func utf7encode(s string) string { + var r string + var code string + + flushcode := func() { + if code == "" { + return + } + var b bytes.Buffer + for _, c := range code { + high, low := utf16.EncodeRune(c) + if high == 0xfffd && low == 0xfffd { + b.WriteByte(byte(c >> 8)) + b.WriteByte(byte(c >> 0)) + } else { + b.WriteByte(byte(high >> 8)) + b.WriteByte(byte(high >> 0)) + b.WriteByte(byte(low >> 8)) + b.WriteByte(byte(low >> 0)) + } + } + r += "&" + utf7encoding.EncodeToString(b.Bytes()) + "-" + code = "" + } + + for _, c := range s { + if c == '&' { + flushcode() + r += "&-" + } else if c >= ' ' && c < 0x7f { + flushcode() + r += string(c) + } else { + code += string(c) + } + } + flushcode() + return r +} diff --git a/imapserver/utf7_test.go b/imapserver/utf7_test.go index a60ea09..8943a37 100644 --- a/imapserver/utf7_test.go +++ b/imapserver/utf7_test.go @@ -2,6 +2,7 @@ package imapserver import ( "errors" + "fmt" "testing" ) @@ -16,6 +17,12 @@ func TestUTF7(t *testing.T) { if (expErr == nil) != (err == nil) || err != nil && !errors.Is(err, expErr) { t.Fatalf("got err %v, expected %v", err, expErr) } + if expErr == nil { + expInput := utf7encode(output) + if expInput != input { + t.Fatalf("encoding, got %s, expected %s", expInput, input) + } + } } check("plain", "plain", nil) @@ -24,10 +31,19 @@ func TestUTF7(t *testing.T) { check("&Jjo-test&Jjo-", "☺test☺", nil) check("&Jjo-test", "☺test", nil) check("&-", "&", nil) - check("&-", "&", nil) check("&Jjo", "", errUTF7UnfinishedShift) // missing closing - check("&Jjo-&-", "", errUTF7SuperfluousShift) // shift just after unshift not allowed, should have been a single shift. check("&AGE-", "", errUTF7UnneededShift) // Just 'a', does not need utf7. check("&☺-", "", errUTF7Base64) check("&YQ-", "", errUTF7OddSized) // Just a single byte 'a' + check("&2AHcNw-", "𐐷", nil) + check(fmt.Sprintf("&%s-", utf7encoding.EncodeToString([]byte{0xdc, 0x00, 0xd8, 0x00})), "", errUTF7BadSurrogate) // Low & high surrogate swapped. + check(fmt.Sprintf("&%s-", utf7encoding.EncodeToString([]byte{0, 1, 0xdc, 0x00})), "", errUTF7BadSurrogate) // ASCII + high surrogate. + check(fmt.Sprintf("&%s-", utf7encoding.EncodeToString([]byte{0, 1, 0xd8, 0x00})), "", errUTF7BadSurrogate) // ASCII + low surrogate. + check(fmt.Sprintf("&%s-", utf7encoding.EncodeToString([]byte{0xd8, 0x00, 0, 1})), "", errUTF7BadSurrogate) // low surrogate + ASCII. + check(fmt.Sprintf("&%s-", utf7encoding.EncodeToString([]byte{0xdc, 0x00, 0, 1})), "", errUTF7BadSurrogate) // high surrogate + ASCII. + + // ../rfc/9051:7967 + check("~peter/mail/&U,BTFw-/&ZeVnLIqe-", "~peter/mail/台北/日本語", nil) + check("&U,BTFw-&ZeVnLIqe-", "", errUTF7SuperfluousShift) } diff --git a/store/account.go b/store/account.go index 9ecb133..1f6bdf4 100644 --- a/store/account.go +++ b/store/account.go @@ -2599,12 +2599,18 @@ func CheckMailboxName(name string, allowInbox bool) (normalizedName string, isIn if strings.HasPrefix(name, "/") || strings.HasSuffix(name, "/") || strings.Contains(name, "//") { return "", false, errors.New("bad slashes in mailbox name") } + + // "%" and "*" are difficult to use with the IMAP LIST command, but we allow mostly + // allow them. ../rfc/3501:1002 ../rfc/9051:983 + if strings.HasPrefix(name, "#") { + return "", false, errors.New("mailbox name cannot start with hash due to conflict with imap namespaces") + } + + // "#" and "&" are special in IMAP mailbox names. "#" for namespaces, "&" for + // IMAP-UTF-7 encoding. We do allow them. ../rfc/3501:1018 ../rfc/9051:991 + for _, c := range name { - switch c { - case '%', '*', '#', '&': - return "", false, fmt.Errorf("character %c not allowed in mailbox name", c) - } - // ../rfc/6855:192 + // ../rfc/3501:999 ../rfc/6855:192 ../rfc/9051:979 if c <= 0x1f || c >= 0x7f && c <= 0x9f || c == 0x2028 || c == 0x2029 { return "", false, errors.New("control characters not allowed in mailbox name") }