Index | Thread | Search

From:
gilles@poolp.org
Subject:
Re: smtpd: table auth offloading
To:
"Omar Polo" <op@omarpolo.com>
Cc:
tech@openbsd.org
Date:
Sun, 26 May 2024 21:00:43 +0000

Download raw body.

Thread
May 24, 2024 9:55 AM, gilles@poolp.org wrote:

> May 24, 2024 9:42 AM, "Omar Polo" <op@omarpolo.com> wrote:
> 
>> On 2024/05/23 21:55:50 +0000, gilles@poolp.org wrote:
>> 
>>> Hello,
>>> 
>>> Just a mail to propose an idea that I discussed with op@ in private.
>>> 
>>> In smtpd, there are two codepaths for authentication:
>>> 
>>> 1- if no table was provided, smtpd relies on bsd_auth(3)
>>> 
>>> 2- if a table is provided, it performs a K_CREDENTIALS lookup which,
>>> given a username as a key, looks up the crypt(3)-ed password that
>>> is then compared to the one that the user provided.
>>> 
>>> When you provide a table to the auth keyword, ie:
>>> 
>>> listen on all [...] auth <foobar>
>>> 
>>> Regardless of the backend used by <foobar> you take the second path,
>>> which is great because any backend that supports key-value lookup is
>>> usable as an authentication backend in smtpd: inline, file, db, ...
>>> 
>>> Problem is that some backends do not expect to be queried this way.
>>> 
>>> For example, OpenLDAP and OpenBSD's ldapd have an existing attribute
>>> userPassword which is mandatory but which we can't use because it is
>>> not in the format we expect and not meant to be fed to crypt(3) like
>>> we do for all other table backends. In such cases, we'd rather let a
>>> table backend do the validation and tell us if it succeeded.
>>> 
>>> This diff introduces a new lookup service K_AUTH which is only meant
>>> to be used by table backends that need custom auth validation. If we
>>> authenticate against a table that supports K_AUTH, then instead of a
>>> K_CREDENTIALS lookup we forward the username:password so the backend
>>> can tell us if authentication succeeded or not.
>>> 
>>> ok ?
>> 
>> after sleeping on this I think that maybe we should document this under
>> smtpd-tables.7 rather than table.5 because this is not about the format
>> of the data, but rather a special mechanism only for proc tables.
> 
> You're right and I found it annoying that auth is described before credentials
> when most people will use credentials so I'm all for moving the blurb.
> 
>> but in any case, it reads fine and I've been surprised by how simple it
>> is!
>> 
>> ok op@
>> 

updated, I decided to not be as verbose about auth tables since its a
developer only feature, moved it to smtpd-tables.7, made >80 lines
shorter.

if no objection, I'll commit tomorrow.


Index: lka.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
diff -u -p -r1.248 lka.c
--- lka.c	20 Jan 2024 09:01:03 -0000	1.248
+++ lka.c	26 May 2024 20:56:02 -0000
@@ -720,6 +720,7 @@ static int
 lka_authenticate(const char *tablename, const char *user, const char *password)
 {
 	struct table		*table;
+	char	       		 offloadkey[LINE_MAX];
 	union lookup		 lk;
 
 	log_debug("debug: lka: authenticating for %s:%s", tablename, user);
@@ -730,7 +731,27 @@ lka_authenticate(const char *tablename, 
 		return (LKA_TEMPFAIL);
 	}
 
-	switch (table_lookup(table, K_CREDENTIALS, user, &lk)) {
+	/* table backend supports authentication offloading */
+	if (table_check_service(table, K_AUTH)) {
+		if (!bsnprintf(offloadkey, sizeof(offloadkey), "%s:%s",
+			user, password)) {
+			log_warnx("warn: key serialization failed for %s:%s",
+			    tablename, user);
+			return (LKA_TEMPFAIL);
+		}
+		switch (table_match(table, K_AUTH, offloadkey)) {
+		case -1:
+			log_warnx("warn: user credentials lookup fail for %s:%s",
+			    tablename, user);
+			return (LKA_TEMPFAIL);
+		case 0:
+			return (LKA_PERMFAIL);
+		default:
+			return (LKA_OK);
+		}
+	}
+
+	switch (table_lookup(table, K_CRE10;rgb:f8f8/f8f8/f2f2DENTIALS, user, &lk)) {
 	case -1:
 		log_warnx("warn: user credentials lookup fail for %s:%s",
 		    tablename, user);
Index: smtpd-api.h
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd-api.h,v
diff -u -p -r1.36 smtpd-api.h
--- smtpd-api.h	23 Dec 2018 16:06:24 -0000	1.36
+++ smtpd-api.h	26 May 2024 20:56:03 -0000
@@ -135,8 +135,9 @@ enum table_service {
 	K_RELAYHOST	= 0x200,	/* returns struct relayhost	*/
 	K_STRING	= 0x400,
 	K_REGEX		= 0x800,
+	K_AUTH		= 0x1000,
 };
-#define K_ANY		  0xfff
+#define K_ANY		  0xffff
 
 enum {
 	PROC_TABLE_OK,
Index: smtpd-tables.7
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtpd-tables.7,v
diff -u -p -r1.1 smtpd-tables.7
--- smtpd-tables.7	7 May 2024 12:13:43 -0000	1.1
+++ smtpd-tables.7	26 May 2024 20:56:03 -0000
@@ -191,6 +191,10 @@ The services and their result format are
 .Bl -tag -width mailaddrmap -compact
 .It Ic alias
 One or more aliases separated by a comma.
+.It Ic auth
+Only usable for check.
+Lookup key is username and cleartext password separated by
+.Sq \&: .
 .It Ic domain
 A domain name.
 .\" XXX are wildcards allowed?
Index: table.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/table.c,v
diff -u -p -r1.52 table.c
--- table.c	7 May 2024 12:10:06 -0000	1.52
+++ table.c	26 May 2024 20:56:03 -0000
@@ -83,6 +83,7 @@ table_service_name(enum table_service s)
 	case K_RELAYHOST:	return "relayhost";
 	case K_STRING:		return "string";
 	case K_REGEX:		return "regex";
+	case K_AUTH:		return "auth";
 	}
 	return "???";
 }
@@ -116,6 +117,8 @@ table_service_from_name(const char *serv
 		return K_STRING;
 	if (!strcmp(service, "regex"))
 		return K_REGEX;
+	if (!strcmp(service, "auth"))
+		return K_AUTH;
 	return (-1);
 }