Re: [Dovecot] Userdb passwd and 'nologin' users
After thinking about this for a while, I think the best solution is simply to remove the shell check unconditionally. I'm not sure if anyone else except me ever wanted it (and I can live with a couple of unnecessary users getting mailboxes). Done for v2.2: http://hg.dovecot.org/dovecot-2.2/rev/4eea2224e16b I did also wonder about using a special "dovecot-skip" GECOS field for this, but maybe not a good idea either. On 1.2.2013, at 0.35, Ben Morrow 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, >- makes the 'nologin' check configurable, >- adds a new optional check that the user owns their home directory. > > 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. > > 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. > > 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? > > Ben > >
Re: [Dovecot] Userdb passwd and 'nologin' users
At 4AM +0100 on 1/02/13 you (Daniel Parthey) wrote:
> Hi Ben,
>
> Ben Morrow wrote:
> > +if (set->check_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;
> > +}
>
> Valid shells are defined in /etc/shells and "locked" users, I would
> strongly discourage from hardcoding a list of no-login shells here.
That list isn't mine, my patch just moves that code from one part of the
file to another and makes it conditional. Personally I don't think
checking the shell is sensible at all, which is why I'm trying to make
it optional.
> Users locked with "passwd -l" can also be detected by a ! at
> the beginning of the password hash.
That is system-specific, and in any case you have to be root (and on
non-BSD systems you have to make a shadow password call) to see the
password field. The userdb shouldn't be doing that: locked users will
already be prevented from logging in with a password because the
password won't match. (Of course, this doesn't cover e.g. Kerberos
logins, though I would usually lock a Kerberos user by telling the KDC
not to issue tickets rather than by locking the passwd account.)
Ben
Re: [Dovecot] Userdb passwd and 'nologin' users
At 1AM +0200 on 1/02/13 you (Timo Sirainen) wrote:
> On 1.2.2013, at 0.35, Ben Morrow 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 +
+++ b/src/auth/userdb-passwd.c Fri Feb 01 18:13:31 2013 +
@@ -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;
Re: [Dovecot] Userdb passwd and 'nologin' users
Hi Ben,
Ben Morrow wrote:
> +if (set->check_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;
> +}
Valid shells are defined in /etc/shells and "locked" users, I would
strongly discourage from hardcoding a list of no-login shells here.
Users locked with "passwd -l" can also be detected by a ! at
the beginning of the password hash.
Regards
Daniel
--
https://plus.google.com/103021802792276734820
Re: [Dovecot] Userdb passwd and 'nologin' users
On 1.2.2013, at 0.35, Ben Morrow 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..
>- 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
}
> 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.
> 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.
> 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
[Dovecot] Userdb passwd and 'nologin' users
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,
- makes the 'nologin' check configurable,
- adds a new optional check that the user owns their home directory.
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.
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.
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?
Ben
diff -r bf80034a547d src/auth/auth-settings.c
--- a/src/auth/auth-settings.c Thu Jan 31 18:27:22 2013 +0200
+++ b/src/auth/auth-settings.c Thu Jan 31 22:11:31 2013 +
@@ -202,6 +202,8 @@
DEF(SET_TIME, failure_delay),
DEF(SET_UINT, first_valid_uid),
DEF(SET_UINT, last_valid_uid),
+DEF(SET_BOOL, check_nologin),
+DEF(SET_BOOL, check_homedir),
DEF(SET_BOOL, verbose),
DEF(SET_BOOL, debug),
@@ -241,6 +243,8 @@
.failure_delay = 2,
.first_valid_uid = 500,
.last_valid_uid = 0,
+.check_nologin = TRUE,
+.check_homedir = FALSE,
.verbose = FALSE,
.debug = FALSE,
diff -r bf80034a547d src/auth/auth-settings.h
--- a/src/auth/auth-settings.h Thu Jan 31 18:27:22 2013 +0200
+++ b/src/auth/auth-settings.h Thu Jan 31 22:11:31 2013 +
@@ -40,6 +40,8 @@
unsigned int failure_delay;
unsigned int first_valid_uid;
unsigned int last_valid_uid;
+bool check_nologin;
+bool check_homedir;
bool verbose, debug, debug_passwords;
const char *verbose_passwords;
diff -r bf80034a547d src/auth/userdb-passwd.c
--- a/src/auth/userdb-passwd.c Thu Jan 31 18:27:22 2013 +0200
+++ b/src/auth/userdb-passwd.c Thu Jan 31 22:11:31 2013 +
@@ -10,6 +10,8 @@
#include "time-util.h"
#include "userdb-template.h"
+#include
+
#define USER_CACHE_KEY "%u"
#define PASSWD_SLOW_WARN_MSECS (10*1000)
#define PASSWD_SLOW_MASTER_WARN_MSECS 50
@@ -76,6 +78,41 @@
}
}
+static bool
+passwd_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;
+
+if (set->check_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;
+}
+
+if (set->check_homedir) {
+int err = errno;
+struct stat st;
+int ok;
+
+/* skip users who don't own their homedirs */
+ok = (stat(pw->pw_dir, &st) >= 0 &&
+S_ISDIR(st.st_mode) &&
+st.st_uid == pw->pw_uid);
+errno = err;
+if (!ok) return FALSE;
+}
+
+ return TRUE;
+}
+
static void passwd_lookup(struct auth_request *auth_request,
userdb_callback_t *callback)
{
@@ -106,6 +143,13 @@
return;
}
+if (!passwd_want_pw(&pw, auth_request->set)) {
+auth_request_log_info(auth_request, "passwd",
+ "user has bad uid or homedir");
+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);
@@ -137,25 +181,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
