From 79199dd84c8149abee909cebfa8393deffa5fad5 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sun, 10 Jun 2018 22:30:37 -0400 Subject: [PATCH] Run as user dropping privileges --- internal/file/filesystem.go | 9 +-------- internal/ingress/controller/checker.go | 8 +++++--- internal/ingress/controller/checker_test.go | 4 ++-- rootfs/Dockerfile | 11 ++++++++++- rootfs/etc/nginx/nginx.conf | 2 +- rootfs/etc/nginx/template/nginx.tmpl | 8 +++++++- test/manifests/ingress-controller/mandatory.yaml | 10 ++++++++-- 7 files changed, 34 insertions(+), 18 deletions(-) diff --git a/internal/file/filesystem.go b/internal/file/filesystem.go index 53d2036a3..3ad2f531b 100644 --- a/internal/file/filesystem.go +++ b/internal/file/filesystem.go @@ -35,7 +35,7 @@ func NewLocalFS() (Filesystem, error) { fs := filesystem.DefaultFs{} for _, directory := range directories { - err := fs.MkdirAll(directory, 0655) + err := fs.MkdirAll(directory, 0777) if err != nil { return nil, err } @@ -97,12 +97,5 @@ func NewFakeFS() (Filesystem, error) { } } - fakeFs.MkdirAll("/run", 0655) - fakeFs.MkdirAll("/proc", 0655) - fakeFs.MkdirAll("/etc/nginx/template", 0655) - - fakeFs.MkdirAll(DefaultSSLDirectory, 0655) - fakeFs.MkdirAll(AuthDirectory, 0655) - return fakeFs, nil } diff --git a/internal/ingress/controller/checker.go b/internal/ingress/controller/checker.go index 0d6eaca7c..92096eee6 100644 --- a/internal/ingress/controller/checker.go +++ b/internal/ingress/controller/checker.go @@ -26,6 +26,8 @@ import ( "github.com/pkg/errors" ) +const nginxPID = "/tmp/nginx.pid" + // Name returns the healthcheck name func (n NGINXController) Name() string { return "nginx-ingress-controller" @@ -58,13 +60,13 @@ func (n *NGINXController) Check(_ *http.Request) error { if err != nil { return errors.Wrap(err, "unexpected error reading /proc directory") } - f, err := n.fileSystem.ReadFile("/run/nginx.pid") + f, err := n.fileSystem.ReadFile(nginxPID) if err != nil { - return errors.Wrap(err, "unexpected error reading /run/nginx.pid") + return errors.Wrapf(err, "unexpected error reading %v", nginxPID) } pid, err := strconv.Atoi(strings.TrimRight(string(f), "\r\n")) if err != nil { - return errors.Wrap(err, "unexpected error reading the PID from /run/nginx.pid") + return errors.Wrapf(err, "unexpected error reading the nginx PID from %v", nginxPID) } _, err = fs.NewProc(pid) diff --git a/internal/ingress/controller/checker_test.go b/internal/ingress/controller/checker_test.go index cb3aca44e..050eb1e46 100644 --- a/internal/ingress/controller/checker_test.go +++ b/internal/ingress/controller/checker_test.go @@ -60,8 +60,8 @@ func TestNginxCheck(t *testing.T) { }) // create pid file - fs.MkdirAll("/run", 0655) - pidFile, err := fs.Create("/run/nginx.pid") + fs.MkdirAll("/tmp", 0655) + pidFile, err := fs.Create(nginxPID) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/rootfs/Dockerfile b/rootfs/Dockerfile index 4f438d556..68254efd1 100644 --- a/rootfs/Dockerfile +++ b/rootfs/Dockerfile @@ -20,7 +20,8 @@ WORKDIR /etc/nginx RUN clean-install \ diffutils \ - dumb-init + dumb-init \ + libcap2-bin # Create symlinks to redirect nginx logs to stdout and stderr docker log collector # This only works if nginx is started with CMD or ENTRYPOINT @@ -30,6 +31,14 @@ RUN mkdir -p /var/log/nginx \ COPY . / +RUN setcap cap_net_bind_service=+ep /usr/sbin/nginx \ + && setcap cap_net_bind_service=+ep /nginx-ingress-controller + +RUN mkdir -p /etc/ingress-controller/ssl /etc/ingress-controller/auth \ + && chown -R www-data.www-data /etc/nginx /etc/ingress-controller + +USER www-data + ENTRYPOINT ["/usr/bin/dumb-init"] CMD ["/nginx-ingress-controller"] diff --git a/rootfs/etc/nginx/nginx.conf b/rootfs/etc/nginx/nginx.conf index bb36624ce..6f8e86b90 100644 --- a/rootfs/etc/nginx/nginx.conf +++ b/rootfs/etc/nginx/nginx.conf @@ -1,5 +1,5 @@ # A very simple nginx configuration file that forces nginx to start. -pid /run/nginx.pid; +pid /tmp/nginx.pid; events {} http {} diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index d2a38e147..7a6e729f6 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -7,6 +7,9 @@ {{ $proxyHeaders := .ProxySetHeaders }} {{ $addHeaders := .AddHeaders }} +# setup custom paths that do not require root access +pid /tmp/nginx.pid; + {{ if $cfg.EnableModsecurity }} load_module /etc/nginx/modules/ngx_http_modsecurity_module.so; {{ end }} @@ -20,7 +23,6 @@ worker_processes {{ $cfg.WorkerProcesses }}; worker_cpu_affinity {{ $cfg.WorkerCpuAffinity }}; {{ end }} -pid /run/nginx.pid; {{ if ne .MaxOpenFiles 0 }} worker_rlimit_nofile {{ .MaxOpenFiles }}; {{ end }} @@ -115,6 +117,10 @@ http { keepalive_timeout {{ $cfg.KeepAlive }}s; keepalive_requests {{ $cfg.KeepAliveRequests }}; + client_body_temp_path /tmp/client-body; + fastcgi_temp_path /tmp/fastcgi-temp; + proxy_temp_path /tmp/proxy-temp; + client_header_buffer_size {{ $cfg.ClientHeaderBufferSize }}; client_header_timeout {{ $cfg.ClientHeaderTimeout }}s; large_client_header_buffers {{ $cfg.LargeClientHeaderBuffers }}; diff --git a/test/manifests/ingress-controller/mandatory.yaml b/test/manifests/ingress-controller/mandatory.yaml index fe17a87db..b349f9f67 100644 --- a/test/manifests/ingress-controller/mandatory.yaml +++ b/test/manifests/ingress-controller/mandatory.yaml @@ -251,6 +251,14 @@ spec: - --publish-service=$(POD_NAMESPACE)/ingress-nginx - --annotations-prefix=nginx.ingress.kubernetes.io - --watch-namespace=${NAMESPACE} + securityContext: + capabilities: + drop: + - ALL + add: + - NET_BIND_SERVICE + # www-data -> 33 + runAsUser: 33 env: - name: POD_NAME valueFrom: @@ -284,5 +292,3 @@ spec: periodSeconds: 10 successThreshold: 1 timeoutSeconds: 1 - securityContext: - privileged: true