[chrony-dev] Re: [PATCH] Privilege Separation - Version 2 - Add helper process

[Bryan Christianson <bryan@xxxxxxxxxxxxx>]

I removed all the stuff to actually compile privops, figuring it would show up when the priv helper is used with a driver.

I have a couple of queries about the next step

1. PRIVOPS_ALL is true when any of the function defines is added to config, I think PRV_Initialise() should be called
from SYS_DropRoot() (guarded by PRIVOPS_ALL) - or would you prefer a call from each driver SYS_xxxxx_DropRoot() ?

2. Not sure where (or even if) PRV_Finalise() should be called.

--
Bryan



> On 16/11/2015, at 12:53 PM, Bryan Christianson <bryan@xxxxxxxxxxxxx> wrote:
> 
> Privileged helper that will perform adjtime(), settimeofday(), bind() on
> behalf of chronyd when running as non-root user.
> 
> Changes since version 1
> 1. Fix variable/struct/function names as suggested
> 2. Avoid calling LOG_FATAL from the helper
> 3. Added #define PRIVOPS_xxxxx for each privileged function
> 4. Transfer fd request and data in single I/O
> 5. helper_main() restructured as suggested.
> 6. remove privops.o from Makefile.in
> 
> Tested compilation and execution on MacOS X
> ---
> logging.h |   1 +
> privops.c | 511 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> privops.h |  59 ++++++++
> 3 files changed, 571 insertions(+)
> create mode 100644 privops.c
> create mode 100644 privops.h
> 
> diff --git a/logging.h b/logging.h
> index b480c42..8fc8640 100644
> --- a/logging.h
> +++ b/logging.h
> @@ -100,6 +100,7 @@ typedef enum {
>   LOGF_Keys,
>   LOGF_Logging,
>   LOGF_Nameserv,
> +  LOGF_PrivOps,
>   LOGF_Rtc,
>   LOGF_Regress,
>   LOGF_Sys,
> diff --git a/privops.c b/privops.c
> new file mode 100644
> index 0000000..5b49b53
> --- /dev/null
> +++ b/privops.c
> @@ -0,0 +1,511 @@
> +/*
> +  chronyd/chronyc - Programs for keeping computer clocks accurate.
> +
> + **********************************************************************
> + * Copyright (C) Bryan Christianson 2015
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + **********************************************************************
> +
> +  =======================================================================
> +
> +  Perform privileged operations over a unix socket to a privileged fork.
> +
> +*/
> +#include "config.h"
> +
> +#include "sysincl.h"
> +#include "conf.h"
> +#include "logging.h"
> +#include "privops.h"
> +#include "util.h"
> +
> +#if PRIVOPS_ANY
> +
> +#define op_ADJTIME        1024
> +#define op_SETTIMEOFDAY   1025
> +#define op_BINDSOCKET     1026
> +
> +union sockaddr_in46 {
> +  struct sockaddr_in in4;
> +#ifdef FEAT_IPV6
> +  struct sockaddr_in6 in6;
> +#endif
> +  struct sockaddr u;
> +};
> +
> +/* daemon request structs */
> +
> +typedef struct {
> +  struct timeval tv;
> +} ReqAdjustTime;
> +
> +typedef struct {
> +  struct timeval tv;
> +} ReqSetTime;
> +
> +typedef struct {
> +  int sock;
> +  socklen_t sa_len;
> +  union sockaddr_in46 sa;
> +} ReqBindSocket;
> +
> +typedef struct {
> +  int op;
> +  union {
> +    ReqAdjustTime adj_tv;
> +    ReqSetTime settime_tv;
> +    ReqBindSocket bind_sock;
> +  } u;
> +} PrvRequest;
> +
> +/* helper response structs */
> +
> +typedef struct {
> +  struct timeval tv;
> +} ResAdjustTime;
> +
> +typedef struct {
> +  char msg[256];
> +} ResFatalMsg;
> +
> +typedef struct {
> +  int fatal_error;
> +  int rc;
> +  int res_errno;
> +  union {
> +    ResFatalMsg fatal_msg;
> +    ResAdjustTime adj_tv;
> +  } u;
> +} PrvResponse;
> +
> +static int helper_fd = -1;
> +
> +static inline int
> +have_helper(void)
> +{
> +  return helper_fd >= 0;
> +}
> +
> +/* ======================================================================= */
> +
> +/* send an open file descriptor plus data over a unix socket */
> +
> +static int
> +send_with_fd(int fd, int fd_to_send, const void *data, int data_len)
> +{
> +  struct msghdr msg;
> +  struct iovec iov;
> +  int cmsglen;
> +  struct cmsghdr *cmsg;
> +  void *ptr_send_fd;
> +  char cmsgbuf[256];
> +
> +  /* data is transferred vio iov */
> +  iov.iov_base = (void *)data;
> +  iov.iov_len = data_len;
> +
> +  msg.msg_name = NULL;
> +  msg.msg_namelen = 0;
> +  msg.msg_iov = &iov;
> +  msg.msg_iovlen = 1;
> +  msg.msg_control = cmsgbuf;
> +  msg.msg_controllen = sizeof(cmsgbuf);
> +  msg.msg_flags = 0;
> +
> +  cmsglen = 0;
> +
> +  cmsg = CMSG_FIRSTHDR(&msg);
> +  memset(cmsg, 0, CMSG_SPACE(sizeof(int)));
> +  cmsglen += CMSG_SPACE(sizeof(int));
> +
> +  cmsg->cmsg_level = SOL_SOCKET;
> +  cmsg->cmsg_type = SCM_RIGHTS;
> +  cmsg->cmsg_len = CMSG_LEN(sizeof(int));
> +
> +  ptr_send_fd = CMSG_DATA(cmsg);
> +  memcpy(ptr_send_fd, &fd_to_send, sizeof(int));
> +
> +  msg.msg_controllen = cmsglen;
> +
> +  if (sendmsg(fd, &msg, 0) != data_len)
> +    return -1;
> +
> +  return 0;
> +}
> +
> +/* ======================================================================= */
> +/* receive data plus optional file descriptor over a unix socket */
> +
> +static int
> +receive_with_fd(int fd, void *data, int data_size, int *fd_recvd)
> +{
> +  int data_bytes;
> +  struct msghdr msg;
> +  struct cmsghdr *cmsg;
> +  struct iovec iov;
> +  char cmsgbuf[256];
> +
> +  *fd_recvd = -1;
> +
> +  iov.iov_base = data;
> +  iov.iov_len = data_size;
> +
> +  msg.msg_name = NULL;
> +  msg.msg_namelen = 0;
> +  msg.msg_iov = &iov;
> +  msg.msg_iovlen = 1;
> +  msg.msg_control = (void *)cmsgbuf;
> +  msg.msg_controllen = sizeof(cmsgbuf);
> +  msg.msg_flags = MSG_WAITALL;
> +
> +  /* read the data */
> +  data_bytes = recvmsg(fd, &msg, 0);
> +  if (data_bytes <= 0)
> +    return -1;
> +
> +  /* extract transferred descriptor (if any) */
> +  for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
> +    if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS)
> +      memcpy(fd_recvd, CMSG_DATA(cmsg), sizeof(int));
> +  }
> +
> +  return data_bytes;
> +}
> +
> +/* ======================================================================= */
> +
> +/* HELPER - prepare fatal error for daemon */
> +static void
> +res_fatal(PrvResponse *res, const char *fmt, ...)
> +{
> +  va_list ap;
> +  va_start(ap, fmt);
> +
> +  memset(res, 0, sizeof(PrvResponse));
> +  res->fatal_error = 1;
> +  vsnprintf(res->u.fatal_msg.msg, sizeof(ResFatalMsg), fmt, ap);
> +
> +  va_end(ap);
> +}
> +
> +/* ======================================================================= */
> +
> +/* HELPER - send response to the fd */
> +
> +static inline int
> +send_response(int fd, const PrvResponse *res)
> +{
> +  if (write(fd, res, sizeof(PrvResponse)) != sizeof(PrvResponse))
> +    return -1;
> +
> +  return 0;
> +}
> +
> +/* ======================================================================= */
> +
> +/* HELPER - perform adjtime() */
> +
> +static void
> +do_adjtime(const PrvRequest *req, PrvResponse *res)
> +{
> +  const struct timeval *delta = &req->u.adj_tv.tv;
> +  struct timeval olddelta;
> +
> +  memset(res, 0, sizeof(PrvResponse));
> +  res->rc = adjtime(delta, &olddelta);
> +  if (res->rc != 0)
> +    res->res_errno = errno;
> +  else
> +    res->u.adj_tv.tv = olddelta;
> +}
> +
> +/* ======================================================================= */
> +
> +/* HELPER - perform settimeofday() */
> +
> +static void
> +do_settimeofday(const PrvRequest *req, PrvResponse *res)
> +{
> +  const struct timeval *tv = &req->u.adj_tv.tv;
> +  memset(res, 0, sizeof(PrvResponse));
> +
> +  res->rc = settimeofday(tv, NULL);
> +  if (res->rc != 0)
> +    res->res_errno = errno;
> +}
> +
> +/* ======================================================================= */
> +
> +/* HELPER - bind port to a socket */
> +static void
> +do_bindsocket(PrvRequest *req, PrvResponse *res)
> +{
> +  unsigned short port;
> +  IPAddr ip;
> +  int sock_fd;
> +  struct sockaddr *sa;
> +  socklen_t sa_len;
> +
> +  sa = &req->u.bind_sock.sa.u;
> +  sa_len = req->u.bind_sock.sa_len;
> +  sock_fd = req->u.bind_sock.sock;
> +
> +  UTI_SockaddrToIPAndPort(sa, &ip, &port);
> +  if (port != 0 && port != CNF_GetNTPPort()) {
> +    res_fatal(res, "PRV_BindSocket, invalid port: %d", port);
> +    return;
> +  }
> +
> +  memset(res, 0, sizeof(PrvResponse));
> +  res->rc = bind(sock_fd, sa, sa_len);
> +  if (res->rc != 0)
> +    res->res_errno = errno;
> +
> +  /* sock is still open on daemon side  but we're done with it in the helper */
> +  close(sock_fd);
> +}
> +
> +/* ======================================================================= */
> +
> +/* HELPER - main loop - action requests from the daemon */
> +
> +static void
> +helper_main(int fd)
> +{
> +  PrvRequest req;
> +  PrvResponse res;
> +  int recvd_fd;
> +
> +  while (1) {
> +    if (receive_with_fd(fd, &req, sizeof(PrvRequest), &recvd_fd) < sizeof(PrvRequest))
> +      /* read error or closed input - we cannot recover - give up */
> +      return;
> +
> +    switch (req.op) {
> +    default:
> +      res_fatal(&res, "Unexpected operator : %d", req.op);
> +      break;
> +
> +    case op_ADJTIME:
> +      do_adjtime(&req, &res);
> +      break;
> +
> +    case op_SETTIMEOFDAY:
> +      do_settimeofday(&req, &res);
> +      break;
> +
> +    case op_BINDSOCKET:
> +      req.u.bind_sock.sock = recvd_fd; /* use transferred descriptor */
> +      do_bindsocket(&req, &res);
> +      break;
> +    }
> +    send_response(fd, &res);
> +  }
> +}
> +
> +/* ======================================================================= */
> +
> +/* DAEMON - read helper response from fd */
> +
> +static int
> +read_response(int fd, PrvResponse *res)
> +{
> +  if (read(fd, res, sizeof(PrvResponse)) < sizeof(PrvResponse))
> +    LOG_FATAL(LOGF_PrivOps, "Error reading from helper fd : %s", strerror(errno));
> +
> +  if (res->fatal_error)
> +    LOG_FATAL(LOGF_PrivOps, "Fatal error in helper: %s", res->u.fatal_msg.msg);
> +
> +  /* if operation failed in the helper, set errno so daemon can print log message */
> +  if (res->rc != 0) {
> +    errno = res->res_errno;
> +    return -1;
> +  }
> +  return 0;
> +}
> +
> +/* ======================================================================= */
> +
> +/* DAEMON - send daemon request to fd and wait for response */
> +
> +static int
> +send_to_helper(int fd, const PrvRequest *req, PrvResponse *res)
> +{
> +  int error;
> +
> +  if (req->op == op_BINDSOCKET)
> +    error = send_with_fd(fd, req->u.bind_sock.sock, req, sizeof(PrvRequest)) != 0;
> +  else
> +    error = write(fd, req, sizeof(PrvRequest)) != sizeof(PrvRequest);
> +
> +  if (error)
> +      LOG_FATAL(LOGF_PrivOps, "Error writing to helper fd : %s", strerror(errno));
> +
> +  return read_response(fd, res);
> +}
> +
> +/* ======================================================================= */
> +
> +/* DAEMON - bind socket to reserved port */
> +
> +#if defined(PRIVOPS_BINDSOCKET)
> +int
> +PRV_BindSocket(int sock, struct sockaddr *address, socklen_t address_len)
> +{
> +  PrvRequest req;
> +  PrvResponse res;
> +  IPAddr ip;
> +  unsigned short port;
> +
> +  if (address->sa_len > address_len)
> +    LOG_FATAL(LOGF_PrivOps, "PRV_BindSocket, incorrect address length: %d", (int)address_len);
> +
> +  UTI_SockaddrToIPAndPort(address, &ip, &port);
> +  if (port != 0 && port != CNF_GetNTPPort())
> +    LOG_FATAL(LOGF_PrivOps, "PRV_BindSocket, invalid port: %d", port);
> +
> +  if (!have_helper())
> +    /* dont use the helper if not required */
> +    return bind(sock, address, address_len);
> +
> +  req.op = op_BINDSOCKET;
> +  req.u.bind_sock.sock = sock;
> +  req.u.bind_sock.sa_len = address_len;
> +  memcpy(&req.u.bind_sock.sa.u, address, address_len);
> +
> +  if (send_to_helper(helper_fd, &req, &res) != 0)
> +    return -1;
> +
> +  return 0;
> +}
> +#endif
> +
> +/* ======================================================================= */
> +
> +/* DAEMON - request adjtime() */
> +
> +#if defined(PRIVOPS_ADJUSTTIME)
> +int
> +PRV_AdjustTime(const struct timeval *delta, struct timeval *olddelta)
> +{
> +  PrvRequest req;
> +  PrvResponse res;
> +
> +  if (!have_helper() || delta == NULL)
> +    /* helper is not running or read adjustment call */
> +    return adjtime(delta, olddelta);
> +
> +  req.op = op_ADJTIME;
> +  req.u.adj_tv.tv = *delta;
> +
> +  if (send_to_helper(helper_fd, &req, &res) != 0)
> +    return -1;
> +
> +  if (olddelta)
> +    *olddelta = res.u.adj_tv.tv;
> +
> +  return 0;
> +}
> +#endif
> +
> +/* ======================================================================= */
> +
> +/* DAEMON - request settimeofday() */
> +
> +#if defined(PRIVOPS_SETTIME)
> +int
> +PRV_SetTime(const struct timeval *tp, const struct timezone *tzp)
> +{
> +  PrvRequest req;
> +  PrvResponse res;
> +
> +  /* only support setting the time */
> +  assert(tp != NULL);
> +  assert(tzp == NULL);
> +
> +  if (!have_helper()) {
> +    /* helper is not running - this will fail if not privileged */
> +    return settimeofday(tp, NULL);
> +  }
> +
> +  req.op = op_SETTIMEOFDAY;
> +  req.u.settime_tv.tv = *tp;
> +
> +  return send_to_helper(helper_fd, &req, &res);
> +}
> +#endif
> +
> +/* ======================================================================= */
> +
> +/* DAEMON - setup socket(s) then fork to run the helper */
> +/* must be called before privileges are dropped */
> +
> +void
> +PRV_Initialise(void)
> +{
> +  pid_t pid;
> +  int sock_pair[2];
> +
> +  if (have_helper())
> +    LOG_FATAL(LOGF_PrivOps, "PRV_start, helper is already running");
> +
> +  if (socketpair(AF_UNIX, SOCK_STREAM, 0, sock_pair) != 0)
> +    LOG_FATAL(LOGF_PrivOps, "PRV_start, socketpair() failed : %s", strerror(errno));
> +
> +  UTI_FdSetCloexec(sock_pair[0]);
> +  UTI_FdSetCloexec(sock_pair[1]);
> +
> +  pid = fork();
> +  if (pid == -1)
> +    LOG_FATAL(LOGF_PrivOps, "PRV_start, fork failed : %s", strerror(errno));
> +
> +  if (pid == 0) {
> +    /* child process */
> +    int fd;
> +    close(sock_pair[0]);
> +
> +    /* close other descriptors inherited from the parent process */
> +    for (fd = 0; fd < 1024; fd++) {
> +      if (fd != sock_pair[1])
> +        close(fd);
> +    }
> +
> +    helper_main(sock_pair[1]);
> +
> +    close(sock_pair[1]);
> +    exit(0);
> +
> +  } else {
> +    /* parent process */
> +    close(sock_pair[1]);
> +    helper_fd = sock_pair[0];
> +  }
> +}
> +
> +/* ======================================================================= */
> +
> +/* DAEMON - graceful shutdown of the helper */
> +
> +void
> +PRV_Finalise(void)
> +{
> +  int status;
> +  close(helper_fd);
> +  helper_fd = -1;
> +  wait(&status);
> +}
> +
> +#endif /* PRIVOPS_ANY */
> diff --git a/privops.h b/privops.h
> new file mode 100644
> index 0000000..d699deb
> --- /dev/null
> +++ b/privops.h
> @@ -0,0 +1,59 @@
> +/*
> +  chronyd/chronyc - Programs for keeping computer clocks accurate.
> +
> + **********************************************************************
> + * Copyright (C) Bryan Christianson 2015
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + **********************************************************************
> +
> +  =======================================================================
> +
> +  Perform privileged operations over a unix socket to a privileged fork.
> +
> +*/
> +
> +#ifndef GOT_PRIVOPS_H
> +#define GOT_PRIVOPS_H
> +
> +#define PRIVOPS_ANY \
> +  defined(PRIVOPS_ADJUSTTIME) || \
> +  defined(PRIVOPS_BINDSOCKET) || \
> +  defined(PRIVOPS_SETTIME)
> +
> +#if defined(PRIVOPS_ADJUSTTIME)
> +  int PRV_AdjustTime(const struct timeval *delta, struct timeval *olddelta);
> +#else
> +  #define PRV_AdjustTime adjtime
> +#endif
> +
> +#if defined(PRIVOPS_BINDSOCKET)
> +  int PRV_BindSocket(int sock, struct sockaddr *address, socklen_t address_len);
> +#else
> +  #define PRV_BindSocket bind
> +#endif
> +
> +#if defined(PRIVOPS_SETTIME)
> +  int PRV_SetTime(const struct timeval *tp, const struct timezone *tzp);
> +#else
> +  #define PRV_SetTime settimeofday
> +#endif
> +
> +#if PRIVOPS_ANY
> +  void PRV_Initialise(void);
> +  void PRV_Finalise(void);
> +#endif
> +
> +#endif
> -- 
> 2.4.9 (Apple Git-60)
> 


--
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.