pam: add pam_sss_gss module for gssapi authentication #5367

pbrezina · 2020-10-14T11:04:39Z

The module configuration and usage should be clear
from its manual page. Suggestions are welcome.

The main use case is to enable password-less authentication
for sudo where smartcards are not possible (because the
reader is not available in the target host).

It is not possible to set the service name or keytab at this
moment so the module relies on host/hostname@REALM where hostname
is provided by SSSD (based on the user's domain) and REALM
is determined by GSSAPI. This principal is already stored in
krb5_keytab location for IPA and AD providers so the functionality
is for free.

Because authorization is done by pam_sss and it is not needed
for the use case, I would implement configurable ticket names
and keytab location only when required by users.

frozencemetery · 2020-10-15T21:14:44Z

(I plan to review this once CI passes.)

pbrezina · 2020-10-16T08:40:03Z

Thank you. Tests pass, rhel7 and rhel8 failures are not related (infrastructure issues).

frozencemetery · 2020-10-20T19:05:59Z

I have started review though this is large and will take a bit.

Though on that subject: this is a fairly sizable change, and I don't see anything in the way of tests for it. Have I missed something?

frozencemetery

C stuff. It looks more than it is - with a couple exceptions, I'm mostly going after defensive coding nits and style.

(I expect to do a second pass, pending information on tests, going more thoroughly into the krb5 stuff.)

src/confdb/confdb.c

src/external/libgssapi_krb5.m4

src/man/sssd.conf.5.xml

src/sss_client/pam_sss_gss.c

frozencemetery · 2020-10-20T20:04:13Z

src/sss_client/pam_sss_gss.c

+    gss_ctx_id_t ctx = GSS_C_NO_CONTEXT;
+    gss_buffer_desc input = GSS_C_EMPTY_BUFFER;
+    gss_buffer_desc output = GSS_C_EMPTY_BUFFER;
+    OM_uint32 flags = GSS_C_MUTUAL_FLAG;


Same thing here with flags.

Can you be more specific?

You hardcoded just the mutual flag in two places. This is an odd choice of flags, so I was hoping you could explain why you were doing this in the code. (And then we'll figure out if it's right or not :).)

src/sss_client/pam_sss_gss.c

pbrezina · 2020-10-27T11:49:34Z

Though on that subject: this is a fairly sizable change, and I don't see anything in the way of tests for it. Have I missed something?

We don't currently have framework that would allow to test this. But this is going to change soon so tests will be added later, possibly by QA.

pbrezina · 2020-10-27T11:50:19Z

Pathes were updated.

pbrezina · 2020-10-27T17:18:00Z

Github hide lots of "conversations" from me so there are many unsolved issues so the patches are not yet ready to be reviewed again.

frozencemetery · 2020-10-28T20:08:01Z

the patches are not yet ready to be reviewed again.

Okay, just ping me when you want another review and I'll take a look.

pbrezina · 2020-10-29T12:11:13Z

It's ready for next round. The remaining question is about the proper flag for gssapi otherwise I answered or resolved everything. The issues were mostly cosmetic since we seem to have different coding habits, but since it does not break SSSD's coding style I respected your suggestions.

frozencemetery

No additional comments. As on the other PR, I'm not checking for memory issues.

sumit-bose · 2020-11-25T12:04:46Z

src/responder/pam/pamsrv_gssapi.c

+{
+    char **list = pam_ctx->gssapi_services;
+
+    if (domain->gssapi_services != NULL) {


Hi,

what about sub-domains? I guess if there is no sub-domain specific setting it would be reasonable to check the parent first before falling back to the pam responder setting?

bye,
Sumit

Do you thing it would be useful to allow inheritance of this option and its explicit configuration per subdomain?

Hi,

I was more thinking about what would be expected. E.g. if you join your client to an AD domain which is part of a forest and set pam_gssapi_services in the [domain/...] section my expectation would be that it works for users from other domains in the forest as well.

With inheritance enabled it would be a bit odd to disable the feature for a sub-domain because you have to use pam_gssapi_services = NotExistingServiceName since iirc we cannot handle empty values correctly.

But currently if you want to allow sub-domain user you have to add pam_gssapi_services to the [pam] section which enables it globally. So if you prefer to avoid inheritance I'm fine with only sub-domain specific configuration, but it should be explained in the man page that domain-specific settings must be set for each domain, including sub-domains, individually.

bye,
Sumit

src/man/pam_sss_gss.8.xml

sumit-bose · 2020-11-25T12:45:41Z

Hi,

while testing this PR it looks like KRB5CCNAME is not respected. If there user uses a different ccache type as defined in /etc/krb5.conf or if with default_ccache_name = FILE:/tmp/krb5cc_%{uid}_XXXXXX the ccache name has a random component authentication does not work.

If this is currently and expected limitation it should be documented in the man page..

bye,
Sumit

pbrezina · 2020-11-26T13:43:31Z

while testing this PR it looks like KRB5CCNAME is not respected. If there user uses a different ccache type as defined in /etc/krb5.conf or if with default_ccache_name = FILE:/tmp/krb5cc_%{uid}_XXXXXX the ccache name has a random component authentication does not work.

Hmm, nice catch. Unfortunately sudo resets the environment before PAM invocation so KRB5CCNAME is not available for the PAM module. It can be made available by adding it to env_keep in /etc/sudoers but this will also make it available in the called process and this is probably not what we want so we will have to patch sudo to keep this variable for PAM. I'll ask sudo upstream what are our options.

However, even if the variable is available, gssapi uses the default location:
pam_sss_gss: Initializing GSSAPI authentication with SSSD
pam_sss_gss: Switching euid from 0 to 91600000
pam_sss_gss: Trying to establish security context
pam_sss_gss: User domain: ipa.vm, Target name: host@master.client.vm
pam_sss_gss: Unable to establish GSS context [maj:0xd0000, min:0x96c73a8d]
pam_sss_gss: GSSAPI: Unspecified GSS failure. Minor code may provide more information
pam_sss_gss: GSSAPI: No Kerberos credentials available (default cache: FILE:/tmp/krb5cc_91600000_XXXXXX)
pam_sss_gss: Switching euid from 91600000 to 0
pam_sss_gss: KRB5CCNAME=FILE:/tmp/krb5cc_91600000_nSfT3j
pam_sss_gss: System error [5]: Input/output error

@frozencemetery Is there any gssapi call I should call to make it use this variable?

abbra · 2020-11-26T14:27:49Z

You can pass ccache in client creds store you pass to gss_acquire_cred_from(). You are already using keytab there, passing ccache will make use of the ccache.

sumit-bose · 2020-11-26T15:50:09Z

while testing this PR it looks like KRB5CCNAME is not respected. If there user uses a different ccache type as defined in /etc/krb5.conf or if with default_ccache_name = FILE:/tmp/krb5cc_%{uid}_XXXXXX the ccache name has a random component authentication does not work.

Hmm, nice catch. Unfortunately sudo resets the environment before PAM invocation so KRB5CCNAME is not available for the PAM module. It can be made available by adding it to env_keep in /etc/sudoers but this will also make it available in the called process and this is probably not what we want so we will have to patch sudo to keep this variable for PAM. I'll ask sudo upstream what are our options.

Hi,

this is during authentication and at this stage the enviroment is not cleaned up.

If I use default_ccache_name = FILE:/tmp/krb5cc_%{uid}_XXXXXX the module fails and sudo will ask for a password, if I check the enviromnet at this stage I see:

  # cat /proc/$(pidof sudo)/environ |tr '\0' '\n' | grep KRB
  KRB5CCNAME=FILE:/tmp/krb5cc_1367201104_K74YJ1

so I guess $KRB5CCNAME is available.

HTH

bye,
Sumit

pbrezina · 2020-11-27T11:13:27Z

No, if I put getenv directly in the module it is empty unless it is whitelisted in env_keep.

sumit-bose · 2020-11-27T11:50:07Z

No, if I put getenv directly in the module it is empty unless it is whitelisted in env_keep.

Ok, thanks, good to know that /proc/PID/environ might not contain the current state.

Nevertheless I think it would be good to try getenv() first and use the ccache from the environment if set and only fall back to the default otherwise.

bye,
Sumit

pbrezina · 2020-11-30T12:25:14Z

KRB5CCNAME is now respected, if it is set in env_keep in sudoers (or ldap rules). And I asked sudo-workers to see if we can make it available to PAM only. I also updated the manpage.

sumit-bose · 2020-12-04T11:11:25Z

KRB5CCNAME is now respected, if it is set in env_keep in sudoers (or ldap rules). And I asked sudo-workers to see if we can make it available to PAM only. I also updated the manpage.

Thanks,

it is working fine now. I would suggest to mention env_keep details in the pam_sss_gss man page since sudo is the main use-case here.

While testing I came across a behavior which can be a bug or a feature and we should decide how to handle and/or document it.

Currently authentication will be successful if you have a TGT in the credential cache which can be used to successfully request a host ticket. This TGT does not have to be associated with the user calling sudo. libkrbr5 provides krb5_aname_to_localname to translate the Kerberos principal to a local user name, SSSD provides the localauth plugin for this but inside of SSSD we can just do a CACHE_REQ_USER_BY_UPN to check if the principal relates to the user trying to authenticate.

A related item are ccache types which can handle multiple TGTs. Currently the 'active' TGT is used and if the PAM responder would check if the principal matches the user pam_sss_gss should either have a way to select the proper TGT if more than one are available or iterate through the available TGTs (which I guess should be avoided). However, I'm not sure if GSSAPI can do this of if plain libkrb5 calls must be used for this. As an alternative an error message can be shown to the user which can point the user to the kswitch utility.

bye,
Sumit

src/db/sysdb_subdomains.c

pbrezina · 2020-12-10T21:29:18Z

Please, see new changes.

Only canonical UPN is returned and it is used only as a hint - if we fail to obtain credentials for the principal we try again and let GSSAPI to choose. This allows user to login with both canonical UPN and basic UPN. (Note: I did not find a way how to test the return code via some macro.)
cache_req_user_by_upn is called after gssapi handshake is done and it checks for username == found user
added new option pam_gssapi_check_upn (defaults to true) that can turn 2. on and off
pam_gssapi_services defaults to "-" meaning disabled to overcome the fact that we can't tell whether the option is set and empty or unset
I updated man page to describe these changes

sumit-bose · 2020-12-11T11:21:43Z

Hi,

thanks for the updates, all my tests are working well. There is a missing </para> in the pam_sss_gss man page.

I like the idea using - to unset an option, we might want to use this with other options in future as well. But I would suggest to mention in the man page entry explicitly that the hyphen/minus character can be used to unset the option.

bye,
Sumit

config.h is required for the definitions to work correctly. Compilation will fail if sss_format.h is included in a file that does not include directly or indirectly config.h

:config: Added `pam_gssapi_services` to list PAM services that can authenticate using GSSAPI

:config: Added `pam_gssapi_check_upn` to enforce authentication only with principal that can be associated with target user.

:feature: New PAM module `pam_sss_gss` for authentication using GSSAPI :packaging: Added `pam_sss_gss.so` PAM module and `pam_sss_gss.8` manual page

pbrezina · 2020-12-11T13:20:09Z

Thank you. Here's a diff:

diff --git a/src/man/pam_sss_gss.8.xml b/src/man/pam_sss_gss.8.xml
index d4bb705e3..ce5b11bff 100644
--- a/src/man/pam_sss_gss.8.xml
+++ b/src/man/pam_sss_gss.8.xml
@@ -46,6 +46,7 @@
             already present in the Kerberos credentials cache or if user's
             ticket granting ticket can be used to get the correct service ticket
             then the user will be authenticated.
+        </para>
         <para>
             If <option>pam_gssapi_check_upn</option> is True (default) then SSSD
             requires that the credentials used to obtain the service tickets can
@@ -183,9 +184,18 @@ auth sufficient pam_sss_gss.so
             <option>[domain_realm]</option> in /etc/krb5.conf like so:
         </para>
         <para>
-            2. Authentication does not work and syslog contains "Can't find
-            client principal $NAME in cache collection": Try to kinit with the
-            required principal name.
+            3. Authentication does not work and syslog contains "No Kerberos
+            credentials available": You don't have any credentials that can be
+            used to obtain the required service ticket. Use kinit or autheticate
+            over SSSD to acquire those credentials.
+        </para>
+        <para>
+            4. Authentication does not work and SSSD sssd-pam log contains "User
+            with UPN [$UPN] was not found." or "UPN [$UPN] does not match target
+            user [$username].": You are using credentials that can not be mapped
+            to the user that is being authenticated. Try to use kswitch to
+            select different principal, make sure you authenticated with SSSD or
+            consider disabling <option>pam_gssapi_check_upn</option>.
         </para>
         <programlisting>
 [domain_realm]
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 69cd4c3a1..d637e2eaa 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -1712,8 +1712,11 @@ p11_uri = library-description=OpenSC%20smartcard%20framework;slot-id=2
                         <para>
                             Comma separated list of PAM services that are
                             allowed to try GSSAPI authentication using
-                            pam_sss_gss.so module. This option can be also set
-                            in domain section.
+                            pam_sss_gss.so module.
+                        </para>
+                        <para>
+                            To disable GSSAPI authentication, set this option
+                            to <quote>-</quote> (dash).
                         </para>
                         <para>
                             Note: This option can also be set per-domain which

sumit-bose · 2020-12-11T14:12:52Z

Hi,

thanks for your patience, ACK. I will set the label when the CI checks are done.

bye,
Sumit

croissanne · 2020-12-11T14:44:57Z

Hai,

So cockpit's perspective to this patch as I understand it:

Cockpit would build an s4u ccache using gssapi. This means that the ccache we'd use has the target user as the client principal (so not the cockpit principal building the cache). As far as I understand that means calling krb5_aname_to_localname in that context would yield the target principal, not the cockpit principal who built the cache.

That said we would not have a TGT for that user, the way s4u works means we'd just have tickets for specific services, in this case the sudo service. But I think that should be fine, as in a "normal" scenario you'd just use that TGT to get a ticket for the sudo service.

Having this option on by default where that client principal needs to match a local user makes sense to me, as I think that's the scenario cockpit would fall under. Cockpit would ssh into a machine with a host ticket, and then escalate using the sudo service ticket, only for that specific user. So I think this should be fine, and gets an ACK from our side :)

sumit-bose · 2020-12-11T15:55:38Z

The rawhide failure is expected and the rhel8 failure is due to an issue in the CI runner.

sumit-bose · 2020-12-16T14:48:54Z

Pushed PR: #5367

master
- d09aa17 - pam: add pam_sss_gss module for gssapi authentication
- fffe316 - pam: add pam_gssapi_check_upn option
- d63172f - pam: add pam_gssapi_services option
- dcc4201 - pam: fix typo in debug message
- a3e2677 - cache_req: add helper to call user by upn search
- 6715b31 - domain: store hostname and keytab path
- 3b0e48c - packet: add sss_packet_set_body
- 45f2eb5 - sss_format.h: include config.h

pbrezina force-pushed the gssapi branch from 785bf55 to 86d25d8 Compare October 14, 2020 11:11

pbrezina added the Waiting for review label Oct 14, 2020

pbrezina force-pushed the gssapi branch 2 times, most recently from 7570194 to 39522e0 Compare October 14, 2020 12:31

croissanne mentioned this pull request Oct 20, 2020

test: Add kerberized sudo to s4u proof of concept cockpit-project/cockpit#14782

Merged

2 tasks

frozencemetery suggested changes Oct 20, 2020

View reviewed changes

pbrezina force-pushed the gssapi branch from 39522e0 to bc06fa6 Compare October 27, 2020 11:49

pbrezina force-pushed the gssapi branch from bc06fa6 to 7e3ced4 Compare October 29, 2020 12:08

pbrezina assigned frozencemetery Nov 2, 2020

frozencemetery reviewed Nov 5, 2020

View reviewed changes

deastoe mentioned this pull request Nov 11, 2020

nss: Use posix_fallocate() to alloc memcache file #5370

Closed

sumit-bose reviewed Nov 25, 2020

View reviewed changes

src/man/pam_sss_gss.8.xml Outdated Show resolved Hide resolved

pbrezina force-pushed the gssapi branch from 7e3ced4 to 321fd8b Compare November 30, 2020 12:12

pbrezina force-pushed the gssapi branch from 76140bb to d66c0a6 Compare December 9, 2020 15:27

sumit-bose reviewed Dec 9, 2020

View reviewed changes

src/db/sysdb_subdomains.c Outdated Show resolved Hide resolved

pbrezina force-pushed the gssapi branch from d66c0a6 to 2cfe91e Compare December 10, 2020 21:25

pbrezina added 5 commits December 11, 2020 13:32

sss_format.h: include config.h

fde618d

config.h is required for the definitions to work correctly. Compilation will fail if sss_format.h is included in a file that does not include directly or indirectly config.h

packet: add sss_packet_set_body

8f653f9

domain: store hostname and keytab path

46ead4b

cache_req: add helper to call user by upn search

826152e

pam: fix typo in debug message

609452d

pbrezina force-pushed the gssapi branch from 2cfe91e to 8efe056 Compare December 11, 2020 13:17

pbrezina added 3 commits December 11, 2020 14:19

pam: add pam_gssapi_services option

47fea8b

:config: Added `pam_gssapi_services` to list PAM services that can authenticate using GSSAPI

pam: add pam_gssapi_check_upn option

1a0f643

:config: Added `pam_gssapi_check_upn` to enforce authentication only with principal that can be associated with target user.

pam: add pam_sss_gss module for gssapi authentication

bcb1170

:feature: New PAM module `pam_sss_gss` for authentication using GSSAPI :packaging: Added `pam_sss_gss.so` PAM module and `pam_sss_gss.8` manual page

pbrezina force-pushed the gssapi branch from 8efe056 to bcb1170 Compare December 11, 2020 13:19

sumit-bose added Accepted Ready to push Ready to push and removed Waiting for review Ready to push Ready to push labels Dec 11, 2020

sumit-bose added Pushed and removed Accepted Ready to push Ready to push labels Dec 16, 2020

sumit-bose closed this Dec 16, 2020

alexey-tikhonov mentioned this pull request Jan 11, 2021

gssapi: default pam_gssapi_services to NULL in domain section and coverity fixes #5453

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pam: add pam_sss_gss module for gssapi authentication #5367

pam: add pam_sss_gss module for gssapi authentication #5367

pbrezina commented Oct 14, 2020

frozencemetery commented Oct 15, 2020

pbrezina commented Oct 16, 2020

frozencemetery commented Oct 20, 2020

frozencemetery left a comment •

edited

frozencemetery Oct 20, 2020

pbrezina Oct 27, 2020

frozencemetery Oct 28, 2020

pbrezina commented Oct 27, 2020

pbrezina commented Oct 27, 2020

pbrezina commented Oct 27, 2020

frozencemetery commented Oct 28, 2020

pbrezina commented Oct 29, 2020

frozencemetery left a comment

sumit-bose Nov 25, 2020

pbrezina Nov 30, 2020

sumit-bose Dec 4, 2020

sumit-bose commented Nov 25, 2020

pbrezina commented Nov 26, 2020

abbra commented Nov 26, 2020

sumit-bose commented Nov 26, 2020

pbrezina commented Nov 27, 2020

sumit-bose commented Nov 27, 2020

pbrezina commented Nov 30, 2020

sumit-bose commented Dec 4, 2020

pbrezina commented Dec 10, 2020

sumit-bose commented Dec 11, 2020

pbrezina commented Dec 11, 2020

sumit-bose commented Dec 11, 2020

croissanne commented Dec 11, 2020

sumit-bose commented Dec 11, 2020

sumit-bose commented Dec 16, 2020

pam: add pam_sss_gss module for gssapi authentication #5367

pam: add pam_sss_gss module for gssapi authentication #5367

Conversation

pbrezina commented Oct 14, 2020

frozencemetery commented Oct 15, 2020

pbrezina commented Oct 16, 2020

frozencemetery commented Oct 20, 2020

frozencemetery left a comment • edited

Choose a reason for hiding this comment

frozencemetery Oct 20, 2020

Choose a reason for hiding this comment

pbrezina Oct 27, 2020

Choose a reason for hiding this comment

frozencemetery Oct 28, 2020

Choose a reason for hiding this comment

pbrezina commented Oct 27, 2020

pbrezina commented Oct 27, 2020

pbrezina commented Oct 27, 2020

frozencemetery commented Oct 28, 2020

pbrezina commented Oct 29, 2020

frozencemetery left a comment

Choose a reason for hiding this comment

sumit-bose Nov 25, 2020

Choose a reason for hiding this comment

pbrezina Nov 30, 2020

Choose a reason for hiding this comment

sumit-bose Dec 4, 2020

Choose a reason for hiding this comment

sumit-bose commented Nov 25, 2020

pbrezina commented Nov 26, 2020

abbra commented Nov 26, 2020

sumit-bose commented Nov 26, 2020

pbrezina commented Nov 27, 2020

sumit-bose commented Nov 27, 2020

pbrezina commented Nov 30, 2020

sumit-bose commented Dec 4, 2020

pbrezina commented Dec 10, 2020

sumit-bose commented Dec 11, 2020

pbrezina commented Dec 11, 2020

sumit-bose commented Dec 11, 2020

croissanne commented Dec 11, 2020

sumit-bose commented Dec 11, 2020

sumit-bose commented Dec 16, 2020

frozencemetery left a comment •

edited