W dniu 22.03.2019 o 11:19, Michał Koc pisze:
W dniu 21.03.2019 o 11:52, Otto Moerbeek pisze:
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.

the diff is ready, what happened here:

1. host => ip manipulation moved to host_ip() function in net.c and is using getaddrinfo()
2. net_set_sa() in net.c modified to use host_ip()

Bets Regards
M.K

Additional range check when comparing memory contents


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      22 Mar 2019 14:27:49 -0000
@@ -30,8 +30,10 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/socket.h>
+#include <arpa/inet.h>
 #include <ctype.h>
 #include <fcntl.h>
+#include <ifaddrs.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -48,6 +50,7 @@ struct cfgstate       cfgstate;
 int    conflen = 0;
 char   *confbuf, *confptr;
 
+int    check_peer_addr(const char *);
 int    yyparse(void);
 int    yylex(void);
 void   yyerror(const char *);
@@ -172,17 +175,8 @@ setting            : INTERFACE STRING
                | PEER STRING
                {
                        struct syncpeer *peer;
-                       int              duplicate = 0;
 
-                       for (peer = LIST_FIRST(&cfgstate.peerlist); peer;
-                            peer = LIST_NEXT(peer, link))
-                               if (strcmp($2, peer->name) == 0) {
-                                       duplicate++;
-                                       break;
-                               }
-                       if (duplicate)
-                               free($2);
-                       else {
+                       if (check_peer_addr($2)) {
                                peer = calloc(1, sizeof *peer);
                                if (!peer) {
                                        log_err("config: calloc(1, %lu) "
@@ -191,10 +185,11 @@ setting           : INTERFACE STRING
                                        YYERROR;
                                }
                                peer->name = $2;
-                       }
-                       LIST_INSERT_HEAD(&cfgstate.peerlist, peer, link);
-                       cfgstate.peercnt++;
-                       log_msg(2, "config: add peer %s", peer->name);
+                               LIST_INSERT_HEAD(&cfgstate.peerlist, peer, 
link);
+                               cfgstate.peercnt++;
+                               log_msg(2, "config: add peer %s", peer->name);
+                       } else
+                               free($2);
                }
                | LISTEN ON STRING af port
                {
@@ -281,6 +276,51 @@ match(char *token)
            sizeof keywords[0], match_cmp);
 
        return k ? k->value : STRING;
+}
+
+int
+check_peer_addr(const char *peer_addr)
+{
+       struct ifaddrs          *ifap = 0, *ifa;
+       struct syncpeer         *peer;
+       struct sockaddr         *sa, *peer_sa;
+
+       sa = host_ip(peer_addr);
+
+       if(!sa)
+               log_msg(2, "config: skip unparseable peer %s", peer_addr);
+
+       if (sa && getifaddrs(&ifap) == 0) {
+               for (ifa = ifap; ifa && sa; ifa = ifa->ifa_next) {
+                       if (!ifa->ifa_addr || ifa->ifa_addr->sa_family != 
sa->sa_family)
+                               continue;
+
+                       if (memcmp(sa, ifa->ifa_addr, sa->sa_len < 
ifa->ifa_addr->sa_len ? sa->sa_len : ifa->ifa_addr->sa_len) == 0) {
+                               log_msg(2, "config: skip local peer %s", 
peer_addr);
+                               free(sa);
+                               sa = 0;
+                       }
+               }
+               freeifaddrs(ifap);
+       }
+
+       if (sa) {
+           for (peer = LIST_FIRST(&cfgstate.peerlist); peer && sa;
+                peer = LIST_NEXT(peer, link)) {
+                   peer_sa = host_ip(peer->name);
+               
+                   if (memcmp(sa, peer_sa, sa->sa_len < peer_sa->sa_len ? 
sa->sa_len : peer_sa->sa_len) == 0) {
+                       log_msg(2, "config: skip duplicate peer %s", peer_addr);
+                       free(sa);
+                       sa = 0;
+                   }
+                   free(peer_sa);
+           }
+       }
+
+       free(sa);
+
+       return sa != 0;
 }
 
 int
Index: net.c
===================================================================
RCS file: /cvs/src/usr.sbin/sasyncd/net.c,v
retrieving revision 1.23
diff -u -p -r1.23 net.c
--- net.c       12 Dec 2015 20:04:23 -0000      1.23
+++ net.c       22 Mar 2019 14:27:49 -0000
@@ -755,32 +755,26 @@ net_read(struct syncpeer *p, u_int32_t *
 static int
 net_set_sa(struct sockaddr *sa, char *name, in_port_t port)
 {
-       struct sockaddr_in      *sin = (struct sockaddr_in *)sa;
-       struct sockaddr_in6     *sin6 = (struct sockaddr_in6 *)sa;
+       struct sockaddr         *peer_sa = host_ip(name);
 
-       if (!name) {
-               /* XXX Assume IPv4 */
-               sa->sa_family = AF_INET;
-               sin->sin_port = htons(port);
-               sin->sin_len = sizeof *sin;
-               return 0;
-       }
+       if (peer_sa) {
 
-       if (inet_pton(AF_INET, name, &sin->sin_addr) == 1) {
-               sa->sa_family = AF_INET;
-               sin->sin_port = htons(port);
-               sin->sin_len = sizeof *sin;
-               return 0;
-       }
+           memcpy(sa, peer_sa, peer_sa->sa_len);
+           free(peer_sa);
+
+           switch(sa->sa_family) {
+           case AF_INET:
+               ((struct sockaddr_in *)sa)->sin_port = htons(port);
+               break;
+
+           case AF_INET6:
+               ((struct sockaddr_in6 *)sa)->sin6_port = htons(port);
+               break;
+           }
 
-       if (inet_pton(AF_INET6, name, &sin6->sin6_addr) == 1) {
-               sa->sa_family = AF_INET6;
-               sin6->sin6_port = htons(port);
-               sin6->sin6_len = sizeof *sin6;
-               return 0;
        }
 
-       return -1;
+       return peer_sa == 0 ? -1 : 0;
 }
 
 static void
@@ -848,4 +842,24 @@ net_check_peers(void *arg)
 {
        net_connect();
        (void)timer_add("peer recheck", 600, net_check_peers, 0);
+}
+
+struct sockaddr *
+host_ip(const char *hostname) {
+       struct addrinfo         *ai = 0;
+       struct sockaddr         *sa = 0;
+
+       if (getaddrinfo(hostname, NULL, NULL, &ai) == 0) {
+               
+               switch (ai->ai_family) {
+               case AF_INET:
+               case AF_INET6:
+                       sa = calloc(1, ai->ai_addr->sa_len);
+                       memcpy(sa, ai->ai_addr, ai->ai_addr->sa_len);
+                       break;
+               }
+               freeaddrinfo(ai);
+       }
+
+       return sa;
 }
Index: net.h
===================================================================
RCS file: /cvs/src/usr.sbin/sasyncd/net.h,v
retrieving revision 1.5
diff -u -p -r1.5 net.h
--- net.h       2 Jun 2006 20:09:43 -0000       1.5
+++ net.h       22 Mar 2019 14:27:49 -0000
@@ -53,6 +53,7 @@ enum CTLTYPE { RESERVED = 0, CTL_STATE, 
 /* net.c */
 void   net_connect(void);
 void   net_disconnect_peer(struct syncpeer *);
+struct sockaddr        *host_ip(const char *);
 
 /* net_ctl.c */
 void   net_ctl_handle_msg(struct syncpeer *, u_int8_t *, u_int32_t);

Reply via email to