Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared xDAV (CalDAV/CardDAV) folders are unreachable (301 redirect) #3251

Open
futzle opened this issue Oct 19, 2020 · 9 comments
Open

Shared xDAV (CalDAV/CardDAV) folders are unreachable (301 redirect) #3251

futzle opened this issue Oct 19, 2020 · 9 comments

Comments

@futzle
Copy link

futzle commented Oct 19, 2020

In 3.0.8, I could have public CalDAV and CardDAV folders in the shared namespace, accessible at URLs like:
https://my-server-name:8443/dav/calendars/SharedCalendar/
https://my-server-name:8443/dav/addressbooks/SharedAddressBooks/

but these are not reachable in version 3.2.4, and redirect to a bogus URL, whether the calendar exists or not:

% curl -i -o - --basic --request GET --user 'debbiep@polyfoam.com.au:MySecretPassword' https://calendar.polyfoam.com.au:8443/dav/calendars/SharedCalendar/  
HTTP/1.1 301 Moved Permanently
Date: Sun, 18 Oct 2020 23:44:15 GMT
Strict-Transport-Security: max-age=600
Location: /dav/calendars/user/debbiep@polyfoam.com.au/(null).SharedCalendar/
Vary: Accept-Encoding
Content-Length: 0

I think that this piece of code is getting called, and it shouldn't be:

if (httpd_userid && !config_getswitch(IMAPOPT_FASTMAILSHARING) &&

@futzle
Copy link
Author

futzle commented Oct 19, 2020

This breaks #2373 even more.

@elliefm
Copy link
Contributor

elliefm commented Oct 19, 2020

Looking at the code and the commit history around it, it looks like it thinks the URL style you're requesting is a "legacy FastmailSharing URL", and is trying to helpfully (but incorrectly) redirect you to the new-style URL to the user's shared calendar -- and in this case, the target user is "(null)" because it's not a user's shared calendar, it's a global one, and we get a junk URL.

But the if statement also checks for tgt->flags != TGT_DAV_SHARED, which looks to me like it's trying to skip this case of global shared calendars (but I don't know that, I'm simply reading and inferring).

I saw on your info-cyrus post that you did an in-place upgrade from 3.0 to 3.2, so I wonder if there's some metadata here that needs updating during the upgrade that isn't properly documented? Can you reproduce this with a newly-created shared calendar?

I'm not really sure whether redirecting in this case is correct but the redirect is broken, or whether redirecting is wrong and we should be skipping it. What happens if you add a patch like this, that I think will bypass the redirect for this case?

diff --git a/imap/http_dav.c b/imap/http_dav.c
index b124bd171c..f94d73513a 100644
--- a/imap/http_dav.c
+++ b/imap/http_dav.c
@@ -764,6 +764,7 @@ HIDDEN int calcarddav_parse_path(const char *path,
     /* Check for FastMail legacy sharing URLs and redirect */
     if (httpd_userid && !config_getswitch(IMAPOPT_FASTMAILSHARING) &&
         tgt->flags != TGT_DAV_SHARED &&
+        tgt->userid != NULL &&
         !mboxname_userownsmailbox(httpd_userid, mboxname)) {
         buf_reset(&redirect_buf);
         buf_printf(&redirect_buf, "%s/%s/%s/%s%c%s",

@elliefm
Copy link
Contributor

elliefm commented Oct 19, 2020

For cross-referencing purposes, the info-cyrus thread is here: https://cyrus.topicbox.com/groups/info/Td603eb775ed9502c-M47f0329655b05b3eff3164b4

@futzle
Copy link
Author

futzle commented Oct 19, 2020

The same behaviour happens with a newly-created calendar. (I'm not sure how TGT_DAV_SHARED gets set but it looks like it's only set at runtime and not a property of any Cyrus database. I may be wrong.)

I should add that this issue is only visible if the user authenticates (since that is one of the tests in the if-statement). Our shared calendars and address books have restricted permissions: I have used cyradm's sam command to give read access only to certain users. A newly-created calendar by default doesn't require authentication so it seems to work until you add --user to the curl command.

Anyway, I have worked around the problem by setting fastmailsharing in imapd.conf, which also causes the if-test to be skipped:

fastmailsharing: yes

I don't quite know what this option is meant to do, but it's only referenced in this one file so I'm happy with deploying this hack to get my users going today.

@dilyanpalauzov
Copy link
Contributor

Since the fastmailsharing option is the answer, why is this still opened?

@futzle
Copy link
Author

futzle commented Oct 31, 2020

Perhaps all that is needed is for the option to be referenced in the section on CaLDAV support, especially this sentence in https://www.cyrusimap.org/imap/download/installation/http/caldav.html, which is now only true (for authenticated users) when this option is set.

The public calendar hierarchy will reside at the toplevel of the shared mailbox namespace

It might also be worth mentioning this option as needing to be considered while upgrading to 3.2 in https://www.cyrusimap.org/imap/download/upgrade.html since it is a change in default behaviour.

Edit: also there is still the matter of whether the diff that @elliefm suggested earlier should be applied. I think it should: as of 3.2.4, the code is passing NULL to a printf("%s"), which on my libc only inserts "(null)" but on another libc may well segfault.

@lowar
Copy link

lowar commented Dec 29, 2020

There is still a remaining problem in 3.2.5 (I don't know if this also happens in 3.2.4, so maybe it is a new problem):

After applying the fastmailsharing workaround I can now initially import calendar entries and a GET on the shared namespace URL will correctly return the calendar as an ICS file.

But trying to read the calendar using the CALDAV protocol (starting with a PROPFIND) will result in no output because the httpd segfaults.

@c-schwamborn
Copy link

I was looking to replace some other caldav/carddav implementations with cyrus as i use it anyways for years as mailserver on several installations. Cyrus seemed a pretty good choice, since it's written in c it promised to be a fast and stable dav backed, as I'm used to with the imap part.

Sadly the picture cyrus presents with group resources for calendars and addressbooks is not what I expected.
Even if the fastmailsharing option would provide a workaround, which it doesn't, since all tests i did, end in a segfault, I strongly disagree with @dilyanpalauzov, that the issue can be closed even if this would work.

In terms of usability, the whole group resource support for the httpd module needs some rework. What I don't understand is why shared calendars and addressbooks not just work like the mailbox counterpart?
Let a shared calendar reside in <shared_mailbox_name>/#calendars/<calendar_name> would provide a user, who has the appropriate permission, to create new calendars/addressbooks within that resource and the dav path would not be that different from the user path /dav/calendars/<shared_mailbox_name>/<calendar_name> which could be redirected to /dav/calendars/user/<user_name>/shared_mailbox_name>.<calendar_name> as cyrus 3.2.x is supposed to do.

Besides that logical question, I couldn't get shared calendars work at all.
I tested the whole thing on debian buster with stock 3.0.8 version, a self compiled 3.0.14 and the 3.2.5 from backports, all with the same result: As soon as I need to authenticate to access a calendar, cyrus httpd segfaults. As a note: The shared calendars don't show up in a clients auto discovery at all.
Funny behavior which puzzled me in my tests was the fact, that a newly created calendar (lets say #calendars/testcal1, with acls: anyone lrs) can be accessed by Thunderbird. Adding write permissions for the test user while TB is still running and I can even add some events. Closing and restarting TB and cyrus httpd segfaults as soon as the shared calendar is accessed.

Is there currently active development regarding the httpd module in cyrus?

@dilyanpalauzov
Copy link
Contributor

The intended calendar sharing workflow is described at https://evertpot.com/webdav-caldav-carddav-sharing/ . This is different from the IMAP sharing workflow (just ACL changes). The fastmailsharing option is one more way of data sharing, which can be classified as deprecated.

In CalDAV, the client obtains from the servers a “calendar-home-sets” (list of super-folders). Each user has one calendar-home-set, where each of its calendars reside. A client, which has no idea about the principles of calendar sharing described above, would see the list of shared calendars, if the client is first told all calendar-home-sets (the one of the authenticated user and the ones of the other users, which share calendars). And then the CalDAV client has to iterate over each returned calendar-home-set to get the list of shared calendars. (Or return the URLs of the shared calendars of the other users, withing the own calendar-home-set). This is not implemented in Cyrus IMAP.

If nobody works on updating the documentation for fastmailsharing, the documentation will not be updated. But if you propose a new wording, it is not foreseenable whether it will ever be considered. It can get attention e.g. in six months and some questions can be raised. But in six months you have no more idea what were the reasons, have forgotten everythings, so you need a lot of time to deep again in the topic. If however the proposed change never gets attention, you will not have to get into the topic again, once you have forgotten the context.

For these reasons I have not submitted patches upstream, but then noticed that I have a lot of local changes, and jumping to a new version is very much work to adapt each change. So I started submitting changes upstream, keep notices on what is so far upstream and what not, and when I have to switch to a new version, I kind of do not have to forward-port the changes, that are already upstream. That said, it is not predictable whether errors fixed upstream, which errors exist on the “stable branch”, will be fixed also on the stable branch.

You can try to make a calendar “public” with the caldavadmin-http module, shown by GET /dav/calendars/user/zzz@domain and click on public there (I have never used this feature and I do not know what it actually does).

The webdav (no caldav/carddav) module is known to cause segfaults, because e.g. Nautilus does not only mount the provided URL, but tries to mount each URL up in the hierarchy, which httpd does not foresee.

As far as your tests lead to segfaults, because you try to authenticate/use the legacy fastmailsharing options is used, you can provide more detailed description how to reproduce these, in particular provide backtrace with debug symbols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants