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

systemd does not invoke pam_end properly #22318

Closed
spaetz opened this issue Jan 31, 2022 · 5 comments · Fixed by #22341
Closed

systemd does not invoke pam_end properly #22318

spaetz opened this issue Jan 31, 2022 · 5 comments · Fixed by #22341
Labels
bug 🐛 Programming errors, that need preferential fixing
Milestone

Comments

@spaetz
Copy link

spaetz commented Jan 31, 2022

This is a spinoff of #17564 (comment) which is about enabling WakeSystem= for user units.

libcap and (previous) PAM author AndrewGMorgan comments that systemd likely does not use the pam API correctly with regard to pam_end(). I cannot claim to understand half of it, but the linked comment contains more pointers to likely code culprits. It would be great if somebody familiar with the systemd code base could look if that is a problem, in order to unblock the WakeSystem progress.

Specifically, the lack of the PAM_DATA_SILENT flag in pam_end (

pam_end(handle, pam_code | flags);
) seems to be wrong.

Thanks for systemd!

@yuwata yuwata added the bug 🐛 Programming errors, that need preferential fixing label Jan 31, 2022
@yuwata yuwata added this to the v251 milestone Jan 31, 2022
@spaetz
Copy link
Author

spaetz commented Jan 31, 2022

Digging deeper, I am a bit puzzled, systemd does invoke pam_end, it just does not add the FLAG PAM_DATA_SILENT, which looking at the documentation does not seem too critical, but then I don't really know PAM...

@AndrewGMorgan
Copy link

The PAM_DATA_SILENT flag is the way that pam_end() communicates to the module stack that this invocation of pam_end() is not the final one, but in the process that is going to directly exec the child. Without this flag, all of the modules assume that PAM is fully shutting down and they should fully delete and cleanup all credentials etc.

@poettering
Copy link
Member

@AndrewGMorgan so in a better world the flag would be named PAM_CALLING_IN_CHILD or so? or maybe PAM_NOT_CALLING_IN_PARENT?

@spaetz
Copy link
Author

spaetz commented Jan 31, 2022

Uhh, thanks for the background info. Comparing to similar flags such as PAM_SILENT which were more about suppressing user logging, this flag seems to have much more serious purposes. Even more than a "makes PAM not take the call too seriously" (paraphrased from the manpage) :-)

@AndrewGMorgan
Copy link

Possibly, but it has been named this way since around (I suspect before) the year 2000.

I seem to recall there were some use cases wanting to have a hook in the PAM stack that executed a callback after the application called setuid() but before it invoked exec*(). There was a desire to drop/wipe references to some kernel objects at this stage etc. but not all of them---the session closing still needed them around in some form.

However, it was normally the case that the terminating (full end) pam_close_session() ... pam_end() for the application was invoked in a process that had never called setuid(). The decision at that time was to use this flag to support a mini-end protocol.

poettering added a commit to poettering/systemd that referenced this issue Feb 1, 2022
yuwata pushed a commit to yuwata/systemd that referenced this issue Feb 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Programming errors, that need preferential fixing
Development

Successfully merging a pull request may close this issue.

4 participants