GNOME Bugzilla – Bug 567701
Teach krb5-auth-dialog to use gnome-keyring
Last modified: 2018-01-10 21:09:15 UTC
Description of problem: This program keeps asking for password, although we have a perfectly good keyring manager available. Version-Release number of selected component (if applicable): 0.7-7.fc9 How reproducible: Always. Steps to Reproduce: 1. Kerberos ticket expires, krb5-auth-dialog pops up to ask for password. Additional info: See Red Hat bug https://bugzilla.redhat.com/show_bug.cgi?id=478728 for patches to 0.7 and 0.8.
Thanks for the patch. Two comments here: * we should use secmem_malloc instead of gmalloc in case we're storing passwords (thats why secmem support got added for 0.8) * it'd be nice to split out the keyring handling into a separate file and to use ka_ wrappers for that to improve readability
Thanks for reviewing! I'll work on it and when I have something new, I'll report back.
I looked at the secmem stuff, but I think we have a different situation here. The password from the keyring gets fetched like this: rc = gnome_keyring_find_password_sync(&krb5_schema, &applet->kr.passwd[i], "application", "krb5-auth-dialog", "prompt", prompts[i].prompt, NULL); So, it's gnome keyring API that allocates memory for the password here - not us. We just free it later, with the appropriate function: gnome_keyring_free_password (applet->kr.passwd[i]); My guess is that gnome keyring may already be using secure memory. I see a lot of references in the ChangeLog to secure memory. The rest of memory is already allocated securely, so we just use that to store passwords into the keyring. Does that work for you?
I just attached a new patch to that RH bug. It is here: https://bugzilla.redhat.com/attachment.cgi?id=329154
That patch that make use of keyring optional: https://bugzilla.redhat.com/attachment.cgi?id=329408
Sorry, this one: https://bugzilla.redhat.com/attachment.cgi?id=329409
Yet another iteration (avoid a possible crash when keyring is not in use and password deletion is attempted): https://bugzilla.redhat.com/attachment.cgi?id=329417
Hi Bojan, this looks nice! One major issue still remains: If we use pkinit trying a wrong PIN even once can be very bad since the smart card might lock itself after 3 wrong PINs so the pk_userid (if set) as well as the principal (if set) needs to be part of the password schema. The dialog offered by gnome-keyring-daemon when using ssh has a checkbox that allows to switch using gnome-kerying automatically on for this key on. Something similar might be needed here too so. This can probably be delayed though.
Not too familiar with smartcards, so I'll need a bit of help here. When you say that the card locks itself after 3 wrong pins - do you mean 3 successive tries with the wrong pin or 3 tries ever with the wrong pin? BTW, the code (as it is now), will try only once with the keyring stored password. If that is not successful, all passwords will be considered bad and removed from the keyring. In other words, one failure will cause the dialog to be displayed again and user will have to enter the password by hand.
| When you say that the card locks itself after 3 wrong pins - do you mean 3 | successive tries with the wrong pin or 3 tries ever with the wrong pin? 3 succdssive tries. | BTW, the code (as it is now), will try only once with the keyring stored | password. If that is not successful, all passwords will be considered bad and | removed from the keyring. In other words, one failure will cause the dialog to | be displayed again and user will have to enter the password by hand. Sure I read that from the patch but one failed attempt is 33% of attempts in a row you have. That's why we have to make sure that we don't waste them needlessly. Adding the principal and pk_userid to the password schema should do the trick.If the user switches pkinit on or off pk_userid will change and we'll show the password dialog.
Isn't that going happen automatically, because the Kerberos prompts will be different? For instance, if principal changes from foo@EXAMPLE.COM to bar@EXAMPLE.COM, the prompt will be changed too, which means that keyring search will not be successful and dialog will be displayed. Or am I misunderstanding this? Not sure exactly how krb5_get_init_creds_opt_set_pkinit works (which also calls auth_dialog_prompter). Do prompts always stay the same there?
Or did you mean to say that prompts for pkinit and regular authentication can be the same, so we need to have two entries in the keyring for each such prompt? In other words, have a flag in the schema as to what each prompt refers to? PS. Sorry about all the dumb questions. Just not too sure how pkinit stuff works...
Just one more question: krb5_get_init_creds_opt_set_pkinit - is that a Heimdal only thing or something? I don't see it in MIT krb5. Or maybe I didn't install all the required headers... Do you have API description handy for this function?
I just attached another patch to that RH bug report: https://bugzilla.redhat.com/attachment.cgi?id=329924 I'm purely guessing here and have no way to test, but I think we ought to delete keyring passwords if krb5_get_init_creds_opt_set_pkinit() returns an error.
To err on the side of caution, I attached a patch that completely avoids the keyring if pkinit is used: https://bugzilla.redhat.com/attachment.cgi?id=329925 Hopefully, this should eliminate accidental locking of smartcards.
Actually, this is more correct: https://bugzilla.redhat.com/attachment.cgi?id=329931 pk_userid may also be empty string (default), in which case we should also not attempt keyring.
Just tested your patch. I have gnome-keyring disabled via gconf (no pkinit) and when the credentials should be refreshed I get: DEBUG: credentials_expiring: Checking expiry <900s DEBUG: credentials_expiring: Expiry @ 1233355553 DEBUG: grab_credentials: Auth failed with -1765328252: Password read interrupted DEBUG: credentials_expiring: Checking expiry <900s DEBUG: credentials_expiring: Expiry @ 1233355553 DEBUG: grab_credentials: Auth failed with -1765328252: Password read interrupted instead of the libnotify message. Could you have a look.
Yeah, that's the result of me not using use_keyring variable inside credentials_expiring function. Misleading, but not really an error. I'll post another patch that you can test shortly.
Try this: https://bugzilla.redhat.com/attachment.cgi?id=330517
Will do, but please use gnome BTS for patches and provide follow up patches since otherwise it's too cumbersome to review the differences.
Created attachment 128043 [details] [review] Use gnome keyring to (optionally) provide passwords for authentication As requested, patch attached.
There has been a release since, without inclusion of this, so obviously no interest. Closing as WONTFIX.
This is still on my TODO list.
Created attachment 134999 [details] [review] Keyring patch against recent trunk This compiles, but I did not actually test it. Hopefully better than nothing.
Tested the new patch. Appears to work.
Created attachment 135008 [details] [review] Keyring patch against recent trunk, with preferences I didn't edit the XML file though.
Created attachment 135060 [details] [review] Keyring patch against recent trunk, with preferences
Created attachment 135064 [details] [review] Keyring patch against recent trunk, with preferences Trunk is a moving target.
Guido, Any time frame for inclusion of this patch? I'd like to be able to put this to rest (i.e. avoid creating more patches, because I'm lazy).
Bojan, Thanks for the update! No need to create more patches, I'll keep track of that. I'm currently trying to get krb5-auth-dialog into gnome 2.28 so I'll focus on cleanup and bug fixes. Adding gnome keyring support will be next however I'd like to move things around a bit before that. (The user should be able to select keyring usage from the password dialog itself and we should change the use_kr_only logic a bit).
Thank you!
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/krb5-auth-dialog/issues/1.