D27935: Make kwallet-pam work with pam_fscrypt

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> aacid wrote in pam_kwallet.c:329
> That is a good question, the old code was kind of prepared for it.
> 
> I am going to say "no" open can not be called before authenticate, if you read https://pubs.opengroup.org/onlinepubs/008329799/pam_open_session.htm it says
> 
> "The pam_open_session() function opens a new session for a user previously authenticated with a call to pam_authenticate()."
> 
> But my pam knowledge is between none and i googled a little, so I would be happy if someone can google a bit more and agree/disagree with me

Alex did add this check in a dedicated commit 634464255a82de55e0288f7e425e50f6c409f51d <https://phabricator.kde.org/R107:634464255a82de55e0288f7e425e50f6c409f51d> and even though I couldn't find a subsequent commit that made use of that preparation I'm sure he must have had a reason to add it. Perhaps it was this:

http://www.linux-pam.org/Linux-PAM-html/mwg-expected-of-module-overview.html#mwg-expected-of-module-overview-1

> The independence of the four groups of service a module can offer means that the module should allow for the possibility that any one of these four services may legitimately be called in **any order**. Thus, the module writer should consider the appropriateness of performing a service without the prior success of some other part of the module.

gnome-keyring, as the most topically relevant example, doesn't explicitly have 'open called before' but its written in a very particular way. Both auth and session can unlock the keyring (and attempt a daemon start) it seems. Not really the same, but there's certainly some nuance to how modules may get called.

So, the original check for sm_open_session in our code may have been not entirely pointless, it does kinda meet that any-order expectation. It probably also wasn't entirely correct though as it didn't meet the independence expectation ^^

Removing it leaves me a bit uneasy without input from someone who knows more about pam and the two of us. At the same time it's clearly not quite right either.

How about leaving it in for 5.18 and drop it for master?
Should there be an unexpected problem we'll at least have months of theoretical testing and a shorter window from 5.19.0 to .1 to hotfix a potential regression.

REPOSITORY
  R107 KWallet PAM Integration

REVISION DETAIL
  https://phabricator.kde.org/D27935

To: aacid
Cc: sitter, security-team, davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200316/986ccfd3/attachment-0001.html>