-
Notifications
You must be signed in to change notification settings - Fork 81
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
authtok: add support for decrypting an auth token #294
base: main
Are you sure you want to change the base?
Conversation
Add support for storing an encrypted auth token in u2f_keys. The auth token is passed to PAM and may be used by other PAM modules. The encryption key is derived using the FIDO2 hmac-secret extension.
Hi, First of all, thank you for the contribution! I have given this a brief look and some thinking and wanted to write those down. As pointed out in the linked issue, there's a set of complexities here I'm worried about:
The first source of problems could potentially be partially alleviated by implementing In any case, I think we'll have to carefully consider these consequences, how to deal with them, and whether the added complexity is worthwhile before being able to move forward. Any thoughts or suggestions? |
I don't think we should implement
Since we also can't verify if the current auth token still works with gnome-keyring (or any other service that the auth token is for), there is not much we can do here. For UV requirement change, we can only detect this case during the next authentication by failing to validate the AEAD tag. At this point the encrypted auth token is already lost. The best we could do is to skip the auth token and report this error to the user. In both of the cases, the user has to manually set up the auth token again with pamu2fcfg. The solution is definitely not ideal, but I can't come up with anything better. We could potentially change pamu2fcfg to support updating existing auth tokens, but I'm not sure if the benefit is worth the added complexity. That being said, I still think this feature would be helpful to some users, since entering password for gnome-keyring defeats the purpose of using YubiKeys in a passwordless setup. |
Thank you for the feedback.
Marking the module as
Good point. For some reason, this use-case escaped me and, as you say, is probably sufficient reason to not do something like that -- at least not without somehow knowing that the user wants to wrap their login credentials.
Ack.
Agreed. At least for a potential first stab at this, the preference would be to keep it as simple as possible and manually dealing with the problem using I have not yet had time to take a closer look at your implementation or try it out for myself. I hope to be able to do so soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay. I've played around with this for a bit and left a couple of mostly high-level comments.
There's a few things we need to take into consideration, I hope I managed to cover most of them in the in-line comments. (It's quite a large diff which might benefit being split into smaller commits/parts/multiple MR:s to ease the design/review process.)
Thanks again for having looked into this!
hi, did test out this pr and it seems to address exactly the remaining issue I have (full debian login with fido2/u2f token) note: did successfully test this:
|
@LDVG I think I addressed your previous concerns. Could you take another look? Thanks! |
@zhihengq only thing I found a bit lacking is the debug output.
|
as an idea (most likely for a follow up pull) maybe it would also be possible to implement |
That's a great idea! Could you help me put it in? Or I can do that when I have some time.
I'm not exactly sure how to make it work. Changing the auth token requires presence of the fido2 device (and possibly human interaction) to encrypt the new token. What happens if the fido2 device is not present? |
ok, will push to your branch.
well, same as other |
ok, can´t push to your branch, so here is the patch: |
Debug log whether authtok is enabled and when token descryption is successful. Yubico#294 (comment)
@LDVG anything left that is blocking this pr? |
Hi, thanks for the ping. While we believe this is a good experimental start if people are finding it useful, this PR is rather big both in terms of LOC and the number of configuration knobs it introduces. Coupled with UX quirks related to e.g. changing your password and having to pass module arguments to pamu2fcfg, that we might want to figure out before shipping something like this, the maintenance cost of this is rather high -- and we have consequently given this feature a low priority. Ideally, we'd like to first understand how we can minimize the footprint of this feature. For example, in terms of code size, this may require first refactoring some parts of pam-u2f (e.g. to move assertion logic so that pamu2fcfg can reuse it). As I alluded to in my early reviews of this PR, one way of going about it could be to split this PR into smaller parts in which we can discuss each problem at a time and test things separately. Again, thanks for the contributions and interest in pam-u2f! |
Add support for storing an encrypted password in u2f_keys. During authentication, the password is decrypted and passed to PAM for other PAM modules.
Problem
When logging in with pam_u2f as a single factor, gnome-keyring cannot be auto-unlocked. This is because no password was provided during authentication, but gnome-keyring requires the user password to decrypt its database. Another user reported the same problem in #283.
Solution
This PR allows pamu2fcfg to optionally store a password for each authenticator. During authentication, pam_u2f retrieves the password and pass it to PAM as
PAM_AUTHTOK
. Subsequent PAM modules such as pam_gnome_keyring will use the password inPAM_AUTHTOK
to auto-unlock the keyring.Passwords are encrypted with AES-256-GCM and stored in the u2f_key file, along with an HMAC salt. The AES encryption key is derived using the FIDO2 HMAC-SECRET extension.
New arguments added:
allowauthtok
: an optional argument that enables this feature.-a, --authtok
: an optional flag to prompt the user for a password to be stored.-p, --pam-arguments=STRING
: an optional argument that specifies the arguments passed to pam_u2f. This is used to ensure users are authenticated in the same way when getting the HMAC secret.Next steps
Update the documentation for this feature.