[sssd PR#5647][opened] krb5_child: Honor Kerberos keytab location

[elkoniu]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: opened

PR body:
"""
Kerberos keytab location can be specified per domain in sssd.conf.
If it is not specified - default path is used: /etc/krb5.keytab
The problem is that default path itself can be redefined for kerberos
by adding entry in krb5.conf:

  [libdefaults]
  default_keytab_name = /<PATH>/krb5.keytab

krb5_child will still use /etc/krb5.keytab as default value which
will cause an error.

This patch adds config checking to krb5_child.
If keytab parameter will be set to /etc/krb5.keytab,
krb5_child will validate it against krb5.conf and eventually
overwritte with value presented there.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

sumit-bose commented:
"""
Hi,

I'm afraid this won't work. With your patch you cannot make a difference if
`/etc/krb5.keytab` is set because the SSSD default for `krb5_keytab` is used or is
`krb5_keytab = /etc/krb5.keytab` is explicitly set in `sssd.conf`. In the first case the
`libkrb5` default value should be used, in the second case the value from `sssd.conf`
should be used.

I would suggest to handle this similar to `ldap_krb5_keytab` where internally `NULL` is
used if the value is not set explicitly in `sssd.conf` and pass `NULL` until
`krb5_kt_resolve()` is called and then call `krb5_kt_default()` instead.

HTH

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-846942925

[sssd PR#5647][synchronized] krb5_child: Honor Kerberos keytab location

[elkoniu]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

sumit-bose commented:
"""
Hi,

thank you for the new version, this is a valid solution. However I would like to ask you
for another change. I wonder if you can just add an empty string `create_send_buffer()` in
case the keytab is not set, similar to `create_tgt_req_send_buffer()`
(https://github.com/SSSD/sssd/blob/master/src/providers/ldap/sdap_child_he... and
https://github.com/SSSD/sssd/blob/master/src/providers/ldap/sdap_child_he...).
And then call `krb5_kt_default()` or `krb5_kt_default_name()` only in `krb5_child`.

One of reasons I'm asking is to be similar to how it is handled in the LDAP provider
and `ldap_child`. `ldap_child` does very similar things as `krb5_child` and it might be
worth to consider to move the functionality of `ldap_child` to `krb5_child` eventually to
reduce code duplication. For this it would be good to do the keytab handling in the same
way.

Another reason is to try to move as much libkrb5 call into `krb5_child` so that eventually
it might be possible to not link the krb5 backend code with libkrb5 and have only
`krb5_child` depending on libkrb5. I'm not sure if the is really possible, but at
least here with getting the default keytab it would be easy to achieve.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-851335285

[sssd PR#5647][+Changes requested] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Changes requested

[sssd PR#5647][+Bugzilla] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Bugzilla

[sssd PR#5647][synchronized] krb5_child: Honor Kerberos keytab location

[elkoniu]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647

[sssd PR#5647][+Waiting for review] krb5_child: Honor Kerberos keytab location

[elkoniu]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Waiting for review

[sssd PR#5647][-Changes requested] krb5_child: Honor Kerberos keytab location

[elkoniu]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: -Changes requested

[sssd PR#5647][synchronized] krb5_child: Honor Kerberos keytab location

[elkoniu]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647

[sssd PR#5647][synchronized] krb5_child: Honor Kerberos keytab location

[elkoniu]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647

[sssd PR#5647][synchronized] krb5_child: Honor Kerberos keytab location

[elkoniu]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

sumit-bose commented:
"""
Hi,

thanks for the update and the comments. I'm fine with the code and the patch works for
me, so ACK. Let me just wait for the CI before setting the label.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-854741406

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

alexey-tikhonov commented:
"""
F34 and Rawhide have:
```
FAILED test_kcm.py::test_kcm_renewals - AttributeError: 'NoneType' object has...
```

Is this relevant to this PR?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-855776524

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

alexey-tikhonov commented:
"""
F34 and Rawhide have:
```
FAILED test_kcm.py::test_kcm_renewals - AttributeError: 'NoneType' object has...
...
FileNotFoundError: [Errno 2] No such file or directory:
'/shared/sssd/ci-build-debug/intg/bld/src/tests/intg/passwd.bak' ->
'/shared/sssd/ci-build-debug/intg/bld/src/tests/intg/passwd'
```

Is this relevant to this PR?
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-855776524

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

alexey-tikhonov commented:
"""
Looks like latest run on master branch on F34 didn't fail this test:
https://sssd-jenkins.rhev-ci-vms.eng.rdu2.redhat.com/job/sssd-ci/job/mast...
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-855786072

[sssd PR#5647][+Changes requested] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Changes requested

[sssd PR#5647][-Waiting for review] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: -Waiting for review

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

sumit-bose commented:
"""
Hi @elkoniu,

can you check if you can reproduce the integration check failure on F34 locally?

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-855788797

[sssd PR#5647][synchronized] krb5_child: Honor Kerberos keytab location

[elkoniu]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

alexey-tikhonov commented:
"""
In current PR CI run: `make-intgcheck:         success` (F34)
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-856116863

[sssd PR#5647][-Changes requested] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: -Changes requested

[sssd PR#5647][+Waiting for review] krb5_child: Honor Kerberos keytab location

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Waiting for review

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[elkoniu]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

elkoniu commented:
"""
Thanks @sumit-bose @alexey-tikhonov and @justin-stephenson for taking a look at this PR.
It turns out the issue was this line:
`DEBUG(SSSDBG_TRACE_FUNC, "krb5_keytab not set in sssd.conf for domain [%s]\n",
kr->dom->name)` in `krb5_child_handler.c +122`. In case of renewal `kr->dom` was
NULL which was leading to crash. In the last update I removed reference to
`kr->dom->name` from debug message.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-856118145

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

sumit-bose commented:
"""
Hi,

thanks for the fix, ACK.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-856444147

[sssd PR#5647][+Accepted] krb5_child: Honor Kerberos keytab location

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Accepted

[sssd PR#5647][-Waiting for review] krb5_child: Honor Kerberos keytab location

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: -Waiting for review

[sssd PR#5647][+Ready to push] krb5_child: Honor Kerberos keytab location

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Ready to push

[sssd PR#5647][+Pushed] krb5_child: Honor Kerberos keytab location

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

Label: +Pushed

[sssd PR#5647][comment] krb5_child: Honor Kerberos keytab location

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5647
Title: #5647: krb5_child: Honor Kerberos keytab location

pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5647

* `master`
    * ecb2ae7a8f78a2b561c67fca5bb97766c0d3ae9c - krb5_child: Honor Kerberos keytab
location

"""

See the full comment at https://github.com/SSSD/sssd/pull/5647#issuecomment-856595565

[sssd PR#5647][closed] krb5_child: Honor Kerberos keytab location

[pbrezina]

   URL: https://github.com/SSSD/sssd/pull/5647
Author: elkoniu
 Title: #5647: krb5_child: Honor Kerberos keytab location
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5647/head:pr5647
git checkout pr5647