git: 5761f8a7de9f - stable/13 - libtacplus: Allow additional AV pairs to be configured.

From: Dag-Erling Smørgrav <des_at_FreeBSD.org>
Date: Wed, 13 Dec 2023 20:06:16 UTC
The branch stable/13 has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=5761f8a7de9fa0248e1d7e2be751de1c6d9aa973

commit 5761f8a7de9fa0248e1d7e2be751de1c6d9aa973
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2023-06-13 16:04:22 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2023-12-13 16:08:13 +0000

    libtacplus: Allow additional AV pairs to be configured.
    
    * Replace hand-rolled input tokenizer with openpam_readlinev() which supports line continuations and has better quoting and escaping.
    * Simplify string handling by merging struct clnt_str and struct srvr_str into just struct tac_str.
    * Each server entry in the configuration file can now have up to 255 AV pairs which will be appended to the ones returned by the server in response to a successful authorization request.
    
    This allows nss_tacplus(8) to be used with servers which do not provide identity information beyond confirming the existence of the user.
    
    This adds a dependency on libpam, however libtacplus is currently only used by pam_tacplus(8) (which is already always used with libpam) and the very recently added nss_tacplus(8) (which is extremely niche).  In the longer term it might be a good idea to split this out into a separate library.
    
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    Reviewed by:    pauamma_gundo.com, markj
    Differential Revision:  https://reviews.freebsd.org/D40285
    Relnotes:       yes
    
    (cherry picked from commit 21850106fdda5269bc881f0e62839dff3d9edf47)
---
 UPDATING                        |   7 +
 lib/libtacplus/Makefile         |   2 +-
 lib/libtacplus/taclib.c         | 347 +++++++++++++++++-----------------------
 lib/libtacplus/taclib_private.h |  63 ++++----
 lib/libtacplus/tacplus.conf.5   |  42 ++---
 share/mk/src.libnames.mk        |   2 +-
 6 files changed, 206 insertions(+), 257 deletions(-)

diff --git a/UPDATING b/UPDATING
index 67809e8fc4dc..21873313b3be 100644
--- a/UPDATING
+++ b/UPDATING
@@ -12,6 +12,13 @@ Items affecting the ports and packages system can be found in
 /usr/ports/UPDATING.  Please read that file before updating system packages
 and/or ports.
 
+20230913:
+	Improvements to libtacplus(8) mean that tacplus.conf(5) now
+	follows POSIX shell syntax rules. This may cause TACACS+
+	authentication to fail if the shared secret contains a single
+	quote, double quote, or backslash character which isn't
+	already properly quoted or escaped.
+
 20230619:
 	To enable pf rdr rules for connections initiated from the host, pf
 	filter rules can be optionally enabled for packets delivered
diff --git a/lib/libtacplus/Makefile b/lib/libtacplus/Makefile
index b658d6ce02ba..43567350aeac 100644
--- a/lib/libtacplus/Makefile
+++ b/lib/libtacplus/Makefile
@@ -27,7 +27,7 @@ LIB=		tacplus
 SRCS=		taclib.c
 INCS=		taclib.h
 CFLAGS+=	-Wall
-LIBADD=		md
+LIBADD=		md pam
 SHLIB_MAJOR=	5
 MAN=		libtacplus.3 tacplus.conf.5
 
diff --git a/lib/libtacplus/taclib.c b/lib/libtacplus/taclib.c
index 6577876ef44f..00bee367ca9f 100644
--- a/lib/libtacplus/taclib.c
+++ b/lib/libtacplus/taclib.c
@@ -34,6 +34,7 @@
 #include <arpa/inet.h>
 
 #include <assert.h>
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <md5.h>
@@ -45,32 +46,34 @@
 #include <string.h>
 #include <unistd.h>
 
+#include <security/pam_appl.h>
+#include <security/openpam.h>
+
 #include "taclib_private.h"
 
 static int		 add_str_8(struct tac_handle *, u_int8_t *,
-			    struct clnt_str *);
+			    struct tac_str *);
 static int		 add_str_16(struct tac_handle *, u_int16_t *,
-			    struct clnt_str *);
+			    struct tac_str *);
 static int		 protocol_version(int, int, int);
 static void		 close_connection(struct tac_handle *);
 static int		 conn_server(struct tac_handle *);
 static void		 crypt_msg(struct tac_handle *, struct tac_msg *);
-static void		*dup_str(struct tac_handle *, const struct srvr_str *,
+static void		*dup_str(struct tac_handle *, const struct tac_str *,
 			    size_t *);
 static int		 establish_connection(struct tac_handle *);
-static void		 free_str(struct clnt_str *);
+static void		 free_str(struct tac_str *);
 static void		 generr(struct tac_handle *, const char *, ...)
 			    __printflike(2, 3);
 static void		 gen_session_id(struct tac_msg *);
 static int		 get_srvr_end(struct tac_handle *);
-static int		 get_srvr_str(struct tac_handle *, const char *,
-				      struct srvr_str *, size_t);
-static void		 init_clnt_str(struct clnt_str *);
-static void		 init_srvr_str(struct srvr_str *);
+static int		 get_str(struct tac_handle *, const char *,
+				      struct tac_str *, size_t);
+static void		 init_str(struct tac_str *);
 static int		 read_timed(struct tac_handle *, void *, size_t,
 			    const struct timeval *);
 static int		 recv_msg(struct tac_handle *);
-static int		 save_str(struct tac_handle *, struct clnt_str *,
+static int		 save_str(struct tac_handle *, struct tac_str *,
 			    const void *, size_t);
 static int		 send_msg(struct tac_handle *);
 static int		 split(char *, char *[], int, char *, size_t);
@@ -88,7 +91,7 @@ static void              create_msg(struct tac_handle *, int, int, int);
  * for the next time.
  */
 static int
-add_str_8(struct tac_handle *h, u_int8_t *fld, struct clnt_str *cs)
+add_str_8(struct tac_handle *h, u_int8_t *fld, struct tac_str *cs)
 {
 	u_int16_t len;
 
@@ -112,7 +115,7 @@ add_str_8(struct tac_handle *h, u_int8_t *fld, struct clnt_str *cs)
  * for the next time.
  */
 static int
-add_str_16(struct tac_handle *h, u_int16_t *fld, struct clnt_str *cs)
+add_str_16(struct tac_handle *h, u_int16_t *fld, struct tac_str *cs)
 {
 	size_t len;
 
@@ -234,7 +237,7 @@ close_connection(struct tac_handle *h)
 static int
 conn_server(struct tac_handle *h)
 {
-	const struct tac_server *srvp = &h->servers[h->cur_server];
+	struct tac_server *srvp = &h->servers[h->cur_server];
 	int flags;
 
 	if ((h->fd = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP)) == -1) {
@@ -355,7 +358,7 @@ crypt_msg(struct tac_handle *h, struct tac_msg *msg)
  * though they have no content.
  */
 static void *
-dup_str(struct tac_handle *h, const struct srvr_str *ss, size_t *len)
+dup_str(struct tac_handle *h, const struct tac_str *ss, size_t *len)
 {
 	unsigned char *p;
 
@@ -402,10 +405,10 @@ establish_connection(struct tac_handle *h)
  * Free a client string, obliterating its contents first for security.
  */
 static void
-free_str(struct clnt_str *cs)
+free_str(struct tac_str *cs)
 {
 	if (cs->data != NULL) {
-		memset(cs->data, 0, cs->len);
+		memset_s(cs->data, cs->len, 0, cs->len);
 		free(cs->data);
 		cs->data = NULL;
 		cs->len = 0;
@@ -456,8 +459,8 @@ get_srvr_end(struct tac_handle *h)
 }
 
 static int
-get_srvr_str(struct tac_handle *h, const char *field,
-	     struct srvr_str *ss, size_t len)
+get_str(struct tac_handle *h, const char *field,
+    struct tac_str *ss, size_t len)
 {
 	if (h->srvr_pos + len > ntohl(h->response.length)) {
 		generr(h, "Invalid length field in %s response from server "
@@ -472,19 +475,12 @@ get_srvr_str(struct tac_handle *h, const char *field,
 }
 
 static void
-init_clnt_str(struct clnt_str *cs)
+init_str(struct tac_str *cs)
 {
 	cs->data = NULL;
 	cs->len = 0;
 }
 
-static void
-init_srvr_str(struct srvr_str *ss)
-{
-	ss->data = NULL;
-	ss->len = 0;
-}
-
 static int
 read_timed(struct tac_handle *h, void *buf, size_t len,
     const struct timeval *deadline)
@@ -597,7 +593,7 @@ recv_msg(struct tac_handle *h)
 }
 
 static int
-save_str(struct tac_handle *h, struct clnt_str *cs, const void *data,
+save_str(struct tac_handle *h, struct tac_str *cs, const void *data,
     size_t len)
 {
 	free_str(cs);
@@ -687,84 +683,14 @@ send_msg(struct tac_handle *h)
 	return 0;
 }
 
-/*
- * Destructively split a string into fields separated by white space.
- * `#' at the beginning of a field begins a comment that extends to the
- * end of the string.  Fields may be quoted with `"'.  Inside quoted
- * strings, the backslash escapes `\"' and `\\' are honored.
- *
- * Pointers to up to the first maxfields fields are stored in the fields
- * array.  Missing fields get NULL pointers.
- *
- * The return value is the actual number of fields parsed, and is always
- * <= maxfields.
- *
- * On a syntax error, places a message in the msg string, and returns -1.
- */
 static int
-split(char *str, char *fields[], int maxfields, char *msg, size_t msglen)
-{
-	char *p;
-	int i;
-	static const char ws[] = " \t";
-
-	for (i = 0;  i < maxfields;  i++)
-		fields[i] = NULL;
-	p = str;
-	i = 0;
-	while (*p != '\0') {
-		p += strspn(p, ws);
-		if (*p == '#' || *p == '\0')
-			break;
-		if (i >= maxfields) {
-			snprintf(msg, msglen, "line has too many fields");
-			return -1;
-		}
-		if (*p == '"') {
-			char *dst;
-
-			dst = ++p;
-			fields[i] = dst;
-			while (*p != '"') {
-				if (*p == '\\') {
-					p++;
-					if (*p != '"' && *p != '\\' &&
-					    *p != '\0') {
-						snprintf(msg, msglen,
-						    "invalid `\\' escape");
-						return -1;
-					}
-				}
-				if (*p == '\0') {
-					snprintf(msg, msglen,
-					    "unterminated quoted string");
-					return -1;
-				}
-				*dst++ = *p++;
-			}
-			*dst = '\0';
-			p++;
-			if (*p != '\0' && strspn(p, ws) == 0) {
-				snprintf(msg, msglen, "quoted string not"
-				    " followed by white space");
-				return -1;
-			}
-		} else {
-			fields[i] = p;
-			p += strcspn(p, ws);
-			if (*p != '\0')
-				*p++ = '\0';
-		}
-		i++;
-	}
-	return i;
-}
-
-int
-tac_add_server(struct tac_handle *h, const char *host, int port,
-    const char *secret, int timeout, int flags)
+tac_add_server_av(struct tac_handle *h, const char *host, int port,
+    const char *secret, int timeout, int flags, const char *const *avs)
 {
 	struct tac_server *srvp;
+	const char *p;
+	size_t len;
+	int i;
 
 	if (h->num_servers >= MAXSERVERS) {
 		generr(h, "Too many TACACS+ servers specified");
@@ -790,8 +716,46 @@ tac_add_server(struct tac_handle *h, const char *host, int port,
 		return -1;
 	srvp->timeout = timeout;
 	srvp->flags = flags;
+	srvp->navs = 0;
+	for (i = 0; avs[i] != NULL; i++) {
+		if (i >= MAXAVPAIRS) {
+			generr(h, "too many AV pairs");
+			goto fail;
+		}
+		for (p = avs[i], len = 0; is_arg(*p); p++)
+			len++;
+		if (p == avs[i] || *p != '=') {
+			generr(h, "invalid AV pair %d", i);
+			goto fail;
+		}
+		while (*p++ != '\0')
+			len++;
+		if ((srvp->avs[i].data = xstrdup(h, avs[i])) == NULL)
+			goto fail;
+		srvp->avs[i].len = len;
+		srvp->navs++;
+	}
 	h->num_servers++;
 	return 0;
+fail:
+	memset_s(srvp->secret, strlen(srvp->secret), 0, strlen(srvp->secret));
+	free(srvp->secret);
+	srvp->secret = NULL;
+	for (i = 0; i < srvp->navs; i++) {
+		free(srvp->avs[i].data);
+		srvp->avs[i].data = NULL;
+		srvp->avs[i].len = 0;
+	}
+	return -1;
+}
+
+int
+tac_add_server(struct tac_handle *h, const char *host, int port,
+    const char *secret, int timeout, int flags)
+{
+	const char *const *avs = { NULL };
+
+	return tac_add_server_av(h, host, port, secret, timeout, flags, avs);
 }
 
 void
@@ -819,12 +783,22 @@ tac_close(struct tac_handle *h)
 	free(h);
 }
 
+static void
+freev(char **fields, int nfields)
+{
+	if (fields != NULL) {
+		while (nfields-- > 0)
+			free(fields[nfields]);
+		free(fields);
+	}
+}
+
 int
 tac_config(struct tac_handle *h, const char *path)
 {
 	FILE *fp;
-	char buf[MAXCONFLINE];
-	int linenum;
+	char **fields;
+	int linenum, nfields;
 	int retval;
 
 	if (path == NULL)
@@ -834,46 +808,23 @@ tac_config(struct tac_handle *h, const char *path)
 		return -1;
 	}
 	retval = 0;
-	linenum = 0;
-	while (fgets(buf, sizeof buf, fp) != NULL) {
-		int len;
-		char *fields[4];
-		int nfields;
-		char msg[ERRSIZE];
+	linenum = nfields = 0;
+	fields = NULL;
+	while ((fields = openpam_readlinev(fp, &linenum, &nfields)) != NULL) {
 		char *host, *res;
 		char *port_str;
 		char *secret;
 		char *timeout_str;
-		char *options_str;
 		char *end;
 		unsigned long timeout;
 		int port;
 		int options;
+		int i;
 
-		linenum++;
-		len = strlen(buf);
-		/* We know len > 0, else fgets would have returned NULL. */
-		if (buf[len - 1] != '\n') {
-			if (len >= sizeof buf - 1)
-				generr(h, "%s:%d: line too long", path,
-				    linenum);
-			else
-				generr(h, "%s:%d: missing newline", path,
-				    linenum);
-			retval = -1;
-			break;
-		}
-		buf[len - 1] = '\0';
-
-		/* Extract the fields from the line. */
-		nfields = split(buf, fields, 4, msg, sizeof msg);
-		if (nfields == -1) {
-			generr(h, "%s:%d: %s", path, linenum, msg);
-			retval = -1;
-			break;
-		}
-		if (nfields == 0)
+		if (nfields == 0) {
+			freev(fields, nfields);
 			continue;
+		}
 		if (nfields < 2) {
 			generr(h, "%s:%d: missing shared secret", path,
 			    linenum);
@@ -882,8 +833,6 @@ tac_config(struct tac_handle *h, const char *path)
 		}
 		host = fields[0];
 		secret = fields[1];
-		timeout_str = fields[2];
-		options_str = fields[3];
 
 		/* Parse and validate the fields. */
 		res = host;
@@ -899,7 +848,10 @@ tac_config(struct tac_handle *h, const char *path)
 			}
 		} else
 			port = 0;
-		if (timeout_str != NULL) {
+		i = 2;
+		if (nfields > i && strlen(fields[i]) > 0 &&
+		    strspn(fields[i], "0123456789") == strlen(fields[i])) {
+			timeout_str = fields[i];
 			timeout = strtoul(timeout_str, &end, 10);
 			if (timeout_str[0] == '\0' || *end != '\0') {
 				generr(h, "%s:%d: invalid timeout", path,
@@ -907,22 +859,17 @@ tac_config(struct tac_handle *h, const char *path)
 				retval = -1;
 				break;
 			}
+			i++;
 		} else
 			timeout = TIMEOUT;
 		options = 0;
-		if (options_str != NULL) {
-			if (strcmp(options_str, "single-connection") == 0)
-				options |= TAC_SRVR_SINGLE_CONNECT;
-			else {
-				generr(h, "%s:%d: invalid option \"%s\"",
-				    path, linenum, options_str);
-				retval = -1;
-				break;
-			}
-		};
-
-		if (tac_add_server(h, host, port, secret, timeout,
-		    options) == -1) {
+		if (nfields > i &&
+		    strcmp(fields[i], "single-connection") == 0) {
+			options |= TAC_SRVR_SINGLE_CONNECT;
+			i++;
+		}
+		if (tac_add_server_av(h, host, port, secret, timeout,
+		    options, (const char *const *)(fields + i)) == -1) {
 			char msg[ERRSIZE];
 
 			strcpy(msg, h->errmsg);
@@ -930,9 +877,10 @@ tac_config(struct tac_handle *h, const char *path)
 			retval = -1;
 			break;
 		}
+		memset_s(secret, strlen(secret), 0, strlen(secret));
+		freev(fields, nfields);
 	}
-	/* Clear out the buffer to wipe a possible copy of a shared secret */
-	memset(buf, 0, sizeof buf);
+	freev(fields, nfields);
 	fclose(fp);
 	return retval;
 }
@@ -1038,17 +986,17 @@ tac_open(void)
 		h->num_servers = 0;
 		h->cur_server = 0;
 		h->errmsg[0] = '\0';
-		init_clnt_str(&h->user);
-		init_clnt_str(&h->port);
-		init_clnt_str(&h->rem_addr);
-		init_clnt_str(&h->data);
-		init_clnt_str(&h->user_msg);
+		init_str(&h->user);
+		init_str(&h->port);
+		init_str(&h->rem_addr);
+		init_str(&h->data);
+		init_str(&h->user_msg);
 		for (i=0; i<MAXAVPAIRS; i++) {
-			init_clnt_str(&(h->avs[i]));
-			init_srvr_str(&(h->srvr_avs[i]));
+			init_str(&(h->avs[i]));
+			init_str(&(h->srvr_avs[i]));
 		}
-		init_srvr_str(&h->srvr_msg);
-		init_srvr_str(&h->srvr_data);
+		init_str(&h->srvr_msg);
+		init_str(&h->srvr_data);
 	}
 	return h;
 }
@@ -1091,8 +1039,8 @@ tac_send_authen(struct tac_handle *h)
 	/* Scan the optional fields in the reply. */
 	ar = &h->response.u.authen_reply;
 	h->srvr_pos = offsetof(struct tac_authen_reply, rest[0]);
-	if (get_srvr_str(h, "msg", &h->srvr_msg, ntohs(ar->msg_len)) == -1 ||
-	    get_srvr_str(h, "data", &h->srvr_data, ntohs(ar->data_len)) == -1 ||
+	if (get_str(h, "msg", &h->srvr_msg, ntohs(ar->msg_len)) == -1 ||
+	    get_str(h, "data", &h->srvr_data, ntohs(ar->data_len)) == -1 ||
 	    get_srvr_end(h) == -1)
 		return -1;
 
@@ -1112,6 +1060,7 @@ tac_send_author(struct tac_handle *h)
 	char dbgstr[64];
 	struct tac_author_request *areq = &h->request.u.author_request;
 	struct tac_author_response *ares = &h->response.u.author_response;
+	struct tac_server *srvp;
 
 	h->request.length =
 		htonl(offsetof(struct tac_author_request, rest[0]));
@@ -1145,23 +1094,25 @@ tac_send_author(struct tac_handle *h)
 	/* Send the message and retrieve the reply. */
 	if (send_msg(h) == -1 || recv_msg(h) == -1)
 		return -1;
+	srvp = &h->servers[h->cur_server];
 
 	/* Update the offset in the response packet based on av pairs count */
 	h->srvr_pos = offsetof(struct tac_author_response, rest[0]) +
 		ares->av_cnt;
 
 	/* Scan the optional fields in the response. */
-	if (get_srvr_str(h, "msg", &h->srvr_msg, ntohs(ares->msg_len)) == -1 ||
-	    get_srvr_str(h, "data", &h->srvr_data, ntohs(ares->data_len)) ==-1)
+	if (get_str(h, "msg", &h->srvr_msg, ntohs(ares->msg_len)) == -1 ||
+	    get_str(h, "data", &h->srvr_data, ntohs(ares->data_len)) ==-1)
 		return -1;
 
 	/* Get each AV pair (just setting pointers, not malloc'ing) */
 	clear_srvr_avs(h);
 	for (i=0; i<ares->av_cnt; i++) {
 		snprintf(dbgstr, sizeof dbgstr, "av-pair-%d", i);
-		if (get_srvr_str(h, dbgstr, &(h->srvr_avs[i]),
+		if (get_str(h, dbgstr, &(h->srvr_avs[i]),
 				 ares->rest[i]) == -1)
 			return -1;
+		h->srvr_navs++;
 	}
 
 	/* Should have ended up at the end */
@@ -1172,7 +1123,7 @@ tac_send_author(struct tac_handle *h)
 	if (!h->single_connect)
 		close_connection(h);
 
-	return ares->av_cnt << 8 | ares->status;
+	return (h->srvr_navs + srvp->navs) << 8 | ares->status;
 }
 
 int
@@ -1206,8 +1157,8 @@ tac_send_acct(struct tac_handle *h)
 
 	/* reply */
 	h->srvr_pos = offsetof(struct tac_acct_reply, rest[0]);
-	if (get_srvr_str(h, "msg", &h->srvr_msg, ntohs(ar->msg_len)) == -1 ||
-	    get_srvr_str(h, "data", &h->srvr_data, ntohs(ar->data_len)) == -1 ||
+	if (get_str(h, "msg", &h->srvr_msg, ntohs(ar->msg_len)) == -1 ||
+	    get_str(h, "data", &h->srvr_data, ntohs(ar->data_len)) == -1 ||
 	    get_srvr_end(h) == -1)
 		return -1;
 
@@ -1270,41 +1221,41 @@ tac_set_av(struct tac_handle *h, u_int index, const char *av)
 char *
 tac_get_av(struct tac_handle *h, u_int index)
 {
-	if (index >= MAXAVPAIRS)
-		return NULL;
-	return dup_str(h, &(h->srvr_avs[index]), NULL);
+	struct tac_server *srvp;
+
+	if (index < h->srvr_navs)
+		return dup_str(h, &h->srvr_avs[index], NULL);
+	index -= h->srvr_navs;
+	srvp = &h->servers[h->cur_server];
+	if (index < srvp->navs)
+		return xstrdup(h, srvp->avs[index].data);
+	return NULL;
 }
 
 char *
 tac_get_av_value(struct tac_handle *h, const char *attribute)
 {
-	int i, len;
-	const char *ch, *end;
-	const char *candidate;
-	int   candidate_len;
+	int i, attr_len;
 	int   found_seperator;
-	struct srvr_str srvr;
+	char *ch, *end;
+	struct tac_str *candidate;
+	struct tac_str value;
+	struct tac_server *srvp = &h->servers[h->cur_server];
 
-	if (attribute == NULL || ((len = strlen(attribute)) == 0))
+	if (attribute == NULL || (attr_len = strlen(attribute)) == 0)
 		return NULL;
 
-	for (i=0; i<MAXAVPAIRS; i++) {
-		candidate = h->srvr_avs[i].data;
-		candidate_len = h->srvr_avs[i].len;
+	for (i = 0; i < h->srvr_navs + srvp->navs; i++) {
+		if (i < h->srvr_navs)
+			candidate = &h->srvr_avs[i];
+		else
+			candidate = &srvp->avs[i - h->srvr_navs];
 
-		/*
-		 * Valid 'srvr_avs' guaranteed to be contiguous starting at 
-		 * index 0 (not necessarily the case with 'avs').  Break out
-		 * when the "end" of the list has been reached.
-		 */
-		if (!candidate)
-			break;
-
-		if (len < candidate_len && 
-		    !strncmp(candidate, attribute, len)) {
+		if (attr_len < candidate->len &&
+		    strncmp(candidate->data, attribute, attr_len) == 0) {
 
-			ch = candidate + len;
-			end = candidate + candidate_len;
+			ch = candidate->data + attr_len;
+			end = candidate->data + candidate->len;
 
 			/*
 			 * Sift out the white space between A and V (should not
@@ -1331,9 +1282,9 @@ tac_get_av_value(struct tac_handle *h, const char *attribute)
 			 * dup_str() will handle srvr.len == 0 correctly.
 			 */
 			if (found_seperator == 1) {
-				srvr.len = end - ch;
-				srvr.data = ch;
-				return dup_str(h, &srvr, NULL);
+				value.len = end - ch;
+				value.data = ch;
+				return dup_str(h, &value, NULL);
 			}
 		}
 	}
@@ -1352,8 +1303,10 @@ static void
 clear_srvr_avs(struct tac_handle *h)
 {
 	int i;
-	for (i=0; i<MAXAVPAIRS; i++)
-		init_srvr_str(&(h->srvr_avs[i]));
+
+	for (i = 0; i < h->srvr_navs; i++)
+		init_str(&(h->srvr_avs[i]));
+	h->srvr_navs = 0;
 }
 
 
diff --git a/lib/libtacplus/taclib_private.h b/lib/libtacplus/taclib_private.h
index 665616f1f200..43203b72f6ca 100644
--- a/lib/libtacplus/taclib_private.h
+++ b/lib/libtacplus/taclib_private.h
@@ -58,30 +58,8 @@
 #define TAC_UNENCRYPTED		0x01
 #define TAC_SINGLE_CONNECT	0x04
 
-struct tac_server {
-	struct sockaddr_in addr;	/* Address of server */
-	char		*secret;	/* Shared secret */
-	int		 timeout;	/* Timeout in seconds */
-	int		 flags;
-};
-
-/*
- * An optional string of bytes specified by the client for inclusion in
- * a request.  The data is always a dynamically allocated copy that
- * belongs to the library.  It is copied into the request packet just
- * before sending the request.
- */
-struct clnt_str {
-	void		*data;
-	size_t		 len;
-};
-
-/*
- * An optional string of bytes from a server response.  The data resides
- * in the response packet itself, and must not be freed.
- */
-struct srvr_str {
-	const void	*data;
+struct tac_str {
+	char		*data;
 	size_t		 len;
 };
 
@@ -171,6 +149,15 @@ struct tac_msg {
 	} u;
 };
 
+struct tac_server {
+	struct sockaddr_in addr;	/* Address of server */
+	char		*secret;	/* Shared secret */
+	int		 timeout;	/* Timeout in seconds */
+	int		 flags;
+	unsigned int	 navs;
+	struct tac_str	 avs[MAXAVPAIRS];
+};
+
 struct tac_handle {
 	int		 fd;		/* Socket file descriptor */
 	struct tac_server servers[MAXSERVERS];	/* Servers to contact */
@@ -180,20 +167,30 @@ struct tac_handle {
 	int		 last_seq_no;
 	char		 errmsg[ERRSIZE];	/* Most recent error message */
 
-	struct clnt_str	 user;
-	struct clnt_str	 port;
-	struct clnt_str	 rem_addr;
-	struct clnt_str	 data;
-	struct clnt_str	 user_msg;
-	struct clnt_str  avs[MAXAVPAIRS];
+	struct tac_str	 user;
+	struct tac_str	 port;
+	struct tac_str	 rem_addr;
+	struct tac_str	 data;
+	struct tac_str	 user_msg;
+	struct tac_str	 avs[MAXAVPAIRS];
 
 	struct tac_msg	 request;
 	struct tac_msg	 response;
 
 	int		 srvr_pos;	/* Scan position in response body */
-	struct srvr_str	 srvr_msg;
-	struct srvr_str	 srvr_data;
-	struct srvr_str  srvr_avs[MAXAVPAIRS];
+	unsigned int	 srvr_navs;
+	struct tac_str	 srvr_msg;
+	struct tac_str	 srvr_data;
+	struct tac_str	 srvr_avs[MAXAVPAIRS];
 };
 
+#define is_alpha(ch) /* alphabetical */					\
+	(((ch) >= 'A' && (ch) <= 'Z') || ((ch) >= 'a' && (ch) <= 'z'))
+#define is_num(ch) /* numerical */					\
+	((ch) >= '0' && (ch) <= '9')
+#define is_alnum(ch) /* alphanumerical */				\
+	(is_alpha(ch) || is_num(ch))
+#define is_arg(ch) /* valid in an argument name */			\
+	(is_alnum(ch) || (ch) == '_' || (ch) == '-')
+
 #endif
diff --git a/lib/libtacplus/tacplus.conf.5 b/lib/libtacplus/tacplus.conf.5
index 9016a25faad0..2653c664d67e 100644
--- a/lib/libtacplus/tacplus.conf.5
+++ b/lib/libtacplus/tacplus.conf.5
@@ -22,7 +22,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd July 29, 1998
+.Dd June 13, 2023
 .Dt TACPLUS.CONF 5
 .Os
 .Sh NAME
@@ -44,23 +44,9 @@ Leading
 white space is ignored, as are empty lines and lines containing
 only comments.
 .Pp
-A TACACS+ server is described by two to four fields on a line.
-The
-fields are separated by white space.
-The
-.Ql #
-character at the beginning of a field begins a comment, which extends
-to the end of the line.
-A field may be enclosed in double quotes,
-in which case it may contain white space and/or begin with the
-.Ql #
-character.
-Within a quoted string, the double quote character can
-be represented by
-.Ql \e\&" ,
-and the backslash can be represented by
-.Ql \e\e .
-No other escape sequences are supported.
+A TACACS+ server is described by a minimum of two fields on a line.
+The fields are separated by whitespace and follow the same rules for
+comments, quoting, escaping, and line continuation as the POSIX shell.
 .Pp
 The first field specifies
 the server host, either as a fully qualified domain name or as a
@@ -81,12 +67,11 @@ An empty secret disables the
 normal encryption mechanism, causing all data to cross the network in
 cleartext.
 .Pp
-The third field contains a decimal integer specifying the timeout
-in seconds for communicating with the server.
+The optional third field may contain a decimal integer specifying the
+timeout in seconds for communicating with the server.
 The timeout applies
 separately to each connect, write, and read operation.
-If this field
-is omitted, it defaults to 3 seconds.
+If this field is omitted, it defaults to 3 seconds.
 .Pp
 The optional fourth field may contain the string
 .Ql single-connection .
@@ -96,6 +81,11 @@ sessions.
 Some older TACACS+ servers become confused if this option
 is specified.
 .Pp
+Any subsequent fields must be of the form
+.Ar attribute Ns = Ns Ar value
+and will be appended to authorization responses as if they had been
+sent by the server.
+.Pp
 Up to 10 TACACS+ servers may be specified.
 The servers are tried in
 order, until a valid response is received or the list is exhausted.
@@ -118,11 +108,13 @@ shared secrets, it should not be readable except by root.
 tacserver.domain.com	OurLittleSecret
 
 # A server using a non-standard port, with an increased timeout and
-# the "single-connection" option.
-auth.domain.com:4333	"Don't tell!!"	15	single-connection
+# the "single-connection" option, and overrides for the for uid, gid
+# and shell attributes.
+auth.domain.com:4333	"Don't tell!!"	15	single-connection \e
+    uid=1001 gid=20 shell="/usr/local/bin/zsh"
 
 # A server specified by its IP address:
-192.168.27.81		$X*#..38947ax-+=
+192.168.27.81		$X*#..38947ax-+=	shell="/sbin/nologin"
 .Ed
 .Sh SEE ALSO
 .Xr libtacplus 3
diff --git a/share/mk/src.libnames.mk b/share/mk/src.libnames.mk
index 054fa9099025..4963a9c02436 100644
--- a/share/mk/src.libnames.mk
+++ b/share/mk/src.libnames.mk
@@ -396,7 +396,7 @@ _DP_c+=		ssp_nonshared
 .endif
 _DP_stats=	sbuf pthread
 _DP_stdthreads=	pthread
-_DP_tacplus=	md
+_DP_tacplus=	md pam
 _DP_nvpair=	spl
 _DP_panelw=	ncursesw
 _DP_rpcsec_gss=	gssapi