On Tue, Jul 12, 2016 at 3:23 AM, Timo Sirainen tss@iki.fi wrote:
You need this to be specific to the one passdb, not everything?
Actually it would not matter to me if all passdb drivers implemented "username_format", or "override_username"; that would make the software even more flexible and powerful.
So auth_username_format=%Ln setting wouldn't work?
Consider the scenario where one has a mix of local users, found in the shadow file, and virtual users, which are in a SQLite database.
The virtual users in the database might be completely different users, i.e. "timo@iki.fi" and "timo@oulu.fi" could legitimately be two completely different users, and the user "timo" in /etc/shadow could be yet another, unrelated person.
According to the specification and examples found at http://wiki2.dovecot.org/AuthDatabase/SQL#Examples, the "password_query" performs the following SELECT:
password_query = SELECT userid AS username, domain, password
FROM users WHERE userid = '%n' AND domain = '%d'
this means that the "domain" column is being used to perform user validation, and indeed, a %n@%d (user@domain) forms a tuple which is unique, so this is a valid use case, since there is enough information from that to identify unique users.
As "user@domain.net" and "user@domain.fi" could be two different users, IMAP login must be set to "user@domain" forma, rather than just using "user" as the login.
The problem is that when the client passes user@domain as login, and the user actually has a real UNIX account and receives e-mail on the system, the lookup in /etc/shadow fails.
Since "auth_username_format=%Ln" overrides everything, that immediately breaks lookups of virtual users in the aforementioned SQLite database.
Currently, it appears that sticking "auth_username_format" in userdb{} or passdb{} blocks is illegal. If the software allowed to specify discrete "auth_username_format" for each userdb{} and passdb{} block, that would immediately solve the problem, and increase the software's flexibility as a beneficial sideeffect. However, since I am not familiar with the code base, I am not even sure how I would go about this.
If you could explain to me what exactly I need to pass into var_expand() function in order to be able to expand placeholders like "%Ln", I might be able to write a patch to implement override_username. That is of course assuming var_expand() is indeed the function doing the %placeholder expansion?
As an aside, if one uses "username_format", in the userdb{} block, "auth" complains thus: auth-worker(5330): Warning: userdb passwd: Move templates args to override_fields setting.
I'd be happy for such a generic patch. I'm not entirely sure what's the best/nicest way to do it though. Maybe temporarily override auth_request->user? Could be ugly, but maybe doable..
Here is what I cooked up:
char *Login = NULL;
if((request->user != NULL) && ((Login = strdup(request->user)) != NULL))
{
if(strtok(Login, "@") != NULL)
{
*spw_r = getspnam(Login);
}
}
else
but that is a hack, and a potentially very dangerous hack, since I don't know what kind of validation has been done on request->user up to that point, so that could be a direct vector of a buffer overflow attack. (If I were attacking dovecot, this is the first place I would hit it at.)
I've tried to be paranoid with all the NULL checking, but I don't trust myself. Nevertheless, I'm attaching the full patch to this e-mail, and would appreciate any feedback from a security standpoint.