On Sun, Jun 22, 2014 at 05:39:34AM -0400, Brad Smith wrote:
> On Sun, Jun 22, 2014 at 05:01:38AM +0200, J??r??mie Courr??ges-Anglas wrote:
> >
> > (Redirecting this to ports@)
> >
> > Could you folks test this patch against dovecot from -stable? I only
> > did compile testing on -current. I don't know how the allocator(s)
> > handle failures nor how would i_realloc handle pwbuf_size ==
> > old_pwbuf_size, but this looks safe.
> >
> >
> > $OpenBSD$
> >
> > Hack: we avoid the actual ERANGE error case by always providing a large
> > enough buffer.
>
> I'd prefer to use the diff I had commited when this issue first came
> up although back then local auth didn't work at all without the hack
> that was added. I don't have a 5.5 system around at the moment so
> please check this builds first and then test as appropriate.
What I see with this diff (thanks to sthen for the package) is no more
auto-of-mem errors. So that is good. But I see this instead:
Jul 4 13:19:17 mx1 dovecot: auth-worker(14261): Error:
bsdauth(ottox,2001:981:aaf3:1:224:1dff:fede:e939): getpwnam() failed:
Operation not permitted
The error code from getpwnam_r for a non-existent user is 1, which is
now interpreted as an errno (EPERM), it seems.
On the client side I see:
xx NO [UNAVAILABLE] Temporary authentication failure
instead of the
xx NO [AUTHENTICATIONFAILED] Authentication failed.
So it can be seen which usernames are valid.
-Otto
>
>
> Index: Makefile
> ===================================================================
> RCS file: /home/cvs/ports/mail/dovecot/Makefile,v
> retrieving revision 1.218.2.1
> diff -u -p -u -p -r1.218.2.1 Makefile
> --- Makefile 12 May 2014 09:40:19 -0000 1.218.2.1
> +++ Makefile 22 Jun 2014 08:34:15 -0000
> @@ -20,7 +20,7 @@ PKGNAME-postgresql= dovecot-postgresql-$
> CATEGORIES= mail
> MASTER_SITES= ${HOMEPAGE}releases/${V_MAJOR}/
>
> -REVISION-server= 0
> +REVISION-server= 1
>
> SHARED_LIBS= dovecot-compression 0.0 \
> dovecot-lda 2.0 \
> Index: patches/patch-src_lib_ipwd_c
> ===================================================================
> RCS file: patches/patch-src_lib_ipwd_c
> diff -N patches/patch-src_lib_ipwd_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_lib_ipwd_c 22 Jun 2014 09:32:58 -0000
> @@ -0,0 +1,129 @@
> +$OpenBSD$
> +
> +Revert http://hg.dovecot.org/dovecot-2.2/raw-rev/349d52c4ca51 to workaround
> +non POSIX compliant getpwnam_r / getpwuid_r functions.
> +
> +--- src/lib/ipwd.c.orig Tue Nov 19 15:36:30 2013
> ++++ src/lib/ipwd.c Sun Jun 22 05:30:14 2014
> +@@ -6,33 +6,41 @@
> +
> + #include <unistd.h>
> +
> +-#define PWBUF_MIN_SIZE 128
> +-#define GRBUF_MIN_SIZE 128
> ++#define DEFAULT_PWBUF_SIZE 16384
> ++#define DEFAULT_GRBUF_SIZE 16384
> +
> + static void *pwbuf = NULL, *grbuf = NULL;
> + static size_t pwbuf_size, grbuf_size;
> +
> + static void pw_init(void)
> + {
> +- size_t old_pwbuf_size = pwbuf_size;
> ++ long size;
> +
> +- if (pwbuf == NULL || errno == ERANGE) {
> +- pwbuf_size = nearest_power(old_pwbuf_size + 1);
> +- if (pwbuf_size < PWBUF_MIN_SIZE)
> +- pwbuf_size = PWBUF_MIN_SIZE;
> +- pwbuf = i_realloc(pwbuf, old_pwbuf_size, pwbuf_size);
> ++ if (pwbuf == NULL) {
> ++ size = sysconf(_SC_GETPW_R_SIZE_MAX);
> ++ if (size < 0)
> ++ size = DEFAULT_PWBUF_SIZE;
> ++
> ++ pwbuf_size = size;
> ++ pwbuf = i_malloc(pwbuf_size);
> + }
> + }
> +
> + static void gr_init(void)
> + {
> +- size_t old_grbuf_size = grbuf_size;
> ++ long size;
> +
> +- if (grbuf == NULL || errno == ERANGE) {
> +- grbuf_size = nearest_power(old_grbuf_size + 1);
> +- if (grbuf_size < PWBUF_MIN_SIZE)
> +- grbuf_size = PWBUF_MIN_SIZE;
> +- grbuf = i_realloc(grbuf, old_grbuf_size, grbuf_size);
> ++ if (grbuf == NULL) {
> ++ size = sysconf(_SC_GETGR_R_SIZE_MAX);
> ++ /* Some BSDs return too low value for this. instead of trying
> ++ to figure out exactly which, just make sure it's at least
> ++ a reasonable size. if the real size is smaller, it doesn't
> ++ matter much that we waste a few kilobytes of memory. */
> ++ if (size < DEFAULT_GRBUF_SIZE)
> ++ size = DEFAULT_GRBUF_SIZE;
> ++
> ++ grbuf_size = size;
> ++ grbuf = i_malloc(grbuf_size);
> + }
> + }
> +
> +@@ -46,16 +54,8 @@ int i_getpwnam(const char *name, struct passwd *pwd_r)
> + {
> + struct passwd *result;
> +
> +- errno = 0;
> +- do {
> +- pw_init();
> +- errno = getpwnam_r(name, pwd_r, pwbuf, pwbuf_size, &result);
> +-#ifdef __OpenBSD__
> +- /* OpenBSD returns 1 for all errors, assume it's ERANGE */
> +- if (errno == 1)
> +- errno = ERANGE;
> +-#endif
> +- } while (errno == ERANGE);
> ++ pw_init();
> ++ errno = getpwnam_r(name, pwd_r, pwbuf, pwbuf_size, &result);
> + if (result != NULL)
> + return 1;
> + if (errno == EINVAL) {
> +@@ -69,16 +69,8 @@ int i_getpwuid(uid_t uid, struct passwd *pwd_r)
> + {
> + struct passwd *result;
> +
> +- errno = 0;
> +- do {
> +- pw_init();
> +- errno = getpwuid_r(uid, pwd_r, pwbuf, pwbuf_size, &result);
> +-#ifdef __OpenBSD__
> +- /* OpenBSD returns 1 for all errors, assume it's ERANGE */
> +- if (errno == 1)
> +- errno = ERANGE;
> +-#endif
> +- } while (errno == ERANGE);
> ++ pw_init();
> ++ errno = getpwuid_r(uid, pwd_r, pwbuf, pwbuf_size, &result);
> + if (result != NULL)
> + return 1;
> + return errno == 0 ? 0 : -1;
> +@@ -88,11 +80,8 @@ int i_getgrnam(const char *name, struct group *grp_r)
> + {
> + struct group *result;
> +
> +- errno = 0;
> +- do {
> +- gr_init();
> +- errno = getgrnam_r(name, grp_r, grbuf, grbuf_size, &result);
> +- } while (errno == ERANGE);
> ++ gr_init();
> ++ errno = getgrnam_r(name, grp_r, grbuf, grbuf_size, &result);
> + if (result != NULL)
> + return 1;
> + return errno == 0 ? 0 : -1;
> +@@ -102,11 +91,8 @@ int i_getgrgid(gid_t gid, struct group *grp_r)
> + {
> + struct group *result;
> +
> +- errno = 0;
> +- do {
> +- gr_init();
> +- errno = getgrgid_r(gid, grp_r, grbuf, grbuf_size, &result);
> +- } while (errno == ERANGE);
> ++ gr_init();
> ++ errno = getgrgid_r(gid, grp_r, grbuf, grbuf_size, &result);
> + if (result != NULL)
> + return 1;
> + return errno == 0 ? 0 : -1;
>
> --
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
