From 707f923fe37d804763ec712b0d4ef7776d7ecb87 Mon Sep 17 00:00:00 2001 From: reyk <> Date: Mon, 12 Oct 2015 06:50:08 +0000 Subject: [PATCH] Move execution of the constraints from the ntp to the parent process. This helps the ntp process to a) give a better pledge(2) and to b) keep the promise of "saving the world again... on time" by removing the delays that have been introduced by expensive constraint forks. The new design offers better privsep but introduces a few more imsgs and runs a little bit more code in the privileged parent. The privileged code is minimal, carefully checked, and does not attempt to "parse" any contents; the forked constraints instantly drop all privileges and pledge to "stdio inet". OK beck@ deraadt@ --- src/usr.sbin/ntpd/config.c | 3 +- src/usr.sbin/ntpd/constraint.c | 451 ++++++++++++++++++++++++--------- src/usr.sbin/ntpd/ntp.c | 56 ++-- src/usr.sbin/ntpd/ntpd.c | 81 ++++-- src/usr.sbin/ntpd/ntpd.h | 23 +- 5 files changed, 427 insertions(+), 187 deletions(-) diff --git a/src/usr.sbin/ntpd/config.c b/src/usr.sbin/ntpd/config.c index dc56b12b..fcf89ddf 100644 --- a/src/usr.sbin/ntpd/config.c +++ b/src/usr.sbin/ntpd/config.c @@ -1,4 +1,4 @@ -/* $OpenBSD: config.c,v 1.27 2015/07/18 00:53:44 bcook Exp $ */ +/* $OpenBSD: config.c,v 1.28 2015/10/12 06:50:08 reyk Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -203,6 +203,7 @@ new_constraint(void) if ((p = calloc(1, sizeof(struct constraint))) == NULL) fatal("new_constraint calloc"); p->id = ++constraint_maxid; + p->fd = -1; return (p); } diff --git a/src/usr.sbin/ntpd/constraint.c b/src/usr.sbin/ntpd/constraint.c index 8fc2cca8..5b8cc21a 100644 --- a/src/usr.sbin/ntpd/constraint.c +++ b/src/usr.sbin/ntpd/constraint.c @@ -1,4 +1,4 @@ -/* $OpenBSD: constraint.c,v 1.18 2015/10/09 03:50:40 deraadt Exp $ */ +/* $OpenBSD: constraint.c,v 1.19 2015/10/12 06:50:08 reyk Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -28,7 +29,6 @@ #include #include -#include #include #include #include @@ -37,8 +37,9 @@ #include #include #include +#include #include -#include +#include #include "log.h" #include "ntpd.h" @@ -50,11 +51,15 @@ struct constraint * constraint_byfd(int); struct constraint * constraint_bypid(pid_t); -int constraint_close(int); +int constraint_close(u_int32_t); void constraint_update(void); void constraint_reset(void); int constraint_cmp(const void *, const void *); +void priv_constraint_close(int, int); +void priv_constraint_child(struct constraint *, struct ntp_addr_msg *, + u_int8_t *, int[2]); + struct httpsdate * httpsdate_init(const char *, const char *, const char *, const char *, const u_int8_t *, size_t); @@ -66,8 +71,10 @@ void *httpsdate_query(const char *, const char *, const char *, char *tls_readline(struct tls *, size_t *, size_t *, struct timeval *); -extern u_int constraint_cnt; +u_int constraint_cnt; extern u_int peer_cnt; +extern struct imsgbuf *ibuf; /* priv */ +extern struct imsgbuf *ibuf_main; /* chld */ struct httpsdate { char *tls_host; @@ -134,12 +141,10 @@ constraint_addr_init(struct constraint *cstr) int constraint_query(struct constraint *cstr) { - int pipes[2]; - struct timeval rectv, xmttv; - void *ctx; - static char hname[NI_MAXHOST]; - time_t now; - struct iovec iov[2]; + time_t now; + struct ntp_addr_msg am; + struct iovec iov[3]; + int iov_cnt = 0; now = getmonotime(); @@ -168,7 +173,7 @@ constraint_query(struct constraint *cstr) /* Reset and retry */ cstr->senderrors = 0; - constraint_close(cstr->fd); + constraint_close(cstr->id); break; case STATE_REPLY_RECEIVED: default: @@ -177,61 +182,82 @@ constraint_query(struct constraint *cstr) } cstr->last = now; - if (getnameinfo((struct sockaddr *)&cstr->addr->ss, - SA_LEN((struct sockaddr *)&cstr->addr->ss), - hname, sizeof(hname), NULL, 0, - NI_NUMERICHOST) != 0) - fatalx("%s getnameinfo %s", __func__, cstr->addr_head.name); + cstr->state = STATE_QUERY_SENT; - log_debug("constraint request to %s", hname); + memset(&am, 0, sizeof(am)); + memcpy(&am.a, cstr->addr, sizeof(am.a)); + + iov[iov_cnt].iov_base = &am; + iov[iov_cnt++].iov_len = sizeof(am); + if (cstr->addr_head.name) { + am.namelen = strlen(cstr->addr_head.name) + 1; + iov[iov_cnt].iov_base = cstr->addr_head.name; + iov[iov_cnt++].iov_len = am.namelen; + } + if (cstr->addr_head.path) { + am.pathlen = strlen(cstr->addr_head.path) + 1; + iov[iov_cnt].iov_base = cstr->addr_head.path; + iov[iov_cnt++].iov_len = am.pathlen; + } + + imsg_composev(ibuf_main, IMSG_CONSTRAINT_QUERY, + cstr->id, 0, -1, iov, iov_cnt); + + return (0); +} + +void +priv_constraint_msg(u_int32_t id, u_int8_t *data, size_t len) +{ + struct ntp_addr_msg am; + struct ntp_addr *h; + struct constraint *cstr; + int pipes[2]; + + if ((cstr = constraint_byid(id)) != NULL) { + log_warnx("IMSG_CONSTRAINT_QUERY repeated for id %d", id); + return; + } + + if (len < sizeof(am)) { + log_warnx("invalid IMSG_CONSTRAINT_QUERY received"); + return; + } + memcpy(&am, data, sizeof(am)); + if (len != (sizeof(am) + am.namelen + am.pathlen)) { + log_warnx("invalid IMSG_CONSTRAINT_QUERY received"); + return; + } + /* Additional imsg data is obtained in the unpriv child */ + + if ((h = calloc(1, sizeof(*h))) == NULL) + fatal("calloc ntp_addr"); + memcpy(h, &am.a, sizeof(*h)); + h->next = NULL; + + cstr = new_constraint(); + cstr->id = id; + cstr->addr = h; + cstr->addr_head.a = h; + constraint_add(cstr); + constraint_cnt++; if (socketpair(AF_UNIX, SOCK_DGRAM, AF_UNSPEC, pipes) == -1) fatal("%s pipes", __func__); - /* Fork child handlers */ + /* + * Fork child handlers and make sure to do any sensitive work in the + * the (unprivileged) child. The parent should not do any parsing, + * certificate loading etc. + */ switch (cstr->pid = fork()) { case -1: cstr->senderrors++; close(pipes[0]); close(pipes[1]); - return (-1); + return; case 0: - setproctitle("constraint from %s", hname); - - if (pledge("stdio inet", NULL) == -1) - err(1, "pledge"); - - /* Child process */ - if (dup2(pipes[1], CONSTRAINT_PASSFD) == -1) - fatal("%s dup2 CONSTRAINT_PASSFD", __func__); - if (pipes[0] != CONSTRAINT_PASSFD) - close(pipes[0]); - if (pipes[1] != CONSTRAINT_PASSFD) - close(pipes[1]); - (void)closefrom(CONSTRAINT_PASSFD + 1); - - if (fcntl(CONSTRAINT_PASSFD, F_SETFD, FD_CLOEXEC) == -1) - fatal("%s fcntl F_SETFD", __func__); - - cstr->fd = CONSTRAINT_PASSFD; - imsg_init(&cstr->ibuf, cstr->fd); - - if ((ctx = httpsdate_query(hname, - CONSTRAINT_PORT, cstr->addr_head.name, cstr->addr_head.path, - conf->ca, conf->ca_len, &rectv, &xmttv)) == NULL) { - /* Abort with failure but without warning */ - exit(1); - } - - iov[0].iov_base = &rectv; - iov[0].iov_len = sizeof(rectv); - iov[1].iov_base = &xmttv; - iov[1].iov_len = sizeof(xmttv); - imsg_composev(&cstr->ibuf, IMSG_CONSTRAINT, 0, 0, -1, iov, 2); - imsg_flush(&cstr->ibuf); - - /* Tear down the TLS connection after sending the result */ - httpsdate_free(ctx); + priv_constraint_child(cstr, &am, data + sizeof(am), pipes); _exit(0); /* NOTREACHED */ @@ -239,56 +265,140 @@ constraint_query(struct constraint *cstr) /* Parent */ close(pipes[1]); cstr->fd = pipes[0]; - cstr->state = STATE_QUERY_SENT; imsg_init(&cstr->ibuf, cstr->fd); break; } +} + +void +priv_constraint_child(struct constraint *cstr, struct ntp_addr_msg *am, + u_int8_t *data, int pipes[2]) +{ + static char hname[NI_MAXHOST]; + struct timeval rectv, xmttv; + struct sigaction sa; + struct passwd *pw; + void *ctx; + struct iovec iov[2]; + int i; + + if (setpriority(PRIO_PROCESS, 0, 0) == -1) + log_warn("could not set priority"); + + /* Init TLS and load cert before chroot() */ + if (tls_init() == -1) + fatalx("tls_init"); + if ((conf->ca = tls_load_file(CONSTRAINT_CA, + &conf->ca_len, NULL)) == NULL) + log_warnx("constraint certificate verification turned off"); + + /* Drop privileges */ + if ((pw = getpwnam(NTPD_USER)) == NULL) + fatalx("unknown user %s", NTPD_USER); + + if (chroot(pw->pw_dir) == -1) + fatal("chroot"); + if (chdir("/") == -1) + fatal("chdir(\"/\")"); + + if (setgroups(1, &pw->pw_gid) || + setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) || + setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) + fatal("can't drop privileges"); + + /* Reset all signal handlers */ + memset(&sa, 0, sizeof(sa)); + sigemptyset(&sa.sa_mask); + sa.sa_flags = SA_RESTART; + sa.sa_handler = SIG_DFL; + for (i = 1; i < _NSIG; i++) + sigaction(i, &sa, NULL); + + if (pledge("stdio inet", NULL) == -1) + fatal("pledge"); + + /* Get name and set process title */ + if (getnameinfo((struct sockaddr *)&cstr->addr->ss, + SA_LEN((struct sockaddr *)&cstr->addr->ss), + hname, sizeof(hname), NULL, 0, + NI_NUMERICHOST) != 0) + fatalx("%s getnameinfo", __func__); + log_debug("constraint request to %s", hname); + setproctitle("constraint from %s", hname); - return (0); + /* Set file descriptors */ + if (dup2(pipes[1], CONSTRAINT_PASSFD) == -1) + fatal("%s dup2 CONSTRAINT_PASSFD", __func__); + if (pipes[0] != CONSTRAINT_PASSFD) + close(pipes[0]); + if (pipes[1] != CONSTRAINT_PASSFD) + close(pipes[1]); + (void)closefrom(CONSTRAINT_PASSFD + 1); + + if (fcntl(CONSTRAINT_PASSFD, F_SETFD, FD_CLOEXEC) == -1) + fatal("%s fcntl F_SETFD", __func__); + + cstr->fd = CONSTRAINT_PASSFD; + imsg_init(&cstr->ibuf, cstr->fd); + + /* Get remaining data from imsg in the unpriv child */ + if (am->namelen) { + if ((cstr->addr_head.name = + get_string(data, am->namelen)) == NULL) + fatalx("invalid IMSG_CONSTRAINT_QUERY name"); + data += am->namelen; + } + if (am->pathlen) { + if ((cstr->addr_head.path = + get_string(data, am->pathlen)) == NULL) + fatalx("invalid IMSG_CONSTRAINT_QUERY path"); + } + + /* Run! */ + if ((ctx = httpsdate_query(hname, + CONSTRAINT_PORT, cstr->addr_head.name, cstr->addr_head.path, + conf->ca, conf->ca_len, &rectv, &xmttv)) == NULL) { + /* Abort with failure but without warning */ + exit(1); + } + + iov[0].iov_base = &rectv; + iov[0].iov_len = sizeof(rectv); + iov[1].iov_base = &xmttv; + iov[1].iov_len = sizeof(xmttv); + imsg_composev(&cstr->ibuf, + IMSG_CONSTRAINT_RESULT, 0, 0, -1, iov, 2); + imsg_flush(&cstr->ibuf); + + /* Tear down the TLS connection after sending the result */ + httpsdate_free(ctx); } void -constraint_check_child(void) +priv_constraint_check_child(pid_t pid, int status) { struct constraint *cstr; - int status; int fail, sig; - pid_t pid; - - do { - pid = waitpid(WAIT_ANY, &status, WNOHANG); - if (pid <= 0) - continue; - fail = sig = 0; - if (WIFSIGNALED(status)) { - sig = WTERMSIG(status); - } else if (WIFEXITED(status)) { - if (WEXITSTATUS(status) != 0) - fail = 1; - } else - fatalx("unexpected cause of SIGCHLD"); - - if ((cstr = constraint_bypid(pid)) != NULL) { - if (sig) - fatalx("constraint %s, signal %d", - log_sockaddr((struct sockaddr *) - &cstr->addr->ss), sig); - if (fail) { - log_debug("no constraint reply from %s" - " received in time, next query %ds", - log_sockaddr((struct sockaddr *) - &cstr->addr->ss), CONSTRAINT_SCAN_INTERVAL); - } - - if (fail || cstr->state < STATE_QUERY_SENT) { - cstr->senderrors++; - constraint_close(cstr->fd); - } - } - } while (pid > 0 || (pid == -1 && errno == EINTR)); + fail = sig = 0; + if (WIFSIGNALED(status)) { + sig = WTERMSIG(status); + } else if (WIFEXITED(status)) { + if (WEXITSTATUS(status) != 0) + fail = 1; + } else + fatalx("unexpected cause of SIGCHLD"); + + if ((cstr = constraint_bypid(pid)) != NULL) { + if (sig) + fatalx("constraint %s, signal %d", + log_sockaddr((struct sockaddr *) + &cstr->addr->ss), sig); + + priv_constraint_close(cstr->fd, fail); + } } struct constraint * @@ -331,18 +441,15 @@ constraint_bypid(pid_t pid) } int -constraint_close(int fd) +constraint_close(u_int32_t id) { struct constraint *cstr; - if ((cstr = constraint_byfd(fd)) == NULL) { - log_warn("%s: fd %d: not found", __func__, fd); + if ((cstr = constraint_byid(id)) == NULL) { + log_warn("%s: id %d: not found", __func__, id); return (0); } - msgbuf_clear(&cstr->ibuf.w); - close(cstr->fd); - cstr->fd = -1; cstr->last = getmonotime(); if (cstr->addr == NULL || (cstr->addr = cstr->addr->next) == NULL) { @@ -360,6 +467,25 @@ constraint_close(int fd) return (constraint_init(cstr)); } +void +priv_constraint_close(int fd, int fail) +{ + struct constraint *cstr; + u_int32_t id; + + if ((cstr = constraint_byfd(fd)) == NULL) { + log_warn("%s: fd %d: not found", __func__, fd); + return; + } + + id = cstr->id; + constraint_remove(cstr); + constraint_cnt--; + + imsg_compose(ibuf, IMSG_CONSTRAINT_CLOSE, id, 0, -1, + &fail, sizeof(fail)); +} + void constraint_add(struct constraint *cstr) { @@ -370,19 +496,31 @@ void constraint_remove(struct constraint *cstr) { TAILQ_REMOVE(&conf->constraints, cstr, entry); + + msgbuf_clear(&cstr->ibuf.w); + if (cstr->fd != -1) + close(cstr->fd); free(cstr->addr_head.name); free(cstr->addr_head.path); free(cstr); } +void +constraint_purge(void) +{ + struct constraint *cstr, *ncstr; + + TAILQ_FOREACH_SAFE(cstr, &conf->constraints, entry, ncstr) + constraint_remove(cstr); +} + int -constraint_dispatch_msg(struct pollfd *pfd) +priv_constraint_dispatch(struct pollfd *pfd) { struct imsg imsg; struct constraint *cstr; ssize_t n; struct timeval tv[2]; - double offset; if ((cstr = constraint_byfd(pfd->fd)) == NULL) return (0); @@ -391,37 +529,26 @@ constraint_dispatch_msg(struct pollfd *pfd) return (0); if ((n = imsg_read(&cstr->ibuf)) == -1 || n == 0) { - constraint_close(pfd->fd); + priv_constraint_close(pfd->fd, 1); return (1); } for (;;) { if ((n = imsg_get(&cstr->ibuf, &imsg)) == -1) { - constraint_close(pfd->fd); + priv_constraint_close(pfd->fd, 1); return (1); } if (n == 0) break; switch (imsg.hdr.type) { - case IMSG_CONSTRAINT: + case IMSG_CONSTRAINT_RESULT: if (imsg.hdr.len != IMSG_HEADER_SIZE + sizeof(tv)) fatalx("invalid IMSG_CONSTRAINT received"); - memcpy(tv, imsg.data, sizeof(tv)); - - offset = gettime_from_timeval(&tv[0]) - - gettime_from_timeval(&tv[1]); - - log_info("constraint reply from %s: offset %f", - log_sockaddr((struct sockaddr *)&cstr->addr->ss), - offset); - - cstr->state = STATE_REPLY_RECEIVED; - cstr->last = getmonotime(); - cstr->constraint = tv[0].tv_sec; - - constraint_update(); + /* forward imsg to ntp child, don't parse it here */ + imsg_compose(ibuf, imsg.hdr.type, + cstr->id, 0, -1, imsg.data, sizeof(tv)); break; default: break; @@ -433,7 +560,71 @@ constraint_dispatch_msg(struct pollfd *pfd) } void -constraint_dns(u_int32_t id, u_int8_t *data, size_t len) +constraint_msg_result(u_int32_t id, u_int8_t *data, size_t len) +{ + struct constraint *cstr; + struct timeval tv[2]; + double offset; + + if ((cstr = constraint_byid(id)) == NULL) { + log_warnx("IMSG_CONSTRAINT_CLOSE with invalid constraint id"); + return; + } + + if (len != sizeof(tv)) { + log_warnx("invalid IMSG_CONSTRAINT received"); + return; + } + + memcpy(tv, data, len); + + offset = gettime_from_timeval(&tv[0]) - + gettime_from_timeval(&tv[1]); + + log_info("constraint reply from %s: offset %f", + log_sockaddr((struct sockaddr *)&cstr->addr->ss), + offset); + + cstr->state = STATE_REPLY_RECEIVED; + cstr->last = getmonotime(); + cstr->constraint = tv[0].tv_sec; + + constraint_update(); +} + +void +constraint_msg_close(u_int32_t id, u_int8_t *data, size_t len) +{ + struct constraint *cstr; + int fail; + + if ((cstr = constraint_byid(id)) == NULL) { + log_warnx("IMSG_CONSTRAINT_CLOSE with invalid constraint id"); + return; + } + + if (len != sizeof(int)) { + log_warnx("invalid IMSG_CONSTRAINT_CLOSE received"); + return; + } + + memcpy(&fail, data, len); + + if (fail) { + log_debug("no constraint reply from %s" + " received in time, next query %ds", + log_sockaddr((struct sockaddr *) + &cstr->addr->ss), CONSTRAINT_SCAN_INTERVAL); + } + + if (fail || cstr->state < STATE_QUERY_SENT) { + cstr->senderrors++; + constraint_close(cstr->id); + } +} + +void +constraint_msg_dns(u_int32_t id, u_int8_t *data, size_t len) { struct constraint *cstr, *ncstr = NULL; u_int8_t *p; @@ -542,7 +733,7 @@ constraint_reset(void) TAILQ_FOREACH(cstr, &conf->constraints, entry) { if (cstr->state == STATE_QUERY_SENT) continue; - constraint_close(cstr->fd); + constraint_close(cstr->id); } conf->constraint_errors = 0; } @@ -583,9 +774,6 @@ httpsdate_init(const char *hname, const char *port, const char *name, { struct httpsdate *httpsdate = NULL; - if (tls_init() == -1) - return (NULL); - if ((httpsdate = calloc(1, sizeof(*httpsdate))) == NULL) goto fail; @@ -785,3 +973,20 @@ tls_readline(struct tls *tls, size_t *lenp, size_t *maxlength, fatal("gettimeofday"); return (buf); } + +char * +get_string(u_int8_t *ptr, size_t len) +{ + size_t i; + char *str; + + for (i = 0; i < len; i++) + if (!(isprint(ptr[i]) || isspace(ptr[i]))) + break; + + if ((str = calloc(1, i + 1)) == NULL) + return (NULL); + memcpy(str, ptr, i); + + return (str); +} diff --git a/src/usr.sbin/ntpd/ntp.c b/src/usr.sbin/ntpd/ntp.c index f7668e08..da6fc354 100644 --- a/src/usr.sbin/ntpd/ntp.c +++ b/src/usr.sbin/ntpd/ntp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ntp.c,v 1.136 2015/10/09 03:54:53 deraadt Exp $ */ +/* $OpenBSD: ntp.c,v 1.137 2015/10/12 06:50:08 reyk Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -31,7 +31,6 @@ #include #include #include -#include #include "ntpd.h" @@ -42,14 +41,13 @@ volatile sig_atomic_t ntp_quit = 0; volatile sig_atomic_t ntp_report = 0; -volatile sig_atomic_t ntp_sigchld = 0; struct imsgbuf *ibuf_main; struct imsgbuf *ibuf_dns; struct ntpd_conf *conf; struct ctl_conns ctl_conns; u_int peer_cnt; u_int sensors_cnt; -u_int constraint_cnt; +extern u_int constraint_cnt; time_t lastreport; void ntp_sighdlr(int); @@ -70,9 +68,6 @@ ntp_sighdlr(int sig) case SIGINFO: ntp_report = 1; break; - case SIGCHLD: - ntp_sigchld = 1; - break; } } @@ -110,13 +105,6 @@ ntp_main(int pipe_prnt[2], int fd_ctl, struct ntpd_conf *nconf, return (pid); } - tls_init(); - - /* Verification will be turned off if CA is not found */ - if ((conf->ca = tls_load_file(CONSTRAINT_CA, - &conf->ca_len, NULL)) == NULL) - log_warnx("constraint certificate verification turned off"); - /* in this case the parent didn't init logging and didn't daemonize */ if (nconf->settime && !nconf->debug) { log_init(nconf->debug); @@ -166,18 +154,8 @@ ntp_main(int pipe_prnt[2], int fd_ctl, struct ntpd_conf *nconf, endservent(); - /* - * XXX - * Unfortunately, the "contraint" processes are forked - * below the "ntp engine". Hence the ntp engine needs - * to be able to fork -> "proc", and the "constraint" - * process will want to open sockets -> "inet". - * - * For many reasons, including fork/exec cost, it would - * be better if constraints were forked from the master - * process, which would then tell the ntp engine. - */ - if (pledge("stdio inet proc", NULL) == -1) + /* The ntp process will want to open NTP client sockets -> "inet" */ + if (pledge("stdio inet", NULL) == -1) err(1, "pledge"); signal(SIGTERM, ntp_sighdlr); @@ -185,7 +163,7 @@ ntp_main(int pipe_prnt[2], int fd_ctl, struct ntpd_conf *nconf, signal(SIGINFO, ntp_sighdlr); signal(SIGPIPE, SIG_IGN); signal(SIGHUP, SIG_IGN); - signal(SIGCHLD, ntp_sighdlr); + signal(SIGCHLD, SIG_DFL); if ((ibuf_main = malloc(sizeof(struct imsgbuf))) == NULL) fatal(NULL); @@ -248,7 +226,7 @@ ntp_main(int pipe_prnt[2], int fd_ctl, struct ntpd_conf *nconf, } new_cnt = PFD_MAX + - peer_cnt + listener_cnt + ctl_cnt + constraint_cnt; + peer_cnt + listener_cnt + ctl_cnt; if (new_cnt > pfd_elms) { if ((newp = reallocarray(pfd, new_cnt, sizeof(*pfd))) == NULL) { @@ -369,9 +347,6 @@ ntp_main(int pipe_prnt[2], int fd_ctl, struct ntpd_conf *nconf, TAILQ_FOREACH(cstr, &conf->constraints, entry) { if (constraint_query(cstr) == -1) continue; - pfd[i].fd = cstr->fd; - pfd[i].events = POLLIN; - i++; } now = getmonotime(); @@ -439,10 +414,6 @@ ntp_main(int pipe_prnt[2], int fd_ctl, struct ntpd_conf *nconf, nfds -= control_dispatch_msg(&pfd[j], &ctl_cnt); } - for (; nfds > 0 && j < i; j++) { - nfds -= constraint_dispatch_msg(&pfd[j]); - } - for (s = TAILQ_FIRST(&conf->ntp_sensors); s != NULL; s = next_s) { next_s = TAILQ_NEXT(s, entry); @@ -451,11 +422,6 @@ ntp_main(int pipe_prnt[2], int fd_ctl, struct ntpd_conf *nconf, } report_peers(ntp_report); ntp_report = 0; - - if (ntp_sigchld) { - constraint_check_child(); - ntp_sigchld = 0; - } } msgbuf_write(&ibuf_main->w); @@ -501,6 +467,14 @@ ntp_dispatch_imsg(void) conf->status.synced = 0; } break; + case IMSG_CONSTRAINT_RESULT: + constraint_msg_result(imsg.hdr.peerid, + imsg.data, imsg.hdr.len - IMSG_HEADER_SIZE); + break; + case IMSG_CONSTRAINT_CLOSE: + constraint_msg_close(imsg.hdr.peerid, + imsg.data, imsg.hdr.len - IMSG_HEADER_SIZE); + break; default: break; } @@ -590,7 +564,7 @@ ntp_dispatch_imsg_dns(void) client_addr_init(peer); break; case IMSG_CONSTRAINT_DNS: - constraint_dns(imsg.hdr.peerid, + constraint_msg_dns(imsg.hdr.peerid, imsg.data, imsg.hdr.len - IMSG_HEADER_SIZE); break; default: diff --git a/src/usr.sbin/ntpd/ntpd.c b/src/usr.sbin/ntpd/ntpd.c index 6622de2b..cc9b3077 100644 --- a/src/usr.sbin/ntpd/ntpd.c +++ b/src/usr.sbin/ntpd/ntpd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ntpd.c,v 1.96 2015/10/09 01:37:09 deraadt Exp $ */ +/* $OpenBSD: ntpd.c,v 1.97 2015/10/12 06:50:08 reyk Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -60,6 +60,8 @@ volatile sig_atomic_t sigchld = 0; struct imsgbuf *ibuf; int timeout = INFTIM; +extern u_int constraint_cnt; + const char *showopt; static const char *ctl_showopt_list[] = { @@ -99,18 +101,22 @@ usage(void) #define POLL_MAX 8 #define PFD_PIPE 0 +#define PFD_MAX 1 int main(int argc, char *argv[]) { struct ntpd_conf lconf; - struct pollfd pfd[POLL_MAX]; + struct pollfd *pfd = NULL; pid_t chld_pid = 0, pid; const char *conffile; - int fd_ctl, ch, nfds; + int fd_ctl, ch, nfds, i, j; int pipe_chld[2]; struct passwd *pw; extern char *__progname; + u_int pfd_elms = 0, new_cnt; + struct constraint *cstr; + void *newp; if (strcmp(__progname, "ntpctl") == 0) { ctl_main(argc, argv); @@ -201,18 +207,42 @@ main(int argc, char *argv[]) signal(SIGHUP, sighdlr); close(pipe_chld[1]); + constraint_purge(); if ((ibuf = malloc(sizeof(struct imsgbuf))) == NULL) fatal(NULL); imsg_init(ibuf, pipe_chld[0]); + constraint_cnt = 0; + while (quit == 0) { + new_cnt = PFD_MAX + constraint_cnt; + if (new_cnt > pfd_elms) { + if ((newp = reallocarray(pfd, new_cnt, + sizeof(*pfd))) == NULL) { + /* panic for now */ + log_warn("could not resize pfd from %u -> " + "%u entries", pfd_elms, new_cnt); + fatalx("exiting"); + } + pfd = newp; + pfd_elms = new_cnt; + } + + memset(pfd, 0, sizeof(*pfd) * pfd_elms); pfd[PFD_PIPE].fd = ibuf->fd; pfd[PFD_PIPE].events = POLLIN; if (ibuf->w.queued) pfd[PFD_PIPE].events |= POLLOUT; - if ((nfds = poll(pfd, 1, timeout)) == -1) + i = PFD_MAX; + TAILQ_FOREACH(cstr, &conf->constraints, entry) { + pfd[i].fd = cstr->fd; + pfd[i].events = POLLIN; + i++; + } + + if ((nfds = poll(pfd, i, timeout)) == -1) if (errno != EINTR) { log_warn("poll error"); quit = 1; @@ -241,6 +271,10 @@ main(int argc, char *argv[]) quit = 1; } + for (j = PFD_MAX; nfds > 0 && j < i; j++) { + nfds -= priv_constraint_dispatch(&pfd[j]); + } + if (sigchld) { if (check_child(chld_pid, "child")) { quit = 1; @@ -269,24 +303,33 @@ main(int argc, char *argv[]) } int -check_child(pid_t pid, const char *pname) +check_child(pid_t chld_pid, const char *pname) { int status, sig; char *signame; + pid_t pid; - if (waitpid(pid, &status, WNOHANG) > 0) { - if (WIFEXITED(status)) { - log_warnx("Lost child: %s exited", pname); - return (1); - } - if (WIFSIGNALED(status)) { - sig = WTERMSIG(status); - signame = strsignal(sig) ? strsignal(sig) : "unknown"; - log_warnx("Lost child: %s terminated; signal %d (%s)", - pname, sig, signame); - return (1); + do { + pid = waitpid(WAIT_ANY, &status, WNOHANG); + if (pid <= 0) { + continue; + } else if (pid == chld_pid) { + if (WIFEXITED(status)) { + log_warnx("Lost child: %s exited", pname); + return (1); + } + if (WIFSIGNALED(status)) { + sig = WTERMSIG(status); + signame = strsignal(sig) ? + strsignal(sig) : "unknown"; + log_warnx("Lost child: %s terminated; " + "signal %d (%s)", pname, sig, signame); + return (1); + } + } else { + priv_constraint_check_child(pid, status); } - } + } while (pid > 0 || (pid == -1 && errno == EINTR)); return (0); } @@ -343,6 +386,10 @@ dispatch_imsg(struct ntpd_conf *lconf) lconf->settime = 0; timeout = INFTIM; break; + case IMSG_CONSTRAINT_QUERY: + priv_constraint_msg(imsg.hdr.peerid, + imsg.data, imsg.hdr.len - IMSG_HEADER_SIZE); + break; default: break; } diff --git a/src/usr.sbin/ntpd/ntpd.h b/src/usr.sbin/ntpd/ntpd.h index 78e16cd9..9d008183 100644 --- a/src/usr.sbin/ntpd/ntpd.h +++ b/src/usr.sbin/ntpd/ntpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ntpd.h,v 1.121 2015/05/20 13:32:39 reyk Exp $ */ +/* $OpenBSD: ntpd.h,v 1.122 2015/10/12 06:50:08 reyk Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -114,6 +114,12 @@ struct ntp_addr_wrap { u_int8_t pool; }; +struct ntp_addr_msg { + struct ntp_addr a; + size_t namelen; + size_t pathlen; +}; + struct ntp_status { double rootdelay; double rootdispersion; @@ -279,8 +285,10 @@ enum imsg_type { IMSG_ADJFREQ, IMSG_SETTIME, IMSG_HOST_DNS, - IMSG_CONSTRAINT, IMSG_CONSTRAINT_DNS, + IMSG_CONSTRAINT_QUERY, + IMSG_CONSTRAINT_RESULT, + IMSG_CONSTRAINT_CLOSE, IMSG_CTL_SHOW_STATUS, IMSG_CTL_SHOW_PEERS, IMSG_CTL_SHOW_PEERS_END, @@ -343,12 +351,17 @@ void set_next(struct ntp_peer *, time_t); /* constraint.c */ void constraint_add(struct constraint *); void constraint_remove(struct constraint *); +void constraint_purge(void); int constraint_init(struct constraint *); int constraint_query(struct constraint *); -int constraint_dispatch_msg(struct pollfd *); -void constraint_check_child(void); int constraint_check(double); -void constraint_dns(u_int32_t, u_int8_t *, size_t); +void constraint_msg_dns(u_int32_t, u_int8_t *, size_t); +void constraint_msg_result(u_int32_t, u_int8_t *, size_t); +void constraint_msg_close(u_int32_t, u_int8_t *, size_t); +void priv_constraint_msg(u_int32_t, u_int8_t *, size_t); +int priv_constraint_dispatch(struct pollfd *); +void priv_constraint_check_child(pid_t, int); +char *get_string(u_int8_t *, size_t); /* util.c */ double gettime_corrected(void);