Passwordless-gdm for sssd-2-9#8267
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the passwordless-gdm feature, which is a significant enhancement. The changes involve a major refactoring of the authentication handling in krb5_child.c to better support multiple authentication methods, and the introduction of a new JSON-based protocol for authentication selection with GDM. The refactoring is well-structured and improves maintainability by separating the request and answer phases of authentication. The new JSON handling logic is implemented in pamsrv_json.c.
My review found a couple of critical issues in the new code that could lead to crashes. One is a classic C bug with a shadowed loop variable, and the other is a potential NULL pointer dereference when handling JSON data. I've provided specific comments and suggestions for fixes.
Overall, this is a great feature addition. Once the identified issues are addressed, this PR should be in good shape.
|
JFTR: I think so massive change and big RFE isn't a best fit for LTM branch... |
|
This needs to incorporate #8296 |
34c110d to
befff69
Compare
|
I rebased on top of sssd-2-9 and included the changes from #8296 in this PR |
|
Hi, so far my tests went well, I opened freeipa/freeipa#8075 to get some broader testing. bye, |
Ah, looks like I have to create a Fedora-43 build as well. |
befff69 to
9c8d54f
Compare
|
Rebased on top of sssd-2-9 |
b8945d2 to
34092f4
Compare
88d8093 to
c7fc0d5
Compare
c7fc0d5 to
2d9d41d
Compare
Include JSON message where applies. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
It returns NULL on error, but this wasn't checked. Fixes: ceeffa9 ("Responder: generate JSON message for GUI") Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Integration with GDM requests two prompts for smartcard so modifying the prompt_config structure. In addition, implement all the functions needed to manipulate the structure for these new prompts. Finally, add unit-tests for the new functions. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
This API gets all the elements with the selected response type data from the response_data linked list. Includes unit tests. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
8df6cd5 to
f54496c
Compare
Implement a set of functions to retrieve the smartcard data and generate the JSON message with it. Implement new unit test and adapt the existing ones to take into account the new data. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Parse GUI reply for smartcard and set the appropriate data in `sss_auth_token` structure. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Several of the functions in `pamsrv_json` had lots of arguments and I'm about to add more for the passkey authentication mechanism. Reduce these arguments by creating a structure that will contain all these data. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Include the certificate data in the JSON messages to set it in the authtok structure more easily. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Include the certificate data in the JSON message to set it in the authtok structure more easily. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
This is needed by `pamsrv_json.c`, so let's make it public. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Implement a set of functions to retrieve the passkey data and generate the JSON message with it. Implement new unit test and adapt the existing ones to take into account the new data. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
`sss_authtok_set_local_passkey_pin` provides a way to set the passkey PIN in the authtok structure for local passkey authentication. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Parse GUI reply for passkey and set the appropriate data in `sss_auth_token` structure. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
During the `preauthentication` phase krb5_child checks for the available authentication methods for the given user, advertises them and the process is kept alive. Once the state is change to `authentication` the same krb5_child process processes the credentials and proceeds with the authentication itself. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
defaults The `pam_p11_allowed_services` option now includes `gdm-switchable-auth` as one of the default allowed PAM services for smartcard authentication. The service was added alongside the other GDM-related services (gdm-smartcard and gdm-password) for logical grouping. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
When a user's password expires after successful JSON authentication, the fallback to traditional password change fails. Add PAM_CLI_FLAGS_CHAUTHTOK_PREAUTH flag to distinguish password change preauth from normal authentication preauth. When this flag is set, the PAM responder skips JSON message generation and returns traditional preauth data instead. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Use `pam_get_auth_types()` to detect the available mechanisms for a user. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Add a note to clarify that 2FA isn't supported in JSON protocol and fix
man page compilation for `pam_json_services` option.
:feature: Unified passwordless login in the GUI. SSSD now supports a
rich authentication selection interface. Users can login with
smartcards, passkey, External IdPs and passwords directly
within the graphical user interface.
:packaging: SSSD now supports authentication mechanism selection through
PAM using a JSON-based protocol. This feature enables
passwordless authentication mechanisms in GUI login
environments that support the protocol.
Feature will be supported by GNOME Display Manager (GDM)
starting with GNOME 50. While currently optimized for GNOME,
the JSON protocol design allows for future support in other
display managers.
authselect is the recommended approach and will handle the
necessary PAM stack modifications automatically starting
with version 1.7 through the new option `with-switch-auth`
which provides a new PAM service called `switchable-auth`.
Manual PAM configuration is also possible.
For more technical details and implementation specifications,
see the design documentation:
SSSD/sssd.io#79
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Port the pre-authentication retry logic from the IPA provider to the krb5 provider, making it available to all krb5-based authentication flows. Relates: 6c1272e ("krb5: Add fallback password change support") Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
The `tokeninfo_matches()` function already handles PAM stacked tokens correctly by processing them through the 2FA single path, so the `answer_otp()` function should allow this token type to proceed. Add SSS_AUTHTOK_TYPE_PAM_STACKED to the allowed authentication token types in `answer_otp()` to restore previous functionality. Fixes: 4cb99a2 ("krb5_child: advertise authentication methods"). Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Justin Stephenson <jstephen@redhat.com> (cherry picked from commit df15165)
When keep-alive sessions transition between command types (e.g., from SSS_PAM_PREAUTH to SSS_PAM_AUTHENTICATE), enterprise principal settings were not being updated, causing parsing inconsistencies in complex AD environments. This change ensures that when the backend sends updated enterprise principal settings for different command types, the principals are correctly re-parsed with the appropriate flags, fixing UPN handling in multi-domain AD environments. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Alejandro López <allopez@redhat.com> Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit dd3cd95)
In contrast to other authentication methods for PKINIT some information about the used Smartcard and certificate are already needed for the pre-authentication step to trigger the MIT Kerberos PKINIT module to get back the information if PKINIT is possible or not and if the Smartcard can be used for authentication. If krb5_child is kept running between the pre-authentication and the authentication step the information given during pre-authentication is used if Smartcard authentication was selected. As long as only a single certificate is available there is no issue. But if there are multiple certificates which all apply to the given mapping and matching rules for the user trying to log in and the user can choose a certificate for authentication the authentication might fail if the certificate use during pre-authentication and the one selected by the user differ. Before the change to keep krb5_child running for all authentication methods this was not an issue since the fresh instance started during the authentication step was using the certificate selected by the user. With this patch krb5_child is restart during the authentication step is Smartcard authentication was selected. Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Justin Stephenson <jstephen@redhat.com> (cherry picked from commit f3a36be)
Commit 1f680ed added ad_pac_common.c and $(NDR_KRB5PAC_LIBS) to sssd_pam unconditionally. So when building --without-samba, sssd_pam fails to link with undefined references to ndr_pull_init_blob and ndr_pull_PAC_DATA. This change qualifies those additions with `BUILD_SAMBA` so the PAC indicator feature is compiled in only when samba support is enabled. Reviewed-by: Sumit Bose <sbose@redhat.com> Reviewed-by: Tomáš Halman <thalman@redhat.com> (cherry picked from commit d0becea)
adcf277 to
9e62f39
Compare
This is the implementation for the so called passwordless-gdm feature. The design page for this feature is available at SSSD/sssd.io#79.
The original patch set was reviewed at #8212 by Justin and Sumit. I've had done some minor modifications to those patches:
As a reminder you can use https://copr.fedorainfracloud.org/coprs/ipedrosa/passwordles-gdm/ for testing and update authselect, mutter, gdm and gnome-shell packages. As for sssd, I'd use the build provided in this PR since it will contain the exact bits that will be shipped in RHEL.
authselect brings a new feature called
with-switchable-auththat you should enable to use this feature. In addition, you should add the following configuration to sssd.conf:Known limitations: