[Dovecot] [patch] 'c' and 'd' in setacl
Hi Timo, Hi *,
I just recognized that the new imap-acl plugin in dovecot 1.2 does not know the obsolete rights 'd' and 'c' when setting. According to RFC 4314 section 2.1.1.:
If a client includes the "d" right in a rights list, then it MUST be
treated as if the client had included every member of the "delete"
right.and
If a client includes the "c" right in a rights list, then it MUST be
treated as if the client had included every member of the "create"
right.Unfortunatly there are actually clients which depend on this behavior.
I attached a rather rough[0] patch which implements this.
cheers sascha
[0] I don't like the use of static indexes witch imap_acl_letter_map but currently I wasn't able to decide on a more elegant solution.
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
Hello everyone,
while I understand (and am glad to see) that the codebase evolved almost beyond recognition since I submitted it, from a superficial glance over the present code, I get the impression that it might be better to employ an additional check along the lines of what I had done in the piece of equivalent 1.1.x code I attached (forgive the Windows-style linebreaks and space-based indentations, I copypasted from PuTTY to Notepad...) to prevent adding the same right to the array twice when parsing a letters string that contains both the obsolete rights and any of the regular rights it aliases to (e.g. "xc"). Of course, something like that might be silently intercepted later in the code by now in a change of action that eluded my sight; in that case, the submitted code should work fine.
Either way, I am happy to see this is still being worked on.
Regards, Matvey.
2009/2/6 Sascha Wilde wilde@intevation.de
Hi Timo, Hi *,
I just recognized that the new imap-acl plugin in dovecot 1.2 does not know the obsolete rights 'd' and 'c' when setting. According to RFC 4314 section 2.1.1.:
If a client includes the "d" right in a rights list, then it MUST be treated as if the client had included every member of the "delete" right.
and
If a client includes the "c" right in a rights list, then it MUST be treated as if the client had included every member of the "create" right.
Unfortunatly there are actually clients which depend on this behavior.
I attached a rather rough[0] patch which implements this.
cheers sascha
[0] I don't like the use of static indexes witch imap_acl_letter_map but currently I wasn't able to decide on a more elegant solution.
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/%7Ewilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On Fri, 2009-02-06 at 12:29 +0100, Sascha Wilde wrote:
I just recognized that the new imap-acl plugin in dovecot 1.2 does not know the obsolete rights 'd' and 'c' when setting.
.. [0] I don't like the use of static indexes witch imap_acl_letter_map but currently I wasn't able to decide on a more elegant solution.
How about instead of
array_append(rights, &imap_acl_letter_map[8].name
something like:
str = MAIL_ACL_CREATE; array_append(rights, &str
or something.
Hi Timo,
thanks for your reply!
Timo Sirainen tss@iki.fi writes:
On Fri, 2009-02-06 at 12:29 +0100, Sascha Wilde wrote:
I just recognized that the new imap-acl plugin in dovecot 1.2 does not know the obsolete rights 'd' and 'c' when setting.
.. [0] I don't like the use of static indexes witch imap_acl_letter_map but currently I wasn't able to decide on a more elegant solution.How about instead of
array_append(rights, &imap_acl_letter_map[8].name
something like:
str = MAIL_ACL_CREATE; array_append(rights, &str
I gave it a try in the attached patch. Actually I considered that my self but I'm (still) not sure if this is 100% legal according to the standard.
Partly this is because I haven't fully understood which information from the array "rights" is retained outside the function (by means of "rights_r"). Partly its because I'm not sure if it's ok to refer to the immediate string value used to initialize an automatic variable outside the function.
In case you are sure the attached version is kosher I would indeed like it better than the first.
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 http://www.intevation.de/~wilde/ http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück; AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
Hi,
On 09.02.2009, Sascha Wilde wrote:
Timo Sirainen tss@iki.fi writes: [...]
How about instead of
array_append(rights, &imap_acl_letter_map[8].name
something like:
str = MAIL_ACL_CREATE; array_append(rights, &str
I gave it a try in the attached patch. Actually I considered that my self but I'm (still) not sure if this is 100% legal according to the standard.
After a bit of googling around, I'm now quite sure that it is legal.
MAIL_ACL_CREATE expands to a string literal, and array_append puts the
address of that literal into the array (by memcpy-ing what &str points to).
According to a draft copy of the C89 standard sting literals have static
storage (quoting from [1], section 3.1.4):
Semantics
A character string literal has static storage duration and type
``array of char ,'' and is initialized with the given characters.So storing the address of the literal in the array is legal.
BTW, the OpenPGP signature on Sascha's mail was broken, probable because of this mailman bug: https://bugs.launchpad.net/mailman/+bug/265967
Bernhard
[1] ANSI C89 draft: http://flash-gordon.me.uk/ansi.c.txt linked to from http://en.wikipedia.org/wiki/ANSI_C
-- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On Mon, 2009-02-09 at 13:01 +0100, Bernhard Herzog wrote:
str = MAIL_ACL_CREATE; array_append(rights, &str
I gave it a try in the attached patch. Actually I considered that my self but I'm (still) not sure if this is 100% legal according to the standard.
After a bit of googling around, I'm now quite sure that it is legal.
Right. I think it's also already done elsewhere in Dovecot.
Although:
static const char *acl_k = MAIL_ACL_CREATE;static const char *acl_x = MAIL_ACL_DELETE;
Wonder if it's more optimal for these to be static or non-static. I think I'll have to try what kind of a difference gcc does with them, or if it just optimizes them to the exact same code.
participants (4)
-
Bernhard Herzog
-
Matvey Soloviev
-
Sascha Wilde
-
Timo Sirainen