On Thu, Mar 21, 2019 at 09:28:28AM +0100, Michał Koc wrote:
> W dniu 21.03.2019 o 07:21, Otto Moerbeek pisze:
> > On Thu, Mar 21, 2019 at 12:51:11AM +0100, Klemens Nanni wrote:
> > > On Tue, Mar 12, 2019 at 03:19:56PM +0100, Otto Moerbeek wrote:
> > > > > I also fixed a case of parsing IPv6 addresses.
> > > > >
> > > > > Anyone willing to ok?
> > > See comments inline.
> > >
> > > > And now also with a lexer bug fixed. Earlier I thougt it was an order
> > > > dependency in the clauses. But is was an order dependency in comment
> > > > lines and empty lines.
> > > > +check_peer_addr(const char *peer_addr)
> > > > +{
> > > > + int valid = 1;
> > > > + short peer_family = AF_UNSPEC;
> > > > + struct ifaddrs *ifap = NULL, *ifa;
> > > > + struct syncpeer *peer;
> > > > + struct sockaddr_in sa;
> > > > + struct sockaddr_in6 sa6;
> > > > +
> > > > + if (inet_pton(AF_INET, peer_addr, &sa.sin_addr) == 1)
> > > > + peer_family = AF_INET;
> > > > +
> > > > + if (peer_family == AF_UNSPEC && inet_pton(AF_INET6, peer_addr,
> > > > + &sa6.sin6_addr) == 1)
> > > > + peer_family = AF_INET6;
> > > `peer_addr' must not be a CIDR network, so I'd go with the more AF
> > > agnostic getaddrinfo(3) and check for res->ai_family in any case...
> > >
> > > > + if (peer_family == AF_UNSPEC) {
> > > > + log_msg(2, "config: skip unparseable peer %s",
> > > > peer_addr);
> > > > + valid = 0;
> > > > + }
> > > ..but `peer_addr' may also be a hostname, so that is not handled by
> > > your diff:
> > >
> > > net.h: char *name; /* FQDN or an IP, from conf */
> > >
> > > getaddrinfo(3) can resolve however, thus inet_pton(3) should not be used
> > > here.
> > >
> > > Either way, this is a job for host_ip() as found in pfctl or bgpd.
> > > Other daemons like iked still have host_v4() and host_v6(). Parsers use
> > > these functions to check for valid addresses, so I'd say sasyncd should
> > > be no exception and adopt the same approach.
> > >
> > > > @@ -325,7 +386,7 @@ yylex(void)
> > > > /* Numerical token? */
> > > > if (isdigit(*confptr)) {
> > > > for (p = confptr; *p; p++)
> > > > - if (*p == '.') /* IP address, or bad input */
> > > > + if (*p == '.' || *p == ':') /* IP address, or
> > > > bad input */
> > > This fixes the parser as in
> > >
> > > # echo peer 2001:db8::1 > conf
> > > # sasyncd -dnv -c conf
> > > config: syntax error
> > > # ./obj/sasyncd -dnv -c conf
> > > configuration OK
> > >
> > > But I have not actually tested this with a proper IPv6 setup since I do
> > > not use sasyncd; did you?
> > >
> > > > @@ -397,6 +458,9 @@ conf_parse_file(char *cfgfile)
> > > > if (*s == '#') {
> > > > while (*s != '\n' && s < buf + conflen)
> > > > s++;
> > > > + while (*s == '\n' && s < buf + conflen)
> > > > + s++;
> > > > + s--;
> > > OK kn for this fix alone.
> > >
> > I'll test an ipv6 seup and commit the lexer parts if those work.
> > Michal, can you work on the first set of comments? I think Klemens is
> > right.
> >
> > -Otto
> >
> Of course, I'll follow the comments soon
>
> Best Regards
> M.K.
>
Meanwhile, I tested a IPv6 setup, it works ok.
So I'm going to commit the diff below,
-Otto
Index: conf.y
===================================================================
RCS file: /cvs/src/usr.sbin/sasyncd/conf.y,v
retrieving revision 1.19
diff -u -p -r1.19 conf.y
--- conf.y 9 Apr 2017 02:40:24 -0000 1.19
+++ conf.y 21 Mar 2019 10:51:46 -0000
@@ -325,7 +325,7 @@ yylex(void)
/* Numerical token? */
if (isdigit(*confptr)) {
for (p = confptr; *p; p++)
- if (*p == '.') /* IP address, or bad input */
+ if (*p == '.' || *p == ':') /* IP address, or bad input
*/
goto is_string;
v = (int)strtol(confptr, (char **)NULL, 10);
yylval.val = v;
@@ -397,6 +397,9 @@ conf_parse_file(char *cfgfile)
if (*s == '#') {
while (*s != '\n' && s < buf + conflen)
s++;
+ while (*s == '\n' && s < buf + conflen)
+ s++;
+ s--;
continue;
}
if (d == buf && isspace(*s))
