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

Non root user #244

Merged
merged 3 commits into from
Jul 7, 2022
Merged

Non root user #244

merged 3 commits into from
Jul 7, 2022

Conversation

zolug
Copy link
Collaborator

@zolug zolug commented Jun 28, 2022

Description

Issue link

Checklist

  • Purpose
    • Bug fix
    • New functionality
    • Documentation
    • Refactoring
    • CI
  • Test
    • Unit test
    • E2E Test
    • Tested manually
  • Introduce a breaking change
    • Yes (description required)
      Meridio images now contain a numeric user that a container will be started with by default. Use "runAsUser: 0" to keep running as root.
    • [] No
  • Introduce changes in the Operator
    • Yes (description required)
      Deployment templates are modified to provide the required securityContext (runAsNonRoot and capabilities)
    • No

@zolug
Copy link
Collaborator Author

zolug commented Jun 29, 2022

/reverify

@zolug
Copy link
Collaborator Author

zolug commented Jun 29, 2022

If implemented the following nsm improvement item could help to get rid of some of the CAP_DAC_OVERRIDE capabilities required to access the nsm-socket: networkservicemesh/cmd-nsmgr#510

Copy link
Member

@LionelJouin LionelJouin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ctraffic container is only a testing container so the changes were not really needed in the yaml nor the dockerfile. Is it possible these changes affect the way to test/debug with the ctraffic container?

Could you modify the documentation for the TAPA integration in the application pod? (debug capabilities can be avoided in this documentation)
https://github.com/Nordix/Meridio/blob/master/docs/user-application.md

Otherwise I can see new capabilities in the proxy, load-balancer, nsp, ipam and fe containers, are they used for debugging or they are required?
If they are required, then please update the list in the documentation:
https://github.com/Nordix/Meridio/blob/master/docs/proxy.md#privileges
https://github.com/Nordix/Meridio/blob/master/docs/load-balancer.md#privileges
https://github.com/Nordix/Meridio/blob/master/docs/nsp.md#privileges
https://github.com/Nordix/Meridio/blob/master/docs/ipam.md#privileges
https://github.com/Nordix/Meridio/blob/master/docs/front-end.md#privileges

Otherwise, I tried the changes, it works fine

@zolug
Copy link
Collaborator Author

zolug commented Jun 30, 2022

The ctraffic container is only a testing container so the changes were not really needed in the yaml nor the dockerfile. Is it possible these changes affect the way to test/debug with the ctraffic container?

Yeah, I know, but it's also the container using/interacting with the tapa sidecar. So I wanted to showcase, that basically no capabilities are required to use it (except for the ones required by the debug tools or maybe the application that would rely on secondary networking).
Initially, I wanted to introduce a dedicated container to interact with the ambassador socket, but I discarded that idea.

Otherwise, tools like mconnect, ctraffic, ping, tcpdump, netstat, ss, strace, netcat (unless wanted to bind to ports < 1024) should be working fine. On the other hand, the container could be still ordered to run as root through the "runAsUser:0" sercurityContext option, in case you think it would be safer because of the e2e tests for example.

Could you modify the documentation for the TAPA integration in the application pod? (debug capabilities can be avoided in this documentation) https://github.com/Nordix/Meridio/blob/master/docs/user-application.md

Sure, I will do that.

Otherwise I can see new capabilities in the proxy, load-balancer, nsp, ipam and fe containers, are they used for debugging or they are required? If they are required, then please update the list in the documentation: https://github.com/Nordix/Meridio/blob/master/docs/proxy.md#privileges https://github.com/Nordix/Meridio/blob/master/docs/load-balancer.md#privileges https://github.com/Nordix/Meridio/blob/master/docs/nsp.md#privileges https://github.com/Nordix/Meridio/blob/master/docs/ipam.md#privileges https://github.com/Nordix/Meridio/blob/master/docs/front-end.md#privileges

Some of them are required by the main binary/binaries. I will update the docs.

Otherwise, I tried the changes, it works fine

Thanks. Have you maybe also tried running e2e tests with it? Just to see, that e.g. ctraffic container provides access to all the necessary tools needed.

@LionelJouin
Copy link
Member

Great thank you. No I haven't tried with the e2e tests, I only tried manually.

zolug added 3 commits July 5, 2022 18:13
Allow TAPA sidecar users running as non-root to access the
ambassador socket without the need of additional privileges/tricks.
-Docker images (except NSC and Remote Vlan NSE) now include a non-root user
-In order to fallback running as root use "runAsUser: 0" for example
-Access of mounted volumes is achieved through fsGRoup mainly
-In case of hostPaths fsGroup is useless, so CAP_DAC_OVERRIDE provides access
 to the shared items if file permissions do not offer expected access mode
 (e.g. nsm-socket lacks write permission for 'others', while spire-socket
 grants read/write/execute permissions)
-file capabilities are applied to supplement running as non-root, because the
 ambient capability set of the container is "cleared" in such cases
-Binaries proxy, load-balancer and tapa need CAP_DAC_OVERRIDE to use nsm-socket
-images contain debug/troubleshooting tools that require capabilites (e.g. ping,
 tcpdump, netstat, ss)
-As nfqlb utilizes shared memory it also requires CAP_IPC_LOCK,CAP_IPC_OWNER
-BIRD requires CAP_NET_BIND_SERVICE if expected to bind to standard BGP port 173.
 Also, interaction between BIRD and frontend is secured by fsGroup and writable
 volume mounts.
-User applications with TAPA sidecar must access the meridio ambassador unix socket.
 This can be achieved through using fsGroups, or in case the ambassador socket has
 rwx permissions for "others", then no additional settings are required.

 Note:
 In development environments (e.g. Kind, xcluster) in case of hostPath based
 persistent storage methods (e.g Rancher's Local Path) the mounted directory's
 file permissions will allow 'rwx' access for "others". So no additional
 capabilities are needed by IPAM and NSP. However access problems are expected,
 if the persistent storage file already exists before an upgrade from a root user
 based deployment (or simply the UID changes).
@zolug
Copy link
Collaborator Author

zolug commented Jul 6, 2022

/reverify

@zolug
Copy link
Collaborator Author

zolug commented Jul 7, 2022

#15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants