From 334cfd5f3bdd4fd742b5d9486880923193c9365d Mon Sep 17 00:00:00 2001 From: Marco van Wieringen Date: Sat, 4 May 2013 20:34:35 +0200 Subject: [PATCH] Implementation of an allowed scriptdir keyword Implement an allowed scriptdir keyword in the filed that sets the directories in which any runscript must be located so we can limit the attack surface of the filedaemon. Currently the filed will execute any script in any directory which makes it a serious security concern by much of the bigger customers security officers. This new keyword implemented per director and a global one which is used as a fallback when a specific one for a specific director is not configured. Fixes #31: Implementation of an allowed scriptdir keyword --- src/filed/filed_conf.c | 23 +++++++++++-------- src/filed/filed_conf.h | 3 ++- src/filed/job.c | 22 +++++++++++++++---- src/lib/runscript.c | 50 +++++++++++++++++++++++++++++++++++++++--- src/lib/runscript.h | 3 ++- 5 files changed, 83 insertions(+), 18 deletions(-) diff --git a/src/filed/filed_conf.c b/src/filed/filed_conf.c index 512d29d5070..b43ff20ecc5 100644 --- a/src/filed/filed_conf.c +++ b/src/filed/filed_conf.c @@ -117,6 +117,7 @@ static RES_ITEM cli_items[] = { { "compatible", store_bool, ITEM(res_client.compatible), 0, 0, "true" }, { "maximumbandwidthperjob", store_speed, ITEM(res_client.max_bandwidth_per_job), 0, 0, NULL }, { "allowbandwidthbursting", store_bool, ITEM(res_client.allow_bw_bursting), 0, 0, "false" }, + { "allowedscriptdir", store_alist_str, ITEM(res_client.allowed_script_dirs), 0, 0, NULL }, { NULL, NULL, { 0 }, 0, 0, NULL } }; @@ -140,6 +141,7 @@ static RES_ITEM dir_items[] = { { "tlsdhfile", store_dir, ITEM(res_dir.tls_dhfile), 0, 0, NULL }, { "tlsallowedcn", store_alist_str, ITEM(res_dir.tls_allowed_cns), 0, 0, NULL }, { "maximumbandwidthperjob", store_speed, ITEM(res_dir.max_bandwidth_per_job), 0, 0, NULL }, + { "allowedscriptdir", store_alist_str, ITEM(res_dir.allowed_script_dirs), 0, 0, NULL }, { NULL, NULL, { 0 }, 0, 0, NULL } }; @@ -255,6 +257,9 @@ void free_resource(RES *sres, int type) if (res->res_dir.tls_allowed_cns) { delete res->res_dir.tls_allowed_cns; } + if (res->res_dir.allowed_script_dirs) { + delete res->res_dir.allowed_script_dirs; + } break; case R_CLIENT: if (res->res_client.working_directory) { @@ -278,14 +283,12 @@ void free_resource(RES *sres, int type) if (res->res_client.FDsrc_addr) { free_addresses(res->res_client.FDsrc_addr); } - if (res->res_client.pki_keypair_file) { free(res->res_client.pki_keypair_file); } if (res->res_client.pki_keypair) { crypto_keypair_free(res->res_client.pki_keypair); } - if (res->res_client.pki_signing_key_files) { delete res->res_client.pki_signing_key_files; } @@ -296,11 +299,9 @@ void free_resource(RES *sres, int type) } delete res->res_client.pki_signers; } - if (res->res_client.pki_master_key_files) { delete res->res_client.pki_master_key_files; } - if (res->res_client.pki_recipients) { X509_KEYPAIR *keypair; foreach_alist(keypair, res->res_client.pki_recipients) { @@ -308,7 +309,6 @@ void free_resource(RES *sres, int type) } delete res->res_client.pki_recipients; } - if (res->res_client.tls_ctx) { free_tls_context(res->res_client.tls_ctx); } @@ -327,12 +327,17 @@ void free_resource(RES *sres, int type) if (res->res_client.verid) { free(res->res_client.verid); } + if (res->res_client.allowed_script_dirs) { + delete res->res_client.allowed_script_dirs; + } break; case R_MSGS: - if (res->res_msgs.mail_cmd) + if (res->res_msgs.mail_cmd) { free(res->res_msgs.mail_cmd); - if (res->res_msgs.operator_cmd) + } + if (res->res_msgs.operator_cmd) { free(res->res_msgs.operator_cmd); + } free_msgs_res((MSGSRES *)res); /* free message resource */ res = NULL; break; @@ -394,6 +399,7 @@ void save_resource(int type, RES_ITEM *items, int pass) Emsg1(M_ABORT, 0, _("Cannot find Director resource %s\n"), res_all.res_dir.hdr.name); } res->res_dir.tls_allowed_cns = res_all.res_dir.tls_allowed_cns; + res->res_dir.allowed_script_dirs = res_all.res_dir.allowed_script_dirs; break; case R_CLIENT: if ((res = (URES *)GetResWithName(R_CLIENT, res_all.res_dir.hdr.name)) == NULL) { @@ -401,11 +407,10 @@ void save_resource(int type, RES_ITEM *items, int pass) } res->res_client.pki_signing_key_files = res_all.res_client.pki_signing_key_files; res->res_client.pki_master_key_files = res_all.res_client.pki_master_key_files; - res->res_client.pki_signers = res_all.res_client.pki_signers; res->res_client.pki_recipients = res_all.res_client.pki_recipients; - res->res_client.messages = res_all.res_client.messages; + res->res_client.allowed_script_dirs = res_all.res_client.allowed_script_dirs; break; default: Emsg1(M_ERROR, 0, _("Unknown resource type %d\n"), type); diff --git a/src/filed/filed_conf.h b/src/filed/filed_conf.h index c4d5a2e6a7d..c763e639a36 100644 --- a/src/filed/filed_conf.h +++ b/src/filed/filed_conf.h @@ -64,6 +64,7 @@ struct DIRRES { char *tls_keyfile; /* TLS Server Key File */ char *tls_dhfile; /* TLS Diffie-Hellman Parameters */ alist *tls_allowed_cns; /* TLS Allowed Clients */ + alist *allowed_script_dirs; /* Only allow to run scripts in this directories */ uint64_t max_bandwidth_per_job; /* Bandwidth limitation (per director) */ TLS_CONTEXT *tls_ctx; /* Shared TLS Context */ }; @@ -97,10 +98,10 @@ struct CLIENTRES { char *tls_ca_certdir; /* TLS CA Certificate Directory */ char *tls_certfile; /* TLS Client Certificate File */ char *tls_keyfile; /* TLS Client Key File */ - X509_KEYPAIR *pki_keypair; /* Shared PKI Public/Private Keypair */ alist *pki_signers; /* Shared PKI Trusted Signers */ alist *pki_recipients; /* Shared PKI Recipients */ + alist *allowed_script_dirs; /* Only allow to run scripts in this directories */ TLS_CONTEXT *tls_ctx; /* Shared TLS Context */ char *verid; /* Custom Id to print in version command */ uint64_t max_bandwidth_per_job; /* Bandwidth limitation (global) */ diff --git a/src/filed/job.c b/src/filed/job.c index d2c7d0a933f..e4f98a7fe21 100644 --- a/src/filed/job.c +++ b/src/filed/job.c @@ -304,7 +304,10 @@ void *handle_client_request(void *dirp) } /* Run the after job */ - run_scripts(jcr, jcr->RunScripts, "ClientAfterJob"); + run_scripts(jcr, jcr->RunScripts, "ClientAfterJob", + (jcr->director->allowed_script_dirs) ? + jcr->director->allowed_script_dirs : + me->allowed_script_dirs); if (jcr->JobId) { /* send EndJob if running a job */ char ed1[50], ed2[50]; @@ -643,7 +646,11 @@ static int runbeforenow_cmd(JCR *jcr) { BSOCK *dir = jcr->dir_bsock; - run_scripts(jcr, jcr->RunScripts, "ClientBeforeJob"); + run_scripts(jcr, jcr->RunScripts, "ClientBeforeJob", + (jcr->director->allowed_script_dirs) ? + jcr->director->allowed_script_dirs : + me->allowed_script_dirs); + if (job_canceled(jcr)) { dir->fsend(FailedRunScript); Dmsg0(100, "Back from run_scripts ClientBeforeJob now: FAILED\n"); @@ -1318,7 +1325,11 @@ static int backup_cmd(JCR *jcr) Jmsg(jcr, M_FATAL, 0, _("VSS was not initialized properly. ERR=%s\n"), be.bstrerror()); } - run_scripts(jcr, jcr->RunScripts, "ClientAfterVSS"); + + run_scripts(jcr, jcr->RunScripts, "ClientAfterVSS", + (jcr->director->allowed_script_dirs) ? + jcr->director->allowed_script_dirs : + me->allowed_script_dirs); } #endif @@ -1597,7 +1608,10 @@ static int restore_cmd(JCR *jcr) Jmsg(jcr, M_WARNING, 0, _("VSS was not initialized properly. VSS support is disabled. ERR=%s\n"), be.bstrerror()); } //free_and_null_pool_memory(jcr->job_metadata); - run_scripts(jcr, jcr->RunScripts, "ClientAfterVSS"); + run_scripts(jcr, jcr->RunScripts, "ClientAfterVSS", + (jcr->director->allowed_script_dirs) ? + jcr->director->allowed_script_dirs : + me->allowed_script_dirs); } #endif diff --git a/src/lib/runscript.c b/src/lib/runscript.c index bc98ac7fea5..fd780f525e2 100644 --- a/src/lib/runscript.c +++ b/src/lib/runscript.c @@ -95,15 +95,48 @@ void free_runscript(RUNSCRIPT *script) free(script); } -int run_scripts(JCR *jcr, alist *runscripts, const char *label) +static inline bool script_dir_allowed(JCR *jcr, RUNSCRIPT *script, alist *allowed_script_dirs) { - Dmsg2(200, "runscript: running all RUNSCRIPT object (%s) JobStatus=%c\n", label, jcr->JobStatus); + char *bp, *allowed_script_dir; + bool allowed = false; + POOL_MEM script_dir(PM_FNAME); + + /* + * If there is no explicit list of allowed dirs allow any dir. + */ + if (!allowed_script_dirs) { + return true; + } + + /* + * Determine the dir the script is in. + */ + pm_strcpy(script_dir, script->command); + if ((bp = strrchr(script_dir.c_str(), '/'))) { + *bp = '\0'; + } + + /* + * Match the path the script is in against the list of allowed script directories. + */ + foreach_alist(allowed_script_dir, allowed_script_dirs) { + if (bstrcasecmp(script_dir.c_str(), allowed_script_dir)) { + allowed = true; + break; + } + } + + return allowed; +} +int run_scripts(JCR *jcr, alist *runscripts, const char *label, alist *allowed_script_dirs) +{ RUNSCRIPT *script; bool runit; - int when; + Dmsg2(200, "runscript: running all RUNSCRIPT object (%s) JobStatus=%c\n", label, jcr->JobStatus); + if (strstr(label, NT_("Before"))) { when = SCRIPT_Before; } else if (bstrcmp(label, NT_("ClientAfterVSS"))) { @@ -121,6 +154,15 @@ int run_scripts(JCR *jcr, alist *runscripts, const char *label) Dmsg2(200, "runscript: try to run %s:%s\n", NPRT(script->target), NPRT(script->command)); runit = false; + if (!script_dir_allowed(jcr, script, allowed_script_dirs)) { + Dmsg1(200, "runscript: Not running script %s because its not in one of the allowed scripts dirs\n", + script->command); + Jmsg(jcr, M_ERROR, 0, _("Runscript: run %s \"%s\" could not execute, " + "not in one of the allowed scripts dirs\n"), label, script->command); + jcr->setJobStatus(JS_ErrorTerminated); + goto bail_out; + } + if ((script->when & SCRIPT_Before) && (when & SCRIPT_Before)) { if ((script->on_success && (jcr->JobStatus == JS_Running || jcr->JobStatus == JS_Created)) || (script->on_failure && (job_canceled(jcr) || jcr->JobStatus == JS_Differences))) { @@ -157,6 +199,8 @@ int run_scripts(JCR *jcr, alist *runscripts, const char *label) script->run(jcr, label); } } + +bail_out: return 1; } diff --git a/src/lib/runscript.h b/src/lib/runscript.h index 15c70088457..df0c5adb91c 100644 --- a/src/lib/runscript.h +++ b/src/lib/runscript.h @@ -95,7 +95,8 @@ RUNSCRIPT *new_runscript(); RUNSCRIPT *copy_runscript(RUNSCRIPT *src); /* launch each script from runscripts*/ -int run_scripts(JCR *jcr, alist *runscripts, const char *name); +int run_scripts(JCR *jcr, alist *runscripts, const char *name, + alist *allowed_script_dirs = NULL); /* free RUNSCRIPT (and all POOLMEM) */ void free_runscript(RUNSCRIPT *script);