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

Authored by jfranklin on Tue, Sep 10, 7:29 PM.

Details

Reviewers
zzag
Group Reviewers
KWin
Plasma
Summary

Greetings,

I am working on improving some of the functionality in the screen locker. Specifically, I am attempting to fix this bug 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 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.

Diff Detail

Repository
R133 KScreenLocker
Lint
Lint Skipped
Unit
Unit Tests Skipped
jfranklin created this revision.Tue, Sep 10, 7:29 PM
Restricted Application added a project: Plasma. · View Herald TranscriptTue, Sep 10, 7:29 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
jfranklin requested review of this revision.Tue, Sep 10, 7:29 PM

I also didn't point this out in my original post, but I fixed a misuse of pam_end(). The status passed to pam_end() should be the returned status of the last PAM library call. This is noted in the documentation for Linux-PAM.