feat: get ticket path from krb5 when updating current user policies #903
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GabrielNagy
force-pushed
the
adsysctl-update-infer-krb5cc
branch
2 times, most recently
from
89d54ad to
255e5ec
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #903 +/- ##
==========================================
+ Coverage 87.61% 87.65% +0.03%
==========================================
Files 75 76 +1
Lines 6694 6713 +19
==========================================
+ Hits 5865 5884 +19
Misses 492 492
Partials 337 337 ☔ View full report in Codecov by Sentry. |
didrocks
requested changes
Feb 9, 2024
didrocks
approved these changes
Feb 14, 2024
We default it to false and require users to explicitly opt-in in order to benefit from the krb5 cached ticket autodetection.
This leverages the libkrb5 API via CGo to determine the default path of the current user's cached ticket. According to the MIT Kerberos documentation[1] the order is: - a prior call to krb5_cc_set_default_name() - the KRB5CCNAME environment variable - the default_ccache_name profile variable - the operating system or build-time default value The C function doesn't guarantee that the ticket is present on disk (or that the ticket is of FILE type for that matter), so we do the additional checks ourselves and return an error as needed. Testing is accomplished via a C mock with a mention that integration tests will look a bit different in the sense that we will not override krb5_init_context for them, since that is a critical path required by samba to work. This is not a huge loss as we do more granular tests at the package level. Similar to the wbclient mock, hoist the compilation logic to testutils in order to make it accessible from the integration tests. [1] https://web.mit.edu/kerberos/www/krb5-latest/doc/appdev/refs/api/krb5_cc_default_name.html
When applying policies for the current user, if the KRB5CCNAME environment variable is empty and the detect_cached_ticket setting is true, try to get the default ticket path from krb5. We don't return an error here but let the daemon handle the result, as even if the ticket cannot be found there might still be a tracked one in the adsys rundir.
And sort dependencies alphabetically, leaving debhelper/dh packages and golang-go first due to their importance.
And reuse it in the winbind package tests too.
GabrielNagy
force-pushed
the
adsysctl-update-infer-krb5cc
branch
from
8303d54 to
0545972
Compare
GabrielNagy
added a commit
that referenced
this pull request
Feb 21, 2024
This is a follow-up to #903 where the functionality to get the default ticket path from `libkrb5` was added. The simplest and safest way to use the functionality above fom the PAM module is to delegate most of the responsibility to a CLI command written in Go, that can return the ticket path of any given user. Because the PAM module runs as root, we need to de-escalate to the given user (using `setuid` since libkrb5 treats `setuid` and `seteuid` separately, so the latter is not enough). The CLI command is specialized for usage from within the PAM module to minimize friction. It reads the configuration file to see if the `detect_cached_ticket` setting is enabled, and avoids returning an error if the ticket path is not present on disk. Thus the PAM module can just execute the command, read its output and silently return if the subprocess exited with 0 and no output was printed. We block authentication if the subprocess returns an error, or if any buffer-allocation or system calls needed to run and read the output are failing. On top of this being a fully backwards-compatible change on the account of the opt-in setting, the impact of not having a ticket on disk is minimized since adsys will then treat the user as a non-domain user. There is the risk of the ticket cache on disk being outdated, which will end up failing the `adsysctl update` operation that happens on login, but ensuring the validity of the existing ticket path pointed to by `libkrb5` is the responsibility of the process that generates/renews the ticket, not adsys. Fixes UDENG-2191
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This leverages the libkrb5 API via CGo to determine the default path of the current user's cached ticket. According to the MIT Kerberos documentation the order is:
krb5_cc_set_default_name()KRB5CCNAMEenvironment variabledefault_ccache_nameprofile variableThe C function doesn't guarantee that the ticket is present on disk (or that the ticket is of
FILEtype for that matter), so we do the additional checks ourselves and return an error as needed.Testing is accomplished via a C mock with a mention that integration tests will look a bit different in the sense that we will not override
krb5_init_contextfor them, since that is a critical path required by samba to work. This is not a huge loss as we do more granular tests at the package level.Similar to the
wbclientmock, hoist the compilation logic totestutilsin order to make it accessible from the integration tests.For now this logic is only used when updating policies for the current user and
KRB5CCNAMEis not set. The functionality is gated behind a new setting,detect_cached_ticket, which we default to false to maintain backwards compatibility. The same implementation will be used for the PAM module (where we will have a CLI command that runssetuidwith the target user before callingad.TicketPath), but I opened this one first for an easier review.(Partly) fixes UDENG-2191