The smtp auth PLAIN specification is weird. It's valid (apparently for

imap, mostly) to provide "username1\0real_username\0password" as your
base64 encoded string for authentication. We currently don't handle
that, instead expecting the first byte to be a NUL.  So fix that up by
scanning for the first string, and ignoring it if it's there. The string
is also stupid in that the last bit (password) may not be NUL
terminated, so pay attention to that in our decoding and make sure that
it's always terminated correctly.

It's been discussed, and this decoding really should happen in the
unauthenticated process, not in the privileged one, but that is another
diff.

Problem found by todd@, who kindly helped me debug this and confirmed
that it now works with kmail, mutt and thunderbird.

"if it makes more stuff work, please commit" jacekm@.

-0- - not a smtpd hacker and I resent your implication.

1 files changed, 29 insertions, 11 deletions

diff --git a/usr.sbin/smtpd/smtpd.c b/usr.sbin/smtpd/smtpd.c
index 7328e0a4707..32b0bc942b7 100644
--- a/usr.sbin/smtpd/smtpd.c
+++ b/usr.sbin/smtpd/smtpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: smtpd.c,v 1.51 2009/03/29 14:18:20 jacekm Exp $	*/
+/*	$OpenBSD: smtpd.c,v 1.52 2009/04/03 05:20:17 oga Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -451,22 +451,40 @@ parent_dispatch_smtp(int fd, short event, void *p)
 		case IMSG_PARENT_AUTHENTICATE: {
 			struct session_auth_req *req;
 			struct session_auth_reply reply;
-			u_int8_t buffer[1024];
-			char *pw_name;
-			char *pw_passwd;
+			char buf[1024];
+			char *user;
+			char *pass;
+			int len;
 
 			req = (struct session_auth_req *)imsg.data;
 
 			reply.session_id = req->session_id;
 			reply.value = 0;
 
-			if (kn_decode_base64(req->buffer, buffer, sizeof(buffer)) != -1) {
-				pw_name = buffer+1;
-				pw_passwd = pw_name+strlen(pw_name)+1;
-
-				if (auth_userokay(pw_name, NULL, "auth-smtp", pw_passwd))
-					reply.value = 1;
-			}
+			/* String is not NUL terminated, leave room. */
+			if ((len = kn_decode_base64(req->buffer, buf,
+			    sizeof(buf) - 1)) == -1)
+				goto out;
+			/* buf is a byte string, NUL terminate. */
+			buf[len] = '\0';
+
+			/*
+			 * Skip "foo" in "foo\0user\0pass", if present.
+			 */
+			user = memchr(buf, '\0', len);
+			if (user == NULL || user >= buf + len - 2)
+				goto out;
+			user++; /* skip NUL */
+
+			pass = memchr(user, '\0', len - (user - buf));
+			if (pass == NULL || pass >= buf + len - 2)
+				goto out;
+			pass++; /* skip NUL */
+
+			if (auth_userokay(user, NULL, "auth-smtp", pass))
+				reply.value = 1;
+
+out:
 			imsg_compose(ibuf, IMSG_PARENT_AUTHENTICATE, 0, 0,
 			    -1, &reply, sizeof(reply));