D27935: Make kwallet-pam work with pam_fscrypt

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> sitter wrote in pam_kwallet.c:329
> 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.

> 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.

Doesn't sound very convincing to me, if there's going to be a regression i'd prefer it to be this month when i still remember this code and i still use kwallet-pam and pam_fscrypt, if you delay it to 5.19.0 or something I will not totally remember this code and may not be using this any of those two either os my motivation to fix it may be pretty small.

I can try adding the code that makes pam_sm_open_session from here if you think it makes sense to have it

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/a62b32b2/attachment-0001.html>