I did not check the code, but went straight to testing. There is one thing that caught my attention. When the number of attempts remaining is less than 3 the user is supposed to get that number before asking for the PIN, so that they are aware that they are in a risky situation. Something like:

$ su - iker@ipa.test
Insert your passkey device, then press ENTER.
You have 3 PIN attempts remaining
Enter PIN:
su: Authentication failure

In general terms, passkey preflight should be run during preauth stage. I don't know how hard it would be to implement it this way but it would make more sense.

I did not check the code, but went straight to testing. There is one thing that caught my attention. When the number of attempts remaining is less than 3 the user is supposed to get that number before asking for the PIN, so that they are aware that they are in a risky situation. Something like:

$ su - iker@ipa.test
Insert your passkey device, then press ENTER.
You have 3 PIN attempts remaining
Enter PIN:
su: Authentication failure

In general terms, passkey preflight should be run during preauth stage. I don't know how hard it would be to implement it this way but it would make more sense.

It is run during preauth stage, I just had to move around the pam_sss code which sends the message to the console. Please try again.

@ikerexxe Please confirm these are the appropriate backport labels to use also

@ikerexxe Please confirm these are the appropriate backport labels to use also

What RHEL versions do you target?

This PR supersedes #7631

I've updated the code to address Alexey's concerns

@ikerexxe, thanks for updates.

Do I understand correctly that the only point of passkey: timeout argument refactor patch is to use 'timeout = 1' for preflight()?

I think it's not enough.

'sssd_pam' should provide timeout as a command line argument to 'passkey_child' and this (perhaps decreased by 1) should be used instead of TIMEOUT define (see 'p11_child' as example).

What about this ^^?

Do I understand correctly that the only point of passkey: timeout argument refactor patch is to use 'timeout = 1' for preflight()?
I think it's not enough.
'sssd_pam' should provide timeout as a command line argument to 'passkey_child' and this (perhaps decreased by 1) should be used instead of TIMEOUT define (see 'p11_child' as example).

Unless you plan to provide a way for this to be configurable I don't see any difference between the actual implementation and your proposal. Thus, I'd prefer to keep it as it is.

Do I understand correctly that the only point of passkey: timeout argument refactor patch is to use 'timeout = 1' for preflight()?
I think it's not enough.
'sssd_pam' should provide timeout as a command line argument to 'passkey_child' and this (perhaps decreased by 1) should be used instead of TIMEOUT define (see 'p11_child' as example).

Unless you plan to provide a way for this to be configurable

It is already configurable via sssd.conf::passkey_child_timeout option.

I don't see any difference between the actual implementation and your proposal.

The issue is that 'passkey_child' doesn't use it.

There are two distinct timeouts happening here: the timeout you're referring to controls how long the PAM responder will wait for the passkey_child process to complete. The other, separate timeout determines how much time the user has to physically connect their FIDO2 token.

There are two distinct timeouts happening here: the timeout you're referring to controls how long the PAM responder will wait for the passkey_child process to complete. The other, separate timeout determines how much time the user has to physically connect their FIDO2 token.

They should be related. At the very least 2nd can't be greater than 1st.
Take 'p11_child' as an example: all internal child timeouts should be less than governing sssd_pam timeout.

Agreed to tackle the timeout topic on a different PR.

@ikerexxe @madhuriupadhye @alexey-tikhonov Hi, can you approve the PR if you have no further comments/concerns?

CI, and more specifically the system tests, are in the red, and I believe this is due to our changes. I have asked Madhuri to review them.

@ikerexxe @madhuriupadhye @alexey-tikhonov Hi, can you approve the PR if you have no further comments/concerns?

IIUC, we need tests to be updated alongside with this PR.
I don't care if it's part of this PR or a separate PR, but it should be ready.

Closing this for now.

I will open a new PR once this PR is ready, I will work with QE to have passkey tests updated alongside these changes also.