dovecot - Jul 2016 - passdb {driver = shadow args = override_username=%variable}

Hello list

I want to implement override_username = %variable for the "shadow"
driver, so that the following works:

passdb {
  driver = shadow
  args = override_username=%Ln
}

by "%variable", I mean "Variables" at
http://wiki.dovecot.org/Variables.

I've hacked up a solution with strtok(3C), but that's a hack and a
possible security hole, and I'd rather not implement it.
What I really want is to be able to leverage Dovecot's var_expand()
call, which, looking at the code, appears to be the function
responsible for %variable expansion and formatting.

The relevant piece of code seems to be this excerpt in auth/passdb-shadow.c:

static enum passdb_result
shadow_lookup(struct auth_request *request, struct spwd **spw_r)
{
        auth_request_log_debug(request, AUTH_SUBSYS_DB, "lookup");

        *spw_r = getspnam(request->user);
        if (*spw_r == NULL) {
                auth_request_log_unknown_user(request, AUTH_SUBSYS_DB);
                return PASSDB_RESULT_USER_UNKNOWN;
        }

when request->user contains 'user at domain.tld', I want the code to
be
able to process "override_username", determine that the format is %Ln
so that request->user is lowercased, and everything after the "@"
sign
is stripped. Therefore, if this user happens to exist in the shadow
file, authentication will succeed.

Currently, the "shadow" driver only appears to support "args =
blocking=no".

Please advise.

On Saturday, July 09, 2016 12:01:43 PM UNIX admin wrote:
> Hello list
> 
> I want to implement override_username = %variable for the
"shadow"
> driver, so that the following works:
> 
> passdb {
>   driver = shadow
>   args = override_username=%Ln
> }
> 
> by "%variable", I mean "Variables" at
http://wiki.dovecot.org/Variables.
That is pretty interesting, and along those lines. Could override_username also
allow one
to block certain usernames from being synced?

My mail server monitoring uses the nobody user account. That triggers syncing
for the
nobody user account. Which always fails syncing. I end up running the following
command
allot to avoid repeat errors in logs.

"doveadm replicator remove nobody"

I have not found a way to block users from passdb. Seems to be all or nothing. I
could see
about having monitoring use an actual account.

-- 
William L. Thomson Jr.
Obsidian-Studios, Inc.
http://www.obsidian-studios.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part.
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20160711/2bd4d780/attachment.sig>

On 09 Jul 2016, at 13:01, UNIX admin <tripivceta at gmail.com>
wrote:
> 
> Hello list
> 
> I want to implement override_username = %variable for the
"shadow"
> driver, so that the following works:
> 
> passdb {
>  driver = shadow
>  args = override_username=%Ln
> }
> 
> by "%variable", I mean "Variables" at
http://wiki.dovecot.org/Variables.
> 
> I've hacked up a solution with strtok(3C), but that's a hack and a
> possible security hole, and I'd rather not implement it.
> What I really want is to be able to leverage Dovecot's var_expand()
> call, which, looking at the code, appears to be the function
> responsible for %variable expansion and formatting.
> 
> The relevant piece of code seems to be this excerpt in
auth/passdb-shadow.c:
> 
> static enum passdb_result
> shadow_lookup(struct auth_request *request, struct spwd **spw_r)
> {
>        auth_request_log_debug(request, AUTH_SUBSYS_DB, "lookup");
> 
>        *spw_r = getspnam(request->user);
>        if (*spw_r == NULL) {
>                auth_request_log_unknown_user(request, AUTH_SUBSYS_DB);
>                return PASSDB_RESULT_USER_UNKNOWN;
>        }
> 
> when request->user contains 'user at domain.tld', I want the
code to be
> able to process "override_username", determine that the format is
%Ln
> so that request->user is lowercased, and everything after the
"@" sign
> is stripped. Therefore, if this user happens to exist in the shadow
> file, authentication will succeed.
> 
> Currently, the "shadow" driver only appears to support "args
= blocking=no".
You need this to be specific to the one passdb, not everything? So
auth_username_format=%Ln setting wouldn't work?

passwd-file driver has username_format parameter, which does this. I've
thought that rather than copy&pasting such code to other drivers, the next
step would be to make it work for all the passdb and userdb drivers, such as:

passdb {
  driver = anything
  args = anything
  username_format = %Ln
}

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..

On 11 Jul 2016, at 19:48, William L. Thomson Jr. <wlt-ml at o-sinc.com>
wrote:
> 
> On Saturday, July 09, 2016 12:01:43 PM UNIX admin wrote:
>> Hello list
>> 
>> I want to implement override_username = %variable for the
"shadow"
>> driver, so that the following works:
>> 
>> passdb {
>>  driver = shadow
>>  args = override_username=%Ln
>> }
>> 
>> by "%variable", I mean "Variables" at
http://wiki.dovecot.org/Variables.
> 
> That is pretty interesting, and along those lines. Could override_username
also allow one
> to block certain usernames from being synced?
> 
> My mail server monitoring uses the nobody user account. That triggers
syncing for the
> nobody user account. Which always fails syncing. I end up running the
following command
> allot to avoid repeat errors in logs.
> 
> "doveadm replicator remove nobody"
> 
> I have not found a way to block users from passdb. Seems to be all or
nothing. I could see
> about having monitoring use an actual account.
If something isn't in first_valid_uid..last_valid_uid it's not included
in the list of users. In v2.3.x tree there's also a commit to check also
first_valid_gid..last_valid_gid, which I'm not sure if it should be part of
v2.2.x.

On Tue, Jul 12, 2016 at 3:23 AM, Timo Sirainen <tss at 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 at iki.fi" and "timo at 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 at 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 at domain.net" and "user at domain.fi" could be two
different
users, IMAP login must be set to "user at domain" forma, rather than
just
using "user" as the login.

The problem is that when the client passes user at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: src-auth-passdb-shadow.c.patch
Type: application/octet-stream
Size: 740 bytes
Desc: not available
URL:
<http://dovecot.org/pipermail/dovecot/attachments/20160712/7e385575/attachment.obj>