Add support for pam-kwallet in kwalletd
Review Request #116555 - Created March 2, 2014 and submitted
This patch adds support for pam-kwallet (in my scratch right now, to be released soon). This is how the new pam works, and why this patch is needed: In order to open the wallet in a secure way we have to try hard to not send the hash on an insecure manner. This is how we achieve that: -pam_kwallet creates a pipe. -pam_kwallet opens a local socket listening somewhere (/tmp/foo.socket for example). -pam_kwallet forks+execv kwallet, passing via arguments the sockets (pipe and local socket). -pam_kwallet sends the hash via the pipe. -kwalletd gets the hash and waits for the environment. -startkde uses "socat" to send the environment to kwalletd. -kwalletd setups the environment before any Qt code is executed. -kwalletd resumes execution. With this way of executing kwallet we get: -pam_kwallet knows to who it is sending the hash (its on child). -hash is never revealed on shared memory (dbus), since pipes are private to the apps. -ptrace is usually disabled so only root can see the hash on the app memory -no Qt code is executed without the proper environment (same as startkde) -if kwalletd is executed normally (not from pam_kwallet) then it is business as usual. The patch also comes with integration tests that simulate how kwalletd is executed in the pam module. For the release team, I would love to add this to 4.13, after all it is innocuous if kwalletd is not executed via pam_module.
|Make this function static like the other ones?||Albert Astals Cid|
The Beta 1 of 4.13 is on Wednesday. Can the maintainer of the code affected by this give a evaluation of how "dangerous" this patch is and his recommendation for the Freeze exception?
For the release time, since this is really like I would like to give you my PoV on what the worse case scenarios are. -If the kwalletd is not executed using pam, then nothing will happen. This patch will do nothing if --pam-login is not passed as an argument. -If the argument is passed and the environment variable is set (which means kwalletd has been executed by the pam module) then we will attempt to read from the specified sockets the required data to open the pam. In the worse case scenario, lets imagine kwalletd crashes or something similar still the user will not notice a thing, since kwalletd will get executed again by libkwallet, and this time it won't have the "--login-pam" argument, so it won't be affected by this patch. So, to summary it: -It is impossible this breaks anything even if you use the pam module.
With my Release Team member hat i don't oppose to this change ending up in 4.13. Please wait until wednesday mid-day to see if anyone has any opposing comment and if not feel free to commit it to 4.13 before the Beta 2 tag.
@Alex, please just make sure to release pam-wallet before next beta ;-) (also, it would be nice to have a CMake indication about pam-wallet dependency - from patch, it looks it's a runtime only dep?)