man: add `sssd-passkey.5` by ikerexxe · Pull Request #7637 · SSSD/sssd

ikerexxe · 2024-10-09T10:00:06Z

Include a new man page for passkey to explain the behaviour of user_verification option in the different scenarios. It is a complex option, so it has been decided to add a table to simplify its understanding.

ikerexxe · 2024-10-09T10:01:29Z

@justin-stephenson and @sumit-bose here is the documentation that we agreed to create yesterday. Please take a look at it when you have some time.

justin-stephenson · 2024-10-09T19:39:38Z

Thanks very much for working on this @ikerexxe

To understand more clearly I did some testing of our current handling of passkey user-verification, specifically with kerberos-enabled passkey auth with IPA.

When IPA require-user-verification == true and my yubikey device has NO PIN set: device auth will fail as we always try to send a PIN to the passkey_child. If 'enter' is pressed without typing any characters then sssd falls back from passkey to password auth, instead of sending an empty PIN string.

When IPA require-user-verification == false and my yubikey device has a PIN set: device auth will fail as no PIN is sent to SSSD and then to the passkey_child.

Is this the expected behavior when a device does not match the IPA server side policy to require user verification?

src/man/sssd-passkey.5.xml

ikerexxe · 2024-10-10T07:20:44Z

When IPA require-user-verification == true and my yubikey device has NO PIN set: device auth will fail as we always try to send a PIN to the passkey_child.

Yes, that's the expected behaviour and that's what I intended to convey with:

sssd.conf	Device	Result
True	User verification is not configured	Authentication fails

I know I mention sssd.conf instead of IPA, but I wanted to simplify the table and taking into account that sssd.conf takes precedence over IPA I thought this was the best to achieve it. If this is not understood, let me know and we will try to improve the text.

If 'enter' is pressed without typing any characters then sssd falls back from passkey to password auth, instead of sending an empty PIN string.

Should we mention this in the man page?

When IPA require-user-verification == false and my yubikey device has a PIN set: device auth will fail as no PIN is sent to SSSD and then to the passkey_child.

This doesn't align with the text I wrote. I based my understanding in the local case, where https://github.com/SSSD/sssd/blob/master/src/passkey_child/passkey_child_credentials.c#L133 is executed, and thus, the PIN is requested. Taking into account that the device won't allow the authentication without the PIN I think this is something that we need to improve in the online/kerberos workflow. As we'll run the preflight option in the preauthentication phase, we can use it to prepare for this use case and request the PIN. WDYT?

sumit-bose · 2024-10-10T09:32:47Z

src/man/sssd-passkey.5.xml

+                        <row>
+                            <entry>True</entry>
+                            <entry>User verification is not configured</entry>
+                            <entry>Authentication fails</entry>


Hi,

I think might be more consistent to handle this case as if no (suitable) device is available at the pre-flight check. Then users have the chance to insert another device which requires a PIN before they enter the PIN.

bye,
Sumit

What I wrote refers to what happens in the authentication phase. By the previous message you mean that I should also explain the preflight phase in the man pages?

By the way, the behaviour that you mention is what is currently proposed in #7631, as errors are ignored at the passkey_child.c level, thus leaving an option for the user to insert another FIDO2 device between the preauth and auth phases.

Hi,

no, the pre-flight is is internal and should not be mentioned in the man page explicitly. What I meant is that "Authentication fails" give the impression that authentication fails immediately but I guess in reality what happens in the authentication phase is more a "User verification is requested but it is expected that authentication fails if no other device is inserted in between".

bye,
Sumit

Can you take a look at the updated text? I think I have reflected what you mention, but I would prefer to have a confirmation.

justin-stephenson · 2024-10-11T14:21:12Z

If 'enter' is pressed without typing any characters then sssd falls back from passkey to password auth, instead of sending an empty PIN string.

Should we mention this in the man page?

We can mention this.

When IPA require-user-verification == false and my yubikey device has a PIN set: device auth will fail as no PIN is sent to SSSD and then to the passkey_child.

This doesn't align with the text I wrote. I based my understanding in the local case, where https://github.com/SSSD/sssd/blob/master/src/passkey_child/passkey_child_credentials.c#L133 is executed, and thus, the PIN is requested. Taking into account that the device won't allow the authentication without the PIN I think this is something that we need to improve in the online/kerberos workflow. As we'll run the preflight option in the preauthentication phase, we can use it to prepare for this use case and request the PIN. WDYT?

Okay, yes I agree that this is a better approach. If UV is false (whether sssd.conf or IPA require-user-verification) and the device has a PIN set then SSSD will automatically determine this when it queries if the device has a PIN set and effectively re-enable UV for this auth(prompt for a PIN). This is also something to clearly state in the man page IMO.

justin-stephenson · 2024-10-11T14:29:24Z

This doesn't align with the text I wrote. I based my understanding in the local case, where https://github.com/SSSD/sssd/blob/master/src/passkey_child/passkey_child_credentials.c#L133 is executed, and thus, the PIN is requested. Taking into account that the device won't allow the authentication without the PIN I think this is something that we need to improve in the online/kerberos workflow. As we'll run the preflight option in the preauthentication phase, we can use it to prepare for this use case and request the PIN. WDYT?

Okay, yes I agree that this is a better approach. If UV is false (whether sssd.conf or IPA require-user-verification) and the device has a PIN set then SSSD will automatically determine this when it queries if the device has a PIN set and effectively re-enable UV for this auth(prompt for a PIN). This is also something to clearly state in the man page IMO.

I suppose this will need to be fixed in the code as part of #7631 or my SSSD PR when it comes.

ikerexxe · 2024-10-14T14:26:55Z

@justin-stephenson I updated the text to include the password fallback and the user verification from the device. Take a look at it when you have some time.

I suppose this will need to be fixed in the code as part of #7631 or my SSSD PR when it comes.

Yes, that would be the best approach.

justin-stephenson · 2024-10-15T13:13:21Z

@justin-stephenson I updated the text to include the password fallback and the user verification from the device. Take a look at it when you have some time.

Looks good to me, thank you Iker.

I suppose this will need to be fixed in the code as part of #7631 or my SSSD PR when it comes.

Yes, that would be the best approach.

alexey-tikhonov · 2024-10-15T13:17:10Z

@ikerexxe, please set backport labels.

ikerexxe · 2024-10-15T14:10:32Z

@ikerexxe, please set backport labels.

I'm taking this PR as a forum to agree on the documentation. Once we all have the same understanding I'll close it and include the commit in #7631, as we'll need to adapt the behaviour a little bit to match the documentation.

src/man/sssd-passkey.5.xml

Include a new man page for passkey to explain the behaviour of `user_verification` option in the different scenarios. It is a complex option, so it has been decided to add a table to simplify its understanding. Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>

ikerexxe · 2024-10-29T10:59:57Z

Hi @sumit-bose,

Are you ok with the current proposal? If so, I'd like to close this PR and move the changes to #7631 to have everything centralized in one place.

sumit-bose · 2024-10-29T12:58:04Z

Hi @sumit-bose,

Are you ok with the current proposal? If so, I'd like to close this PR and move the changes to #7631 to have everything centralized in one place.

Hi,

thank you for the reminder, I just added a comment to one of the conversations, but feel free to ignore it, if you think the current sentence is clear enough. Otherwise I do not have further comments. Please go on and move the changes to #7631.

bye,
Sumit

ikerexxe · 2024-10-29T15:07:26Z

thank you for the reminder, I just added a comment to one of the conversations, but feel free to ignore it, if you think the current sentence is clear enough. Otherwise I do not have further comments. Please go on and move the changes to #7631.

Updated, so I'm closing this PR and moving the changes to the other one. Thank you both for your reviews!

ikerexxe mentioned this pull request Oct 9, 2024

passkey: implement preflight option #7631

Closed

justin-stephenson reviewed Oct 9, 2024

View reviewed changes