From 9d45d8a24bb3d4dd61411a5218fa69ff401b6eaa Mon Sep 17 00:00:00 2001 From: Daniel Roethlisberger Date: Sun, 30 Sep 2018 00:03:46 +0200 Subject: [PATCH] Combine openfile and pcap privsep interfaces --- log.c | 8 +++--- opts.c | 79 ++++++++++++++++++++----------------------------------- opts.h | 4 +-- privsep.c | 46 +++++++++----------------------- privsep.h | 2 +- 5 files changed, 47 insertions(+), 92 deletions(-) diff --git a/log.c b/log.c index 43130ff9..ee822d7d 100644 --- a/log.c +++ b/log.c @@ -918,7 +918,7 @@ log_content_dir_opencb(void *fh) if ((ctx->u.dir.fd = privsep_client_openfile(content_clisock, ctx->u.dir.filename, - 0, 0)) == -1) { + 0)) == -1) { log_err_printf("Opening logdir file '%s' failed: %s (%i)\n", ctx->u.dir.filename, strerror(errno), errno); return -1; @@ -959,7 +959,7 @@ log_content_spec_opencb(void *fh) if ((ctx->u.spec.fd = privsep_client_openfile(content_clisock, ctx->u.spec.filename, - 1, 0)) == -1) { + 1)) == -1) { log_err_printf("Opening logspec file '%s' failed: %s (%i)\n", ctx->u.spec.filename, strerror(errno), errno); return -1; @@ -1290,7 +1290,7 @@ log_pcap_dir_opencb(void *fh) if ((ctx->pcap.u.dir.fd = privsep_client_openfile(content_clisock, ctx->pcap.u.dir.filename, - 0, 1)) == -1) { + 0)) == -1) { log_err_printf("Opening pcapdir file '%s' failed: %s (%i)\n", ctx->pcap.u.dir.filename, strerror(errno), errno); @@ -1328,7 +1328,7 @@ log_pcap_spec_opencb(void *fh) if ((ctx->pcap.u.spec.fd = privsep_client_openfile(content_clisock, ctx->pcap.u.spec.filename, - 1, 1)) == -1) { + 1)) == -1) { log_err_printf("Opening pcapspec file '%s' failed: %s (%i)\n", ctx->pcap.u.spec.filename, strerror(errno), errno); diff --git a/opts.c b/opts.c index 55f598ca..942e051b 100644 --- a/opts.c +++ b/opts.c @@ -895,9 +895,8 @@ void opts_set_user(opts_t *opts, const char *argv0, const char *optarg) { if (!sys_isuser(optarg)) { - fprintf(stderr, "%s: '%s' is not an " - "existing user\n", - argv0, optarg); + fprintf(stderr, "%s: '%s' is not an existing user\n", + argv0, optarg); exit(EXIT_FAILURE); } if (opts->dropuser) @@ -913,9 +912,8 @@ opts_set_group(opts_t *opts, const char *argv0, const char *optarg) { if (!sys_isgroup(optarg)) { - fprintf(stderr, "%s: '%s' is not an " - "existing group\n", - argv0, optarg); + fprintf(stderr, "%s: '%s' is not an existing group\n", + argv0, optarg); exit(EXIT_FAILURE); } if (opts->dropgroup) @@ -930,20 +928,15 @@ void opts_set_jaildir(opts_t *opts, const char *argv0, const char *optarg) { if (!sys_isdir(optarg)) { - fprintf(stderr, "%s: '%s' is not a " - "directory\n", - argv0, optarg); + fprintf(stderr, "%s: '%s' is not a directory\n", argv0, optarg); exit(EXIT_FAILURE); } if (opts->jaildir) free(opts->jaildir); opts->jaildir = realpath(optarg, NULL); if (!opts->jaildir) { - fprintf(stderr, "%s: Failed to " - "canonicalize '%s': " - "%s (%i)\n", - argv0, optarg, - strerror(errno), errno); + fprintf(stderr, "%s: Failed to canonicalize '%s': %s (%i)\n", + argv0, optarg, strerror(errno), errno); exit(EXIT_FAILURE); } log_dbg_printf("Chroot: %s\n", opts->jaildir); @@ -988,20 +981,15 @@ void opts_set_contentlogdir(opts_t *opts, const char *argv0, const char *optarg) { if (!sys_isdir(optarg)) { - fprintf(stderr, "%s: '%s' is not a " - "directory\n", - argv0, optarg); + fprintf(stderr, "%s: '%s' is not a directory\n", argv0, optarg); exit(EXIT_FAILURE); } if (opts->contentlog) free(opts->contentlog); opts->contentlog = realpath(optarg, NULL); if (!opts->contentlog) { - fprintf(stderr, "%s: Failed to " - "canonicalize '%s': " - "%s (%i)\n", - argv0, optarg, - strerror(errno), errno); + fprintf(stderr, "%s: Failed to canonicalize '%s': %s (%i)\n", + argv0, optarg, strerror(errno), errno); exit(EXIT_FAILURE); } opts->contentlog_isdir = 1; @@ -1010,7 +998,8 @@ opts_set_contentlogdir(opts_t *opts, const char *argv0, const char *optarg) } static void -opts_set_logbasedir(const char *argv0, const char *optarg, char **basedir, char **log) +opts_set_logbasedir(const char *argv0, const char *optarg, + char **basedir, char **log) { char *lhs, *rhs, *p, *q; size_t n; @@ -1018,13 +1007,10 @@ opts_set_logbasedir(const char *argv0, const char *optarg, char **basedir, char free(*basedir); if (*log) free(*log); - if (log_content_split_pathspec(optarg, &lhs, - &rhs) == -1) { - fprintf(stderr, "%s: Failed to split " - "'%s' in lhs/rhs: " - "%s (%i)\n", - argv0, optarg, - strerror(errno), errno); + if (log_content_split_pathspec(optarg, &lhs, &rhs) == -1) { + fprintf(stderr, "%s: Failed to split '%s' in lhs/rhs:" + " %s (%i)\n", argv0, optarg, + strerror(errno), errno); exit(EXIT_FAILURE); } /* eliminate %% from lhs */ @@ -1037,19 +1023,14 @@ opts_set_logbasedir(const char *argv0, const char *optarg, char **basedir, char *q = '\0'; /* all %% in lhs resolved to % */ if (sys_mkpath(lhs, 0777) == -1) { - fprintf(stderr, "%s: Failed to create " - "'%s': %s (%i)\n", - argv0, lhs, - strerror(errno), errno); + fprintf(stderr, "%s: Failed to create '%s': %s (%i)\n", + argv0, lhs, strerror(errno), errno); exit(EXIT_FAILURE); } *basedir = realpath(lhs, NULL); if (!*basedir) { - fprintf(stderr, "%s: Failed to " - "canonicalize '%s': " - "%s (%i)\n", - argv0, lhs, - strerror(errno), errno); + fprintf(stderr, "%s: Failed to canonicalize '%s': %s (%i)\n", + argv0, lhs, strerror(errno), errno); exit(EXIT_FAILURE); } /* count '%' in basedir */ @@ -1073,8 +1054,7 @@ opts_set_logbasedir(const char *argv0, const char *optarg, char **basedir, char } *q = '\0'; /* lhs contains encoded realpathed basedir */ - if (asprintf(log, - "%s/%s", lhs, rhs) < 0) + if (asprintf(log, "%s/%s", lhs, rhs) < 0) oom_die(argv0); free(lhs); free(rhs); @@ -1083,7 +1063,8 @@ opts_set_logbasedir(const char *argv0, const char *optarg, char **basedir, char void opts_set_contentlogpathspec(opts_t *opts, const char *argv0, const char *optarg) { - opts_set_logbasedir(argv0, optarg, &opts->contentlog_basedir, &opts->contentlog); + opts_set_logbasedir(argv0, optarg, &opts->contentlog_basedir, + &opts->contentlog); opts->contentlog_isdir = 0; opts->contentlog_isspec = 1; log_dbg_printf("ContentLogPathSpec: basedir=%s, %s\n", @@ -1132,20 +1113,15 @@ void opts_set_pcaplogdir(opts_t *opts, const char *argv0, const char *optarg) { if (!sys_isdir(optarg)) { - fprintf(stderr, "%s: '%s' is not a " - "directory\n", - argv0, optarg); + fprintf(stderr, "%s: '%s' is not a directory\n", argv0, optarg); exit(EXIT_FAILURE); } if (opts->pcaplog) free(opts->pcaplog); opts->pcaplog = realpath(optarg, NULL); if (!opts->pcaplog) { - fprintf(stderr, "%s: Failed to " - "canonicalize '%s': " - "%s (%i)\n", - argv0, optarg, - strerror(errno), errno); + fprintf(stderr, "%s: Failed to canonicalize '%s': %s (%i)\n", + argv0, optarg, strerror(errno), errno); exit(EXIT_FAILURE); } opts->pcaplog_isdir = 1; @@ -1156,7 +1132,8 @@ opts_set_pcaplogdir(opts_t *opts, const char *argv0, const char *optarg) void opts_set_pcaplogpathspec(opts_t *opts, const char *argv0, const char *optarg) { - opts_set_logbasedir(argv0, optarg, &opts->pcaplog_basedir, &opts->pcaplog); + opts_set_logbasedir(argv0, optarg, &opts->pcaplog_basedir, + &opts->pcaplog); opts->pcaplog_isdir = 0; opts->pcaplog_isspec = 1; log_dbg_printf("PcapLogPathSpec: basedir=%s, %s\n", diff --git a/opts.h b/opts.h index f452bed2..06d911f1 100644 --- a/opts.h +++ b/opts.h @@ -99,10 +99,10 @@ typedef struct opts { char *conffile; char *connectlog; char *contentlog; - char *contentlog_basedir; /* static part of logspec, for privsep srv */ + char *contentlog_basedir; /* static part of logspec for privsep srv */ char *masterkeylog; char *pcaplog; - char *pcaplog_basedir; /* static part of pcap logspec, for privsep srv */ + char *pcaplog_basedir; /* static part of pcap logspec for privsep srv */ char *mirrorif; char *mirrortarget; char mirrortarget_ether[ETHER_ADDR_LEN]; diff --git a/privsep.c b/privsep.c index 65f58a3e..bcb25022 100644 --- a/privsep.c +++ b/privsep.c @@ -65,8 +65,6 @@ #define PRIVSEP_REQ_OPENFILE_P 2 /* open content log file w/mkpath */ #define PRIVSEP_REQ_OPENSOCK 3 /* open socket and pass fd */ #define PRIVSEP_REQ_CERTFILE 4 /* open cert file in certgendir */ -#define PRIVSEP_REQ_OPENPCAPFILE_P 5 /* open pcap log file */ -#define PRIVSEP_REQ_OPENPCAPFILE 6 /* open pcap log file w/mkpath */ /* response byte */ #define PRIVSEP_ANS_SUCCESS 0 /* success */ @@ -140,13 +138,16 @@ privsep_server_signal_handler(int sig) static int WUNRES privsep_server_openfile_verify(opts_t *opts, char *fn, int mkpath) { - if (mkpath && !opts->contentlog_isspec) + if (mkpath && !(opts->contentlog_isspec || opts->pcaplog_isspec)) return -1; - if (!mkpath && !opts->contentlog_isdir) + if (!mkpath && !(opts->contentlog_isdir || opts->pcaplog_isdir)) return -1; - if (strstr(fn, mkpath ? opts->contentlog_basedir - : opts->contentlog) != fn || - strstr(fn, "/../")) + if (strstr(fn, opts->contentlog_isspec ? opts->contentlog_basedir + : opts->contentlog) != fn && + strstr(fn, opts->pcaplog_isspec ? opts->pcaplog_basedir + : opts->pcaplog) != fn) + return -1; + if (strstr(fn, "/../")) return -1; return 0; } @@ -190,20 +191,6 @@ privsep_server_openfile(char *fn, int mkpath) return fd; } -static int WUNRES -privsep_server_openpcapfile_verify(opts_t *opts, char *fn, int mkpath) -{ - if (mkpath && !opts->pcaplog_isspec) - return -1; - if (!mkpath && !opts->pcaplog_isdir) - return -1; - if (strstr(fn, mkpath ? opts->pcaplog_basedir - : opts->pcaplog) != fn || - strstr(fn, "/../")) - return -1; - return 0; -} - static int WUNRES privsep_server_opensock_verify(opts_t *opts, void *arg) { @@ -328,16 +315,11 @@ privsep_server_handle_req(opts_t *opts, int srvsock) return 1; } case PRIVSEP_REQ_OPENFILE_P: - case PRIVSEP_REQ_OPENPCAPFILE_P: mkpath = 1; /* fall through */ - case PRIVSEP_REQ_OPENFILE: - case PRIVSEP_REQ_OPENPCAPFILE: { + case PRIVSEP_REQ_OPENFILE: { char *fn; int fd; - int (*verifyfunc)(opts_t *, char *, int) = - (req[0] == PRIVSEP_REQ_OPENFILE_P || req[0] == PRIVSEP_REQ_OPENFILE) ? - privsep_server_openfile_verify : privsep_server_openpcapfile_verify; if (n < 2) { ans[0] = PRIVSEP_ANS_INVALID; @@ -360,7 +342,7 @@ privsep_server_handle_req(opts_t *opts, int srvsock) } memcpy(fn, req + 1, n - 1); fn[n - 1] = '\0'; - if (verifyfunc(opts, fn, mkpath) == -1) { + if (privsep_server_openfile_verify(opts, fn, mkpath) == -1) { free(fn); ans[0] = PRIVSEP_ANS_DENIED; if (sys_sendmsgfd(srvsock, ans, 1, -1) == -1) { @@ -673,18 +655,14 @@ privsep_server(opts_t *opts, int sigpipe, int srvsock[], size_t nsrvsock, } int -privsep_client_openfile(int clisock, const char *fn, int mkpath, int pcapreq) +privsep_client_openfile(int clisock, const char *fn, int mkpath) { char ans[PRIVSEP_MAX_ANS_SIZE]; char req[1 + strlen(fn)]; int fd = -1; ssize_t n; - if (pcapreq) { - req[0] = mkpath ? PRIVSEP_REQ_OPENPCAPFILE_P : PRIVSEP_REQ_OPENPCAPFILE; - } else { - req[0] = mkpath ? PRIVSEP_REQ_OPENFILE_P : PRIVSEP_REQ_OPENFILE; - } + req[0] = mkpath ? PRIVSEP_REQ_OPENFILE_P : PRIVSEP_REQ_OPENFILE; memcpy(req + 1, fn, sizeof(req) - 1); if (sys_sendmsgfd(clisock, req, sizeof(req), -1) == -1) { diff --git a/privsep.h b/privsep.h index 8e562872..b59dab44 100644 --- a/privsep.h +++ b/privsep.h @@ -34,7 +34,7 @@ int privsep_fork(opts_t *, int[], size_t); -int privsep_client_openfile(int, const char *, int, int); +int privsep_client_openfile(int, const char *, int); int privsep_client_opensock(int, const proxyspec_t *spec); int privsep_client_certfile(int, const char *); int privsep_client_close(int);