On Wednesday, August 17, 2011 01:49:34 AM you wrote:
> Well there's a difference, I see "user.base" not "user^base". Sounds
> like there's some difference between our imapd.confs. Although by this
> point I would expect the name to be internalised.
Yep, you are right, my imapd.conf contains the "unixhierarchysep:
true" setting, that is why I was confused. Internally all hierarchy separators
are dots.
> I don't use --trace-children, although I don't see why that should
> matter. Are you getting any log messages from Valgrind running in the
> child imapd process at all?
Not at all, tried it with the proper input as well.
> That sounds better. Another option would be returning early from the
> function. Or using mboxname_to_parts() to do the parsing instead.
Just for reference; the original code of the mboxlist_is_owner function seems
to be extracted (and modified) from the original code base, check the commit
http://git.cyrusimap.org/cyrus-
imapd/commit/?id=4412656e218a42559964ccdce06e8daefb8197c5 here:
@@ -1351,15 +1384,30 @@ int mboxlist_setacl(const char *name, const char
*identifier,
identifier = ident;
}
- if (!strncmp(name+domainlen, "user.", 5) &&
- (!(cp = strchr(userid, '.')) || (cp - userid) > useridlen) &&
- !strncmp(name+domainlen+5, userid, useridlen) &&
- (name[domainlen+5+useridlen] == '\0' ||
- name[domainlen+5+useridlen] == '.')) {
Patch attached for this commit (cyrus-imapd-acl-patch-correction-2.patch),
this should work fine, hope you do not get valgrind errors this time :)
Kristóf
PS Something else came up during manual testing, which is most probably beyond
the scope of this patch and the original commit.
I enabled virtual domains, relevant lines of the imapd.conf file:
virtdomains: yes
defaultdomain: net.lan
admins: admin [email protected]
Playing around with cyradm, I get the following:
[root@intradevel-aiesec cyrus-imapd]# cyradm localhost --user
[email protected]
Password:
intradevel-aiesec.net.lan> listmailbox
admin (\HasNoChildren)
intradevel-aiesec.net.lan> createmailbox user/base
intradevel-aiesec.net.lan> listmailbox
admin (\HasNoChildren) user/base (\HasNoChildren)
intradevel-aiesec.net.lan> listacl user/base
[email protected] lrswipkxtecdan
intradevel-aiesec.net.lan> setacl user/base base all
intradevel-aiesec.net.lan> listacl user/base
base lrswipkxtecda
[email protected] lrswipkxtecdan
intradevel-aiesec.net.lan> setacl user/base base none
intradevel-aiesec.net.lan> listacl user/base
base lkxca
[email protected] lrswipkxtecdan
intradevel-aiesec.net.lan>
intradevel-aiesec.net.lan> setacl user/base [email protected] none
intradevel-aiesec.net.lan> listacl user/base
base lkxca
[email protected] lkxca
My question: who is this "base" user without a domain part in this case?
Someone from the default domain? That should not happen, I guess.
diff --git a/imap/mboxlist.c b/imap/mboxlist.c
index 9995f28..164da19 100644
--- a/imap/mboxlist.c
+++ b/imap/mboxlist.c
@@ -1343,15 +1343,26 @@ int mboxlist_renamemailbox(const char *oldname, const char *newname,
static int mboxlist_is_owner(const char *name, int domainlen,
const char *user, int userlen)
{
- char *cp = NULL;
+ char *dot_position = NULL;
- int enough_length = !strncmp(name+domainlen, "user.", 5);
- int user_complete = (!(cp = strchr(user, '.')) || (cp - user) > userlen);
- int match_strings = !strncmp(name+domainlen+5, user, userlen);
- int match_length = (name[domainlen+5+userlen] == '\0' ||
- name[domainlen+5+userlen] == '.');
-
- return (enough_length && user_complete && match_strings && match_length);
+ int is_user_mbox = !strncmp(name+domainlen, "user.", 5);
+ if(!is_user_mbox)
+ return 0;
+
+ dot_position = strchr(user, '.');
+ if (dot_position && (dot_position - user) <= userlen)
+ return 0;
+
+ int starts_with_user = !strncmp(name+domainlen+5, user, userlen);
+ if(!starts_with_user)
+ return 0;
+
+ int is_exactly_user = (name[domainlen+5+userlen] == '\0' ||
+ name[domainlen+5+userlen] == '.');
+ if(!is_exactly_user)
+ return 0;
+
+ return 1;
}
/*
@@ -1415,6 +1426,7 @@ int mboxlist_setacl(const char *name, const char *identifier,
except for "anonymous", "anyone", the global admin
and users in the default domain */
if ((cp = strchr(identifier, '@'))) {
+ identifierlen = cp - identifier;
if (rights &&
((domain && strncasecmp(cp+1, domain, strlen(cp+1))) ||
(!domain && (!config_defdomain ||
