Skip to content

feat: impersonator middleware feature/usability improvements#353

Closed
leo9800 wants to merge 4 commits intomar10:masterfrom
leo9800:impersonator-refactor
Closed

feat: impersonator middleware feature/usability improvements#353
leo9800 wants to merge 4 commits intomar10:masterfrom
leo9800:impersonator-refactor

Conversation

@leo9800
Copy link
Copy Markdown
Contributor

@leo9800 leo9800 commented Dec 12, 2025

this patch tackles 3 major issues of the current implementation of impersonator:

1. handling additional groups

unix users could retrieve groups they belongs with id command, it is common that a user belongs to multiple additional groups than the one which named identical to him/herself. here below is an example retrieving my groups on my linux workstation:

$ id
uid=1000(leo) gid=1000(leo) groups=1000(leo),150(wireshark),970(libvirt),971(docker),998(wheel)

the current implementation, wsgidav only call os.setegid(1000) when impersonating as leo on this machine, while other groups, incl. wireshark, docker, etc, should also be added.

this patch handles such cases with os.initgroups() to properly add additional groups accordingly (to /etc/group) when impersonating.

2. rejecting system users

uids <= 999 on unix systems are generally preserved for system daemon uses, a sysadmin may not want someone being capable to impersonate as such users. (probably due to misconfiguration of domain controller, etc)

this patch adds an option to reject impersonating-as-system-users attemps:

impersonator:
    reject_system_users: true

3. docker capability issues

docker's --cap-add (or cap_add: in docker-compose.yml) does not add the capabilities to the ambient set, therefore the capability is dropped upon fork()/execve(), (which is behind gunicorn's worker spawning) rendering the approach described in #343 (comment) not feasible.

this patch included a statically-linked helper program in C to achieve this in containers.

suid-wrapper nobody wsgidav -c /path/to/wsgidav.yaml

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.66%. Comparing base (1a63cc0) to head (ae33f5f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #353   +/-   ##
=======================================
  Coverage   43.66%   43.66%           
=======================================
  Files          31       31           
  Lines        4967     4967           
=======================================
  Hits         2169     2169           
  Misses       2798     2798           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mar10
Copy link
Copy Markdown
Owner

mar10 commented Jan 25, 2026

Thanks and sorry for the delay.
Currently I don't have much resources for this project and I cannot fully assess the potential security implications.
Since you implemented the original 'impersonator' as separate, optional (opt-in) middleware marked as 'experimental, that was ok, but the docker and c extension should rather be moved in a separate location.

@mar10 mar10 self-requested a review January 25, 2026 08:14
@mar10 mar10 marked this pull request as draft January 25, 2026 08:14
@leo9800 leo9800 force-pushed the impersonator-refactor branch from 44a2fcd to 484d22f Compare February 2, 2026 16:11
Copy link
Copy Markdown
Owner

@mar10 mar10 left a comment

Choose a reason for hiding this comment

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

Thanks and sorry for the delay.
Currently I don't have much resources for this project and I cannot fully assess the potential security implications.
Since you implemented the original 'impersonator' as separate, optional (opt-in) middleware marked as 'experimental, that was ok, but the docker and c extension should rather be moved in a separate location.

leo9800 added 4 commits April 9, 2026 14:09
… system users (uid <= 999)

sample_wsgidav.yaml: configuration entry for the feature above

Signed-off-by: Leo <i@hardrain980.com>
…d CAP_SETGID

docker `--add-cap` (or `add_cap` in docker-compose.yml) could not set ambient capabilities
and therefore the SUID/SGID caps are not inherited by the child process spawned by gunicorn.

this wrapper mimics what `capsh` do in mar10#343

Signed-off-by: Leo <i@hardrain980.com>
…ages

Signed-off-by: Leo <i@hardrain980.com>
@leo9800 leo9800 force-pushed the impersonator-refactor branch from 484d22f to ae33f5f Compare April 9, 2026 06:10
@leo9800 leo9800 closed this Apr 12, 2026
@leo9800 leo9800 deleted the impersonator-refactor branch April 12, 2026 15:47
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