PAM: Also cache SSS_PAM_PREAUTH

[jhrozek]

Related: https://pagure.io/SSSD/sssd/issue/3960

Even if cached_auth_timeout was set, the pam responder would still forward
the preauthentication requests to the back end. This could trigger unwanted
traffic towards the KDCs.

[sumit-bose]

Hi Jakub,

the patch is working as expected, I only added a minor comment to the code.

While testing I came across two issues where I wonder if you would like to fix them with this patch as well.

First, if a wrong password is given cached authentication currently does not fail but falls back to online authentication. I think this behavior make sense, but might be unexpected. A sentence in the man page describing this behavior would be useful imo.

Second, there is no clear debug message from the PAM responder that cached authentication is used. Instead there is

(Fri May  3 13:05:11 2019) [sssd[pam]] [pam_reply] (0x0200): pam_reply called with result [4]: Systemfehler.

and later on there are some message from sysdb_cache_auth(). Maybe in pam_reply() the called with result message can be skipped for cached auth and a more suitable message can be shown?

Both are not related to the issue at hand so feel free to open a new ticket or ignore them.

bye,
Sumit

[jhrozek]

Thank you, I'll look at the System Error. I saw it in my testing, but I stopped after I realised this was not caused by my patches. I should have at least filed a ticket :-)

About the fallback to online auth - I would actually not expect this myself. But at the same time, I also see no pressing reason to change this behaviour (if we agreed it should be changed),, so I agree making this clear in the man page is good enough for now. If our users or customers would be irritated by the behaviour, we can change it later.

[jhrozek]

Thank you for the comments, here is the diff:

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index c12fef731..03d3a104c 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -3046,7 +3046,9 @@ subdomain_inherit = ldap_purge_cache_timeout
                             Specifies time in seconds since last successful
                             online authentication for which user will be
                             authenticated using cached credentials while
-                            SSSD is in the online mode.
+                            SSSD is in the online mode. If the credentials
+                            are incorrect, SSSD falls back to online
+                            authentication.
                         </para>
                         <para>
                             This option's value is inherited by all trusted
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index a70b342b1..89bdb78a1 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -808,8 +808,9 @@ static void pam_reply(struct pam_auth_req *preq)
         pam_verbosity = DEFAULT_PAM_VERBOSITY;
     }

-    DEBUG(SSSDBG_FUNC_DATA,
-          "pam_reply called with result [%d]: %s.\n",
+    DEBUG(SSSDBG_TRACE_ALL,
+          "pam_reply initially called with result [%d]: %s. "
+          "this result might be changed during processing\n",
           pd->pam_status, pam_strerror(NULL, pd->pam_status));

     if (pd->cmd == SSS_PAM_AUTHENTICATE
@@ -891,6 +892,7 @@ static void pam_reply(struct pam_auth_req *preq)
             break;
 /* TODO: we need the pam session cookie here to make sure that cached
  * authentication was successful */
+        case SSS_PAM_PREAUTH:
         case SSS_PAM_SETCRED:
         case SSS_PAM_ACCT_MGMT:
         case SSS_PAM_OPEN_SESSION:
@@ -1072,6 +1074,8 @@ static void pam_reply(struct pam_auth_req *preq)
     }

 done:
+    DEBUG(SSSDBG_FUNC_DATA, "Returning [%d]: %s to the client\n",
+          pd->pam_status, pam_strerror(NULL, pd->pam_status));
     sss_cmd_done(cctx, preq);
 }

@@ -2022,8 +2026,7 @@ static bool pam_can_user_cache_auth(struct sss_domain_info *domain,
         return false;
     }

-    if (domain->cache_credentials == false
-            || domain->cached_auth_timeout <= 0) {
+    if (!domain->cache_credentials || domain->cached_auth_timeout <= 0) {
         return false;
     }

[jhrozek]

On 5/3/19 1:24 PM, sumit-bose wrote: Hi Jakub, the patch is working as expected, I only added a minor comment to the code. While testing I came across two issues where I wonder if you would like to fix them with this patch as well. First, if a wrong password is given cached authentication currently does not fail but falls back to online authentication. I think this behavior make sense, but might be unexpected. A sentence in the man page describing this behavior would be useful imo.

OK, added.

Second, there is no clear debug message from the PAM responder that cached authentication is used. Instead there is |(Fri May 3 13:05:11 2019) [sssd[pam]] [pam_reply] (0x0200): pam_reply called with result [4]: Systemfehler. |

|I thought the message might be sometimes useful, so I only changed its level to be lowest debug level and reworded the message to make it clear that this is an intermediate result only.| ||

and later on there are some message from |sysdb_cache_auth()|. Maybe in |pam_reply()| the |called with result| message can be skipped for cached auth and a more suitable message can be shown? Both are not related to the issue at hand so feel free to open a new ticket or ignore them.

I also added PREAUTH to the list of PAM commands that just return PAM_SUCCESS during cached authentication. I hope that's correct.

[sumit-bose]

Hi,

the patch works for me and the log messages are looking less irritating now.

About:

I also added PREAUTH to the list of PAM commands that just return PAM_SUCCESS during cached authentication. I hope that's correct.

That's fine, pam_sss ignores all error during pre-auth and falls back default behavior, but having PAM_SUCCESS here make things more clear.

ACK

bye,
Sumit

[jhrozek]

• master: c911562
• sssd-1-16: 0a637ff