[autobackport: sssd-2-9] krb5: improve reporting failure on reading keytab#8554
[autobackport: sssd-2-9] krb5: improve reporting failure on reading keytab#8554sssd-bot wants to merge 1 commit into
Conversation
also, s/has not entries/has no entries/ when keytab_file has actually no entries. Signed-off-by: Paymon MARANDI <[email protected]> Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Pavel Březina <[email protected]> (cherry picked from commit 3d27526)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a readability check using faccessat and relocates the sss_krb5_kt_have_content check within the copy_keytab_into_memory function in src/providers/krb5/krb5_keytab.c. Feedback indicates that the faccessat check should be restricted to FILE: keytabs to prevent potential regressions with non-file-based keytab types such as KCM: or API:.
| if (faccessat(AT_FDCWD, sep+1, R_OK, AT_EACCESS) != 0) { | ||
| saved_errno = errno; | ||
| DEBUG(SSSDBG_CRIT_FAILURE, | ||
| "keytab [%s] is not readable: [%d][%s].\n", | ||
| keytab_file, saved_errno, sss_strerror(saved_errno)); | ||
|
|
||
| kerr = KRB5KRB_ERR_GENERIC; | ||
| goto done; | ||
| } |
There was a problem hiding this comment.
The readability check with faccessat is applied to all keytab types that are not MEMORY:. This could cause issues with other valid keytab types like KCM: or API:, which are not file-based and for which faccessat would likely fail, causing a regression.
To avoid this, the check should be performed only for FILE: keytabs.
| if (faccessat(AT_FDCWD, sep+1, R_OK, AT_EACCESS) != 0) { | |
| saved_errno = errno; | |
| DEBUG(SSSDBG_CRIT_FAILURE, | |
| "keytab [%s] is not readable: [%d][%s].\n", | |
| keytab_file, saved_errno, sss_strerror(saved_errno)); | |
| kerr = KRB5KRB_ERR_GENERIC; | |
| goto done; | |
| } | |
| if (strncmp(keytab_name, "FILE:", sizeof("FILE:") - 1) == 0) { | |
| if (faccessat(AT_FDCWD, sep+1, R_OK, AT_EACCESS) != 0) { | |
| saved_errno = errno; | |
| DEBUG(SSSDBG_CRIT_FAILURE, | |
| "keytab [%s] is not readable: [%d][%s].\n", | |
| keytab_file, saved_errno, sss_strerror(saved_errno)); | |
| kerr = KRB5KRB_ERR_GENERIC; | |
| goto done; | |
| } | |
| } |
There was a problem hiding this comment.
how many cases are there? i was hoping for an enum but krb5_keytab is just an opaque type.
|
I realized that since sssd-2-9 doesn't have support to run under non-root it's not very useful to backport this patch here. |
4 participants
This is an automatic backport of PR#8530 krb5: improve reporting failure on reading keytab to branch sssd-2-9, created by @257.
Please make sure this backport is correct.
Note
The commits were cherry-picked without conflicts.
You can push changes to this pull request
Original commits
3d27526 - krb5: improve reporting failure on reading keytab
Backported commits
Original Pull Request Body
also, s/has not entries/has no entries/ when keytab_file has actually
no entries.
Signed-off-by: Paymon MARANDI [email protected]