URL: https://github.com/SSSD/sssd/pull/268 Author: sumit-bose Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT Action: opened
PR body: """ This patchset got lost when I prepared the certificate mapping patch set.
Applications like gdm with enabled Smartcard support will try to determine the user based on data from the certificate or expected that it is done for them and hence do not prompt for a user name. If a certificate is mapped to multiple accounts this cannot work because it is not clear which account should be used. In this case a 'user name hint' must be given by the user to help the application to find the right user.
To get a consistent user experience while still only prompt for a PIN in environments where a certificate is uniquely mapped to only a single user IPA offers the 'ipa certmapconfig-mod --promptusername=BOOL' command to switch 'user name hinting' on or off.
To test the behavior 'sssctl user-checks' can be used. /etc/pam.d/smartcart-auth should be configured for SSSD and like: ... auth required pam_env.so auth sufficient pam_sss.so allow_missing_name auth required pam_deny.so ...
If now a Smartcard in inserted where the certificate is mapped to multiple users you will see:
user: action: auth service: gdm-smartcard
testing pam_authenticate
PIN for AD.DEVEL Admin (OpenSC Card) User name hint: pam_authenticate for user []: Authentication failure
PAM Environment: - PKCS11_LOGIN_TOKEN_NAME=AD.DEVEL Admin (OpenSC Card)
if no user name hint is given and
user: action: auth service: gdm-smartcard
testing pam_authenticate
PIN for AD.DEVEL Admin (OpenSC Card) User name hint: scuser pam_authenticate for user [scuser]: Success
PAM Environment: - PKCS11_LOGIN_TOKEN_NAME=AD.DEVEL Admin (OpenSC Card)
if a suitable user name hint was given. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/268/head:pr268 git checkout pr268
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
abbra commented: """ Is there a technical reason sssd cannot discover what to do without `allow_missing_name` option to pam_sss? I'd prefer to avoid modifying PAM config files... """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-300711379
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
sumit-bose commented: """
Is there a technical reason sssd cannot discover what to do without allow_missing_name option to pam_sss? I'd prefer to avoid modifying PAM config files...
Recent version of authconfig will do the modifications for you and you have to do the change becasue by default pam_pkcs11 will be in the PAM configuration (but this might change in future).
The reason for the option is that by default a missing user name is unexpteded and the pam module will prompt for since in most cases the user name cannot be derived by the other credentials given by the user. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-300713713
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
abbra commented: """ Still, why you cannot make that decision without an option's help? Sorry, I don't see a difference -- why by seeing a certificate `pam_sss` cannot defer decision to decide whether to accept missing name or not to a backend (SSSD) and then act correspondingly? """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-300717514
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
sumit-bose commented: """
Still, why you cannot make that decision without an option's help? Sorry, I don't see a difference -- why by seeing a certificate pam_sss cannot defer decision to decide whether to accept missing name or not to a backend (SSSD) and then act correspondingly?
Because in general I would expect that something is wrong when there is no user name. Typically (currently the only exception is gdm with enabled Smartcard mode) application request the user name on their own and then use it as an argument to pam_start(). Since this is about authentication I'd prefer to error out early instead of sending potentially bogus data to SSSD. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-300733240
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
abbra commented: """ I opened RFE https://pagure.io/SSSD/sssd/issue/3396 to discuss details of this. I believe "sending potentially bogus data to SSSD" is not an argument -- any process can open a socket to SSSD and talk nonsense there if they wanted to spam you with bogus data. Let's discuss actual limitations that prevent you from implementing SSSD-side processing in the RFE ticket. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-300748868
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
jhrozek commented: """ Looks like this PR must be rebased, therefore I'm setting Changes Requested. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-304274559
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/268 Author: sumit-bose Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/268/head:pr268 git checkout pr268
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
sumit-bose commented: """ Rebased to current master which made the last patch obsolete. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-304298269
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
jhrozek commented: """ @fidencio not trying to be too pushy, but these patches should be applied to downstream as well, could you review them? Or if you're busy, I can take a look as well, just tell me your preference.. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-304683135
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
fidencio commented: """ @jhrozek: feel free to also do the review. I'm setting up an environment now in order to test it right now, but one more pair of eyes would be nice. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-304721941
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
fidencio commented: """ I still haven't tested the code, but gave it a quite good read.
There's basically a few comments made "inline" and the most part of those are actually doubts than pointing out issues. The code, in general, looks good (although my OCD got actuvated a few times due to some indentations).
I'm firing a CI build and will test it later Today. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-304776474
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
fidencio commented: """ CI: http://sssd-ci.duckdns.org/logs/job/70/58/summary.html (passed) """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-304788184
URL: https://github.com/SSSD/sssd/pull/268 Author: sumit-bose Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/268/head:pr268 git checkout pr268
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
fidencio commented: """ This version works as expected!
The way I tested the patches:
- promptusername: False ``` [root@client x86_64]# sssctl user-checks "" -a auth -s gdm-smartcard user: action: auth service: gdm-smartcard
testing pam_authenticate
Password: pam_authenticate for user []: Authentication failure
PAM Environment: - no env - ```
- promptusername: True ``` [root@client x86_64]# sssctl user-checks "" -a auth -s gdm-smartcard user: action: auth service: gdm-smartcard
testing pam_authenticate
PIN for PIV Card Holder pin (PIV_II) User name hint: alice pam_authenticate for user [alice]: Success
PAM Environment: - PKCS11_LOGIN_TOKEN_NAME=PIV Card Holder pin (PIV_II) ```
However, I may have found an issue that is not related to this series (as it still happens on master). So, let me ask you whether do you think it's a bug or something expected (due to a miss configuration, maybe?) ...
In case alice doesn't have a password set, `sssctl user-checks alice@sc.ff -a auth -s gdm-smartcard ` works as expected and the authentication is successful just using the PIN.
In case alice does have a password set, `sssctl user-checks alice@sc.ff -a auth -s gdm-smartcard ` still asked for the PIN (which seems correct), but the authentication fails.
As this possible issue is *not* related to this patch set ... I'll ack the series after running coverity/ci on those patches. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-305298996
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
sumit-bose commented: """ Yes, I think this is unrelated. I assume you just set the password as admin but did not use it as the user. In this case the password is expired and must be reset by the user. It looks that although PKINIT was used for authentication the KDC reply still contains information about the expired password and SSSD sends a reply to the PAM client to request a new password.
Can you try to call 'kinit alice@sc.ff' to set a new password and then check Smartcard authentication again. If it then works it is the issue I assumed above and I can try to fix it in a different ticket. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-305430782
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
fidencio commented: """ @sumit-bose, you nailed it. That's exactly the issue you described. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-305433730
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
fidencio commented: """ As I have not noticed any new warning on coverity and as CI passed successfully (although I will not share the link due to issues with our internal CI) and according to the tests done above, I am ACKing this patch set. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-305434511
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
sumit-bose commented: """ jfyi, I opend https://pagure.io/SSSD/sssd/issue/3419 to track the issue with the expired password. """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-305454746
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
lslebodn commented: """ master: * b130adaa3934d0531aca0f32961ab8b4cc720820 * ee7e72a65d323636600ffda271d5b5c4ddbc78b1 * 32474fa2f0a6dc09386bab405fc3461cb3dd12ac * 6073cfc40747cd6d3142f0f98b880fc390dd7aad * a192a1d72e92dae3e71e062b333e51a5095a0395 * 89ff140d7ab92fce52d6730a7d27c8d73c7d9e4a * 749963195393efa3a4f9b168dd02fbcc68976ba3 """
See the full comment at https://github.com/SSSD/sssd/pull/268#issuecomment-305518786
URL: https://github.com/SSSD/sssd/pull/268 Author: sumit-bose Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/268/head:pr268 git checkout pr268
URL: https://github.com/SSSD/sssd/pull/268 Title: #268: pam_sss: add support for SSS_PAM_CERT_INFO_WITH_HINT
Label: +Pushed
sssd-devel@lists.fedorahosted.org
-
abbra -
fidencio
-
jhrozek
-
lslebodn
-
sumit-bose