Bug 1911689

It looks user named "user" is clashing with "user" namespace here:

https://github.com/cyrusimap/cyrus-imapd/blame/master/imap/mboxname.c#L740

doc: https://www.cyrusimap.org/imap/concepts/features/namespaces.html

Still investigating to find out how this is supposed to work (e.g. 'squatter -C /etc/imapd.conf -r -u user' doesn't crash on assert, but unsure it leads to the expected result yet)


On RHEL7.9 (no assert crash) cscope finds "user" string handled by: 

~~~
Text string: "user"

  File                                 Line
...
b cyrus-imapd-2.4.17/imap/mboxname.c     87 "user",
c cyrus-imapd-2.4.17/imap/mboxname.c    599 else if (!strncmp(name, "user", 4) &&
d cyrus-imapd-2.4.17/imap/mboxname.c    685 "user", namespace->hier_sep);
~~~

On RHEL8 cscope finds "user" string handled by: 

~~~
Text string: "user"

  File                                   Line
...
5 cyrus-imapd-3.0.7/imap/mboxname.c       538 if (strarray_size(mbname->boxes) > 1 && !strcmpsafe(strarray_nth(mbname->boxes, 0), "user")) {
6 cyrus-imapd-3.0.7/imap/mboxname.c       710 if (!strcmpsafe(strarray_nth(mbname->boxes, 0), "user")) {
7 cyrus-imapd-3.0.7/imap/mboxname.c       969 if (category == MBNAME_OTHERUSER) return "user";
8 cyrus-imapd-3.0.7/imap/mboxname.c      1023 if (strarray_size(boxes) == 1 && !strcmpsafe(toplevel, "user")) {
9 cyrus-imapd-3.0.7/imap/mboxname.c      1120 /* note "user" precisely appears here, but no need to special case it
a cyrus-imapd-3.0.7/imap/mboxname.c      1133 buf_appendcstr(&buf, "user");
b cyrus-imapd-3.0.7/imap/mboxname.c      1259 "user", namespace->hier_sep);
c cyrus-imapd-3.0.7/imap/mboxname.c      1568 if (!strcmp(name, "user")) return IMAP_MAILBOX_BADNAME;
d cyrus-imapd-3.0.7/imap/mboxname.c      1694 strarray_unshift(boxes, "user");
e cyrus-imapd-3.0.7/imap/mboxname.c      1956 if (!name[0] || !strcmp(name+domainlen, "user"))
~~~

gdb script setting breakpoints on the above locations:

~~~
# cat cyrus-gdbscript.txt
b main
r -C /etc/imapd.conf -r user
b imap/mboxname.c:538
b imap/mboxname.c:710
b imap/mboxname.c:969
b imap/mboxname.c:1023
b imap/mboxname.c:1133
b imap/mboxname.c:1258
b imap/mboxname.c:1568
b imap/mboxname.c:1694
b imap/mboxname.c:1956
#
~~~

And it hits it on the mentioned location not present in RHEL7 code and free()s the mbname structure holding "user" argument mailbox string finally ending up on the assert():

~~~
# gdb /usr/sbin/squatter -x ~/cyrus-gdbscript.txt
...
Breakpoint 1, 0x0000555555558b55 in main ()
Breakpoint 2 at 0x7ffff7380d81: file imap/mboxname.c, line 538.
Breakpoint 3 at 0x7ffff738161e: file imap/mboxname.c, line 710.
Breakpoint 4 at 0x7ffff738222a: file imap/mboxname.c, line 969.
Breakpoint 5 at 0x7ffff73824a8: file imap/mboxname.c, line 1023.
Breakpoint 6 at 0x7ffff7382cca: file imap/mboxname.c, line 1133.
Breakpoint 7 at 0x7ffff73833d2: file imap/mboxname.c, line 1258.
Breakpoint 8 at 0x7ffff7383f2e: file imap/mboxname.c, line 1568.
Breakpoint 9 at 0x7ffff73844bd: file imap/mboxname.c, line 1694.
Breakpoint 10 at 0x7ffff738518d: file imap/mboxname.c, line 1956.
(gdb) c
Continuing.
cyrus/squatter[9386]: DBERROR: reading /var/lib/imap/db/skipstamp, assuming the worst: No such file or directory

Breakpoint 7, mboxname_init_namespace (namespace=0x55555575b040, isadmin=1) at imap/mboxname.c:1258
1258            sprintf(namespace->prefix[NAMESPACE_USER], "%s%c",
(gdb) c
Continuing.

Breakpoint 3, mbname_from_extname (extname=0x7fffffffe00e "user", ns=0x55555575b040, userid=0x0) at imap/mboxname.c:710
710         if (!strcmpsafe(strarray_nth(mbname->boxes, 0), "user")) {
(gdb) n
711             free(strarray_shift(mbname->boxes));
(gdb) p *mbname->boxes->data
$1 = 0x5555557781c0 "user"
(gdb)

(gdb) c
Continuing.
fatal error: Internal error: assertion failed: lib/cyrusdb_twoskip.c: 2339: keylen
[Inferior 1 (process 9386) exited with code 0113]
(gdb)
~~~

Created attachment 1858432 [details]
proposed patch

The below command no longer works since RHEL8.0 which in previous versions (RHEL7 and older) led to (re)indexing mailbox for user "user":

~~~
# squatter -C /etc/imapd.conf -r user
~~~

This is no longer possible due to the below patch which introduced a major change in mailbox naming code:

https://github.com/cyrusimap/cyrus-imapd/commit/65c9129253cfb22542c906330aecfb3068484a0b

The correct way of doing the same now is (note the '-u' option):

~~~
# squatter -C /etc/imapd.conf -r -u user

or by working around the user namespace clash:

# squatter -C /etc/imapd.conf -r user.user
# squatter -C /etc/imapd.conf -r user/user
~~~

Man page:

~~~
       squatter [ -C config-file ] [ -a ] [ -i ] [-N name] [-S *seconds*] [ **-r 
 ]  -u user...

...

       -u     Extra  options refer to usernames (e.g. foo) rather than
              mailbox names.  This feature was introduced in version 3.0.
~~~

The attached proposed patch is changing the code in a way it doesn't crash on assert(), but informs that the "user" mailbox name is invalid:

~~~
# /usr/sbin/squatter -C /etc/imapd.conf -r user
cyrus/squatter[156773]: DBERROR: reading /var/lib/imap/db/skipstamp, assuming the worst: No such file or directory
Mailbox user: Invalid mailbox name
cyrus/squatter[156773]: indexing mailboxes
cyrus/squatter[156773]: done indexing mailboxes
#
~~~

This happens only for "user" as it clashes with code that wrongly detects "user" as 'user.' or 'user/' namespace:

https://www.cyrusimap.org/imap/concepts/features/namespaces.html

The culprit code:

~~~
 546 EXPORTED mbname_t *mbname_from_extname(const char *extname, const struct namespace *ns, const char *userid)
 547 {
...
 709     /* now look for user */
 710     if (!strcmpsafe(strarray_nth(mbname->boxes, 0), "user")) {
 711         free(strarray_shift(mbname->boxes));
 712         mbname->localpart = strarray_shift(mbname->boxes);
 713         if (crossdomains && mbname->localpart) {
 714             char *p = strchr(mbname->localpart, '@');
 715             if (p) {
 716                 *p = '\0';
 717                 if (strcmpsafe(p+1, config_defdomain))
 718                     mbname->domain = xstrdup(p+1);
 719             }
 720             else if (cdother) {
 721                 mbname->domain = xstrdupnull(mbname_domain(userparts));
 722             }
 723         }
 724         goto done;
 725     }
~~~

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (cyrus-imapd bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:7651