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

starter: introduce possibility to keep capability NET_BIND_SERVICE #650

Merged

Conversation

mhofstetter
Copy link
Member

Currently, the "starter" drops all capabilities before starting the Envoy process itself. This is mainly to prevent the Envoy process from having the capabilities NET_ADMIN, SYS_ADMIN etc.

But this change also comes with the drawback that it prevents Envoy from binding to privileged ports - because the capability NET_BIND_SERVICE gets dropped as well.

Therefore, this commit introduces a new flag --keep-cap-net-bind-service to the starter. If this flag is present, the capability NET_BIND_SERVICE is kept in the privileged and effective set for the Envoy process.

@mhofstetter mhofstetter force-pushed the pr/mhofstetter/cap-keep-netbindservice branch 2 times, most recently from 9140ddf to 4ce0a7b Compare March 18, 2024 14:30
starter/main.cc Outdated Show resolved Hide resolved
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/cap-keep-netbindservice branch 2 times, most recently from 46ab5fa to 79fe198 Compare April 5, 2024 13:16
Copy link
Member

@jrajahalme jrajahalme left a comment

Choose a reason for hiding this comment

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

Some style issues and one bug.

starter/main.cc Outdated Show resolved Hide resolved
starter/main.cc Outdated Show resolved Hide resolved
starter/main.cc Outdated Show resolved Hide resolved
starter/main.cc Outdated Show resolved Hide resolved
starter/main.cc Show resolved Hide resolved
starter/main.cc Show resolved Hide resolved
@mhofstetter
Copy link
Member Author

Some style issues and one bug.

Thanks Jarno! i incorporated your feedback.

@mhofstetter mhofstetter force-pushed the pr/mhofstetter/cap-keep-netbindservice branch from dc15efd to 5518409 Compare April 22, 2024 14:54
@mhofstetter
Copy link
Member Author

rebased to main to fix issues with integration tests ? 🙏 🤞

@mhofstetter mhofstetter marked this pull request as ready for review April 23, 2024 08:49
@mhofstetter mhofstetter requested a review from a team as a code owner April 23, 2024 08:49
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/cap-keep-netbindservice branch 3 times, most recently from e9d992b to 242b9fc Compare April 23, 2024 12:03
Currently, the "starter" drops all capabilities before starting the
Envoy process itself. This is mainly to prevent the Envoy process
from having the capabilities `NET_ADMIN`, `SYS_ADMIN` etc.

But this change also comes with the drawback that it prevents Envoy from
binding to privileged ports - because the capability `NET_BIND_SERVICE`
gets dropped as well.

Therefore, this commit introduces a new flag `--keep-cap-net-bind-service` to
the starter. If this flag is present, the capability `NET_BIND_SERVICE` is kept
in the privileged and effective set for the Envoy process.

Signed-off-by: Marco Hofstetter <[email protected]>
With this commit, the end of the arguments for the starter need to be
signaled with `--`. It is followed by the actual arguments for the Envoy
process.

For backwards compatibility reasons, all arguments are handled as Envoys
if the separator `--` isn't passed to the starter.

Signed-off-by: Marco Hofstetter <[email protected]>
@mhofstetter mhofstetter force-pushed the pr/mhofstetter/cap-keep-netbindservice branch from 242b9fc to 7076e6f Compare April 23, 2024 12:07
@jrajahalme jrajahalme added this pull request to the merge queue Apr 23, 2024
Merged via the queue into cilium:main with commit 682395d Apr 23, 2024
5 checks passed
@mhofstetter mhofstetter removed the dont-merge/preview-only DON'T MERGE label Apr 24, 2024
@mhofstetter mhofstetter deleted the pr/mhofstetter/cap-keep-netbindservice branch April 24, 2024 06:30
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Apr 24, 2024
This commit updates Envoy (Cilium Proxy) to the latest version from
(`ciilum/proxy` - `main`) that includes support to keep the capability
`NET_BIND_SERVICE`.

Relates to: cilium/proxy#650

Signed-off-by: Marco Hofstetter <[email protected]>
mhofstetter added a commit to mhofstetter/proxy that referenced this pull request Apr 25, 2024
Currently, in case of not passing the argument delimiter `--`, the arguments are
added before the actual program name. This results in errors starting up Envoy.

This commit fixes this by inserting the arguments at the end.

Fixes: cilium#650

Signed-off-by: Marco Hofstetter <[email protected]>
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Apr 25, 2024
This commit updates Envoy (Cilium Proxy) to the latest version from
(`ciilum/proxy` - `main`) that includes support to keep the capability
`NET_BIND_SERVICE`.

Relates to: cilium/proxy#650

Signed-off-by: Marco Hofstetter <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Apr 25, 2024
Currently, in case of not passing the argument delimiter `--`, the arguments are
added before the actual program name. This results in errors starting up Envoy.

This commit fixes this by inserting the arguments at the end.

Fixes: #650

Signed-off-by: Marco Hofstetter <[email protected]>
mhofstetter added a commit to mhofstetter/cilium that referenced this pull request Apr 25, 2024
This commit updates Envoy (Cilium Proxy) to the latest version from
(`ciilum/proxy` - `main`) that includes support to keep the capability
`NET_BIND_SERVICE`.

Relates to: cilium/proxy#650

Signed-off-by: Marco Hofstetter <[email protected]>
github-merge-queue bot pushed a commit to cilium/cilium that referenced this pull request Apr 29, 2024
This commit updates Envoy (Cilium Proxy) to the latest version from
(`ciilum/proxy` - `main`) that includes support to keep the capability
`NET_BIND_SERVICE`.

Relates to: cilium/proxy#650

Signed-off-by: Marco Hofstetter <[email protected]>
@sayboras sayboras mentioned this pull request May 1, 2024
2 tasks
jrajahalme pushed a commit that referenced this pull request May 3, 2024
Currently, in case of not passing the argument delimiter `--`, the arguments are
added before the actual program name. This results in errors starting up Envoy.

This commit fixes this by inserting the arguments at the end.

Fixes: #650

Signed-off-by: Marco Hofstetter <[email protected]>
jrajahalme pushed a commit that referenced this pull request May 3, 2024
[ upstream commit 174c6af ]

Currently, in case of not passing the argument delimiter `--`, the arguments are
added before the actual program name. This results in errors starting up Envoy.

This commit fixes this by inserting the arguments at the end.

Fixes: #650

Signed-off-by: Marco Hofstetter <[email protected]>
jrajahalme pushed a commit that referenced this pull request May 3, 2024
[ upstream commit 174c6af ]

Currently, in case of not passing the argument delimiter `--`, the arguments are
added before the actual program name. This results in errors starting up Envoy.

This commit fixes this by inserting the arguments at the end.

Fixes: #650

Signed-off-by: Marco Hofstetter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants