At 1AM +0200 on 1/02/13 you (Timo Sirainen) wrote:
> On 1.2.2013, at 0.35, Ben Morrow <[email protected]> wrote:
>
> > I am running Dovecot with system users (userdb passwd), but some of
> > those users don't have shell accounts on the IMAP server so their shell
> > on that machine is set to /usr/sbin/nologin. Currently I am using
> > maildirs and this is not a problem, but I am in the process of switching
> > to dbox which means I will need a cronjob running 'doveadm purge -A'.
> >
> > During testing I found that those users with a 'nologin' shell are not
> > included in the list returned by the userdb iterator, and that the
> > iterator doesn't honour the first/last_valid_uid settings. This
> > inconsistency seems undesirable, so the attached patch
> >
> > - makes lookup perform the same checks as iteration,
>
> Hmmh. You could also just have them aliased to other users, so this
> wouldn't be necessary..
I don't understand what you mean. Alias them where?
> > - makes the 'nologin' check configurable,
> > - adds a new optional check that the user owns their home directory.
>
> These settings are passwd-specific, so they would have to something like:
>
> userdb {
> driver = passwd
> args = check-nologin=n check-home=y
> }
OK. New patch attached.
> > The last check was the one performed by qmail, and seems to me to be a
> > more reliable 'is this a real user' check than a nologin shell.
>
> It also performs disk I/O, slowing down the lookup.
Hmm. OK, I've left that part out: my real users are segregated by UID
anyway, so all I really care about is getting rid of the nologin check.
(I would be perfectly happy if the check were just removed altogether.)
> > If this patch is applied, the release notes for the next release should
> > probably mention that system users with a 'nologin' shell will no longer
> > be allowed to log in to IMAP until the 'auth_check_nologin' setting is
> > changed from true to false.
>
> The default will in any case be the same as it is now.
Well, yes; but authentication will now check for a nologin shell by
default, which it didn't before, so the visible behaviour will have
changed.
> > Also, there seem to be two first/last_valid_uid settings:
> > first_valid_uid itself, which is honoured by the storage subsystem, and
> > auth_first_valid_uid, which is honoured by the 'passwd' userdb. Is this
> > intentional?
>
> Nope, that's a bug. Fixed that in v2.2:
> http://hg.dovecot.org/dovecot-2.2/rev/18661d1d6ed0
Cool. Will that be backported to 2.1 at some point?
Ben
diff -r 8f7dc71ad982 src/auth/userdb-passwd.c
--- a/src/auth/userdb-passwd.c Fri Feb 01 17:46:08 2013 +0000
+++ b/src/auth/userdb-passwd.c Fri Feb 01 18:13:31 2013 +0000
@@ -20,6 +20,8 @@
struct userdb_module module;
struct userdb_template *tmpl;
+ unsigned int nologin:1;
+
unsigned int fast_count, slow_count;
unsigned int slow_warned:1;
};
@@ -27,6 +29,8 @@
struct passwd_userdb_iterate_context {
struct userdb_iterate_context ctx;
struct passwd_userdb_iterate_context *next_waiting;
+
+ unsigned int nologin:1;
};
static struct passwd_userdb_iterate_context *cur_userdb_iter = NULL;
@@ -76,6 +80,29 @@
}
}
+static bool
+passwd_want_pw(struct passwd *pw, const struct auth_settings *set,
+ unsigned int nologin)
+{
+ /* skip entries not in valid UID range.
+ they're users for daemons and such. */
+ if (pw->pw_uid < (uid_t)set->first_valid_uid)
+ return FALSE;
+ if (pw->pw_uid > (uid_t)set->last_valid_uid && set->last_valid_uid != 0)
+ return FALSE;
+
+ if (nologin) {
+ /* skip entries that don't have a valid shell.
+ they're again probably not real users. */
+ if (strcmp(pw->pw_shell, "/bin/false") == 0 ||
+ strcmp(pw->pw_shell, "/sbin/nologin") == 0 ||
+ strcmp(pw->pw_shell, "/usr/sbin/nologin") == 0)
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
static void passwd_lookup(struct auth_request *auth_request,
userdb_callback_t *callback)
{
@@ -106,6 +133,13 @@
return;
}
+ if (!passwd_want_pw(&pw, auth_request->set, module->nologin)) {
+ auth_request_log_info(auth_request, "passwd",
+ "user has bad uid or shell");
+ callback(USERDB_RESULT_USER_UNKNOWN, auth_request);
+ return;
+ }
+
auth_request_set_field(auth_request, "user", pw.pw_name, NULL);
auth_request_init_userdb_reply(auth_request);
@@ -125,11 +159,15 @@
userdb_iter_callback_t *callback, void *context)
{
struct passwd_userdb_iterate_context *ctx;
+ struct userdb_module *_module = auth_request->userdb->userdb;
+ struct passwd_userdb_module *module =
+ (struct passwd_userdb_module *)_module;
ctx = i_new(struct passwd_userdb_iterate_context, 1);
ctx->ctx.auth_request = auth_request;
ctx->ctx.callback = callback;
ctx->ctx.context = context;
+ ctx->nologin = module->nologin;
setpwent();
if (cur_userdb_iter == NULL)
@@ -137,25 +175,6 @@
return &ctx->ctx;
}
-static bool
-passwd_iterate_want_pw(struct passwd *pw, const struct auth_settings *set)
-{
- /* skip entries not in valid UID range.
- they're users for daemons and such. */
- if (pw->pw_uid < (uid_t)set->first_valid_uid)
- return FALSE;
- if (pw->pw_uid > (uid_t)set->last_valid_uid && set->last_valid_uid != 0)
- return FALSE;
-
- /* skip entries that don't have a valid shell.
- they're again probably not real users. */
- if (strcmp(pw->pw_shell, "/bin/false") == 0 ||
- strcmp(pw->pw_shell, "/sbin/nologin") == 0 ||
- strcmp(pw->pw_shell, "/usr/sbin/nologin") == 0)
- return FALSE;
- return TRUE;
-}
-
static void passwd_iterate_next(struct userdb_iterate_context *_ctx)
{
struct passwd_userdb_iterate_context *ctx =
@@ -173,7 +192,7 @@
errno = 0;
while ((pw = getpwent()) != NULL) {
- if (passwd_iterate_want_pw(pw, set)) {
+ if (passwd_want_pw(pw, set, ctx->nologin)) {
_ctx->callback(pw->pw_name, _ctx->context);
return;
}
@@ -217,9 +236,12 @@
module->module.cache_key = USER_CACHE_KEY;
module->tmpl = userdb_template_build(pool, "passwd", args);
module->module.blocking = TRUE;
+ module->nologin = TRUE;
if (userdb_template_remove(module->tmpl, "blocking", &value))
module->module.blocking = strcasecmp(value, "yes") == 0;
+ if (userdb_template_remove(module->tmpl, "nologin", &value))
+ module->nologin = strcasecmp(value, "yes") == 0;
/* FIXME: backwards compatibility */
if (!userdb_template_is_empty(module->tmpl))
i_warning("userdb passwd: Move templates args to override_fields setting");
