Dear team,

I have the following use-case on some of my routers: ntpd will
opportunistically select a source address, regardless of whether that
source address is actually a globally routable IP address. Most of the
time this is great, but not in some deployment scenarios.

For instance, IP addresses from the AMS-IX or DE-CIX peering LANs cannot
be expected to be functional source IP addresses for Internet use, as
many ISPs drop those prefixes at their border. If the NTP server happens
to be across the Peering LAN, you'll probably want to use an IP address
from your own advertised IP space, like the router's loopback address.

Thus, I set out to patch ntpd so a source address can be explicitly
specified. This is probably mostly useful for machines which have a
bunch of IPs spread over multiple interfaces like bgp routers.

However, in implementing this feature, I somewhat bumped my head against
"how to properly accomodate dual-stack IPv4 + IPv6 environments from a
user-interface point of view?".

If you configure the following:

    "servers ntp.ring.nlnog.net local-address 165.254.255.27"

Do you expect that any AAAA records resolved for ntp.ring.nlnog.net are
ignored, and for the queries to the A records behind ntp.ring.nlnog.net
"165.254.255.27" is used as source address? Or would one expect that for
IPv4 queries "165.254.255.27" is used, and for IPv6 queries "whatever"
is used (kind of like the current behavior)?

Or, should I make it so that you can do the following:

    "servers ntp.ring.nlnog.net \
        local-address 165.254.255.27,2001:728:1808::26"

or allow one to repeat the 'local-address' keyword:

    "servers ntp.ring.nlnog.net \
        local-address 165.254.255.27 \
        local-address 2001:728:1808::26"

or use different keywords for ipv4 and ipv6:

    "servers ntp.ring.nlnog.net \
        local-address4 165.254.255.27 \
        local-address6 2001:728:1808::26"

It feels like a balance must be struck between between deterministic
behaviour, and the software attempting to get the time from somewhere,
somehow. Any guidance on the topic would be appreciated!

Personally I think i am somewhat in favor of the comma separated
approach: "local-address 165.254.255.27,2001:728:1808::26" - if you only
specifiy one address, that AFI will be determinstic, and the software
will just automatically select a source address for the other AFI. If
you specify two addresses (comma separated, one IPv4 and one IPv6),
you'll have determinstic behavior for both AFIs.

Look forward to your feedback.

Kind regards,

Job

---
 src/usr.sbin/ntpd/client.c    |  6 ++++++
 src/usr.sbin/ntpd/ntp.c       |  1 +
 src/usr.sbin/ntpd/ntpd.conf.5 |  1 +
 src/usr.sbin/ntpd/ntpd.h      |  1 +
 src/usr.sbin/ntpd/parse.y     | 41 ++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/src/usr.sbin/ntpd/client.c b/src/usr.sbin/ntpd/client.c
index ddbb1281..31ff782d 100644
--- a/src/usr.sbin/ntpd/client.c
+++ b/src/usr.sbin/ntpd/client.c
@@ -137,11 +137,17 @@ client_query(struct ntp_peer *p)
 
        if (p->query->fd == -1) {
                struct sockaddr *sa = (struct sockaddr *)&p->addr->ss;
+               struct sockaddr *la = (struct sockaddr *)&p->local_addr;
 
                if ((p->query->fd = socket(p->addr->ss.ss_family, SOCK_DGRAM,
                    0)) == -1)
                        fatal("client_query socket");
 
+               if(p->addr->ss.ss_family == la->sa_family)
+                       if (bind(p->query->fd, la, SA_LEN(la)) == -1)
+                               fatal("couldn't bind to local-address: %s",
+                                   log_sockaddr(la));
+
                if (connect(p->query->fd, sa, SA_LEN(sa)) == -1) {
                        if (errno == ECONNREFUSED || errno == ENETUNREACH ||
                            errno == EHOSTUNREACH || errno == EADDRNOTAVAIL) {
diff --git a/src/usr.sbin/ntpd/ntp.c b/src/usr.sbin/ntpd/ntp.c
index f3366640..f22f1ca4 100644
--- a/src/usr.sbin/ntpd/ntp.c
+++ b/src/usr.sbin/ntpd/ntp.c
@@ -521,6 +521,7 @@ ntp_dispatch_imsg_dns(void)
                                if (peer->addr_head.pool) {
                                        npeer = new_peer();
                                        npeer->weight = peer->weight;
+                                       npeer->local_addr = peer->local_addr;
                                        h->next = NULL;
                                        npeer->addr = h;
                                        npeer->addr_head.a = h;
diff --git a/src/usr.sbin/ntpd/ntpd.conf.5 b/src/usr.sbin/ntpd/ntpd.conf.5
index 6e4e0012..07bc2174 100644
--- a/src/usr.sbin/ntpd/ntpd.conf.5
+++ b/src/usr.sbin/ntpd/ntpd.conf.5
@@ -131,6 +131,7 @@ A stratum value other than the default of 1 can be assigned 
using the
 keyword.
 .It Xo Ic server Ar address
 .Op Ic weight Ar weight-value
+.Op Ic local-address Ar address
 .Xc
 Specify the IP address or the hostname of an NTP
 server to synchronize to.
diff --git a/src/usr.sbin/ntpd/ntpd.h b/src/usr.sbin/ntpd/ntpd.h
index 613b29b2..c1d7fa6e 100644
--- a/src/usr.sbin/ntpd/ntpd.h
+++ b/src/usr.sbin/ntpd/ntpd.h
@@ -153,6 +153,7 @@ struct ntp_peer {
        struct ntp_query                *query;
        struct ntp_offset                reply[OFFSET_ARRAY_SIZE];
        struct ntp_offset                update;
+       struct sockaddr_storage          local_addr;
        enum client_state                state;
        time_t                           next;
        time_t                           deadline;
diff --git a/src/usr.sbin/ntpd/parse.y b/src/usr.sbin/ntpd/parse.y
index 6d507957..8bdd28f6 100644
--- a/src/usr.sbin/ntpd/parse.y
+++ b/src/usr.sbin/ntpd/parse.y
@@ -65,6 +65,7 @@ struct opts {
        int             stratum;
        int             rtable;
        char            *refstr;
+       struct sockaddr_storage local_addr;
 } opts;
 void           opts_default(void);
 
@@ -82,7 +83,7 @@ typedef struct {
 
 %token LISTEN ON CONSTRAINT CONSTRAINTS FROM
 %token SERVER SERVERS SENSOR CORRECTION RTABLE REFID STRATUM WEIGHT
-%token ERROR
+%token ERROR LOCALADDR
 %token <v.string>              STRING
 %token <v.number>              NUMBER
 %type  <v.addr>                address url
@@ -94,6 +95,7 @@ typedef struct {
 %type  <v.opts>                refid
 %type  <v.opts>                stratum
 %type  <v.opts>                weight
+%type  <v.opts>                local_addr
 %%
 
 grammar                : /* empty */
@@ -153,6 +155,7 @@ main                : LISTEN ON address listen_opts {
 
                                p = new_peer();
                                p->weight = $3.weight;
+                               p->local_addr = $3.local_addr;
                                p->addr = h;
                                p->addr_head.a = h;
                                p->addr_head.pool = 1;
@@ -190,6 +193,7 @@ main                : LISTEN ON address listen_opts {
                        }
 
                        p->weight = $3.weight;
+                       p->local_addr = $3.local_addr;
                        p->addr_head.a = p->addr;
                        p->addr_head.pool = 0;
                        p->addr_head.name = strdup($2->name);
@@ -348,6 +352,7 @@ server_opts_l       : server_opts_l server_opt
                | server_opt
                ;
 server_opt     : weight
+               | local_addr
                ;
 
 sensor_opts    :       { opts_default(); }
@@ -403,6 +408,38 @@ weight             : WEIGHT NUMBER {
                        }
                        opts.weight = $2;
                }
+               ;
+
+local_addr     : LOCALADDR STRING {
+                       if (opts.local_addr.ss_family != AF_UNSPEC) {
+                               yyerror("local-address specified more than 
once");
+                               YYERROR;
+                       }
+                       struct sockaddr_storage ss;
+                       struct sockaddr_in sin4 = {
+                               .sin_family = AF_INET,
+                               .sin_len = sizeof(struct sockaddr_in)
+                       };
+                       struct sockaddr_in6 sin6 = {
+                               .sin6_family = AF_INET6,
+                               .sin6_len = sizeof(struct sockaddr_in6)
+                       };
+                       bzero(&ss, sizeof(ss));
+                       if (inet_pton(AF_INET, $2, &sin4.sin_addr) == 1)
+                               memcpy(&ss, &sin4, sin4.sin_len);
+                       else if (inet_pton(AF_INET6, $2, &sin6.sin6_addr) == 1)
+                               memcpy(&ss, &sin6, sin6.sin6_len);
+                       else {
+                               yyerror("invalid IPv4 or IPv6 address: %s\n",
+                                   $2);
+                               free($2);
+                               YYERROR;
+                       }
+                       opts.local_addr = ss;
+                       free($2);
+               }
+               ;
+
 rtable         : RTABLE NUMBER {
                        if ($2 < 0 || $2 > RT_TABLEID_MAX) {
                                yyerror("rtable must be between 1"
@@ -421,6 +458,7 @@ opts_default(void)
        memset(&opts, 0, sizeof opts);
        opts.weight = 1;
        opts.stratum = 1;
+       opts.local_addr.ss_family = AF_UNSPEC;
 }
 
 struct keywords {
@@ -460,6 +498,7 @@ lookup(char *s)
                { "correction",         CORRECTION},
                { "from",               FROM},
                { "listen",             LISTEN},
+               { "local-address",      LOCALADDR},
                { "on",                 ON},
                { "refid",              REFID},
                { "rtable",             RTABLE},

Reply via email to