From: Omar Polo Subject: Re: smtpd/makemap vs ipv6 addresses in table(5)s To: Gilles Chehade Cc: tech@openbsd.org Date: Wed, 21 Feb 2024 19:53:28 +0100 On Tue Feb 20, 2024 at 10:59 PM CET, Gilles Chehade wrote: > On Tue, Feb 20, 2024 at 05:10:52PM +0100, Omar Polo wrote: > > [...] > > 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? > > > > I dunno if it should be more strict but that would already be a good > start and an improvement over today. One downside I realized only later is that it still allows gibberish after the ip addr, as in "[::1]foo". > > [...] > > > > thoughs? > > > > Generally ok with this but also starting to wonder if makemap and static > tables parsing would not benefit from a regex(3)-based line parser to be > honest. It's not used anywhere an expensive regex would be an issue, and > it would be more maintainable than having to read it every ten year when > someone hits a case we didn't hit before. > > Dunno, just opening the discussion Not sure if a regexp is needed here, but for sure it could be interesting to eventually settle on some other details of the parser. For example, I these confusing: foo :: bar becomes "foo" -> ":: bar" foo::bar becomes "foo" -> ":bar" foo:: bar becomes "foo" -> "bar" so, some details should be agreed upon regardless of the implementation of the parser. my diff also introduces another ambiguity: what to do when there is no closing ']'? does it makes sense for keys to include '[' outside of ipv6 addresses? (I guess no, but don't know for sure.) Here's another try, where I may have gone too far, which considers a parse error if there is no closing ] and also improves IMHO the splitting of key-value. Using the same example table as before, now it becomes: 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 | sort [::1] bar [::2] bar foo1 bar foo2 bar foo3 bar foo4 : bar foo5 :: bar foo6 :bar foo7 : bar i.e. it splits on the first space or colon, and then trims the spaces. Multiple colons are not trimmed. Does anybody have aliases lines with "foo : bar"? diffstat /home/op/w/smtpd M table.5 | 10+ 7- M util.c | 15+ 23- 2 files changed, 25 insertions(+), 30 deletions(-) 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,37 +885,29 @@ 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 (*keyp == '[') { + if ((valp = strchr(keyp, ']')) == NULL) { + *malformed = 1; + return (0); } - if (*type == T_NONE) - *type = T_LIST; - } + valp++; + } else + valp = keyp + strcspn(keyp, " \t:"); + 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) { - while (*valp) { - if (!isspace((unsigned char)*valp) && - !(*valp == ':' && - isspace((unsigned char)*(valp + 1)))) - break; - ++valp; - } - if (*valp == '\0') - valp = NULL; + if (*valp != '\0') { + *valp++ = '\0'; + valp += strspn(valp, " \t"); } - if (valp == NULL) + if (*valp == '\0') *malformed = 1; *key = keyp; -- Sent from