passwordless-GDM: smartcard and passkey #12
Conversation
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>
ikerexxe
force-pushed
the
gdm-krb5-auth-sel
branch
from
b167fbf to
89541da
Compare
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>
These new options are needed by the GDM integration, but they can be
reused for CLI prompting.
:config: New options to tune smartcard prompting: 'init_prompt' and
'pin_prompt'.
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
ikerexxe
force-pushed
the
gdm-krb5-auth-sel
branch
from
89541da to
098ae69
Compare
ikerexxe
force-pushed
the
gdm-krb5-auth-sel
branch
from
098ae69 to
d22a6f3
Compare
justin-stephenson
left a comment
justin-stephenson
left a comment
There was a problem hiding this comment.
I did a first round of review, some only minor comments are in-line. Overall it LGTM, I have 2 comments about the UX - I only tested passkey, not smart card auth:
-
Adding an IPA user with
ipa user-add pkuser --user-auth-type=passkeythen at the GDM login screen clicking the gear icon in the bottom right it showsPasswordandPasskeybut I expect it would showPasskeyonly. -
Adding an IPA user with
ipa user-add pkuser --user-auth-type=passkey --user-auth-type=password, at the GDM login screen clicking the gear icon in the bottom right it shows onlyPasswordunder login methods, while the password prompt says "Insert your passkey device...". I would expect GDM login methods to showPasskeyandPassword.
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>
ikerexxe
force-pushed
the
gdm-krb5-auth-sel
branch
from
d22a6f3 to
0e95eeb
Compare
This was a config mistake on my part when |
justin-stephenson
left a comment
justin-stephenson
left a comment
There was a problem hiding this comment.
Changes LGTM, thank you (small note I primarily focused on the passkey part).
|
Hi @ikerexxe Can you please re-upload SSSD build to your COPR https://copr.fedorainfracloud.org/coprs/ipedrosa/passwordles-gdm/ again? The rpms for latest build |
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>
This new option is needed by the GDM integration, but it can be reused for CLI prompting. :config: New option to tune passkey prompting: 'pin_prompt'. 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>
`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>
Due to the difficulty of having a single source for the prompts strings for both CLI and GUI, it has been decided to leave them fixed and use the strings proposed by Allan in the mockups design. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
ikerexxe
force-pushed
the
gdm-krb5-auth-sel
branch
from
baeecec to
f5c7b35
Compare
|
In my latest tests with
|
|
Major changes in the latest version:
Apart from that I also run the test at SSSD#8159 locally to make sure that smartcard auth is working. Finally, build is available in COPR repo. |
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>
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
When authenticating with a passkey, different PAM code paths within SSSD
can result in the `authtok` containing data even when the user did not
enter a PIN. Depending on the flow (e.g., triggered by `gdm` vs. `su`),
this data might be an empty string or non-printable characters like `^L`
(form feed).
The previous code had two issues:
1. It only checked if the `authtok` was non-empty
(`sss_authtok_get_type(...) != SSS_AUTHTOK_TYPE_EMPTY`). If user
verification was disabled, this check would incorrectly pass for
these 'junk' `authtok` values. This caused SSSD to prepare and send
an erroneous PIN to the passkey helper.
2. In the case where the `authtok` *was* correctly empty, the check
would fail, `write_buf_len` would remain 0, and the `if
(write_buf_len != 0)` block containing the `write_pipe_send` call
would be skipped. This stalled the authentication flow, as the
callback to continue the process was never set.
This patch fixes both issues:
1. The `user_verification` setting is now stored in the state struct.
The logic is updated to only prepare the PIN buffer if the `authtok`
is non-empty *and* user verification is required
(`state->user_verification != PAM_PASSKEY_VERIFICATION_OFF`).
2. The `write_pipe_send` call is moved outside the conditional block so
it always runs. This ensures that the asynchronous child
communication (via `passkey_child_write_done`) is always triggered,
even if the write buffer is empty (0-length).
This resolves both failure modes: junk PINs are no longer sent when
verification is off, and the auth flow no longer stalls when no PIN is
present.
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>
ikerexxe
force-pushed
the
gdm-krb5-auth-sel
branch
from
5779157 to
ce6c432
Compare
|
Major changes in the latest version:
This is ready for review again ;) |
|
Hi, thank you for the last update, controlling the available authentication types work much better now. I would suggest to mention the limitation of the JSON protocol with respect to two-factor authentication and the man page entry of bye, |
Add a note to clarify that 2FA isn't supported in JSON protocol and fix man page compilation for `pam_json_services` option. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
Updated in a new commit (last one). You can disregard CI failures, Fedora server doesn't seem to respond to the |
|
Hi, thanks for the updates, (so far) I have no further comments. bye, |
|
CI failures seem to be unrelated so I'm merging. |
Extension for the passwordless-GDM feature where the smartcard and passkey authentication methods have been added.
The design page is available at SSSD/sssd.io#79.
You can use https://copr.fedorainfracloud.org/coprs/ipedrosa/passwordles-gdm/ for testing. As a reminder you should update sssd, mutter, gdm and gnome-shell packages and you also need to include the following configuration option in
/etc/sssd/sssd.conf:Known limitations:
PIN requestandPIN attempts leftinformation to GDM. This is temporary and once this PR or the other one are merged, I'll update the PR to make these functionality available.