D23849: Do not allow the greeter to send the same password multiple times (Fix GPG smart card access)

[Jason Franklin]

jfranklin created this revision.
jfranklin added reviewers: KWin, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
jfranklin requested review of this revision.

REVISION SUMMARY
  Greetings,
  
  I am working on improving some of the functionality in the screen locker.  Specifically, I am attempting to fix this bug <https://lists.gnupg.org/pipermail/gnupg-devel/2019-September/034443.html> reported to the gnupg-devel mailing list.  See also a bug that I reported to the Debian package maintainer: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=934185.  That bug report has not received a response, so I've come here.
  
  Some background: The libpam-poldi <https://packages.debian.org/buster/libpam-poldi> package for Debian allows for 2FA with GnuPG smart cards into a Linux workstation.  This functionality is very imporant to me and to my organization.  This patch will fix a problem with the conversation between Poldi and KScreenLocker.
  
  To reproduce the bug using libpam-poldi and the screen locker:
  
  1. arrive at the screen locker
  2. remove and re-insert the GPG smart card
  3. enter a PIN less than 6 characters in length
  4. Bam! The screen locker will stop responding and your `/var/log/auth.log` file will fill up with "PIN too short" messages
  
  For GPG smart card users, this issue is critical, because it means the machine can become unusable if you accidentally hit enter too early.
  
  I will summarize my solution here:
  
  1. The libpam-poldi maintainer discovered that a `PAM_TEXT_INFO` message followed by a `PAM_PROMPT_ECHO_OFF` from the PAM module would result in an infinite loop in the conversation between `kcheckpass` and the greeter.
  2. The infinite loop was caused by the fact that the greeter would keep sending the password that was initially entered over and over without prompting the user again, even though the PAM module was requesting another prompt.
  3. Seeing this, I searched for a compact way to ensure that the password would only be passed to `kcheckpass` once and that passing another password would *require* a prompt.
  4. This was accomplished by clearing the password after sending it from the greeter to `kcheckpass`.
  5. Now, a subsequent request for another password will return the `nullptr`, which will abort the current PAM conversation.  Naturally, this has the same effect as starting the conversation over again (we're at the screen locker, after all), so we continue waiting for input again.  This explains the fall through in the switch statement (see diff).
  
  I worked hard to come up with a solution that would have a minimal impact on the code.  I think this patch accomplishes this, and it certainly fixes my immediate problem.  It also does not seem to introduce any regressions.  I hope testing will reveal these if they are there.
  
  Thanks,
  Jason Franklin

TEST PLAN
  I need all the help testing/reviewing this that I can get.  It works very well in my set up so far.

REPOSITORY
  R133 KScreenLocker

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

AFFECTED FILES
  greeter/authenticator.cpp
  kcheckpass/checkpass_pam.c

To: jfranklin, #kwin, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190910/288e7d8b/attachment-0001.html>