cyrusimap / cyrus-imapd

Cyrus IMAP is an email, contacts and calendar server
http://cyrusimap.org
Other
522 stars 145 forks source link

Incorrect "From" parsing on CalDAV invites #4955

Open amessina opened 5 days ago

amessina commented 5 days ago

KOrganizer creates CalDAV events with ORGANIZER;CN=Anthony Messina <Anthony Messina>:MAILTO: amessina@example.com (with the name duplicated inside <> the CN). While this is unfortunate, Cyrus improperly parses the organizer and sends out invitations From: "Anthony Messina <AnthonyMessina@example.com>" <amessina@example.com>, compressing the name and appending the domain, resulting in <AnthonyMessina@example.com>

This creates issues for other clients (recipients), which attempt to send replies to <AnthonyMessina@example.com> instead of <amessina@example.com>

While I don't know why Korganizer duplicates the ORGANIZER name, Cyrus should not convert the CN into an email address for the From: of the invite.

Note, Cyrus does not alter the ORGANIZER in the attached invite itself.

rsto commented 4 days ago

Thanks for reporting this. I have created https://github.com/cyrusimap/cyrus-imapd/pull/4956 to fix this.

Before merging this, I first want to make we sure we have added test for the expected outcome to our test suite. I did unfortunately run into issues implementing this test that I first need to resolve. I outlined these issues in the pull request.

amessina commented 4 days ago

Thank you for taking this up so quickly.

Looking at #4956, I need to clarify. It's actually not KOrganizer, but Cyrus that currently tries to convert what's between angle brackets <> in the CN. Cyrus changes the following

ORGANIZER;CN=Anthony Messina <Anthony Messina>:MAILTO:
 amessina@example.com

into a fake email address <AnthonyMessina@example.com> -- then creates a "From:" line for the invite with two email addresses (note the compression of my name and the appended domain, within the CN) without proper quoting:

Anthony Messina <AnthonyMessina@messinet.com> <amessina@messinet.com>
  1. Cyrus should not convert anything in the CN (into anything else)
  2. Cyrus should double-quote the entire CN in the "From:" line of the invite.

In this particular case, that means the final "From:" line sent out by Cyrus in the invite should look like:

"Anthony Messina <Anthony Messina>" <amessina@example.com>

In the received invites, the one from KOrganizer is correct:

From: "Anthony Messina <Anthony Messina>" <amessina@example.com>
To: Messinet Secure Services <webmaster@example.com>
Subject: Cyrus invite test CN/MAILTO parsing
Date: Wed, 03 Jul 2024 06:28:34 -0500
Message-ID: <4429334.MNUckLgytD@example.com>
User-Agent: KOrganizer 6.1.0
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="nextPart4787234.vXUDI8C0e8"
Content-Transfer-Encoding: 7Bit

and the one from Cyrus is not:

From: Anthony Messina <AnthonyMessina@example.com> <amessina@example.com>
To: Messinet Secure Services <webmaster@example.com>
Subject: Invitation: Cyrus invite test CN/MAILTO parsing
Date: Wed, 03 Jul 2024 06:28:34 -0500
Message-ID: <cyrus-caldav-419391-1720006114-0@example.com>
Content-Type: multipart/mixed;
    boundary="example.com=_419391=_1720006114=_577160721_M"
iMIP-Content-ID: <83c399ae-f4d3-43d8-8c6d-06736ce6381c@example.com>
Auto-Submitted: auto-generated
MIME-Version: 1.0

Files attached: InvitationFromCyrus.ics.txt InvitationFromKOrganizer.ics.txt

rsto commented 4 days ago

I can replicate that for this line

ORGANIZER;CN=Anthony Messina <Anthony Messina>:MAILTO:amessina@example.com

Cyrus generates a From address

Anthony Messina <Anthony Messina> <amessina@example.com>

I can not replicate that it appends the domain or strips whitespace from the angle-bracketed name part, as you documented:

Anthony Messina <AnthonyMessina@example.com> <amessina@example.com>

What Cyrus currently generates is bogus and we'll need to fix that. But the mangling of the angle-bracket value seems to happen somewhere else.

1. Cyrus should not convert anything in the CN (into anything else)

I only partially agree with this. While RFC 5545 puts no restrictions on the CN parameter value, it looks reasonable and safer to me to strip anything that would resemble an email address. Otherwise it would allow to make an iMIP message appear to an end user as if the invite is coming from an email address that's different from the real originator.

2. Cyrus should double-quote the entire CN in the "From:" line of the invite.

Cyrus should certainly ensure that the CN value in the From: header is properly encoded and quoted if necessary. It currently fails at that.

I misunderstood the role of KOrganizer in all this. It sounds to me as if you are the one who decided to use Anthony Messina <Anthony Messina> as the name? If so, could you please explain to me the purpose of the angle bracketed part, I just want to better understand the use case.

amessina commented 4 days ago

I did not choose to put Anthony Messina <Anthony Messina> in the invite. That's an issue with KOrganizer which I reported here: https://bugs.kde.org/show_bug.cgi?id=489620

I am using defaultdomain and here is the rest of my /etc/imapd.conf

admins: cyrus
allowplaintext: yes
autocreate_inbox_folders: Archive|Drafts|Saved|Sent|Spam|Templates|Trash
autocreate_quota: 2048000
autocreate_sieve_folders: Spam
autocreate_subscribe_folders: Archive|Drafts|Saved|Sent|Spam|Templates|Trash
configdirectory: /var/lib/imap
conversations: yes
conversations_db: twoskip
defaultpartition: default
defaultsearchtier: t1
duplicate_db_path: /run/cyrus/db/deliver.db
hashimapspool: yes
idlesocket: /run/cyrus/socket/idle
lmtpsocket: /run/cyrus/socket/lmtp
mboxname_lockpath: /run/cyrus/lock
notifysocket: /run/cyrus/socket/notify
popminpoll: 1
proc_path: /run/cyrus/proc
ptscache_db_path: /run/cyrus/db/ptscache.db
#sasl_minimum_layer: 56
search_engine: xapian
search_index_headers: no
servername: chicago.example.com
sievedir: /var/lib/imap/sieve
anysievefolder: yes
statuscache_db_path: /run/cyrus/db/statuscache.db
sync_log: yes
sync_log_channels: squatter
syslog_prefix: cyrus
#tls_client_ca_dir: /etc/ssl/certs
#tls_client_ca_file: /etc/pki/tls/certs/ca-bundle.trust.crt
#tls_client_certs: off
#tls_required: yes
#tls_server_cert: /etc/pki/tls/certs/cyrus-imapd.ecdsa.pem
#tls_server_key: /etc/pki/tls/private/cyrus-imapd.ecdsa.pem
#tls_sessions_db_path: /run/cyrus/db/tls_sessions.db
#tls_session_timeout: 1440
#tls_versions: tls1_2 tls1_3
sasl_pwcheck_method: saslauthd
sasl_auto_transition: no
xlist-archive: Archive
xlist-trash: Trash
sasl_mech_list: GS2-KRB5 GSSAPI PLAIN
partition-default: /var/spool/imap
xlist-drafts: Drafts
t1searchpartition-default: /var/lib/imap/search
xlist-sent: Sent
xlist-junk: Spam

httpmodules: jmap  admin caldav carddav freebusy ischedule jmap webdav
defaultdomain: example.com
debug: 1
virtdomains: userid
rsto commented 4 days ago

@amessina Could it be that your email server is doing the expansion from <Anthony Messina> to <AnthonyMessina@example.com>? I came to learn that Postfix is likely doing this for the bogus addresses that Cyrus currently sends out.

amessina commented 3 days ago

@amessina Could it be that your email server is doing the expansion from <Anthony Messina> to <AnthonyMessina@example.com>? I came to learn that Postfix is likely doing this for the bogus addresses that Cyrus currently sends out.

I think you are onto something and we're both half right. When I create the invite, KOrganizer sends it's own invitation email, and Cyrus also sends one. They both route through the same Postfix instance.

The From line that is sent directly from KOrganizer does not exhibit this issue From: "Anthony Messina <Anthony Messina>" <amessina@example.com>

but From line sent from Cyrus does: From: Anthony Messina <AnthonyMessina@messinet.com> <amessina@messinet.com>

Notice the lack of quoting on the one from Cyrus. If it's being sent (to Postfix via sendmail) from Cyrus without the entire CN being quoted (Anthony Messina <Anthony Messina>), I bet Postfix does remove the space and append Postfix's default domain, and the real email address is also present in the From: line as one long line (no quotes).

Perhaps it's now much simpler as Cyrus shouldn just ensure proper quoting is done for CN processing when required by the RFC (when specials are in the name portion).

rsto commented 3 days ago

Perhaps it's now much simpler as Cyrus shouldn just ensure proper quoting is done for CN processing when required by the RFC (when specials are in the name portion).

Agreed, I updated the pull request accordingly.

amessina commented 3 days ago

Thank you for such quick handling @rsto. I'm currently running cyrus-imapd-3.8.3-1.fc40.x86_64 and look forward to this, hopefully in the next release.

dilyanpalauzov commented 3 days ago

Also quotation IMIP related — https://github.com/cyrusimap/cyrus-imapd/pull/2693:

http_caldav_sched.c:imip_send_sendmail() quote the display part of email addresses, when necessary

Sending an invitation to

ATTENDEE;CN="A, B":mailto:b.a@examle.org

resulted email: To: A, B <b.a@examle.org>

A, B must be quoted → To: "A, B" <b.a@examle.org>