Skip to content

krb5: improve reporting failure on reading keytab#8530

Open
257 wants to merge 1 commit intoSSSD:masterfrom
257:improve-reporting-when-running-as-sssd
Open

krb5: improve reporting failure on reading keytab#8530
257 wants to merge 1 commit intoSSSD:masterfrom
257:improve-reporting-when-running-as-sssd

Conversation

@257
Copy link

@257 257 commented Mar 17, 2026

also, s/has not entries/has no entries/ when keytab_file has actually
no entries.

Signed-off-by: Paymon MARANDI paymon@encs.concordia.ca

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request introduces a valuable check for keytab file readability, which enhances error reporting and robustness. This is a good improvement for handling cases where the keytab file exists but cannot be accessed by the process. However, a typo was introduced in the debug message for this new check.

@257 257 force-pushed the improve-reporting-when-running-as-sssd branch from 27cb9fe to fa4a69d Compare March 17, 2026 15:01
@alexey-tikhonov
Copy link
Member

Perhaps tests need update:

[  FAILED  ] tests: 2 test(s), listed below:
[  FAILED  ] test_copy_keytab
[  FAILED  ] test_copy_keytab_order

But what is your use case? Typically 'krb5_child' has cap_dac_read_search to read keytab.

@257 257 closed this Mar 17, 2026
@257
Copy link
Author

257 commented Mar 17, 2026

Perhaps tests need update:

[  FAILED  ] tests: 2 test(s), listed below:
[  FAILED  ] test_copy_keytab
[  FAILED  ] test_copy_keytab_order

will have a look

But what is your use case? Typically 'krb5_child' has cap_dac_read_search to read keytab.

cap_dac_read_search seems to require at least ownership? i can't really tell from the man page so i'm not sure why it didn't apply here.

in our case, our logs would show:
(2026-03-17 9:28:46): [ldap_child[7149]] [copy_keytab_into_memory] (0x0020): [RID#3] keytab [FILE:/etc/krb5.keytab] has not entries.

actual problem was that the build pipeline (homegrown, based on mkosi) included /etc/krb5.keytab in the image that had wrong owner/permission:
-rw------- 1 root root /etc/krb5.keytab

obviously logs didn't help in our troubleshooting hence this patch. (although it pointed to the right direction)

chown'ing that file to: -rw-rw---- 1 root sssd 213 Mar 16 17:26 /etc/krb5.keytab fixed the problem for us.

a side note, I have two questions:

  • we would like to move to systemd-creds for passing the keytab file to sssd and would like to know if community has already tried or seen an "implementaiton" of that?
    -cap_dac_read_search didn't help in our case, man page says:
CAP_DAC_READ_SEARCH
              •  Bypass file read permission checks and directory read
                 and execute permission checks;
              •  invoke [open_by_handle_at(2)](https://man7.org/linux/manpages/man2/open_by_handle_at.2.html);
              •  use the [linkat(2)](https://man7.org/linux/man-pages/man2/linkat.2.html) AT_EMPTY_PATH flag to create a link to a file referred to by a file descriptor.

does that mean that the ownership already has to be set for the user?

@257 257 reopened this Mar 17, 2026
@alexey-tikhonov
Copy link
Member

actual problem was that the build pipeline (homegrown, based on mkosi) included /etc/krb5.keytab in the image that had wrong owner/permission: -rw------- 1 root root /etc/krb5.keytab

That's actually pretty typical ownership.

Can you check output of getcap /usr/libexec/sssd/* in your custom image?
Normally it should be:

/usr/libexec/sssd/krb5_child cap_dac_read_search,cap_setgid,cap_setuid=p
/usr/libexec/sssd/ldap_child cap_dac_read_search=p
/usr/libexec/sssd/sssd_pam cap_dac_read_search=p

I suspect file capabilities are lost in your image.

obviously logs didn't help in our troubleshooting hence this patch.

Thank you.

chown'ing that file to: -rw-rw---- 1 root sssd 213 Mar 16 17:26 /etc/krb5.keytab fixed the problem for us.

This also works, in this case SSSD binaries in your image don't need 'cap_dac_read_search'.

* we would like to move to `systemd-creds` for passing the keytab file to sssd and would like to know if community has already tried or seen an "implementaiton" of that?

That's an interesting option, we didn't explore it yet.

  -`cap_dac_read_search` didn't help in our case, man page says:

...

does that mean that the ownership already has to be set for the user?

No, it's not required. cap_dac_read_search allows to fully bypass DAC controls

@257
Copy link
Author

257 commented Mar 18, 2026

Can you check output of getcap /usr/libexec/sssd/* in your custom image? Normally it should be:

/usr/libexec/sssd/krb5_child cap_dac_read_search,cap_setgid,cap_setuid=p
/usr/libexec/sssd/ldap_child cap_dac_read_search=p
/usr/libexec/sssd/sssd_pam cap_dac_read_search=p

comes up empty! in fact i ended up chown'in /usr/libexec/sssd/sssd_pam to get it to run and was going to submit a patch for that too!

then i saw this and this

build script (gentoo's portage) indeed passes --with-sssd-user=sssd

I suspect file capabilities are lost in your image.

they're indeed lost, great call; thanks.

i cooked up something for the distro, waiting for their feedback.

@257 257 force-pushed the improve-reporting-when-running-as-sssd branch from fa4a69d to 74a4f89 Compare March 18, 2026 16:36
@alexey-tikhonov
Copy link
Member

i cooked up something for the distro, waiting for their feedback.

You might want to sync with @salahcoronya on this.

@alexey-tikhonov alexey-tikhonov self-assigned this Mar 19, 2026
Copilot AI review requested due to automatic review settings March 19, 2026 17:37
@257 257 force-pushed the improve-reporting-when-running-as-sssd branch from 74a4f89 to fbd100f Compare March 19, 2026 17:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to improve diagnostics when a Kerberos keytab cannot be read and fixes a grammar issue in the “empty keytab” log message.

Changes:

  • Added an explicit readability check for the keytab before checking for content.
  • Updated log message grammar from “has not entries” to “has no entries”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +123 to +127
kerr = access(keytab_file, R_OK);
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "keytab [%s] is not readable by us.\n",
keytab_file);
return kerr;
Comment on lines +123 to +128
kerr = access(keytab_file, R_OK);
if (kerr != 0) {
DEBUG(SSSDBG_CRIT_FAILURE, "keytab [%s] is not readable by us.\n",
keytab_file);
return kerr;
}
@257 257 force-pushed the improve-reporting-when-running-as-sssd branch from fbd100f to ce925b9 Compare March 19, 2026 17:47
@alexey-tikhonov
Copy link
Member

@257, try running make check locally to see what unit tests are failing.

I don't say this means patch is wrong, but probably a change of behavior that requires test update as well.

also, s/has not entries/has no entries/ when keytab_file has actually
no entries.

Signed-off-by: Paymon MARANDI <paymon@encs.concordia.ca>
@257 257 force-pushed the improve-reporting-when-running-as-sssd branch from ce925b9 to 30104e0 Compare March 20, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants