Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

man: fix default value for pam_passkey_auth #7227

Closed
wants to merge 1 commit into from

Conversation

ikerexxe
Copy link
Contributor

@ikerexxe ikerexxe commented Mar 4, 2024

The default was changed to true, but the man page wasn't updated.

Fixes: c76ba34

@ikerexxe ikerexxe added passkey Issues and PRs related to 'passkey' feature backport-to-stable Trivial labels Mar 4, 2024
Copy link
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

thanks, now the man page matches the code at https://github.com/SSSD/sssd/blob/master/src/responder/pam/pamsrv.c#L52, ACK.

bye,
Sumit

@alexey-tikhonov
Copy link
Member

Why is 'pam_passkey_auth = true' but 'pam_cert_auth = false' by default?

I'm also not sure about 'Fixes: commit-hash' notation - script that prepares release notes most probably won't like it.

The default was changed to true in
c76ba34 ("PAM: Passkey kerberos preauth
support"), but the man page wasn't updated.

Signed-off-by: Iker Pedrosa <[email protected]>
@ikerexxe
Copy link
Contributor Author

ikerexxe commented Mar 5, 2024

Why is 'pam_passkey_auth = true' but 'pam_cert_auth = false' by default?

No idea 😅

I'm also not sure about 'Fixes: commit-hash' notation - script that prepares release notes most probably won't like it.

I changed it, thanks for the feedback.

@alexey-tikhonov
Copy link
Member

Why is 'pam_passkey_auth = true' but 'pam_cert_auth = false' by default?

No idea 😅

Why do we want 'passkey_auth' enabled by default?

Doesn't this trigger additional steps in krb pre-auth for users who doesn't use passkey?

@ikerexxe
Copy link
Contributor Author

ikerexxe commented Mar 5, 2024

Why do we want 'passkey_auth' enabled by default?

Doesn't this trigger additional steps in krb pre-auth for users who doesn't use passkey?

No, you'd still need to enable passkey authentication per user (--user-auth-type=passkey). So, even if you registered a passkey for a user, you'd still need to enable the authentication mechanism for the user.

@alexey-tikhonov
Copy link
Member

alexey-tikhonov commented Mar 5, 2024

Why do we want 'passkey_auth' enabled by default?
Doesn't this trigger additional steps in krb pre-auth for users who doesn't use passkey?

No, you'd still need to enable passkey authentication per user (--user-auth-type=passkey). So, even if you registered a passkey for a user, you'd still need to enable the authentication mechanism for the user.

It's enough to have it enabled locally to have additional pre-auth step:

https://issues.redhat.com/browse/RHEL-21786?focusedId=24266630&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-24266630
(sorry for private link)

Besides this being visible in the logs (and thus rendering questions) this probably (? I don't know) adds some latency.

@sumit-bose
Copy link
Contributor

Why is 'pam_passkey_auth = true' but 'pam_cert_auth = false' by default?

Hi,

Smartcard authentication isn't enabled by default because checking the presence of a Smartcard might cause delays in the order of a few seconds for every authentication attempt.

HTH

bye,
Sumit

@pbrezina
Copy link
Member

Pushed PR: #7227

  • master
    • 5841348 - man: fix default value for pam_passkey_auth
  • sssd-2-9
    • 10c49b1 - man: fix default value for pam_passkey_auth

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Mar 15, 2024
@pbrezina pbrezina closed this Mar 15, 2024
@ikerexxe ikerexxe deleted the fix_man branch January 7, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
passkey Issues and PRs related to 'passkey' feature Pushed Trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants