mbentry->ext_name not set for proxying GETMETADATA

[MichaelMenge]

Version Cyrus Iampd 3.0.8
mupdate_config: standard

Sending a GETMETADATA command to a frondend server the imapd connection
fails with an assertion error:

imtest -a YYYYYYY -s mailserv08
verify error:num=19:self signed certificate in certificate chain
TLS connection established: TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)
S: * OK [CAPABILITY IMAP4rev1 LITERAL+ ID ENABLE MUPDATE=mupdate://mupdate.mail.localhost/ AUTH=PLAIN AUTH=LOGIN SASL-IR] mailserv08.uni-tuebingen.de Cyrus IMAP 3.0.8 server ready
Please enter your password: 
C: A01 AUTHENTICATE PLAIN XXXXXXXXXXXXXXXXXXX
S: A01 OK [CAPABILITY IMAP4rev1 LITERAL+ ID ENABLE ACL RIGHTS=kxten QUOTA NAMESPACE UIDPLUS NO_ATOMIC_RENAME UNSELECT CHILDREN MULTIAPPEND BINARY CATENATE CONDSTORE ESEARCH SEARCH=FUZZY SORT SORT=MODSEQ SORT=DISPLAY SORT=UID THREAD=ORDEREDSUBJECT THREAD=REFERENCES THREAD=REFS ANNOTATEMORE ANNOTATE-EXPERIMENT-1 METADATA LIST-EXTENDED LIST-STATUS LIST-MYRIGHTS LIST-METADATA WITHIN QRESYNC SCAN XLIST XMOVE MOVE SPECIAL-USE CREATE-SPECIAL-USE DIGEST=SHA1 X-REPLICATION URLAUTH URLAUTH=BINARY MUPDATE=mupdate://mupdate.mail.localhost/ LOGINDISABLED COMPRESS=DEFLATE X-QUOTA=STORAGE X-QUOTA=MESSAGE X-QUOTA=X-ANNOTATION-STORAGE X-QUOTA=X-NUM-FOLDERS IDLE] Success (tls protection) SESSIONID=<fe-11227-1555495095-1-1247971645018351147>
Authenticated.
Security strength factor: 256
a GETMETADATA INBOX (/shared/vendor/cmu/cyrus-imapd/size)
* BYE Fatal error: Internal error: assertion failed: imap/imap_proxy.c: 1407: server && mbox_pat && entry_pat && attribute_pat
Connection closed.

gdb output

(gdb) break annotate_fetch_proxy
Breakpoint 1 at 0x40f82b: file imap/imap_proxy.c, line 1407.
(gdb) cont
Continuing.

Breakpoint 1, annotate_fetch_proxy (server=0x2153cb0 "ma08.mail.localhost", mbox_pat=0x0, entry_pat=0x7fff23b54a60, attribute_pat=0x7fff23b54a50) at imap/imap_proxy.c:1407
1407	    assert(server && mbox_pat && entry_pat && attribute_pat);
(gdb) bt
#0  annotate_fetch_proxy (server=0x2153cb0 "ma08.mail.localhost", mbox_pat=0x0, entry_pat=0x7fff23b54a60, attribute_pat=0x7fff23b54a50) at imap/imap_proxy.c:1407
#1  0x00007f1112e12f25 in annotate_state_fetch (state=0x2154f80, entries=0x7fff23b54a60, attribs=0x7fff23b54a50, callback=0x428eae <getmetadata_response>, rock=0x7fff23b549f0) at imap/annotate.c:2338
#2  0x00000000004288ce in annot_fetch_cb (astate=0x2154f80, rock=0x7fff23b549c0) at imap/imapd.c:9573
#3  0x0000000000428bf4 in apply_mailbox_array (state=0x2154f80, mboxes=0x7fff23b54a80, proc=0x428888 <annot_fetch_cb>, rock=0x7fff23b549c0) at imap/imapd.c:9679
#4  0x0000000000429b5c in cmd_getmetadata (tag=0x2152c70 "a") at imap/imapd.c:10050
#5  0x0000000000412c48 in cmdloop () at imap/imapd.c:1565
#6  0x0000000000411327 in service_main (argc=2, argv=0x2109030, envp=0x7fff23b577e0) at imap/imapd.c:1000
#7  0x0000000000445252 in main (argc=6, argv=0x7fff23b577a8, envp=0x7fff23b577e0) at master/service.c:634

mbox_pat is state->mbentry->ext_name in annotate_state_fetch
and atate->mbentry->ext_name in annot_fetch_cb

I don't know where mbox_pat / mbentry->ext_name should have been set.

[MichaelMenge]

As fetching the Size Information in Cyrus imapd 2.4 I tried the GETANNOTATION command:
which worked

imtest -a YYYYYYY -s mailserv08
verify error:num=19:self signed certificate in certificate chain
TLS connection established: TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)
S: * OK [CAPABILITY IMAP4rev1 LITERAL+ ID ENABLE MUPDATE=mupdate://mupdate.mail.localhost/ AUTH=PLAIN AUTH=LOGIN SASL-IR] mailserv08.uni-tuebingen.de Cyrus IMAP 3.0.8 server ready
Please enter your password: 
C: A01 AUTHENTICATE PLAIN XXXXXXXXXXXXXXXXXXX
S: A01 OK [CAPABILITY IMAP4rev1 LITERAL+ ID ENABLE ACL RIGHTS=kxten QUOTA NAMESPACE UIDPLUS NO_ATOMIC_RENAME UNSELECT CHILDREN MULTIAPPEND BINARY CATENATE CONDSTORE ESEARCH SEARCH=FUZZY SORT SORT=MODSEQ SORT=DISPLAY SORT=UID THREAD=ORDEREDSUBJECT THREAD=REFERENCES THREAD=REFS ANNOTATEMORE ANNOTATE-EXPERIMENT-1 METADATA LIST-EXTENDED LIST-STATUS LIST-MYRIGHTS LIST-METADATA WITHIN QRESYNC SCAN XLIST XMOVE MOVE SPECIAL-USE CREATE-SPECIAL-USE DIGEST=SHA1 X-REPLICATION URLAUTH URLAUTH=BINARY MUPDATE=mupdate://mupdate.mail.localhost/ LOGINDISABLED COMPRESS=DEFLATE X-QUOTA=STORAGE X-QUOTA=MESSAGE X-QUOTA=X-ANNOTATION-STORAGE X-QUOTA=X-NUM-FOLDERS IDLE] Success (tls protection) SESSIONID=<fe-16074-1555495571-1-14306282044106911439>
Authenticated.
Security strength factor: 256

a GETANNOTATION "INBOX" "/vendor/cmu/cyrus-imapd/size" "value"
* ANNOTATION INBOX "/vendor/cmu/cyrus-imapd/size" ("value.shared" "681174812")
a OK Completed

(gdb) break annotate_fetch_proxy
Breakpoint 1 at 0x40f82b: file imap/imap_proxy.c, line 1407.
(gdb) cont
Continuing.

Breakpoint 1, annotate_fetch_proxy (server=0x25cb960 "ma08.mail.localhost", mbox_pat=0x25e1cf0 "INBOX", entry_pat=0x7fffb9218a40, attribute_pat=0x7fffb9218a30) at imap/imap_proxy.c:1407
1407	    assert(server && mbox_pat && entry_pat && attribute_pat);
(gdb) bt
#0  annotate_fetch_proxy (server=0x25cb960 "ma08.mail.localhost", mbox_pat=0x25e1cf0 "INBOX", entry_pat=0x7fffb9218a40, attribute_pat=0x7fffb9218a30) at imap/imap_proxy.c:1407
#1  0x00007ff14228ef25 in annotate_state_fetch (state=0x25e3b60, entries=0x7fffb9218a40, attribs=0x7fffb9218a30, callback=0x4286e5 <getannotation_response>, rock=0x0) at imap/annotate.c:2338
#2  0x00000000004288ce in annot_fetch_cb (astate=0x25e3b60, rock=0x7fffb9218a00) at imap/imapd.c:9573
#3  0x0000000000428a1f in apply_cb (data=0x7fffb9216a70, rock=0x7fffb92179b0) at imap/imapd.c:9626
#4  0x00007ff1422cf4fb in find_cb (rockp=0x7fffb9217870, key=0x7fffb9217390 "user.YYYYYYY.INBOX.", keylen=12, 
    data=0x7ff1302a733c "%(A %(YYYYYYY lrswipkxtecda cyrus lrswipkxtecda) P ssd S ma08.mail.localhost T r M 1541603269)", datalen=94) at imap/mboxlist.c:2410
#5  0x00007ff141fb8e86 in cyrusdb_forone (db=0x259b450, key=0x7fffb9217390 "user.YYYYYYY.INBOX.", keylen=12, p=0x7ff1422ceff7 <find_p>, cb=0x7ff1422cf2b2 <find_cb>, rock=0x7fffb9217870, tid=0x0)
    at lib/cyrusdb.c:258
#6  0x00007ff1422d06c0 in mboxlist_do_find (rock=0x7fffb9217870, patterns=0x7fffb9217950) at imap/mboxlist.c:2802
#7  0x00007ff1422d0d18 in mboxlist_findallmulti (namespace=0x6593a0 <imapd_namespace>, patterns=0x7fffb9217950, isadmin=0, userid=0x25c9aa0 "YYYYYYY", auth_state=0x25cb0d0, proc=0x428900 <apply_cb>, 
    rock=0x7fffb92179b0) at imap/mboxlist.c:2942
#8  0x00007ff1422d0d8f in mboxlist_findall (namespace=0x6593a0 <imapd_namespace>, pattern=0x25e1150 "INBOX", isadmin=0, userid=0x25c9aa0 "YYYYYYY", auth_state=0x25cb0d0, proc=0x428900 <apply_cb>, 
    rock=0x7fffb92179b0) at imap/mboxlist.c:2955
#9  0x0000000000428b13 in apply_mailbox_pattern (state=0x25e3b60, pattern=0x25e1150 "INBOX", proc=0x428888 <annot_fetch_cb>, data=0x7fffb9218a00) at imap/imapd.c:9645
#10 0x0000000000428e0b in cmd_getannotation (tag=0x25e3aa0 "a", mboxpat=0x25e1150 "INBOX") at imap/imapd.c:9739
#11 0x0000000000412c0f in cmdloop () at imap/imapd.c:1558
#12 0x0000000000411327 in service_main (argc=2, argv=0x2598030, envp=0x7fffb921b750) at imap/imapd.c:1000
#13 0x0000000000445252 in main (argc=6, argv=0x7fffb921b718, envp=0x7fffb921b750) at master/service.c:634

So the bt differs at cmd_getannotation / cmd_getmetadata but reunite in annot_fetch_cb
where mbentry->ext_name is set in for the cmd_getmetadata path but not for cmd_getannotation
path

[elliefm]

See also #2472

[elliefm]

So, the problem is that this mbentry was created by mboxlist_lookup, which only sets the fields from the mailbox list (but external names need more context -- userid and namespace -- so the ext_name field is left blank). But then we just pass that mbentry around and everything further down assumes the ext_name field is populated.

It looks like nothing in Cyrus ever (at present) initialises the ext_name field of an mbentry (seriously! try ack ext_name). But mboxlist_entry_copy() will copy it if the source had it set, and mboxlist_entry_free() will clean it up, so it looks like it's safe to just set it ourselves (we already know it -- it was our input)

[elliefm]

Added a check for getmetadata to the Murder.frontend_commands test, which reproduced the error. And added a5767f0 (master) and 80b1a70 (3.0) that seem to fix it.

Can you try this out with a real world setup and confirm if it's properly fixed? (I wonder if there's a nuance that's still broken that I haven't seen because Cassadane's murder setup is quite simple.)

[MichaelMenge]

@elliefm I applied the patch to 3.0.11 and after some testing on my test systems I installed
it on the production systems today. The application that triggered the bug is working fine.
I have not seen any obvious new problems, so I regard this fix as working.
Thank you for fixing it

[elliefm]

The test I added for this trips the same assertion on 2.5, so I guess that's affected too, but I doubt the 3.0/master fix will cherry-pick cleanly, so I'm gonna just let it slide for now