From: Omar Polo <op@omarpolo.com>
Subject: smtpd/makemap vs ipv6 addresses in table(5)s
To: tech@openbsd.org
Date: Tue, 20 Feb 2024 17:10:52 +0100

Now that the parser was unified, let's fix the original issue.  The
parser splits the line on the ':' character too for key-value tables,
which means that ipv6 addresses are misparsed.  The "::1 localhost"
example given at the bottom of table(5) is actually parsed as key "" and
value ":1 localhost".

For list tables, the special comment "# @list" can be used to turn off
the automatic key/value splitting, but for key-valued tables the parser
needs to be fixed.

Furthermore, smtpd serializes ipv6 addresses by wrapping them in braces
before querying the tables, so even if "::1 localhost" worked, it would
still not match at runtime as ::1 doesn't match [::1].

The manpage also lists ipv6:::1 as example, that is similarly broken.

So, I'm proposing to require ipv6 addresses to be wrapped in braces in
table(5) files, and to amend the parser to skip over to the matching ]
if the line starts with [.  Is it enough or should it be more strict?

Before the diff: (only the first two entries are interesting, the rest
is just there to make sure I'm not breaking existing behaviours)

	suzaku% cat t
	[::1]: bar
	[::2]   bar
	foo1    bar
	foo2:   bar
	foo3:bar
	foo4 :  bar
	foo5 :: bar
	foo6::bar
	foo7:: bar
	suzaku% makemap t && makemap -U t.db
	makemap: t:2: duplicate entry for [

after:

	suzaku% ln -s smtpctl/obj/smtpctl makemap
	suzaku% ./makemap t && ./makemap -U t.db | sort
	[::1]   bar
	[::2]   bar
	foo1    bar
	foo2    bar
	foo3    bar
	foo4    bar
	foo5    :: bar
	foo6    :bar
	foo7    bar

with the exception of the [::1] and [::2] lines, the rest of the table
is still parsed exactly as before.

thoughs?

diff /home/op/w/smtpd
commit - 0e1879aafc8d52781267651a6455236cadaf2b54
path + /home/op/w/smtpd
blob - 65dca359f38d257db5d8c948374a8433c15e1a9d
file + table.5
--- table.5
+++ table.5
@@ -173,10 +173,11 @@ of addresses in the table until a match is found.
 A netaddr table can contain exact addresses or netmasks, and looks as follow:
 .Bd -literal -offset indent
 192.168.1.1
-::1
-ipv6:::1
+[::1]
 192.168.1.0/24
 .Ed
+.Pp
+IPv6 addresses must be enclosed in square brackets.
 .Ss Userinfo tables
 Userinfo tables are used in rule context to specify an alternate userbase,
 mapping virtual users to local system users by UID, GID and home directory.
@@ -214,11 +215,11 @@ A source table looks as follow:
 .Bd -literal -offset indent
 192.168.1.2
 192.168.1.3
-::1
-::2
-ipv6:::3
-ipv6:::4
+[::1]
+[::2]
 .Ed
+.Pp
+IPv6 address must be enclosed in square brackets.
 .Ss Mailaddr tables
 Mailaddr tables are lists of email addresses.
 They can be used in the following contexts:
@@ -254,10 +255,12 @@ outgoing connection.
 .Pp
 The format is a mapping from inet4 or inet6 addresses to hostnames:
 .Bd -literal -offset indent
-::1		localhost
+[::1]		localhost
 127.0.0.1	localhost
 88.190.23.165	www.opensmtpd.org
 .Ed
+.Pp
+IPv6 addresses must be enclosed in square brackets.
 .Sh SEE ALSO
 .Xr smtpd.conf 5 ,
 .Xr makemap 8 ,
blob - d481162c8973a288e6470ba1ef1367ad487b8d2d
file + util.c
--- util.c
+++ util.c
@@ -855,7 +855,7 @@ int
 parse_table_line(FILE *fp, char **line, size_t *linesize,
     int *type, char **key, char **val, int *malformed)
 {
-	char	*keyp, *valp, *p;
+	char	*keyp, *valp;
 	ssize_t	 linelen;
 
 	*key = NULL;
@@ -885,26 +885,26 @@ parse_table_line(FILE *fp, char **line, size_t *linesi
 		return 0;
 	}
 
-	if (*type == T_NONE) {
-		for (p = keyp; *p; p++) {
-			if (*p == ' ' || *p == '\t' || *p == ':') {
-				*type = T_HASH;
-				break;
-			}
-		}
-		if (*type == T_NONE)
-			*type = T_LIST;
+	if (*keyp == '[' && (valp = strchr(keyp, ']')) != NULL)
+		valp++;
+	else
+		valp = keyp;
+	for (; *valp; ++valp) {
+		if (*valp == ' ' || *valp == '\t' || *valp == ':')
+			break;
 	}
 
+	if (*type == T_NONE)
+		*type = (*valp == '\0') ? T_LIST : T_HASH;
+
 	if (*type == T_LIST) {
 		*key = keyp;
 		return (0);
 	}
 
 	/* T_HASH */
-	valp = keyp;
-	strsep(&valp, " \t:");
-	if (valp) {
+	if (*valp != '\0') {
+		*valp++ = '\0';
 		while (*valp) {
 			if (!isspace((unsigned char)*valp) &&
 			    !(*valp == ':' &&
@@ -912,10 +912,8 @@ parse_table_line(FILE *fp, char **line, size_t *linesi
 				break;
 			++valp;
 		}
-		if (*valp == '\0')
-			valp = NULL;
 	}
-	if (valp == NULL)
+	if (*valp == '\0')
 		*malformed = 1;
 
 	*key = keyp;