You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "jvgutierrez (via GitHub)" <gi...@apache.org> on 2024/11/15 19:28:50 UTC
[PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
jvgutierrez opened a new pull request, #11872:
URL: https://github.com/apache/trafficserver/pull/11872
Follow up of https://github.com/apache/trafficserver/pull/11855, that rendered unusable ATS as root when spawned via traffic_manager. As mentioned on #11869, traffic_manager drops privileges when spawned as root and as consequence by the time that traffic_server attempts to call initgroups() it gets an EPERM error.
Fixes #11869
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "midchildan (via GitHub)" <gi...@apache.org>.
midchildan commented on PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#issuecomment-2484723370
I believe this fix will reintroduce the problem in #11855. traffic_manager spawns traffic_server with an unprivileged effective UID and privileged real UID. With this change, traffic_server will keep root's supplementary groups because its EUID won't equal 0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "midchildan (via GitHub)" <gi...@apache.org>.
midchildan commented on code in PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#discussion_r1847729165
##########
src/tscore/ink_cap.cc:
##########
@@ -156,8 +156,10 @@ impersonate(const struct passwd *pwd, ImpersonationLevel level)
#endif
// Always repopulate the supplementary group list for the new user.
- if (initgroups(pwd->pw_name, pwd->pw_gid) != 0) {
- Fatal("switching to user %s, failed to initialize supplementary groups ID %ld", pwd->pw_name, (long)pwd->pw_gid);
+ if (geteuid() == 0) { // check that we have enough rights to call initgroups()
+ if (initgroups(pwd->pw_name, pwd->pw_gid) != 0) {
Review Comment:
I think `initgroups()` should be called at the end of `impersonate()` too so that the supplementary groups would be repopulated when elevating privileges instead of dropping them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "ezelkow1 (via GitHub)" <gi...@apache.org>.
ezelkow1 commented on PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#issuecomment-2489681545
@jvgutierrez if you are happy with this can you merge it so then I can pick it back for a 9.2.x release tomorrow?
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "midchildan (via GitHub)" <gi...@apache.org>.
midchildan commented on PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#issuecomment-2484780053
My bad, I got mixed up with a separate workaround I had for https://github.com/apache/trafficserver/issues/11869.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "jvgutierrez (via GitHub)" <gi...@apache.org>.
jvgutierrez merged PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "ezelkow1 (via GitHub)" <gi...@apache.org>.
ezelkow1 commented on PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#issuecomment-2491667260
cherry picked to 9.2.x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "jvgutierrez (via GitHub)" <gi...@apache.org>.
jvgutierrez commented on code in PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#discussion_r1847013734
##########
src/tscore/ink_cap.cc:
##########
@@ -156,8 +156,10 @@ impersonate(const struct passwd *pwd, ImpersonationLevel level)
#endif
// Always repopulate the supplementary group list for the new user.
- if (initgroups(pwd->pw_name, pwd->pw_gid) != 0) {
- Fatal("switching to user %s, failed to initialize supplementary groups ID %ld", pwd->pw_name, (long)pwd->pw_gid);
+ if (geteuid() != 0) { // check that we have enough rights to call initgroups()
Review Comment:
:sweat_smile: you're totally right, fixed :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#issuecomment-2517821664
Cherry-picked to v10.0.x
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "jvgutierrez (via GitHub)" <gi...@apache.org>.
jvgutierrez commented on code in PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#discussion_r1847952505
##########
src/tscore/ink_cap.cc:
##########
@@ -156,8 +156,10 @@ impersonate(const struct passwd *pwd, ImpersonationLevel level)
#endif
// Always repopulate the supplementary group list for the new user.
- if (initgroups(pwd->pw_name, pwd->pw_gid) != 0) {
- Fatal("switching to user %s, failed to initialize supplementary groups ID %ld", pwd->pw_name, (long)pwd->pw_gid);
+ if (geteuid() == 0) { // check that we have enough rights to call initgroups()
+ if (initgroups(pwd->pw_name, pwd->pw_gid) != 0) {
Review Comment:
that would require to add `CAP_SETGID` to `RestrictCapabilities()`? maybe it's worth moving this discussion to another PR/issue and solve #11869 with this one
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Invoke initgroups() iff we got enough privileges (#11869) [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on code in PR #11872:
URL: https://github.com/apache/trafficserver/pull/11872#discussion_r1846987755
##########
src/tscore/ink_cap.cc:
##########
@@ -156,8 +156,10 @@ impersonate(const struct passwd *pwd, ImpersonationLevel level)
#endif
// Always repopulate the supplementary group list for the new user.
- if (initgroups(pwd->pw_name, pwd->pw_gid) != 0) {
- Fatal("switching to user %s, failed to initialize supplementary groups ID %ld", pwd->pw_name, (long)pwd->pw_gid);
+ if (geteuid() != 0) { // check that we have enough rights to call initgroups()
Review Comment:
This should be `geteuid() == 0`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org