From: gilles@poolp.org Subject: Re: smtpd/makemap vs ipv6 addresses in table(5)s To: "Omar Polo" , tech@openbsd.org Date: Sun, 03 Mar 2024 08:34:34 +0000 As discussed in private, ok gilles@ with this diff in 7.5, just for the record March 1, 2024 10:37 PM, "Omar Polo" wrote: > friendly ping, anyone has an opinion? > > Omar Polo wrote: > >> 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;