Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run Ngninx Kubernetes Ingress controller with readOnlyRootFilesystem: true #1677

Closed
abhisek-dwivedi opened this issue Jun 18, 2021 · 10 comments · Fixed by #3548
Closed

Run Ngninx Kubernetes Ingress controller with readOnlyRootFilesystem: true #1677

abhisek-dwivedi opened this issue Jun 18, 2021 · 10 comments · Fixed by #3548
Labels
enhancement Pull requests for new features/feature enhancements proposal An issue that proposes a feature request
Milestone

Comments

@abhisek-dwivedi
Copy link

abhisek-dwivedi commented Jun 18, 2021

Is your feature request related to a problem? Please describe.

Users should be able to run nginx Kubernetes ingress controller with minimum permissions in the security context. Current configuration runs with AllowPrivilegeEscalation: true and readOnlyRootFilesystem: false but there should be a way to restrict the permissions to a bare minimum.

Describe the solution you'd like

I was able to remove AllowPrivilegeEscalation: true by changing the hard code privileged ports (80, 443) in code & templates and building a custom docker image out of it. But for the readOnlyRootFilesystem: false workaround I'm trying to change /etc/nginx directory where all configurations are stored/created to one emptyDir volume.

But after these changes, I'm facing sock connection failure and other issues in IC pod.
Eg:
F0616 14:26:09.191702 1 manager.go:284] Could not get newest config version: could not get expected version: 0 after 4s

Is it possible to run IC with readOnlyRootFilesystem: true ? or some specific steps are required to do so ?

Aha! Link: https://nginx.aha.io/features/IC-97

@github-actions
Copy link

Hi @abhisek-dwivedi thanks for reporting!

Be sure to check out the docs while you wait for a human to take a look at this 🙂

Cheers!

@pleshakov
Copy link
Contributor

Hi @abhisek-dwivedi

Is it possible to run IC with readOnlyRootFilesystem: true ? or some specific steps are required to do so ?

We haven't tested if it's possible to run an IC pod with readOnlyRootFilesystem: true, so we don't have any specific steps or documentation.

Note that in addition to /etc/nginx folder, the following folders are also used for writes:

  • /var/cache/nginx - NGINX stores cache related files if caching is enabled
  • /var/lib/nginx - NGINX stores various temp files here. Also, the IC configures NGINX to open a number of auxiliary unix socket in that folder.

@pleshakov pleshakov added the proposal An issue that proposes a feature request label Jun 21, 2021
@abhisek-dwivedi
Copy link
Author

abhisek-dwivedi commented Jun 22, 2021

Okay, thanks.

Would it be possible to merge all these folders to just one common folder by setting some variable in code and mounting that path as an emptyDir volume externally?
If not, how about mounting 3 emptyDir volumes for each path wherever IC pod writes.

Not sure if that's going to work. Let me know your thoughts.

Thanks

@brianehlert
Copy link
Collaborator

Hello @abhisek-dwivedi
I understand the readOnlyRootFileSystem ask.
I am thinking more about what you are trying to achieve.
Would support for the nginx user directive or running nginx as non-root meet the desired outcome?
Or, is the primary issue around "AllowPrivilegeEscalation"

I am really trying to fully understand what you are trying to accomplish as opposed to focusing on one solution that you are asking about. As there are other possible approaches.

@abhisek-dwivedi
Copy link
Author

Yeah totally agree that there are other possible good approaches. I was just playing around with IC to make it work as per my use case.

I have a product that runs along with IC, so you can say there is a hard dependency on it. But now the problem is that my product runs with minimum Pod Security permission without AllowPrivilegeEscalation and readOnlyRootFileSystem. But now there are some customer's k8s environments that are tightly constrained using PSP or any other Policy engine that doesn't allow the above permission. So in that case IC controller breaks or not allowed to run. As a result, my product functionality breaks.

Also, normal helm chart test/linter tools flag these kinds of permission as well.

FYI: I am able to get pass-through root user and AllowPrivilegeEscalation permission by making some changes in the code and docker file. Now my IC runs with a non-root user and AllowPrivilegeEscalation: false but I am stuck on removing readOnlyRootFileSystem permission

@pleshakov
Copy link
Contributor

Note about AllowPrivilegeEscalation

To allow NGINX to bind to privileged ports like 80 and 443, we add the NET_BIND_SERVICE capability to the nginx binary (https://github.com/nginxinc/kubernetes-ingress/blob/master/build/Dockerfile#L267) and also allow this capability in the pod security context -- https://github.com/nginxinc/kubernetes-ingress/blob/master/deployments/deployment/nginx-ingress.yaml#L42-L46 When the IC container starts, the container runtime launches the IC process, which in turn starts NGINX. Because permitted capabilities of NGINX are different from permitted capabilities of IC (the IC process is not allowed to bind to privileged ports), it is necessary to configure allowPrivilegeEscalation: true.

IC process (cat /proc/1/status | grep Cap in the container):

CapInh: 0000000000000400
CapPrm: 0000000000000000
CapEff: 0000000000000000
CapBnd: 0000000000000400
CapAmb: 0000000000000000

NGINX master process (cat /proc/<pid-of-NGINX-master>/status | grep Cap in the container):

CapInh: 0000000000000400
CapPrm: 0000000000000400
CapEff: 0000000000000400
CapBnd: 0000000000000400
CapAmb: 0000000000000000

@brianehlert brianehlert added the enhancement Pull requests for new features/feature enhancements label Jul 2, 2021
@brianehlert brianehlert added this to the Candidates milestone Aug 10, 2021
@wer-anders
Copy link

I also want to have the nginx-controller container work with a readOnlyFilesysstem. My quick-and-dirty solution is

  1. to move all files in the respecting paths to a temporary path in the container,
  2. mount the paths in kubernetes with emptydir
  3. update the entrypoint to copy back the files before starting nginx.

My entrypoint (make it executable):

#!/bin/bash

cp -a /usr/local/nginx/etc/* /etc/nginx
cp -a /usr/local/nginx/lib/* /var/lib/nginx
cp -a /usr/local/nginx/ingress-controller/* /etc/ingress-controller

/usr/bin/dumb-init $@

My Dockerfile

FROM ingress-nginx/controller:v0.49.0@sha256:e9707504ad0d4c119036b6d41ace4a33596139d3feb9ccb6617813ce48c3eeef

USER 0
RUN mv /etc/nginx /usr/local/nginx/etc;
mv /var/lib/nginx /usr/local/nginx/lib;
mv /etc/ingress-controller /usr/local/nginx/ingress-controller;
mkdir -p /etc/nginx /var/lib/nginx /etc/ingress-controller;
chown 101:101 /etc/nginx /var/lib/nginx /etc/ingress-controller;

USER 101
COPY --chown=101:101 entrypoint /

ENTRYPOINT ["/entrypoint"]

Kubernetes adjustments:

      volumeMounts:
        ...
        - name: tmp
          mountPath: /tmp
        - name: etc
          mountPath: /etc/nginx
        - name: ingresscontroller
          mountPath: /etc/ingress-controller
        - name: cache
          mountPath: /var/cache/nginx
        - name: lib
          mountPath: /var/lib/nginx

  volumes:
    ...
    - name: tmp
      emptyDir: {}
    - name: etc
      emptyDir: {}
    - name: ingresscontroller
      emptyDir: {}
    - name: cache
      emptyDir: {}
    - name: lib
      emptyDir: {}

Doing it like this you can also run this container the old way.

@brianehlert
Copy link
Collaborator

@sigv
Copy link
Contributor

sigv commented Feb 17, 2023

I am proposing a PR to configure a readonly rootfs.

However this feature request sneaks in a second item: allowPrivilegeEscalation: true.
As previously explained, this is done for Nginx to bind ports 80 and 443.
However, Docker v20.10.0 (released 2020-12-08) should support binding privileged ports with no capabilities due to moby/moby#41030 merged as moby/moby@888da28 which sets sysctl net.ipv4.ip_unprivileged_port_start=0.
Similarly, Enabling Unsafe Sysctls in K8s documentation states net.ipv4.ip_unprivileged_port_start is considered a safe sysctl since Kubernetes 1.22.
Should I spin off a second tracking issue for evaluation of removing NET_BIND_SERVICE and allowPrivilegeEscalation from Nginx IC, for removal of setcap and more?


NB! Moved to #3544 (comment).

@brianehlert
Copy link
Collaborator

Lets separate the concerns of readOnlyRoot from allowPrivelegeEscalation
I think that will make it easier to handle.

We have another submitter that is looking at privilege escalation. #3544

And we also have another capability in the works to make defining listeners on additional ports much easier, specifically for HTTP/S that will mesh nicely with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Pull requests for new features/feature enhancements proposal An issue that proposes a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants