[Dovecot] v1.2.alpha1 released
http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz.sig
This is the only v1.2.alpha announcement I'm going to send to dovecot-news list. Next announcement will be for either v1.2.beta1 or v1.2.rc1.
I'm hoping that v1.2 stabilizes pretty soon. There are no huge changes like there were in 0.99->1.0 or 1.0->1.1, so most of the bugs are probably related to new features. I'm already running v1.2 myself and it's gone through heavy stress testing with my imaptest tool, so I'm expecting it to be stable enough for real world testing with not-all-that-important servers. It's safe to jump between v1.1 and v1.2 at any time or even have them concurrently accessing the same mailboxes.
Dovecot v1.2 has a lot of new IMAP extensions implemented. The biggest change comes from CONDSTORE extension which adds a new concept of "modification sequences" (modseq for short). Each message has a modseq that increases whenever a flag is changed or when the message is expunged. The client can keep track of the highest seen modseq and request only changes done since the modseq. See QRESYNC (RFC 5162) for more information. Dovecot starts keeping track of the modseqs only when the first "CONDSTORE/QRESYNC enabling command" is executed. Before that no disk space is used for modseqs. When modseqs are enabled dovecot.index file uses 8 bytes per message more space.
Other implemented IMAP extensions are: ESEARCH, SEARCHRES, WITHIN, ID and CONTEXT=SEARCH.
Added thread indexes to optimize IMAP THREAD command. Only the first half of threading is currently optimized, the rest will hopefully follow later.
Added a non-standard X-REFERENCES2 threading algorithm where threads are sorted by their latest message instead of the thread root message. There is also no base subject merging. This may be renamed and become part of SEARCH=INTHREAD extension some day.
SEARCH=INTHREAD is still a draft, but the INTHREAD search key is already implemented by Dovecot v1.2. This is mostly useful with virtual mailboxes.
Virtual mailboxes: http://wiki.dovecot.org/Plugins/Virtual
Then there are also some internal API changes which will make some things easier. For example lib-storage now allows handling multiple different users at the same time.
On Fri, Sep 05, 2008 at 08:49:22PM +0300, Timo Sirainen wrote:
http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz.sig
This is the only v1.2.alpha announcement I'm going to send to dovecot-news list. Next announcement will be for either v1.2.beta1 or v1.2.rc1.
FYI:
pkgsrc users can find the 1.2 alpha series in pkgsrc-wip/dovecot-devel. dovecot-sieve can be used from pkgsrc/mail/dovecot-sieve since there is no sieve 1.2 plugin (the 1.1 plugin works with dovecot 1.2).
GeertTimo Sirainen schrieb:
http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz.sig
This is the only v1.2.alpha announcement I'm going to send to dovecot-news list. Next announcement will be for either v1.2.beta1 or v1.2.rc1.
I'm hoping that v1.2 stabilizes pretty soon. There are no huge changes like there were in 0.99->1.0 or 1.0->1.1, so most of the bugs are probably related to new features. I'm already running v1.2 myself and it's gone through heavy stress testing with my imaptest tool, so I'm expecting it to be stable enough for real world testing with not-all-that-important servers. It's safe to jump between v1.1 and v1.2 at any time or even have them concurrently accessing the same mailboxes.
Dovecot v1.2 has a lot of new IMAP extensions implemented. The biggest change comes from CONDSTORE extension which adds a new concept of "modification sequences" (modseq for short). Each message has a modseq that increases whenever a flag is changed or when the message is expunged. The client can keep track of the highest seen modseq and request only changes done since the modseq. See QRESYNC (RFC 5162) for more information. Dovecot starts keeping track of the modseqs only when the first "CONDSTORE/QRESYNC enabling command" is executed. Before that no disk space is used for modseqs. When modseqs are enabled dovecot.index file uses 8 bytes per message more space.
Other implemented IMAP extensions are: ESEARCH, SEARCHRES, WITHIN, ID and CONTEXT=SEARCH.
Added thread indexes to optimize IMAP THREAD command. Only the first half of threading is currently optimized, the rest will hopefully follow later.
Added a non-standard X-REFERENCES2 threading algorithm where threads are sorted by their latest message instead of the thread root message. There is also no base subject merging. This may be renamed and become part of SEARCH=INTHREAD extension some day.
SEARCH=INTHREAD is still a draft, but the INTHREAD search key is already implemented by Dovecot v1.2. This is mostly useful with virtual mailboxes.
Virtual mailboxes: http://wiki.dovecot.org/Plugins/Virtual
Then there are also some internal API changes which will make some things easier. For example lib-storage now allows handling multiple different users at the same time.
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ?
-- Best Regards
MfG Robert Schetterer
Germany/Munich/Bavaria
On Sat, 2008-09-06 at 09:42 +0200, Robert Schetterer wrote:
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ?
It's not in my plans, but Kolab people are probably going to need it, so maybe they'll implement it. :)
And what do you mean by "not plugin"? It's most likely going to be a plugin in any case.
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 09:42 +0200, Robert Schetterer wrote:
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ?
It's not in my plans, but Kolab people are probably going to need it, so maybe they'll implement it. :)
And what do you mean by "not plugin"? It's most likely going to be a plugin in any case.
Sorry Timo i missposted you *g, the problem is the file based acl ( whatever plugin or not ), i dont know any webmail etc which gives the users themselfs a chance to make their own acls, editing files i.e via ftp is very outdated, what dovecot needs is edit acls direct via the client over imap like its in cyrus, so we can use horde or squirrel webmail
-- Best Regards
MfG Robert Schetterer
Germany/Munich/Bavaria
On Sat, 2008-09-06 at 10:05 +0200, Robert Schetterer wrote:
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 09:42 +0200, Robert Schetterer wrote:
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ?
It's not in my plans, but Kolab people are probably going to need it, so maybe they'll implement it. :)
And what do you mean by "not plugin"? It's most likely going to be a plugin in any case.
Sorry Timo i missposted you *g, the problem is the file based acl ( whatever plugin or not ), i dont know any webmail etc which gives the users themselfs a chance to make their own acls, editing files i.e via ftp is very outdated, what dovecot needs is edit acls direct via the client over imap like its in cyrus, so we can use horde or squirrel webmail
The problem isn't really about making IMAP commands available for the ACL file creation, that's pretty easy to implement. The problem is that once the ACLs have been given the mailboxes should be visible to other users. That's more difficult to implement.
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 10:05 +0200, Robert Schetterer wrote:
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 09:42 +0200, Robert Schetterer wrote:
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ? It's not in my plans, but Kolab people are probably going to need it, so maybe they'll implement it. :)
And what do you mean by "not plugin"? It's most likely going to be a plugin in any case.
Sorry Timo i missposted you *g, the problem is the file based acl ( whatever plugin or not ), i dont know any webmail etc which gives the users themselfs a chance to make their own acls, editing files i.e via ftp is very outdated, what dovecot needs is edit acls direct via the client over imap like its in cyrus, so we can use horde or squirrel webmail
The problem isn't really about making IMAP commands available for the ACL file creation, that's pretty easy to implement. The problem is that once the ACLs have been given the mailboxes should be visible to other users. That's more difficult to implement.
Hi Timo,
i understand, but that would dove make a really dreamy imap server, especially in i.e full isp virtual setups cause its a well needed feature for smaller workgroups, sharing and delegating acls over user and public mailboxes over imap controlled by users themselfes or by a an admin user for me its a bleeding edge which can only be done by cyrus at present ( which i dont like so much by other reasons ) so i hope its comming in dove sometime in the future
-- Best Regards
MfG Robert Schetterer
Germany/Munich/Bavaria
On Sat, 2008-09-06 at 11:11 +0300, Timo Sirainen wrote:
On Sat, 2008-09-06 at 10:05 +0200, Robert Schetterer wrote:
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 09:42 +0200, Robert Schetterer wrote:
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ?
It's not in my plans, but Kolab people are probably going to need it, so maybe they'll implement it. :)
And what do you mean by "not plugin"? It's most likely going to be a plugin in any case.
Sorry Timo i missposted you *g, the problem is the file based acl ( whatever plugin or not ), i dont know any webmail etc which gives the users themselfs a chance to make their own acls, editing files i.e via ftp is very outdated, what dovecot needs is edit acls direct via the client over imap like its in cyrus, so we can use horde or squirrel webmail
The problem isn't really about making IMAP commands available for the ACL file creation, that's pretty easy to implement. The problem is that once the ACLs have been given the mailboxes should be visible to other users. That's more difficult to implement.
Well, I actually started it today since it's needed for replication: http://hg.dovecot.org/dovecot-1.2/rev/6dd0c6755afe
Mailboxes can't be listed yet (and I'm not planning on implementing that anytime soon), but if you add the wanted mailboxes to subscriptions they should be usable by clients. Configuration goes like:
namespace shared { separator = / # %%u gets expanded to the remote user. Instead of %%u you can # also use %%n and %%d. prefix = shared/%%u/ location = Maildir:/home/%%u/Maildir:INDEX=~/Maildir/shared/%%u }
I didn't bother testing it with ACL plugin yet, but I guess it should work.
And as for the IMAP ACL commands then.. Well, they shouldn't be too difficult to implement I think. Someone else could try to do it though. :)
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 11:11 +0300, Timo Sirainen wrote:
On Sat, 2008-09-06 at 10:05 +0200, Robert Schetterer wrote:
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 09:42 +0200, Robert Schetterer wrote:
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ? It's not in my plans, but Kolab people are probably going to need it, so maybe they'll implement it. :)
And what do you mean by "not plugin"? It's most likely going to be a plugin in any case.
Sorry Timo i missposted you *g, the problem is the file based acl ( whatever plugin or not ), i dont know any webmail etc which gives the users themselfs a chance to make their own acls, editing files i.e via ftp is very outdated, what dovecot needs is edit acls direct via the client over imap like its in cyrus, so we can use horde or squirrel webmail The problem isn't really about making IMAP commands available for the ACL file creation, that's pretty easy to implement. The problem is that once the ACLs have been given the mailboxes should be visible to other users. That's more difficult to implement.
Well, I actually started it today since it's needed for replication: http://hg.dovecot.org/dovecot-1.2/rev/6dd0c6755afe
Mailboxes can't be listed yet (and I'm not planning on implementing that anytime soon), but if you add the wanted mailboxes to subscriptions they should be usable by clients. Configuration goes like:
namespace shared { separator = / # %%u gets expanded to the remote user. Instead of %%u you can # also use %%n and %%d. prefix = shared/%%u/ location = Maildir:/home/%%u/Maildir:INDEX=~/Maildir/shared/%%u }
I didn't bother testing it with ACL plugin yet, but I guess it should work.
And as for the IMAP ACL commands then.. Well, they shouldn't be too difficult to implement I think. Someone else could try to do it though. :) Hi Timo, youre great, i will test this soon
-- Best Regards
MfG Robert Schetterer
Germany/Munich/Bavaria
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 11:11 +0300, Timo Sirainen wrote:
On Sat, 2008-09-06 at 10:05 +0200, Robert Schetterer wrote:
Timo Sirainen schrieb:
On Sat, 2008-09-06 at 09:42 +0200, Robert Schetterer wrote:
Hi Timo, what about imap acl full implemented ( not plugin ) is there a chance to have it in v1.2 ? It's not in my plans, but Kolab people are probably going to need it, so maybe they'll implement it. :)
And what do you mean by "not plugin"? It's most likely going to be a plugin in any case.
Sorry Timo i missposted you *g, the problem is the file based acl ( whatever plugin or not ), i dont know any webmail etc which gives the users themselfs a chance to make their own acls, editing files i.e via ftp is very outdated, what dovecot needs is edit acls direct via the client over imap like its in cyrus, so we can use horde or squirrel webmail The problem isn't really about making IMAP commands available for the ACL file creation, that's pretty easy to implement. The problem is that once the ACLs have been given the mailboxes should be visible to other users. That's more difficult to implement.
Well, I actually started it today since it's needed for replication: http://hg.dovecot.org/dovecot-1.2/rev/6dd0c6755afe
Mailboxes can't be listed yet (and I'm not planning on implementing that anytime soon), but if you add the wanted mailboxes to subscriptions they should be usable by clients. Configuration goes like:
namespace shared { separator = / # %%u gets expanded to the remote user. Instead of %%u you can # also use %%n and %%d. prefix = shared/%%u/ location = Maildir:/home/%%u/Maildir:INDEX=~/Maildir/shared/%%u }
I didn't bother testing it with ACL plugin yet, but I guess it should work.
And as for the IMAP ACL commands then.. Well, they shouldn't be too difficult to implement I think. Someone else could try to do it though. :)
Hi Timo if i use %%u %u works
i get failure
dovecot: Sep 17 01:26:03 Error: child 15148 (imap) killed with signal 6 dovecot: Sep 17 01:26:05 Panic: IMAP(robert@schetterer.com): shared mailbox list: Can't return path for 'users/' dovecot: Sep 17 01:26:05 Error: IMAP(robert@schetterer.com): Raw backtrace: /usr/lib/dovecot/imap [0x80e9531] -> /usr/lib/dovecot/imap [0x80e95b2] -> /usr/lib/dovecot/imap [0x80e8f49] -> /usr/lib/dovecot/imap [0x809b8e1] -> /usr/lib/dovecot/modules/imap/lib01_acl_plugin.so [0xb7d9a8ae] -> /usr/lib/dovecot/modules/imap/lib01_acl_plugin.so(acl_object_init_from_name+0x1d) [0xb7d98e2d] -> /usr/lib/dovecot/modules/imap/lib01_acl_plugin.so(acl_backend_init+0x176) [0xb7d99556] -> /usr/lib/dovecot/modules/imap/lib01_acl_plugin.so(acl_mailbox_list_created+0xde) [0xb7d9cece] -> /usr/lib/dovecot/modules/imap/lib20_mail_log_plugin.so [0xb7ef50a4] -> /usr/lib/dovecot/imap(mailbox_list_init+0x13c) [0x80b211c] -> /usr/lib/dovecot/imap [0x809bc17] -> /usr/lib/dovecot/imap(mail_storage_create+0x114) [0x80b0c74] -> /usr/lib/dovecot/imap(mail_namespaces_init+0x29e) [0x80ace0e] -> /usr/lib/dovecot/imap(main+0x3e3) [0x806f173] -> /lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe0) [0xb7db7450] -> /usr/lib/dovecot/imap [0x805ef21] dovecot: Sep 17 01:26:05 Error: child 15152 (imap) killed with signal 6
-- Best Regards
MfG Robert Schetterer
Germany/Munich/Bavaria
On Wed, 2008-09-17 at 01:31 +0200, Robert Schetterer wrote:
dovecot: Sep 17 01:26:05 Panic: IMAP(robert@schetterer.com): shared mailbox list: Can't return path for 'users/'
Fixed:
http://hg.dovecot.org/dovecot-1.2/rev/fd4091d53627 http://hg.dovecot.org/dovecot-1.2/rev/909ed7cd98a5
Timo Sirainen schrieb:
On Wed, 2008-09-17 at 01:31 +0200, Robert Schetterer wrote:
dovecot: Sep 17 01:26:05 Panic: IMAP(robert@schetterer.com): shared mailbox list: Can't return path for 'users/'
Fixed:
http://hg.dovecot.org/dovecot-1.2/rev/fd4091d53627 http://hg.dovecot.org/dovecot-1.2/rev/909ed7cd98a5
Hi Timo, i will test this
-- Best Regards
MfG Robert Schetterer
Germany/Munich/Bavaria
Robert Schetterer schrieb:
Timo Sirainen schrieb:
On Wed, 2008-09-17 at 01:31 +0200, Robert Schetterer wrote:
dovecot: Sep 17 01:26:05 Panic: IMAP(robert@schetterer.com): shared mailbox list: Can't return path for 'users/'
Fixed:
http://hg.dovecot.org/dovecot-1.2/rev/fd4091d53627 http://hg.dovecot.org/dovecot-1.2/rev/909ed7cd98a5
Hi Timo, i will test this
seems to be fixed
-- Best Regards
MfG Robert Schetterer
Germany/Munich/Bavaria
Timo Sirainen <tss@iki.fi> writes:
Well, I actually started it today since it's needed for replication: http://hg.dovecot.org/dovecot-1.2/rev/6dd0c6755afe
Mailboxes can't be listed yet (and I'm not planning on implementing that anytime soon), but if you add the wanted mailboxes to subscriptions they should be usable by clients. Configuration goes like:
namespace shared { separator = / # %%u gets expanded to the remote user. Instead of %%u you can # also use %%n and %%d. prefix = shared/%%u/ location = Maildir:/home/%%u/Maildir:INDEX=~/Maildir/shared/%%u }
Sounds great, and it's an essential feature we need to make Dovecot work with Kolab Server.
Is there a %%h, too? So that, if we have
mail_location = maildir:~
we can say:
namespace shared { separator = / prefix = users/%%u/ location = Maildir:%%h:INDEX=~/Maildir/shared/%%u }
To make user-mailboxess accessible for other users? If not, how hard would it be to implement?
Another (more specific) problem in this context: Is is it possible to determine a users home calling an external program like checkpassword? This would be needed in an setup, where the users $HOME is set by an checkpassword program to an compute value, to access another users mailbox.
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 Intevation GmbH, Osnabrück http://www.intevation.de/~wilde/ Amtsgericht Osnabrück, HR B 18998 http://www.intevation.de/ Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On Tue, 2008-09-30 at 10:46 +0200, Sascha Wilde wrote:
namespace shared { separator = / # %%u gets expanded to the remote user. Instead of %%u you can # also use %%n and %%d. prefix = shared/%%u/ location = Maildir:/home/%%u/Maildir:INDEX=~/Maildir/shared/%%u }
Sounds great, and it's an essential feature we need to make Dovecot work with Kolab Server.
Is there a %%h, too? So that, if we have
mail_location = maildir:~ .. Another (more specific) problem in this context: Is is it possible to determine a users home calling an external program like checkpassword? This would be needed in an setup, where the users $HOME is set by an checkpassword program to an compute value, to access another users mailbox.
This would require doing a userdb lookup from dovecot-auth the same way as deliver or expire-tool does it. So sure it'd be possible, but I'm not really interested in implementing it yet. I think expire-tool is currently using copy&pasted code from deliver, those could be merged into some library function and then the namespace code could easily use the same function.
Timo Sirainen <tss@iki.fi> writes:
On Tue, 2008-09-30 at 10:46 +0200, Sascha Wilde wrote:
namespace shared { separator = / # %%u gets expanded to the remote user. Instead of %%u you can # also use %%n and %%d. prefix = shared/%%u/ location = Maildir:/home/%%u/Maildir:INDEX=~/Maildir/shared/%%u }
Sounds great, and it's an essential feature we need to make Dovecot work with Kolab Server.
Is there a %%h, too? So that, if we have
mail_location = maildir:~ .. Another (more specific) problem in this context: Is is it possible to determine a users home calling an external program like checkpassword? This would be needed in an setup, where the users $HOME is set by an checkpassword program to an compute value, to access another users mailbox.
This would require doing a userdb lookup from dovecot-auth the same way as deliver or expire-tool does it.
I'm not quite sure what you mean by "this" here, are you referring to the proposed `%%h' variable, too or only to my more specific problem with computer HOME paths?
So sure it'd be possible, but I'm not really interested in implementing it yet. I think expire-tool is currently using copy&pasted code from deliver, those could be merged into some library function and then the namespace code could easily use the same function.
But is deliver currently able to utilize an external program to get user data?
From reading the docs I got the impression that userdb only allows to use data supplied by an arbitrary program by the "Prefetch" backend in combination with an checkpassword passdb, and that deliver can't use this mechanism as the user doesn't login when deliver is run.
So I guess what is needed is a new userdb backend which is explicitly runs an arbitrary external program to get the user data (instead of caching the passdb results).
What do you think?
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 Intevation GmbH, Osnabrück http://www.intevation.de/~wilde/ Amtsgericht Osnabrück, HR B 18998 http://www.intevation.de/ Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On Sep 30, 2008, at 6:08 PM, Sascha Wilde wrote:
Is there a %%h, too? So that, if we have
mail_location = maildir:~ .. Another (more specific) problem in this context: Is is it possible
to determine a users home calling an external program like
checkpassword? This would be needed in an setup, where the users $HOME is set by an checkpassword program to an compute value, to access another users mailbox.This would require doing a userdb lookup from dovecot-auth the same
way as deliver or expire-tool does it.I'm not quite sure what you mean by "this" here, are you referring to the proposed `%%h' variable, too or only to my more specific problem with computer HOME paths?
I think it's the same thing.
So sure it'd be possible, but I'm not really interested in implementing it yet. I think expire-tool is currently using copy&pasted code from deliver, those could be merged into some library function and then the namespace code could easily
use the same function.But is deliver currently able to utilize an external program to get user data?
deliver will do the userdb lookup from dovecot-auth, which in turn can
use the external program.
So I guess what is needed is a new userdb backend which is explicitly runs an arbitrary external program to get the user data (instead of caching the passdb results).
Right. Perhaps the passdb checkpassword code could be used as userdb
too, just with an added extra variable specifying if it's a passdb or
a userdb lookup. Or maybe instead of sending "user \0 pass \0" it'd
just send "user". I'm not really sure. In any case I think the reply
should be handled somewhat differently so that the checkpassword can't
accidentally think it's doing a userdb lookup while it's really doing
a passdb lookup and return success.
Timo Sirainen <tss@iki.fi> writes:
On Sep 30, 2008, at 6:08 PM, Sascha Wilde wrote:
Is there a %%h, too? So that, if we have
mail_location = maildir:~ .. Another (more specific) problem in this context: Is is it possible to determine a users home calling an external program like checkpassword? This would be needed in an setup, where the users $HOME is set by an checkpassword program to an compute value, to access another users mailbox.
This would require doing a userdb lookup from dovecot-auth the same way as deliver or expire-tool does it.
I'm not quite sure what you mean by "this" here, are you referring to the proposed `%%h' variable, too or only to my more specific problem with computer HOME paths?
I think it's the same thing.
Is it? I might be wrong, but i thought for configurations where userdb doesn't depend on the passdb implementing %%h as the home directory of user %%u should be straight forward. Or am I missing something?
[...]
So I guess what is needed is a new userdb backend which is explicitly runs an arbitrary external program to get the user data (instead of caching the passdb results).
Right. Perhaps the passdb checkpassword code could be used as userdb too,
God, so we will try to go this way.
just with an added extra variable specifying if it's a passdb or a userdb lookup. Or maybe instead of sending "user \0 pass \0" it'd just send "user". I'm not really sure. In any case I think the reply should be handled somewhat differently so that the checkpassword can't accidentally think it's doing a userdb lookup while it's really doing a passdb lookup and return success.
Ack. I or someone else from the Kolab/Dovecot team will write a short proposal on the list as soon as we have one... ;-)
cheers sascha
Sascha Wilde OpenPGP key: 4BB86568 Intevation GmbH, Osnabrück http://www.intevation.de/~wilde/ Amtsgericht Osnabrück, HR B 18998 http://www.intevation.de/ Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
On Sep 30, 2008, at 6:48 PM, Sascha Wilde wrote:
On Sep 30, 2008, at 6:08 PM, Sascha Wilde wrote:
Is there a %%h, too? So that, if we have
mail_location = maildir:~ .. Another (more specific) problem in this context: Is is it possible to determine a users home calling an external program like checkpassword? This would be needed in an setup, where the users $HOME is set
by an checkpassword program to an compute value, to access another users mailbox.This would require doing a userdb lookup from dovecot-auth the same way as deliver or expire-tool does it.
I'm not quite sure what you mean by "this" here, are you referring
to the proposed `%%h' variable, too or only to my more specific problem with computer HOME paths?I think it's the same thing.
Is it? I might be wrong, but i thought for configurations where
userdb doesn't depend on the passdb implementing %%h as the home directory of user %%u should be straight forward. Or am I missing something?
I guess I just misunderstood what you meant. All I meant was that %%h
expansion would always have to be done using a userdb lookup.
Timo Sirainen <tss@iki.fi> writes:
On Sep 30, 2008, at 6:08 PM, Sascha Wilde wrote: [...]
So I guess what is needed is a new userdb backend which is explicitly runs an arbitrary external program to get the user data (instead of caching the passdb results).
Right. Perhaps the passdb checkpassword code could be used as userdb too, just with an added extra variable specifying if it's a passdb or a userdb lookup.
I just started to work on this feature and for testing purpose I wrote a very simple dummy checkpassword program. But I have a problem setting the UID and GID:
I'm using:
userdb_uid=12345
userdb_gid=12345
EXTRA="userdb_uid userdb_gid"
export userdb_uid userdb_gid EXTRAaccording to http://wiki.dovecot.org/PasswordDatabase/CheckPassword but then I get an internal login failure. From the dovecot log:
Oct 08 12:42:02 burlywood3 <info> dovecot[3804]: auth(default): prefetch(1@example.com,192.168.11.254): success Oct 08 12:42:02 burlywood3 <info> dovecot[3804]: auth(default): master out: USER 31@example.com home=/kolab/var/dovecot/spool/1@example.com/home uid=0 gid=0 uid=19415 gid=19415 Oct 08 12:42:02 burlywood3 <error> dovecot[3804]: uid specified multiple times for 1@example.com
So am I missing something or is this dovecot extension currently broken?
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
On Wed, 2008-10-08 at 12:54 +0200, Sascha Wilde wrote:
I just started to work on this feature and for testing purpose I wrote a very simple dummy checkpassword program. But I have a problem setting the UID and GID:
I'm using:
userdb_uid=12345 userdb_gid=12345 EXTRA="userdb_uid userdb_gid" export userdb_uid userdb_gid EXTRAaccording to http://wiki.dovecot.org/PasswordDatabase/CheckPassword but
I guess it worked more or less accidentally at some point. Changed now so it should really work: http://hg.dovecot.org/dovecot-1.2/rev/a38778911fa9
Timo Sirainen <tss@iki.fi> writes:
On Wed, 2008-10-08 at 12:54 +0200, Sascha Wilde wrote:
I just started to work on this feature and for testing purpose I wrote a very simple dummy checkpassword program. But I have a problem setting the UID and GID:
I'm using:
userdb_uid=12345 userdb_gid=12345 EXTRA="userdb_uid userdb_gid" export userdb_uid userdb_gid EXTRAaccording to http://wiki.dovecot.org/PasswordDatabase/CheckPassword but
I guess it worked more or less accidentally at some point. Changed now so it should really work: http://hg.dovecot.org/dovecot-1.2/rev/a38778911fa9
Thanks, works now for me as expected.
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
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes:
On Sep 30, 2008, at 6:08 PM, Sascha Wilde wrote: [...]
So I guess what is needed is a new userdb backend which is explicitly runs an arbitrary external program to get the user data (instead of caching the passdb results).
Right. Perhaps the passdb checkpassword code could be used as userdb too, just with an added extra variable specifying if it's a passdb or a userdb lookup.
I just started to work on this feature
I have made a first rough sketch of the userdb-checkpassword back end, basically by copying the code of the passdb version. Now I stumbled upon the note "FIXME: if we ever do some other kind of forking, this needs fixing" in sigchld_handler(). The only problem I experienced is, that the handler dosen't return, when the signaling child wasn't listed in the modules clients -- but simply replacing the continue with return like this: --- a/src/auth/passdb-checkpassword.c Fri Oct 10 12:06:06 2008 +0200 +++ b/src/auth/passdb-checkpassword.c Fri Oct 10 21:34:36 2008 +0200 @@ -165,7 +165,7 @@ "with signal %d", dec2str(pid), WTERMSIG(status)); } - continue; + return; } if (WIFSIGNALED(status)) { seems to fix this. The next handler is called and every thing seems to work well (at least at a first very short test). Is there any reason not to do so? And are there any other issues you had in mind writing the FIXME? 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
Sascha Wilde <wilde@intevation.de> writes:
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes:
On Sep 30, 2008, at 6:08 PM, Sascha Wilde wrote: [...]
So I guess what is needed is a new userdb backend which is explicitly runs an arbitrary external program to get the user data (instead of caching the passdb results).
Right. Perhaps the passdb checkpassword code could be used as userdb too, just with an added extra variable specifying if it's a passdb or a userdb lookup.
I just started to work on this feature
I have made a first rough sketch of the userdb-checkpassword back end, basically by copying the code of the passdb version. Now I stumbled upon the note "FIXME: if we ever do some other kind of forking, this needs fixing" in sigchld_handler().
The only problem I experienced is, that the handler dosen't return, when the signaling child wasn't listed in the modules clients -- but simply replacing the continue with return like this:
Please ignore this statement -- wrong test, wrong result ...
The proposed change is not sufficient, the only reason why my test didn't fail was, that I used an other passdb backend, so that my userdb backend was the only one registering a SIGCHILD handler...
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
On Oct 11, 2008, at 10:52 AM, Sascha Wilde wrote:
I have made a first rough sketch of the userdb-checkpassword back
end, basically by copying the code of the passdb version. Now I stumbled upon the note "FIXME: if we ever do some other kind of forking, this needs fixing" in sigchld_handler().The only problem I experienced is, that the handler dosen't return,
when the signaling child wasn't listed in the modules clients -- but
simply replacing the continue with return like this:Please ignore this statement -- wrong test, wrong result ...
The proposed change is not sufficient, the only reason why my test didn't fail was, that I used an other passdb backend, so that my
userdb backend was the only one registering a SIGCHILD handler...
This might be helpful:
http://hg.dovecot.org/icecap/file/401c2bc71594/src/lib/child-wait.h http://hg.dovecot.org/icecap/file/401c2bc71594/src/lib/child-wait.c
Idea would be that you'd have two child_waits, one for passdb and one
for userdb. Then you'd add all the PIDs to that wait structure. I
guess it should also have a plain child_wait_new() without a PID.
Timo Sirainen <tss@iki.fi> writes:
On Oct 11, 2008, at 10:52 AM, Sascha Wilde wrote:
The proposed change is not sufficient, the only reason why my test didn't fail was, that I used an other passdb backend, so that my userdb backend was the only one registering a SIGCHILD handler...
This might be helpful:
http://hg.dovecot.org/icecap/file/401c2bc71594/src/lib/child-wait.h http://hg.dovecot.org/icecap/file/401c2bc71594/src/lib/child-wait.c
Thanks! Seems indeed helpful. I have changed passdb-checkpassword to use the child-wait stuff, see attached patch. (I have put child-wait.[ch] into src/lib/)
Now I'll try to do the same for my userdb, so that they should work at the same time -- stay tuned...
cheer 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
Sascha Wilde <wilde@intevation.de> writes:
Thanks! Seems indeed helpful. I have changed passdb-checkpassword to use the child-wait stuff, see attached patch. (I have put child-wait.[ch] into src/lib/)
Doh! Forgot to attach the patch (which isn't to bad as it was faulty anyway...).
This time I really attached it!
Now I'll try to do the same for my userdb, so that they should work at the same time -- stay tuned...
I have a first working beta. I'll put a repository with the changes on line soon...
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
Sascha Wilde <wilde@intevation.de> writes:
Sascha Wilde <wilde@intevation.de> writes:
Now I'll try to do the same for my userdb, so that they should work at the same time -- stay tuned...
I have a first working beta. I'll put a repository with the changes on line soon...
Done. You can find all the stuff here: http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/
The addition of child_wait and changes to passdb-checkpassword are in changeset 53b57b123f74, the new userdb back end is in a4d3ea1e52bc.
Next steps will be cleaning up and documentation (at least in the sample config).
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
As announced in MID <s7w4p3ec6pw.fsf@intevation.de> I wrote[0] a new userdb back end, which uses a checkpassword like program to retrieve user data.
This is needed to get computed user data without authentication for the LDA or the yet to be implemented %%h variable in shared user folder name spaces...
The back end needs a special checkpassword program which follows the qmail semantics but additionally provides the user data without password verification when the environment variable AUTHORIZED is set.[1]
I have done some code cleanup (mainly factoring out common code of the passdb and userdb back ends) and you can found the current version, alongside with our acl-plugin enhancements, here:
http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/
Timo, what would be needed to get the new back end upstream?
cheers sascha
[0] Well mostly copy and paste from the existing passdb-checkpassword... [1] The variable name needs some evaluation: it seems, that courier knows an environment variable AUTHENTICATED, which might be a good choice for us, too -- on the other hand it might be totally wrong. ;-)
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
On Fri, 2008-10-17 at 19:04 +0200, Sascha Wilde wrote:
The back end needs a special checkpassword program which follows the qmail semantics but additionally provides the user data without password verification when the environment variable AUTHORIZED is set.[1]
I have done some code cleanup (mainly factoring out common code of the passdb and userdb back ends) and you can found the current version, alongside with our acl-plugin enhancements, here:
http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/
Timo, what would be needed to get the new back end upstream?
Some small things:
rename checkpassword-common.c to db-checkpassword.c so it's consistent with others.
userdb checkpassword is a new dovecot-specific extension, so you can drop all vpopmail etc. exit code handlers. Just 3 needed: success, user doesn't exist and internal error (also being the default).
a valid userdb checkpassword script shouldn't be a valid passdb checkpassword script to avoid accidents. I guess this could be done by
Require userdb scripts to set USERDB environment.
checkpassword-reply checks if USERDB environment is set. If it is, return exit code 2 instead of 0.
userdb-checkpassword.c's success exit code is 2. exit code 0 would produce failure.
Hmm. Or perhaps instead of USERDB change the AUTHORIZED environment's value to something else.
Timo Sirainen <tss@iki.fi> writes:
On Fri, 2008-10-17 at 19:04 +0200, Sascha Wilde wrote:
http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/
Timo, what would be needed to get the new back end upstream?
Some small things:
- rename checkpassword-common.c to db-checkpassword.c so it's consistent with others.
[x] done
- userdb checkpassword is a new dovecot-specific extension, so you can drop all vpopmail etc. exit code handlers. Just 3 needed: success, user doesn't exist and internal error (also being the default).
[x] done
Currently the code handles only two cases: success and (any kind of)
error. The passdb-checkpassword stuff seems not to handle "user
doesn't exist" in any special way, so I don't see why the userdb
backend should.
- a valid userdb checkpassword script shouldn't be a valid passdb checkpassword script to avoid accidents. I guess this could be done by
I don't agree here. I think it would be ok to have only one checkpassword executable to handle both cases.
Require userdb scripts to set USERDB environment.
checkpassword-reply checks if USERDB environment is set. If it is, return exit code 2 instead of 0.
userdb-checkpassword.c's success exit code is 2. exit code 0 would produce failure.
Hmm. Or perhaps instead of USERDB change the AUTHORIZED environment's value to something else.
I fully agree that it is a very good idea that, if AUTHORIZED is set checkpassword-reply should return something != 0 at success and userdb-checkpassword should expect this very value.
I'll implement that.
I don't understand why the checkpassword program[0] should change the environment in any way.
cheers sascha
[0] I guess that's what you mean by "userdb scripts"
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
On Mon, 2008-10-20 at 17:26 +0200, Sascha Wilde wrote:
Currently the code handles only two cases: success and (any kind of) error. The passdb-checkpassword stuff seems not to handle "user doesn't exist" in any special way, so I don't see why the userdb backend should.
The difference is that userdb lookups need to know if the user exists or if the error is only temporary. That determines if deliver returns EX_TEMPFAIL or EX_NOUSER.
- a valid userdb checkpassword script shouldn't be a valid passdb checkpassword script to avoid accidents. I guess this could be done by
I don't agree here. I think it would be ok to have only one checkpassword executable to handle both cases.
Yes, but a checkpassword script written to handle *only* userdb lookups shouldn't be a valid passdb script.
Require userdb scripts to set USERDB environment.
checkpassword-reply checks if USERDB environment is set. If it is, return exit code 2 instead of 0.
userdb-checkpassword.c's success exit code is 2. exit code 0 would produce failure.
Hmm. Or perhaps instead of USERDB change the AUTHORIZED environment's value to something else.
I fully agree that it is a very good idea that, if AUTHORIZED is set checkpassword-reply should return something != 0 at success and userdb-checkpassword should expect this very value.
I'll implement that.
I don't understand why the checkpassword program[0] should change the environment in any way.
The idea was that if there's a checkpassword script that handles only userdb lookups and it's tried to be used as passdb checkpassword, it would fail because checkpassword-reply sees AUTHORIZED=2 environment, which would cause it to return 2 which would cause passdb checkpassword to fail the authentication. The rest of it is to just get this idea working for both passdb and userdb lookups.
Timo Sirainen <tss@iki.fi> writes:
On Mon, 2008-10-20 at 17:26 +0200, Sascha Wilde wrote:
Currently the code handles only two cases: success and (any kind of) error. The passdb-checkpassword stuff seems not to handle "user doesn't exist" in any special way, so I don't see why the userdb backend should.The difference is that userdb lookups need to know if the user exists or if the error is only temporary. That determines if deliver returns EX_TEMPFAIL or EX_NOUSER.
Ah, I see. I'll implement it accordingly.
- a valid userdb checkpassword script shouldn't be a valid passdb checkpassword script to avoid accidents. I guess this could be done by
I don't agree here. I think it would be ok to have only one checkpassword executable to handle both cases.
Yes, but a checkpassword script written to handle *only* userdb lookups shouldn't be a valid passdb script.
Ok, we can agree on that. But I think it would be sufficient to say that such an userdb only checkpassword script MUST fail if AUTHORIZED is not set.
Require userdb scripts to set USERDB environment.
checkpassword-reply checks if USERDB environment is set. If it is, return exit code 2 instead of 0.
userdb-checkpassword.c's success exit code is 2. exit code 0 would produce failure.
Hmm. Or perhaps instead of USERDB change the AUTHORIZED environment's value to something else.
I fully agree that it is a very good idea that, if AUTHORIZED is set checkpassword-reply should return something != 0 at success and userdb-checkpassword should expect this very value.
I'll implement that.
I don't understand why the checkpassword program[0] should change the environment in any way.
The idea was that if there's a checkpassword script that handles only userdb lookups and it's tried to be used as passdb checkpassword, it would fail because checkpassword-reply sees AUTHORIZED=2 environment, which would cause it to return 2 which would cause passdb checkpassword to fail the authentication.
I understand the idea now, but see above: we need the (userdb only) checkpassword script to follow our rules anyway, so instead of doing magic to the environment and checking for this in checkpassword-reply it should be sufficient for the script to fail if AUTHORIZED wasn't set.
Or am I missing something?
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
On Oct 20, 2008, at 7:08 PM, Sascha Wilde wrote:
I understand the idea now, but see above: we need the (userdb only) checkpassword script to follow our rules anyway, so instead of doing magic to the environment and checking for this in checkpassword- reply it should be sufficient for the script to fail if AUTHORIZED wasn't set.
Or am I missing something?
The problem is that you said that AUTHORIZED is set automatically when
userdb checkpassword script is called. So the script doesn't have to
set it manually. That makes the script automatically work as userdb
script (because AUTHORIZED is set automatically) and as passdb script
(because AUTHORIZED isn't set automatically). That kind of breaks the
idea.
Timo Sirainen <tss@iki.fi> writes:
On Oct 20, 2008, at 7:08 PM, Sascha Wilde wrote:
I understand the idea now, but see above: we need the (userdb only) checkpassword script to follow our rules anyway, so instead of doing magic to the environment and checking for this in checkpassword- reply it should be sufficient for the script to fail if AUTHORIZED wasn't set.
Or am I missing something?
The problem is that you said that AUTHORIZED is set automatically when userdb checkpassword script is called.
Yes, the userdb back end sets AUTHORIZED.
So the script doesn't have to set it manually.
Yes, the script doesn't change the environment in any other way than any qmail checkpassword script would.
That makes the script automatically work as userdb script (because AUTHORIZED is set automatically)
...yes, when it is called by the userdb backend...
and as passdb script (because AUTHORIZED isn't set automatically).
...when it is called by the passdb backend, yes.
That kind of breaks the idea.
Sorry I don't get it. The case we want to prevent is that a userdb only checkpassword gets accidentally abused by passdb for authorization, right?
Your solution is:
1. The userdb-only checkpassword script changes the environment in
some way.
2. checkpassword-reply detects the change and returns with an exit
code != 0
3. The passdb backend sees its child's exit code is != 0 and so the
authorization has failedMy solution:
1. The userdb-only checkpassword script sees no AUTHORIZED in the
environment and returns with an exit code != 0[0]
2. The passdb backend sees its child's exit code is != 0 and so the
authorization has failedSo whats the functional difference?
cheers sascha
[0] and != 2 as this is what the userdb backend expects for success as we decided.
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
On Oct 20, 2008, at 8:00 PM, Sascha Wilde wrote:
My solution:
- The userdb-only checkpassword script sees no AUTHORIZED in the environment and returns with an exit code != 0[0]
You assume that the script actually checks this. There's no
requirement that a userdb-only script needs to bother doing it. The
use of AUTHORIZED environment is necessary only if the script wants to
handle both passdb and userdb.
Timo Sirainen <tss@iki.fi> writes:
On Oct 20, 2008, at 8:00 PM, Sascha Wilde wrote:
My solution:
- The userdb-only checkpassword script sees no AUTHORIZED in the environment and returns with an exit code != 0[0]
You assume that the script actually checks this.
More than that, I defined that it MUST do so. As you said, it's a new variant, so _we_ can define how it has to behave.
There's no requirement that a userdb-only script needs to bother doing it. The use of AUTHORIZED environment is necessary only if the script wants to handle both passdb and userdb.
But you are requiring the userdb-only checkpassword program to set AUTHORIZED (or any other environment variable) to a specific value. Why should a developer ignoring my requirement bother to obey yours?
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
On Oct 20, 2008, at 8:57 PM, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Oct 20, 2008, at 8:00 PM, Sascha Wilde wrote:
My solution:
- The userdb-only checkpassword script sees no AUTHORIZED in the environment and returns with an exit code != 0[0]
You assume that the script actually checks this.
More than that, I defined that it MUST do so. As you said, it's a new variant, so _we_ can define how it has to
behave.
People are badly behaving :) There's nothing that's technically
forcing to check that.
There's no requirement that a userdb-only script needs to bother
doing it. The use of AUTHORIZED environment is necessary only if the script wants to handle both passdb and userdb.But you are requiring the userdb-only checkpassword program to set AUTHORIZED (or any other environment variable) to a specific value.
Why should a developer ignoring my requirement bother to obey yours?
If you aren't changing AUTHORIZED environment to a specific value, the
userdb lookup will fail because checkpassword-reply sees that it's not
set correctly. So the handling goes like:
userdb lookup: userdb-only checkpassword script setting
AUTHORIZED=2 -> checkpassword returns 2 -> dovecot-auth assumes okpassdb lookup: userdb-only checkpassword script setting
AUTHORIZED=2 -> checkpassword returns 2 -> dovecot-auth fails the
passdb lookupuserdb lookup: passdb-only checkpassword script doesn't set
AUTHORIZED=2 -> checkpassword returns 0 -> dovecot-auth fails the
userdb lookuppassdb lookup: passdb-only checkpassword script doesn't set
AUTHORIZED=2 -> checkpassword returns 0 -> dovecot-auth assumes ok
All of this forces that the checkpassword script developer either
handles the AUTHORIZED environment correctly or it doesn't work at
all. And it prevents admin from accidentally using the script wrong.
Timo Sirainen <tss@iki.fi> writes: [...]
All of this forces that the checkpassword script developer either handles the AUTHORIZED environment correctly or it doesn't work at all. And it prevents admin from accidentally using the script wrong.
Ok, you convinced me that your concept has the advantage of forcing the checkpassword script author to try to implement all aspects of the spec.
I'll implement it this way, as it isn't hard to do anyway.
But I have to say, that I'm not really sure about the merits of trying to force other developers to do the right thing like this. The developer can still change the environment only when AUTHORIZED is set (why would he do something like that you ask? Maybe he did just copy and paste from a combined passdb and userdb checkpassword), the admin writing his own broken userdb checkpassword script still can use it for passdb, it only won't work for userdb (he will use prefetch until he finds the bug...). After all, nothing is foolproof to a sufficiently talented fool. ;-)
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
On Oct 20, 2008, at 10:19 PM, Sascha Wilde wrote:
Ok, you convinced me that your concept has the advantage of forcing
the checkpassword script author to try to implement all aspects of the spec. .. After all, nothing is foolproof to a sufficiently talented fool. ;-)
Sure, but I like it better if it's foolproof to at least less talented
fools :)
Ever since I took these Human-Computer-Interfacing classes I've
started thinking about ways to make things more easier (and
foolproof). There was this one example about how difficult it was to
design a web page (about 10 years ago) that told the user to maximize
the browser window and then click continue button to start the web
application. Most people succeeded initially but getting the rest of
the people to succeed was a lot more difficult. Unfortunately I don't
have the URL for that page now. Anyway I think those people who failed
at first could be thought of as fools, but with some more design it
was possible to make it foolproof :)
On Oct 20, 2008, at 10:40 PM, Timo Sirainen wrote:
Ever since I took these Human-Computer-Interfacing classes I've
started thinking about ways to make things more easier (and
foolproof). There was this one example about how difficult it was to
design a web page (about 10 years ago) that told the user to
maximize the browser window and then click continue button to start
the web application. Most people succeeded initially but getting the
rest of the people to succeed was a lot more difficult.
Unfortunately I don't have the URL for that page now. Anyway I think
those people who failed at first could be thought of as fools, but
with some more design it was possible to make it foolproof :)
Found it :) http://www.asktog.com/columns/000maxscrns.html
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes: [...]
All of this forces that the checkpassword script developer either handles the AUTHORIZED environment correctly or it doesn't work at all. And it prevents admin from accidentally using the script wrong.
Ok, you convinced me that your concept has the advantage of forcing the checkpassword script author to try to implement all aspects of the spec.
I'll implement it this way, as it isn't hard to do anyway.
Ok. 8ebf5a64e6eb includes the discussed changes.
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
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes: [...]
All of this forces that the checkpassword script developer either handles the AUTHORIZED environment correctly or it doesn't work at all. And it prevents admin from accidentally using the script wrong.
Ok, you convinced me that your concept has the advantage of forcing
the checkpassword script author to try to implement all aspects of the spec.I'll implement it this way, as it isn't hard to do anyway.
Ok. 8ebf5a64e6eb includes the discussed changes.
The code is now in dovecot-1.2 tree. I did some minor changes, mostly
related to getting coding style consistent with the rest of Dovecot.
It probably would have been possible to have the passdb and userdb
share more code, but it's good enough now. :)
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
[userdb-checkpassword]
The code is now in dovecot-1.2 tree.
Thank you, that's great! The only thing I'm missing is the addition to the example.conf I made. (I have to admit it was only a stub, though)
I did some minor changes, mostly related to getting coding style consistent with the rest of Dovecot. It probably would have been possible to have the passdb and userdb share more code, but it's good enough now. :)
I agree. Unfortunately I had no time to factor out the more tricky parts. (There is still a bunch of features we have to implement till November...)
Apropos implementing features:
I had a look at the deliver code to figure out how to get userdb data From the auth server (I need this in shared-storage.c to implement %%h).
There are more than 250LOC in deliver/auth-client.c and I wonder if there is already a higher level api for auth clients? I would have expected something like this in lib-auth, but the stuff in there seems not to be what I'm looking for. Any hints?
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
On Wed, 2008-10-22 at 16:15 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
[userdb-checkpassword]
The code is now in dovecot-1.2 tree.
Thank you, that's great! The only thing I'm missing is the addition to the example.conf I made. (I have to admit it was only a stub, though)
http://hg.dovecot.org/dovecot-1.2/rev/4497c58eaca8 adds some other missing changes too. I also decided to change AUTHORIZED=YES to AUTHORIZED=1 initially. I did also think about yes -> done or yes -> userdb or something similar, but 1 -> 2 seemed the best.
There are more than 250LOC in deliver/auth-client.c and I wonder if there is already a higher level api for auth clients? I would have expected something like this in lib-auth, but the stuff in there seems not to be what I'm looking for. Any hints?
plugins/expire-tool/auth-client.c has copy&pasted that code also.. So it would be nice if it was put to lib-auth :)
Timo Sirainen <tss@iki.fi> writes:
On Wed, 2008-10-22 at 16:15 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
[userdb-checkpassword]
The code is now in dovecot-1.2 tree.
Thank you, that's great! The only thing I'm missing is the addition to the example.conf I made. (I have to admit it was only a stub, though)
http://hg.dovecot.org/dovecot-1.2/rev/4497c58eaca8 adds some other missing changes too. I also decided to change AUTHORIZED=YES to AUTHORIZED=1 initially. I did also think about yes -> done or yes -> userdb or something similar, but 1 -> 2 seemed the best.
Ack.
There are more than 250LOC in deliver/auth-client.c and I wonder if there is already a higher level api for auth clients? I would have expected something like this in lib-auth, but the stuff in there seems not to be what I'm looking for. Any hints?
plugins/expire-tool/auth-client.c has copy&pasted that code also.. So it would be nice if it was put to lib-auth :)
Ok, I'll consider doing so... :)
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
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes:
On Wed, 2008-10-22 at 16:15 +0200, Sascha Wilde wrote:
There are more than 250LOC in deliver/auth-client.c and I wonder if there is already a higher level api for auth clients? I would have expected something like this in lib-auth, but the stuff in there seems not to be what I'm looking for. Any hints?
plugins/expire-tool/auth-client.c has copy&pasted that code also.. So it would be nice if it was put to lib-auth :)
Ok, I'll consider doing so... :)
Having a first look it turns out to be less straight forward then I hoped it would be. While there are significant amounts of code similar in deliver/auth-client.c and expire/auth-client.c they differ in many aspects:
1.) It seems that some code in deliver/auth-client.c has been revised after it was copied to expire/auth-client.c, this is a small problem as I would expect simply using the newer code to be the right thing[tm].
2.) The exported interface in the respective auth-client.h files is different. The solution would be to figure out what the right interface would be and change the current code to use it. My problem I'm not sure what the right interface would look like, for my use the one in expire/auth-client.h looks more compelling, what do you think?
3.) The deliver version does more than I need, and most certainly more than it should in the generic case: the most obvious example is that it sets up RESTRICT_* environment and calls restrict_access_by_env(TRUE); which surely is nothing I want to do in my code...
My current plan is to take only the code from deliver/auth-client and check which parts I need, factor these out to new file in lib-auth (unfortunately lib-auth/auth-client already exists) and finally ask the author of the expire plugin to change his code so that it uses the new stuff in lib-auth (I doubt that I will have the time to do this on my own).
Obviously a god answer on 2. is badly needed... ;-)
One final question: All the code saves the gathered user data by setting the environment accordingly (especially HOME, which is the one of interest for my code) -- but in my case I'm requesting the data for foreign user so setting HOME wouldn't be good idea.
I see two possible solutions:
- Simple and stupid: save HOME, call the client-auth code, read HOME and reset its value to the saved one.
- Clean but grows the API: export another function from auth-client, which does not set env-vars but returns the requested data in some struct.
Any thoughts on that?
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
On Thu, 2008-10-23 at 16:18 +0200, Sascha Wilde wrote:
1.) It seems that some code in deliver/auth-client.c has been revised after it was copied to expire/auth-client.c, this is a small problem as I would expect simply using the newer code to be the right thing[tm].
Yes, I haven't really looked at expire/auth-client.c much lately.
2.) The exported interface in the respective auth-client.h files is different. The solution would be to figure out what the right interface would be and change the current code to use it. My problem I'm not sure what the right interface would look like, for my use the one in expire/auth-client.h looks more compelling, what do you think?
Perhaps something like:
struct auth_user_reply { uid_t uid; gid_t gid; const char *home, *chroot; ARRAY_TYPE(const_string) extra_fields; };
struct auth_connection *auth_connection_init(const char *auth_socket); void auth_connection_deinit(struct auth_connection *conn);
/* Returns -1 = error, 0 = user not found, 1 = ok */ int auth_connection_lookup(struct auth_connection *conn, const char *user, struct auth_user_reply *reply_r);
I'm not sure about the struct, but maybe something like that. deliver would then use the struct to set up environment etc.
3.) The deliver version does more than I need, and most certainly more than it should in the generic case: the most obvious example is that it sets up RESTRICT_* environment and calls restrict_access_by_env(TRUE); which surely is nothing I want to do in my code...
Right. And in general putting all the stuff to environment directly isn't that good. With v1.3's config rewrite I'm hoping to get rid of all this environment usage.
finally ask the author of the expire plugin to change his code
That'd basically be me.
- Clean but grows the API: export another function from auth-client, which does not set env-vars but returns the requested data in some struct.
Yep.
Timo Sirainen <tss@iki.fi> writes:
On Thu, 2008-10-23 at 16:18 +0200, Sascha Wilde wrote: [...]
2.) The exported interface in the respective auth-client.h files is different. The solution would be to figure out what the right interface would be [...] Perhaps something like: [api sketch]
Looks good to me. Especially as it solves the "put everything in the environment" problem in a way I like... :-)
I'm not sure about the struct, but maybe something like that. deliver would then use the struct to set up environment etc.
3.) The deliver version does more than I need, and most certainly more than it should in the generic case: the most obvious example is that it sets up RESTRICT_* environment and calls restrict_access_by_env(TRUE); which surely is nothing I want to do in my code...
Right. And in general putting all the stuff to environment directly isn't that good. With v1.3's config rewrite I'm hoping to get rid of all this environment usage.
Ok, so I'll touch it as few as possible and leave it in the deliver specific files.
finally ask the author of the expire plugin to change his code
That'd basically be me.
:-)
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
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes:
On Thu, 2008-10-23 at 16:18 +0200, Sascha Wilde wrote: [...]
2.) The exported interface in the respective auth-client.h files is different. The solution would be to figure out what the right interface would be [...] Perhaps something like: [api sketch]
Looks good to me.
One more detail: as lib-auth/auth-client.c already exists. Would it be a good idea to put the new stuff in the same file? And in case not, any suggestions what a new file could be named?
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
On Oct 23, 2008, at 9:15 PM, Sascha Wilde wrote:
as lib-auth/auth-client.c already exists. Would it be a good idea to put the new stuff in the same file? And in case not, any suggestions what a new file could be named?
Hmm. auth-client.c is about performing authentication as a client.
What you're doing is about doing a userdb lookup and connecting to
dovecot-auth as a master. So different file, but I'm not really sure
about the name. Perhaps auth-master.c and auth_master_init/deinit()
auth_master_user_lookup() function?
Timo Sirainen <tss@iki.fi> writes:
Hmm. auth-client.c is about performing authentication as a client. What you're doing is about doing a userdb lookup and connecting to dovecot-auth as a master. So different file, but I'm not really sure about the name. Perhaps auth-master.c and auth_master_init/deinit() auth_master_user_lookup() function?
Ok, I used auth-master.* -- the new code is in changeset f5ce17153a3d in my kolab-branch at http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/ and I made deliver use it in 94b00e377a25.
I had no time for thorough testing, but in my test-setup it seems to work like before, so at least I didn't break it completely... ;-)
Now I have to go back and finally implement the %%h feature for shared name spaces.
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
On Fri, 2008-10-24 at 16:19 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
Hmm. auth-client.c is about performing authentication as a client. What you're doing is about doing a userdb lookup and connecting to dovecot-auth as a master. So different file, but I'm not really sure about the name. Perhaps auth-master.c and auth_master_init/deinit() auth_master_user_lookup() function?
Ok, I used auth-master.* -- the new code is in changeset f5ce17153a3d in my kolab-branch at http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/ and I made deliver use it in 94b00e377a25.
I had no time for thorough testing, but in my test-setup it seems to work like before, so at least I didn't break it completely... ;-)
A couple of things:
It disconnects after each lookup. Not good if multiple lookups are done. Then again it probably shouldn't keep the connection alive forever since the imap connections can run for ages and most of the necessary lookups are probably done close to each others. Maybe timeout after 1 minute of idling?
conn->to is for auth request timeout. It should be removed after io_loop_run() so if 1. is fixed it won't leak timeouts. (The same conn->to could actually be used for the two timeouts - one value when looking up, another value when idling.)
Would be nice to get rid of the getenv()s :) The MAIL_CHROOT handling could be moved to deliver (use it if reply->chroot == NULL). The debug could be a parameter to auth_master_init().
You're leaking memory. Cleanest fix would be to add pool_t pool parameter to auth_master_user_lookup() and allocate memory only from it (also p_array_init(&reply->extra_fields) would be cleaner to do inside the lookup code than require it to be done externally).
Timo Sirainen <tss@iki.fi> writes:
On Fri, 2008-10-24 at 16:19 +0200, Sascha Wilde wrote:
Ok, I used auth-master.* -- the new code is in changeset f5ce17153a3d in my kolab-branch at http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/ and I made deliver use it in 94b00e377a25.
I had no time for thorough testing, but in my test-setup it seems to work like before, so at least I didn't break it completely... ;-)
A couple of things:
- It disconnects after each lookup. Not good if multiple lookups are done. Then again it probably shouldn't keep the connection alive forever since the imap connections can run for ages and most of the necessary lookups are probably done close to each others. Maybe timeout after 1 minute of idling?
I agree that this is something that should be optimized, but I was under the impression, that the current behavior of deliver was just like that -- maybe I'm mistaken, I haven't double-checked that...
- conn->to is for auth request timeout. It should be removed after io_loop_run() so if 1. is fixed it won't leak timeouts. (The same conn->to could actually be used for the two timeouts - one value when looking up, another value when idling.)
Ack. Unfortunately I'll have to put a working prototype of the "%%h"-feature together before I'll have time to look into that...
- Would be nice to get rid of the getenv()s :) The MAIL_CHROOT handling could be moved to deliver (use it if reply->chroot == NULL). The debug could be a parameter to auth_master_init().
You are right, and as I moved/left most of the env stuff in deliver/auth-client anyway it is only consequent to handle those two the same -- I'll make this change.
- You're leaking memory.
Um, yes. *blush* -- at least I added the free for the connection shortly after my announcement... ;-)
Cleanest fix would be to add pool_t pool parameter to auth_master_user_lookup() and allocate memory only from it
I think a free_auth_user_reply function might be preferable.
But I have to admit, that I didn't look deeply enough into the memory pool management in dovecot to really know whats The Right Thing To Do[tm].
Btw, on dedicated vs. default resources, I wasn't quite sure if it was a good idea to use the default ioloop. Any thoughts on that?
(also p_array_init(&reply->extra_fields) would be cleaner to do inside the lookup code than require it to be done externally).
Hmm, the idea was to only fill the extra_fields array when it was initialized, but maybe it isn't worth the trouble...
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
On Sun, 2008-10-26 at 18:33 +0100, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Fri, 2008-10-24 at 16:19 +0200, Sascha Wilde wrote:
Ok, I used auth-master.* -- the new code is in changeset f5ce17153a3d in my kolab-branch at http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/ and I made deliver use it in 94b00e377a25.
I had no time for thorough testing, but in my test-setup it seems to work like before, so at least I didn't break it completely... ;-)
A couple of things:
- It disconnects after each lookup. Not good if multiple lookups are done. Then again it probably shouldn't keep the connection alive forever since the imap connections can run for ages and most of the necessary lookups are probably done close to each others. Maybe timeout after 1 minute of idling?
I agree that this is something that should be optimized, but I was under the impression, that the current behavior of deliver was just like that -- maybe I'm mistaken, I haven't double-checked that...
deliver does always only one lookup, so it doesn't matter. But for IMAP if you have shared mailboxes from multiple users it'll do multiple lookups.
- conn->to is for auth request timeout. It should be removed after io_loop_run() so if 1. is fixed it won't leak timeouts. (The same conn->to could actually be used for the two timeouts - one value when looking up, another value when idling.)
Ack. Unfortunately I'll have to put a working prototype of the "%%h"-feature together before I'll have time to look into that...
Well, I could probably get these missing things done too.
Cleanest fix would be to add pool_t pool parameter to auth_master_user_lookup() and allocate memory only from it
I think a free_auth_user_reply function might be preferable.
But I have to admit, that I didn't look deeply enough into the memory pool management in dovecot to really know whats The Right Thing To Do[tm].
The idea behind Dovecot's memory allocations is that you shouldn't have to go through all the trouble of doing lots of memory frees. Because 1) it's easy to cause memory leaks then, 2) it requires more code and makes it uglier, 3) possibly increases memory fragmentation.
So with memory pools you just allocate all the memory from the pool and finally simply free the pool. That takes care of all these 1-3) issues. It could use slightly more memory, but especially for these kind of short living allocations it really doesn't matter.
Btw, on dedicated vs. default resources, I wasn't quite sure if it was a good idea to use the default ioloop. Any thoughts on that?
For deliver it doesn't matter, but for imap you really should create a new ioloop or things will probably break.
(also p_array_init(&reply->extra_fields) would be cleaner to do inside the lookup code than require it to be done externally).
Hmm, the idea was to only fill the extra_fields array when it was initialized, but maybe it isn't worth the trouble...
See above - it's only a short living lookup and this makes code slightly cleaner since the allocation is done only in one place. :)
Timo Sirainen <tss@iki.fi> writes:
On Sun, 2008-10-26 at 18:33 +0100, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Fri, 2008-10-24 at 16:19 +0200, Sascha Wilde wrote:
Ok, I used auth-master.* -- the new code is in changeset f5ce17153a3d in my kolab-branch at http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/ and I made deliver use it in 94b00e377a25.
I had no time for thorough testing, but in my test-setup it seems to work like before, so at least I didn't break it completely... ;-)
A couple of things:
- It disconnects after each lookup. Not good if multiple lookups are done. Then again it probably shouldn't keep the connection alive forever since the imap connections can run for ages and most of the necessary lookups are probably done close to each others. Maybe timeout after 1 minute of idling?
I agree that this is something that should be optimized, but I was under the impression, that the current behavior of deliver was just like that -- maybe I'm mistaken, I haven't double-checked that...
deliver does always only one lookup, so it doesn't matter. But for IMAP if you have shared mailboxes from multiple users it'll do multiple lookups.
Ack.
- conn->to is for auth request timeout. It should be removed after io_loop_run() so if 1. is fixed it won't leak timeouts. (The same conn->to could actually be used for the two timeouts - one value when looking up, another value when idling.)
Ack. Unfortunately I'll have to put a working prototype of the "%%h"-feature together before I'll have time to look into that...
Well, I could probably get these missing things done too.
This would be really great and highly appreciated! I just didn't dare to ask... :-)
Cleanest fix would be to add pool_t pool parameter to auth_master_user_lookup() and allocate memory only from it
I think a free_auth_user_reply function might be preferable.
But I have to admit, that I didn't look deeply enough into the memory pool management in dovecot to really know whats The Right Thing To Do[tm].
The idea behind Dovecot's memory allocations is that you shouldn't have to go through all the trouble of doing lots of memory frees. Because 1) it's easy to cause memory leaks then, 2) it requires more code and makes it uglier, 3) possibly increases memory fragmentation.
So with memory pools you just allocate all the memory from the pool and finally simply free the pool. That takes care of all these 1-3) issues. It could use slightly more memory, but especially for these kind of short living allocations it really doesn't matter.
Than I don't really see the problem with the current code -- I understand that all the memory it uses (with i_strdup and friends) is allocated from the default pool, which I assume will be freed eventually.
If the goal of an dedicated pool is to free the memory early the code using the auth-master API will have to allocate and free this pool, I don't see the advantage here... But then, on a second thought I _do_ see the advantage of a consistent way to do things like this. ;-)
Btw, on dedicated vs. default resources, I wasn't quite sure if it was a good idea to use the default ioloop. Any thoughts on that?
For deliver it doesn't matter, but for imap you really should create a new ioloop or things will probably break.
Yes, I know (already made this mistake)... ;-) The question is, should the ioloop be an extra argument to auth_master_init?
(also p_array_init(&reply->extra_fields) would be cleaner to do inside the lookup code than require it to be done externally).
Hmm, the idea was to only fill the extra_fields array when it was initialized, but maybe it isn't worth the trouble...
See above - it's only a short living lookup and this makes code slightly cleaner since the allocation is done only in one place. :)
Ok, I'll make this change.
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
The idea behind Dovecot's memory allocations is that you shouldn't
have to go through all the trouble of doing lots of memory frees.
Because 1) it's easy to cause memory leaks then, 2) it requires more code and
makes it uglier, 3) possibly increases memory fragmentation.So with memory pools you just allocate all the memory from the pool
and finally simply free the pool. That takes care of all these 1-3)
issues. It could use slightly more memory, but especially for these kind of short living allocations it really doesn't matter.Than I don't really see the problem with the current code -- I understand that all the memory it uses (with i_strdup and friends) is allocated from the default pool, which I assume will be freed eventually.
Well, "default pool" isn't really a proper memory pool.
p_malloc(default_pool) = p_malloc(system_pool) = i_malloc() =
calloc(). It will only get freed when the process is killed.
If the goal of an dedicated pool is to free the memory early the code using the auth-master API will have to allocate and free this pool, I don't see the advantage here... But then, on a second thought I _do_ see the advantage of a consistent way to do things like this. ;-)
I was thinking something like:
pool = pool_alloconly_create("userdb lookup", 512); auth_master_lookup(auth, pool, &reply); // do stuff with reply pool_unref(&pool);
Or I suppose auth_master_init() could allocate the pool internally and
call p_clear() at the beginning of each lookup. Hmm.
Btw, on dedicated vs. default resources, I wasn't quite sure if it
was a good idea to use the default ioloop. Any thoughts on that?For deliver it doesn't matter, but for imap you really should
create a new ioloop or things will probably break.Yes, I know (already made this mistake)... ;-) The question is, should the ioloop be an extra argument to auth_master_init?
I don't think there's any benefit in doing that.
Timo Sirainen <tss@iki.fi> writes:
The idea behind Dovecot's memory allocations is that you shouldn't have to go through all the trouble of doing lots of memory frees. Because 1) it's easy to cause memory leaks then, 2) it requires more code and makes it uglier, 3) possibly increases memory fragmentation.
So with memory pools you just allocate all the memory from the pool and finally simply free the pool. That takes care of all these 1-3) issues. It could use slightly more memory, but especially for these kind of short living allocations it really doesn't matter.
Than I don't really see the problem with the current code -- I understand that all the memory it uses (with i_strdup and friends) is allocated from the default pool, which I assume will be freed eventually.
Well, "default pool" isn't really a proper memory pool. p_malloc(default_pool) = p_malloc(system_pool) = i_malloc() = calloc(). It will only get freed when the process is killed.
Hmm, that would be after the end of the imap connection (deliver should be no problem anyway) -- yes this could indeed take some time...
If the goal of an dedicated pool is to free the memory early the code using the auth-master API will have to allocate and free this pool, I don't see the advantage here... But then, on a second thought I _do_ see the advantage of a consistent way to do things like this. ;-)
I was thinking something like:
pool = pool_alloconly_create("userdb lookup", 512); auth_master_lookup(auth, pool, &reply); // do stuff with reply pool_unref(&pool);
Or I suppose auth_master_init() could allocate the pool internally and call p_clear() at the beginning of each lookup. Hmm.
I think I prefer the first version, as in the second proposal replies From older lookups would become destroyed with every new lookup, which IMO would not be really evident to the user of the API.
Btw, on dedicated vs. default resources, I wasn't quite sure if it was a good idea to use the default ioloop. Any thoughts on that?
For deliver it doesn't matter, but for imap you really should create a new ioloop or things will probably break.
Yes, I know (already made this mistake)... ;-) The question is, should the ioloop be an extra argument to auth_master_init?
I don't think there's any benefit in doing that.
Ok, thanks for your input. :)
I'll put together an a little improved version this afternoon.
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
Ok, as discussed I have made some changes (hopefully improvements) in the new "auth-master" API for userdb requests...
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes: [...]
- Would be nice to get rid of the getenv()s :) The MAIL_CHROOT handling could be moved to deliver (use it if reply->chroot == NULL). The debug could be a parameter to auth_master_init().
You are right, and as I moved/left most of the env stuff in deliver/auth-client anyway it is only consequent to handle those two the same -- I'll make this change.
Done, the last getenv()s have been moved to the deliver code.
- You're leaking memory.
Um, yes. *blush* -- at least I added the free for the connection shortly after my announcement... ;-)
Cleanest fix would be to add pool_t pool parameter to auth_master_user_lookup() and allocate memory only from it
Done.
I thought for a moment of putting the pool (de)allocation into auth_master_init and auth_master_deinit -- but that turned out to be to quirky, especially with the existing deliver code...
(also p_array_init(&reply->extra_fields) would be cleaner to do inside the lookup code than require it to be done externally).
Done, too.
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
Sascha Wilde <wilde@intevation.de> writes:
Ok, as discussed I have made some changes (hopefully improvements) in the new "auth-master" API for userdb requests...
And using this new I finally put a first alpha version of the missing %%h variable for shared name spaces together.
See http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/rev/e83efa40a1dc
With this it is possible to define a name space like this:
namespace shared { separator = / prefix = users/%%u/ location = Maildir:/%%h/maildir subscriptions = no }
where %%h is substituted with the home directory of user %%u, which is needed e.g. when the mail location is configured as:
mail_location = maildir:~/maildir
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
On Tue, 2008-10-28 at 14:11 +0100, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
Ok, as discussed I have made some changes (hopefully improvements) in the new "auth-master" API for userdb requests...
And using this new I finally put a first alpha version of the missing %%h variable for shared name spaces together.
See http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/rev/e83efa40a1dc
The auth connection could be kept alive longer than for one lookup if you put the auth_connection to shared_storage. That'd require creating struct shared_storage, but it should be pretty easy (see e.g. struct cydir_storage or maildir_storage).
There's no need to i_new() struct auth_user_reply. It could just be allocated from stack. And I suppose memset(&reply, 0, sizeof(reply)) before using.
Use t_strdup(reply->home) and you won't leak memory (it gets freed "later"). The current i_strdup() will leak memory.
You could check if %h is even needed before doing the lookup. Just a while ago I added var_has_key() that makes it easy. If %h is used and the home lookup fails, fail the namespace creation.
Use mail_storage_set_critical() rather than i_error() (and i_info()). Although looks like I'm also using i_error() there already, I should fix it. :)
Hmm. The auth-master.h API could use a small change too: auth_master_init() shouldn't try to connect to auth socket yet (so it would never fail). auth_master_user_lookup() instead would try to connect there if the connection is missing.
On Fri, 2008-10-31 at 17:12 +0200, Timo Sirainen wrote:
Hmm. The auth-master.h API could use a small change too: auth_master_init() shouldn't try to connect to auth socket yet (so it would never fail). auth_master_user_lookup() instead would try to connect there if the connection is missing.
I did this and some other changes to the auth-master API and committed it.
On Fri, 2008-10-31 at 17:12 +0200, Timo Sirainen wrote:
See http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/rev/e83efa40a1dc
The auth connection could be kept alive longer than for one lookup if you put the auth_connection to shared_storage. That'd require creating struct shared_storage, but it should be pretty easy (see e.g. struct cydir_storage or maildir_storage).
Actually shared_storage already existed, somehow I missed that. Anyway I did the things I mentioned and some other stuff and committed it.
Timo Sirainen <tss@iki.fi> writes:
On Fri, 2008-10-31 at 17:12 +0200, Timo Sirainen wrote:
See http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/rev/e83efa40a1dc
The auth connection could be kept alive longer than for one lookup if you put the auth_connection to shared_storage. That'd require creating struct shared_storage, but it should be pretty easy (see e.g. struct cydir_storage or maildir_storage).
Actually shared_storage already existed, somehow I missed that. Anyway I did the things I mentioned and some other stuff and committed it.
Thanks a lot!
I'll test it in our environment...
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
Sascha Wilde <wilde@intevation.de> writes:
Timo Sirainen <tss@iki.fi> writes:
On Fri, 2008-10-31 at 17:12 +0200, Timo Sirainen wrote:
See http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/rev/e83efa40a1dc
The auth connection could be kept alive longer than for one lookup if you put the auth_connection to shared_storage. That'd require creating struct shared_storage, but it should be pretty easy (see e.g. struct cydir_storage or maildir_storage).
Actually shared_storage already existed, somehow I missed that. Anyway I did the things I mentioned and some other stuff and committed it.
Thanks a lot!
I'll test it in our environment...
Works like charm -- at least in the few tests I did...
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
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
[userdb-checkpassword] [...] The code is now in dovecot-1.2 tree.
Unfortunately there is one tiny, but essential change missing:
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
On Thu, 2008-10-23 at 13:13 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
[userdb-checkpassword] [...] The code is now in dovecot-1.2 tree.
Unfortunately there is one tiny, but essential change missing:
Oh. I guess I should have bothered to test it. :) I added the code to main.c now instead. I'll try merging changes differently the next time.
Timo Sirainen <tss@iki.fi> writes:
On Thu, 2008-10-23 at 13:13 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
[userdb-checkpassword] [...] The code is now in dovecot-1.2 tree.
Unfortunately there is one tiny, but essential change missing:
Oh. I guess I should have bothered to test it. :) I added the code to main.c now instead. I'll try merging changes differently the next time.
Thanks. v2.1alpha4? ;-)
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
On Thu, 2008-10-23 at 18:55 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Thu, 2008-10-23 at 13:13 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes:
[userdb-checkpassword] [...] The code is now in dovecot-1.2 tree.
Unfortunately there is one tiny, but essential change missing:
Oh. I guess I should have bothered to test it. :) I added the code to main.c now instead. I'll try merging changes differently the next time.
Thanks. v2.1alpha4? ;-)
Nah. checkpassword users are rare. :)
Timo Sirainen <tss@iki.fi> writes:
On Thu, 2008-10-23 at 18:55 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Thu, 2008-10-23 at 13:13 +0200, Sascha Wilde wrote:
Timo Sirainen <tss@iki.fi> writes:
On Oct 21, 2008, at 5:27 PM, Sascha Wilde wrote:
Sascha Wilde <wilde@intevation.de> writes: > [userdb-checkpassword] [...] The code is now in dovecot-1.2 tree.
Unfortunately there is one tiny, but essential change missing:
Oh. I guess I should have bothered to test it. :) I added the code to main.c now instead. I'll try merging changes differently the next time.
Thanks. v2.1alpha4? ;-)
Nah. checkpassword users are rare. :)
But important! I don't know if I'm allowed to tell which customer I'm thinking of, though... ;-)
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
Timo Sirainen said the following on 05/09/08 19:49:
http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz
Is it possibile to log also the shutdown of Dovecot deamon along with startup?
Ciao, luigi
-- / +--[Luigi Rosa]-- \
First draw the curves; then plot the data.
On Sun, 2008-09-07 at 11:37 +0200, Luigi Rosa wrote:
Timo Sirainen said the following on 05/09/08 19:49:
http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz
Is it possibile to log also the shutdown of Dovecot deamon along with startup?
dovecot: Sep 07 11:06:41 Warning: Killed with signal 15
Isn't enough?
dovecot: Sep 07 11:06:41 Warning: Shutting down (Killed with signal 15)
Maybe?
Timo Sirainen said the following on 07/09/08 11:39:
dovecot: Sep 07 11:06:41 Warning: Killed with signal 15
Isn't enough?
dovecot: Sep 07 11:06:41 Warning: Shutting down (Killed with signal 15)
Uhm... Does not appear with my setting:
syslog_facility = local5
Do I have to enable anything else?
Ciao, luigi
-- / +--[Luigi Rosa]-- \
I stayed up all night playing poker with tarot cards. I got a full house and four people died. --Steven Wright
On Sun, 2008-09-07 at 11:46 +0200, Luigi Rosa wrote:
Timo Sirainen said the following on 07/09/08 11:39:
dovecot: Sep 07 11:06:41 Warning: Killed with signal 15
Isn't enough?
dovecot: Sep 07 11:06:41 Warning: Shutting down (Killed with signal 15)
Uhm... Does not appear with my setting:
syslog_facility = local5
Do I have to enable anything else?
You're probably looking at the wrong file. Info and warnings/errors may be logged to different files. http://wiki.dovecot.org/Logging
Timo Sirainen said the following on 07/09/08 11:53:
You're probably looking at the wrong file. Info and warnings/errors may be logged to different files. http://wiki.dovecot.org/Logging
If I have this lines in dovecot.conf:
syslog_facility = local5 #log_path = /var/log/dovecot
And this line in syslog.conf
local5.* /var/log/dovecot
According to wiki page, all log lines should go to /var/log/dovecot via syslog service, and this is exactly what happens. In fact in the past I found warning and errors logged to this file via syslog.
It is very strange that all events are logged except the shutdown.
Ciao, luigi
-- / +--[Luigi Rosa]-- \
According to the obituary notices, a mean and unimportant person never dies.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Timo Sirainen said the following on 07/09/08 12:11:
It is very strange that all events are logged except the shutdown.
Maybe try running with
strace -o log ./dovecot -F
I tried and this line has been logged
Sep 7 12:19:02 mail dovecot: Dovecot v1.2.alpha1 starting up (core dumps disabled)
Normally when I start Dovecot I got this line:
Sep 7 12:19:51 mail dovecot: Dovecot v1.2.alpha1 starting up
When I killed -9 the dovecot task (not strace, but dovecot), the program ended, but no line has been logged.
This happens also in 1.x where x<2
Ciao, luigi
/ +--[Luigi Rosa]-- \
It's time you learn that freedom is never a gift. It has to be earned. --Kirk, The Return of the Archons -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFIw6uA3kWu7Tfl6ZQRAg90AKC4/cDR5jNLdp9ND6KcDL5bz3HIngCfUIcs iPs7jwwjph7T6zKxDsq4SFc= =ew0m -----END PGP SIGNATURE-----
On Sun, 2008-09-07 at 12:22 +0200, Luigi Rosa wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Timo Sirainen said the following on 07/09/08 12:11:
It is very strange that all events are logged except the shutdown.
Maybe try running with
strace -o log ./dovecot -F
I tried and this line has been logged
Sep 7 12:19:02 mail dovecot: Dovecot v1.2.alpha1 starting up (core dumps disabled)
Normally when I start Dovecot I got this line:
Sep 7 12:19:51 mail dovecot: Dovecot v1.2.alpha1 starting up
There's a ulimit -c difference.
When I killed -9 the dovecot task (not strace, but dovecot), the program ended, but no line has been logged.
Well, sure if you're going to kill -9 it then it doesn't log anything. There's no way Dovecot can do anything because you had kernel kill it directly without any kind of a notification..
Timo Sirainen schreef:
http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz http://dovecot.org/releases/1.2/alpha/dovecot-1.2.alpha1.tar.gz.sig
Since an alpha version of v1.2 is released I now publish the new urls for the Dovecot v1.2 Debian autobuilder. The instructions are still the same as described at http://wiki.dovecot.org/PrebuiltBinaries , but the /etc/apt/sources.list lines are as follows:
deb http://xi.rename-it.nl/debian/ experimental-auto main deb-src http://xi.rename-it.nl/debian/ experimental-auto main
These automatically built Debian packages are a little more experimental than the 1.1 versions because these also include the new and not yet released Sieve implementation.
I will provide tarballs for v1.2 ManageSieve some time this week.
Regards,
Stephan.
participants (6)
-
Geert Hendrickx
-
Luigi Rosa
-
Robert Schetterer
-
Sascha Wilde
-
Stephan Bosch
-
Timo Sirainen