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

Allow configuring runAsGroup attribute of sidecar and init containers #11773

Closed
nico151999 opened this issue Dec 15, 2023 · 9 comments · Fixed by #11924
Closed

Allow configuring runAsGroup attribute of sidecar and init containers #11773

nico151999 opened this issue Dec 15, 2023 · 9 comments · Fixed by #11924

Comments

@nico151999
Copy link
Contributor

nico151999 commented Dec 15, 2023

What problem are you trying to solve?

I would like all containers of my pod to define a runAsGroup attribute in their respective security contexts. I only found a way to configure the runAsUser attribute both via Helm chart and via annotation config.linkerd.io/proxy-uid. However, I could not find a way to define the group ID.

How should the problem be solved?

Make the group ID configurable just like the user ID. I.e. implement a way to consider a config.linkerd.io/proxy-gid annotation when patching a pod and also add the option to the helm chart.

Any alternatives you've considered?

None that wouldn't be too hacky or clash with Linkerd mechanisms

How would users interact with this feature?

Configure the group ID of the proxy and init containers via Helm chart and/or annotation

Would you like to work on this feature?

Yes

@kflynn
Copy link
Member

kflynn commented Jan 4, 2024

@nico151999 This sounds like a good thing to have, you interested in putting together a PR? 🙂

@nico151999
Copy link
Contributor Author

@kflynn Thanks for the reply. Good to hear you would be open for a PR. Yes, I am interested. I will consider it in an upcoming sprint.

@kflynn
Copy link
Member

kflynn commented Jan 5, 2024

@nico151999 Great! Looking forward to seeing it. 🙂

@nico151999
Copy link
Contributor Author

@kflynn Could you have a look at this PR so that a new proxy-init release can be made for my draft PR to have a new image tag that can be referenced? Thanks!

@mateiidavid
Copy link
Member

mateiidavid commented Feb 9, 2024

@nico151999 thanks for working on the change. I've had a look at both PRs. Everything looks good at a high level, but it's a little bit hard for me to navigate the changes without having a bit more context into why the changes are necessary to begin with. This is the kind of thing that looks very straightforward to implement but might have some unintended side effects.

Would you mind elaborating on what your concrete use cases are in using runAsGroup and how you intend to use it? Why doesn't runAsUser suffice to restrict permissions?

@nico151999
Copy link
Contributor Author

Thanks for taking a look @mateiidavid. From a technical perspective there is no specific case that comes to my mind when I would need a runAsGroup setting. From a security perspective there are some companies requiring you to set both runAsUser and runAsGroup in your manifest. For example, if you operate on a K8s cluster like this and an OPA gatekeeper enforces both attributes to be set you cannot deploy Linkerd on these clusters. Also, I would claim Linkerd to be more complete if not only runAsUser but both would we configurable. When it comes to reviewing the PRs most of the files are only extended by the respective group ID attribute. There is not much logic in it. I cannot promise there to be no side effects but my adaptions to proxy-init have worked well so far on my dev cluster and it still seemed to do its job without any issues.

@nico151999
Copy link
Contributor Author

@mateiidavid I did some research on the reason why setting runAsGroup is recommended by many security departments. The sources I found targeting the issue of potentially unset runAsGroup settings seem plausible and explain why this setting makes it even more unlikely for the container and the host system to be compromised. A good reference I found is this. The author explains why it can lower the potential threats to the system if runAsUser, runAsGroup and fsGroup are set. Of course, a user can currently set these on the pod level but defining it on the container level allows for more controllability and at least lowers the risk for the linkerd sidecar container to be compromised which would be a considerable security gain already.
I would be happy to hear back from you, get a review of my PRs and merge the changes. Thanks!

@yzapf
Copy link

yzapf commented Apr 9, 2024

Hi guys, I had a talk to one of your colleagues at KubeCon in Paris about this topic (I'm sorry I forgot his name, but it was a nice talk). Can you give us a current status if we can expect this feature soon or do you have any concerns about the implementation where we can assist?

@mateiidavid
Copy link
Member

@nico151999 @yzapf sorry for the delay on this. I replied to the proxy-init PR. We're happy to push this forward, and the PR looks good. Once we can pass CI and get it on the latest main, we're ok to merge the patch.

alpeb added a commit that referenced this issue May 23, 2024
Fixes #11773

Make the proxy's GUID configurable via `proxy.gid` which defaults to `-1`, in which case the GUID is not set.
Also added ability to set the GUID for proxy-init and the core and extension controllers.

---------

Signed-off-by: Nico Feulner <[email protected]>
Co-authored-by: Alejandro Pedraza <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants