[master] ad6bf9c Varnishd needs to run the systems C-compiler to compile the VCL code.
Poul-Henning Kamp
phk at FreeBSD.org
Mon Jul 28 11:09:01 CEST 2014
commit ad6bf9c0e51954cc45fee92d484e95c666d99685
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Mon Jul 28 09:03:06 2014 +0000
Varnishd needs to run the systems C-compiler to compile the VCL code.
For security reasons, we run the C-compiler in a sandbox process
which by default uses the same (non-)privileges as the other sandboxes
(VCL compiler, test-loader process and the worker process).
On some systems access to the C-compiler is limited, also for reasons
of security, and varnishd will fail to compile VCL code, unless all
the sandboxes are given access to the C-compiler.
Add a new parameter "group_cc" which adds a single gid to the grouplist
of the sandbox which executes the cc_command, for the benefit of such
systems.
Do some slightly related polishing of the docs/help-texts in this area
while here anyway.
Fixes #1521
diff --git a/bin/varnishd/common/params.h b/bin/varnishd/common/params.h
index 35c0a7a..2ce95c6 100644
--- a/bin/varnishd/common/params.h
+++ b/bin/varnishd/common/params.h
@@ -63,6 +63,10 @@ struct params {
char *group;
gid_t gid;
+ /* Extra group for compiler access */
+ char *group_cc;
+ gid_t gid_cc;
+
/* TTL used for lack of anything better */
double default_ttl;
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index e8a1763..9feea1a 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -90,6 +90,10 @@ static const char PROTECTED_TEXT[] =
"\n\n"
"NB: This parameter is protected and can not be changed.";
+static const char ONLY_ROOT_TEXT[] =
+ "\n\n"
+ "NB: This parameter only works of varnishd is run as root.";
+
/*--------------------------------------------------------------------*/
@@ -265,6 +269,8 @@ mcf_param_show(struct cli *cli, const char * const *av, void *priv)
mcf_wrap(cli, WIZARD_TEXT);
if (pp->flags & PROTECTED)
mcf_wrap(cli, PROTECTED_TEXT);
+ if (pp->flags & ONLY_ROOT)
+ mcf_wrap(cli, ONLY_ROOT_TEXT);
VCLI_Out(cli, "\n\n");
}
}
@@ -556,6 +562,10 @@ MCF_DumpRstParam(void)
printf("%swizard", q);
q = ", ";
}
+ if (pp->flags & ONLY_ROOT) {
+ printf("%sonly_root", q);
+ q = ", ";
+ }
printf("\n");
}
printf("\n");
diff --git a/bin/varnishd/mgt/mgt_param.h b/bin/varnishd/mgt/mgt_param.h
index f60180b..1b8204a 100644
--- a/bin/varnishd/mgt/mgt_param.h
+++ b/bin/varnishd/mgt/mgt_param.h
@@ -47,6 +47,7 @@ struct parspec {
#define WIZARD (1<<4)
#define PROTECTED (1<<5)
#define OBJ_STICKY (1<<6)
+#define ONLY_ROOT (1<<7)
const char *def;
const char *units;
};
@@ -56,6 +57,7 @@ tweak_t tweak_bytes;
tweak_t tweak_bytes_u;
tweak_t tweak_double;
tweak_t tweak_group;
+tweak_t tweak_group_cc;
tweak_t tweak_listen_address;
tweak_t tweak_poolparam;
tweak_t tweak_string;
diff --git a/bin/varnishd/mgt/mgt_param_tbl.c b/bin/varnishd/mgt/mgt_param_tbl.c
index 6651a93..bcab043 100644
--- a/bin/varnishd/mgt/mgt_param_tbl.c
+++ b/bin/varnishd/mgt/mgt_param_tbl.c
@@ -53,11 +53,19 @@
struct parspec mgt_parspec[] = {
{ "user", tweak_user, NULL, NULL, NULL,
"The unprivileged user to run as.",
- MUST_RESTART,
+ MUST_RESTART | ONLY_ROOT,
"" },
{ "group", tweak_group, NULL, NULL, NULL,
"The unprivileged group to run as.",
- MUST_RESTART,
+ MUST_RESTART | ONLY_ROOT,
+ "" },
+ { "group_cc", tweak_group_cc, NULL, NULL, NULL,
+ "On some systems the C-compiler is restricted so not"
+ " everybody can run it. This parameter makes it possible"
+ " to add an extra group to the sandbox process which runs the"
+ " cc_command, in order to gain access to such a restricted"
+ " c-compiler.",
+ ONLY_ROOT,
"" },
{ "default_ttl", tweak_timeout, &mgt_param.default_ttl,
"0", NULL,
diff --git a/bin/varnishd/mgt/mgt_param_tweak.c b/bin/varnishd/mgt/mgt_param_tweak.c
index 7d9fb4f..e757ec6 100644
--- a/bin/varnishd/mgt/mgt_param_tweak.c
+++ b/bin/varnishd/mgt/mgt_param_tweak.c
@@ -433,6 +433,38 @@ tweak_group(struct vsb *vsb, const struct parspec *par, const char *arg)
return (0);
}
+/*--------------------------------------------------------------------
+ * XXX: see comment for tweak_user, same thing here.
+ */
+
+int
+tweak_group_cc(struct vsb *vsb, const struct parspec *par, const char *arg)
+{
+ struct group *gr;
+
+ (void)par;
+ if (arg != NULL) {
+ if (*arg != '\0') {
+ gr = getgrnam(arg);
+ if (gr == NULL) {
+ VSB_printf(vsb, "Unknown group");
+ return(-1);
+ }
+ REPLACE(mgt_param.group_cc, gr->gr_name);
+ mgt_param.gid_cc = gr->gr_gid;
+ } else {
+ REPLACE(mgt_param.group_cc, "");
+ mgt_param.gid_cc = 0;
+ }
+ } else if (strlen(mgt_param.group_cc) > 0) {
+ VSB_printf(vsb, "%s (%d)",
+ mgt_param.group_cc, (int)mgt_param.gid_cc);
+ } else {
+ VSB_printf(vsb, "<not set>");
+ }
+ return (0);
+}
+
/*--------------------------------------------------------------------*/
static void
diff --git a/bin/varnishd/mgt/mgt_sandbox.c b/bin/varnishd/mgt/mgt_sandbox.c
index 83a2aeb..973ec83 100644
--- a/bin/varnishd/mgt/mgt_sandbox.c
+++ b/bin/varnishd/mgt/mgt_sandbox.c
@@ -51,6 +51,7 @@
#include <grp.h>
#include <stdio.h>
#include <syslog.h>
+#include <string.h>
#include <unistd.h>
#include "mgt/mgt.h"
@@ -62,14 +63,27 @@
static void __match_proto__(mgt_sandbox_f)
mgt_sandbox_unix(enum sandbox_e who)
{
- (void)who;
- if (geteuid() == 0) {
- XXXAZ(setgid(mgt_param.gid));
- XXXAZ(initgroups(mgt_param.user, mgt_param.gid));
- XXXAZ(setuid(mgt_param.uid));
- } else {
+#define NGID 10
+ gid_t gid_list[NGID];
+ int i;
+
+ if (geteuid() != 0) {
REPORT0(LOG_INFO, "Not running as root, no priv-sep");
+ return;
}
+
+ XXXAZ(setgid(mgt_param.gid));
+ XXXAZ(initgroups(mgt_param.user, mgt_param.gid));
+
+ if (who == SANDBOX_CC && strlen(mgt_param.group_cc) > 0) {
+ /* Add the optional extra group for the C-compiler access */
+ i = getgroups(NGID, gid_list);
+ assert(i >= 0);
+ gid_list[i++] = mgt_param.gid_cc;
+ XXXAZ(setgroups(i, gid_list));
+ }
+
+ XXXAZ(setuid(mgt_param.uid));
}
#endif
diff --git a/bin/varnishtest/tests/r01518.vtc b/bin/varnishtest/tests/r01518.vtc
new file mode 100644
index 0000000..11791ae
--- /dev/null
+++ b/bin/varnishtest/tests/r01518.vtc
@@ -0,0 +1,30 @@
+varnishtest "304 body handling with ESI"
+
+server s1 {
+ rxreq
+ txresp -hdr "Last-Modified: Wed, 04 Jun 2014 08:48:52 GMT" \
+ -body {<html><esi:include src="/bar"/>}
+
+ rxreq
+ expect req.url == "/bar"
+ txresp -body {<html>}
+
+} -start
+
+varnish v1 -vcl+backend {
+ sub vcl_backend_response {
+ set beresp.do_esi = true;
+ }
+} -start
+
+client c1 {
+ txreq
+ rxresp
+ expect resp.bodylen == 12
+ expect resp.status == 200
+
+ txreq -hdr "If-Modified-Since: Wed, 04 Jun 2014 08:48:52 GMT"
+ rxresp
+ expect resp.status == 304
+ expect resp.bodylen == 0
+} -run
diff --git a/doc/sphinx/reference/varnishd.rst b/doc/sphinx/reference/varnishd.rst
index 633ea43..e1d02bb 100644
--- a/doc/sphinx/reference/varnishd.rst
+++ b/doc/sphinx/reference/varnishd.rst
@@ -226,6 +226,15 @@ restart
reload
The VCL programs must be reloaded for this parameter to take effect.
+experimental
+ We're not really sure about this parameter, tell us what you find.
+
+wizard
+ Do not touch unless you *really* know what you're doing.
+
+only_root
+ Only works if varnishd is running as root.
+
Here is a list of all parameters, current as of last time we
remembered to update the manual page. This text is produced from the
same text you will find in the CLI if you use the param.show command,
More information about the varnish-commit
mailing list