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
