krb5: improve reporting failure on reading keytab#8530
krb5: improve reporting failure on reading keytab#8530alexey-tikhonov merged 1 commit intoSSSD:masterfrom
Conversation
![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
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
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
27cb9fe to
fa4a69d
Compare
|
Perhaps tests need update: But what is your use case? Typically 'krb5_child' has |
will have a look
in our case, our logs would show: actual problem was that the build pipeline (homegrown, based on obviously logs didn't help in our troubleshooting hence this patch. (although it pointed to the right direction) chown'ing that file to: a side note, I have two questions:
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? |
That's actually pretty typical ownership. Can you check output of I suspect file capabilities are lost in your image.
Thank you.
This also works, in this case SSSD binaries in your image don't need 'cap_dac_read_search'.
That's an interesting option, we didn't explore it yet.
...
No, it's not required. |
comes up empty! in fact i ended up chown'in build script (gentoo's portage) indeed passes
they're indeed lost, great call; thanks. i cooked up something for the distro, waiting for their feedback. |
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
fa4a69d to
74a4f89
Compare
You might want to sync with @salahcoronya on this. |
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
74a4f89 to
fbd100f
Compare
There was a problem hiding this comment.
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.
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
fbd100f to
ce925b9
Compare
|
@257, try running I don't say this means patch is wrong, but probably a change of behavior that requires test update as well. |
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
ce925b9 to
30104e0
Compare
i'm trying diff --git i/contrib/fedora/bashrc_sssd w/contrib/fedora/bashrc_sssd
index 48cba8cca..e48c2d6f7 100644
--- i/contrib/fedora/bashrc_sssd
+++ w/contrib/fedora/bashrc_sssd
@@ -7,8 +7,14 @@
SSS_ARCH=$(uname -m)
# Determine the lib and libdir locations
-SSS_LIB=$(rpm --eval %{_lib})
-SSS_LIBDIR=$(rpm --eval %{_libdir})
+rpm="$(which rpm)"
+if [ $? -eq 0 ]; then
+ SSS_LIB=$(rpm --eval %{_lib})
+ SSS_LIBDIR=$(rpm --eval %{_libdir})
+else
+ SSS_LIB=/usr/lib
+ SSS_LIBDIR=/usr/lib
+fi
# Add the following line to your .bashrc if you want SSSD to throw errors on
# compiler warnings (recommended)but it fails: ...
Makefile:48647: warning: overriding recipe for target 'src/examples/logrotate'
Makefile:13324: warning: ignoring old recipe for target 'src/examples/logrotate'
./sbus_generate.sh /home/pmn/src/github/257/sssd/x86_64/..
GEN stap_generated_probes.h
Generating sbus code for: sbus/codegen/dbus.xml
Generating sbus code for: sss_iface/sss_iface.xml
Generating sbus code for: responder/ifp/ifp_iface/ifp_iface.xml
dtrace: failed to compile script ../src/systemtap/sssd_probes.d: line 78: syntax error near end of input
make: *** [Makefile:48480: stap_generated_probes.h] Error 1it will go to the end if i also remove Makefile:13324: warning: ignoring old recipe for target 'src/examples/logrotate'
PASS: src/tests/pysss_murmur-test.py3.sh
PASS: src/tests/pysss-test.py3.sh
PASS: src/tests/pyhbac-test.py3.sh
PASS: src/tests/double_semicolon_test
PASS: src/config/SSSDConfigTest.py3.sh
PASS: src/tests/whitespace_test
============================================================================
Testsuite summary for sssd 2.13.0
============================================================================
# TOTAL: 6
# PASS: 6
# SKIP: 0
# XFAIL: 0
# FAIL: 0
# XPASS: 0
# ERROR: 0
============================================================================
make[4]: Leaving directory '/usr/src/github/257/sssd/x86_64'
make[3]: Leaving directory '/usr/src/github/257/sssd/x86_64'
make[2]: Leaving directory '/usr/src/github/257/sssd/x86_64'
Making check in src/tests/cwrap
make[2]: Entering directory '/usr/src/github/257/sssd/x86_64/src/tests/cwrap'
make
make[3]: Entering directory '/usr/src/github/257/sssd/x86_64/src/tests/cwrap'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/usr/src/github/257/sssd/x86_64/src/tests/cwrap'
make check-TESTS
make[3]: Entering directory '/usr/src/github/257/sssd/x86_64/src/tests/cwrap'
make[4]: Entering directory '/usr/src/github/257/sssd/x86_64/src/tests/cwrap'
============================================================================
Testsuite summary for sssd 2.13.0
============================================================================
# TOTAL: 0
# PASS: 0
# SKIP: 0
# XFAIL: 0
# FAIL: 0
# XPASS: 0
# ERROR: 0
============================================================================
make[4]: Leaving directory '/usr/src/github/257/sssd/x86_64/src/tests/cwrap' |
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
3 times, most recently
from
7c03fd5 to
068baf7
Compare
this fixes that^: diff --git i/src/systemtap/sssd_probes.d w/src/systemtap/sssd_probes.d
index 91abd0105..0728495cb 100644
--- i/src/systemtap/sssd_probes.d
+++ w/src/systemtap/sssd_probes.d
@@ -74,4 +74,4 @@ provider sssd {
int target, int method);
probe dp_req_done(const char *dp_req_name, int target, int method,
int ret, const char *errorstr);
-}
+};dtrace version? here is mine: :sssd/x86_64 dtrace -V
dtrace: D 2.0.5 |
@alexey-tikhonov, two fedora builds are still failing, could you have look please? |
No, all 'rpm-build:fedora-*' CI jobs look green. |
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
068baf7 to
7aa1e6a
Compare
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
7aa1e6a to
96718a0
Compare
257
force-pushed
the
improve-reporting-when-running-as-sssd
branch
2 times, most recently
from
3ca5b6b to
dc3ae1f
Compare
|
ACK |
|
Note: Covscan is green. |
also, s/has not entries/has no entries/ when keytab_file has actually no entries. Signed-off-by: Paymon MARANDI <paymon@encs.concordia.ca> Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Pavel Březina <pbrezina@redhat.com>
sssd-bot
force-pushed
the
improve-reporting-when-running-as-sssd
branch
from
dc3ae1f to
d762fef
Compare
alexey-tikhonov
added
no-backport
also, s/has not entries/has no entries/ when keytab_file has actually
no entries.
Signed-off-by: Paymon MARANDI paymon@encs.concordia.ca