GNOME Bugzilla – Bug 580068
gnome-keyring-daemon doesn't work with many simultaneius connections
Last modified: 2009-07-27 15:55:01 UTC
Please describe the problem: I use shmux to do several tasks on different (40+) servers in parallel. shmux opens independent ssh sessions to the servers, all sessions use gnome-keyring-daemon for auth. In several (about 15/40, sometimes more, sometimes less) cases I get "Agent admitted failure" message and the session is not connected. There are no errors if I connect in smaller (say, 10 servers per turn) connection packs. There are no errors if my id_rsa is not password protected (even on 50+ simultaneous connections). And there are no errors if I use ssh-agent instead of gnome-keyring-daemon Steps to reproduce: This is almost the same testcase (but easily reproducible): 1. Generate a password protected ssh key and add it to local authorized_keys (and to gnome-keyring-daemon as a ssh-agent too) 2. for n in `seq 10`; do ssh localhost "echo $n" & done 3. for n in `seq 20`; do ssh localhost "echo $n" & done Actual results: I expect both steps 2-3 to produce the same results (showing numbers in random order without errors) Expected results: The step 3 has enough simultaneous connections to make gnome-keyring-daemon shoot errors sometimes Does this happen every time? The testcase — yes, the success count is about 25-30 on my desktop computer with real servers (all other connections are reported as connection errors) Other information:
Is there anything on this bug? It looks like a race condition somewhere in accessing the private key. Actually when this happens, in gck_private_key_real_acquire_crypto_sexp I get self->pv->sexp == NULL and self->pv->sexp_uses == 0.
Actually this one solves the problem, but perhaps there is some better place to add a mutex. --- ./pkcs11/ssh-agent/gck-ssh-agent-ops.c.orig 2009-05-20 04:13:06.668931425 +0400 +++ ./pkcs11/ssh-agent/gck-ssh-agent-ops.c 2009-05-20 04:14:19.061499214 +0400 @@ -818,6 +818,7 @@ return hash; } +G_LOCK_DEFINE(sign_request); static gboolean op_sign_request (GckSshAgentCall *call) { @@ -888,7 +889,9 @@ g_return_val_if_fail (session, FALSE); /* Do the magic */ + G_LOCK(sign_request); result = gp11_session_sign (session, key, mech, hash, n_hash, &n_result, &error); + G_UNLOCK(sign_request); g_object_unref (session); g_object_unref (key);
Thanks for doing that research. I can see the problem now. It's a mistake in the architecture of loading the actual private RSA key. Sadly, I don't think this can be addressed in 2.26.3, as the changes are too big.
I've re-architected how keys get unlocked, among other things. This should fix the above problem you experienced. Any testing you can do is very appreciated. daemon/pkcs11/gkr-pkcs11-auth-ep.c | 414 ++---------------------- daemon/pkcs11/gkr-pkcs11-auth.c | 167 ++--------- daemon/pkcs11/gkr-pkcs11-auth.h | 16 +- pkcs11/gck/Makefile.am | 1 + pkcs11/gck/gck-attributes.c | 17 + pkcs11/gck/gck-attributes.h | 5 +- pkcs11/gck/gck-authenticator.c | 402 +++++++++++++++++++++++ pkcs11/gck/gck-authenticator.h | 77 +++++ pkcs11/gck/gck-certificate-key.c | 10 +- pkcs11/gck/gck-certificate-key.h | 3 +- pkcs11/gck/gck-certificate-trust.c | 18 +- pkcs11/gck/gck-certificate-trust.h | 3 +- pkcs11/gck/gck-certificate.c | 11 +- pkcs11/gck/gck-key.c | 8 +- pkcs11/gck/gck-key.h | 5 +- pkcs11/gck/gck-login.c | 1 + pkcs11/gck/gck-manager.c | 15 +- pkcs11/gck/gck-module.c | 108 ++++++- pkcs11/gck/gck-object.c | 300 +++++++++++------ pkcs11/gck/gck-object.h | 36 ++- pkcs11/gck/gck-private-key.c | 95 ++++-- pkcs11/gck/gck-private-key.h | 9 +- pkcs11/gck/gck-public-key.c | 10 +- pkcs11/gck/gck-session.c | 149 +++++++-- pkcs11/gck/gck-session.h | 8 + pkcs11/gck/gck-sexp.c | 3 +- pkcs11/gck/gck-sexp.h | 2 +- pkcs11/gck/gck-types.h | 1 + pkcs11/gck/gck-util.c | 11 + pkcs11/gck/gck-util.h | 2 + pkcs11/gck/tests/Makefile.am | 3 + pkcs11/gck/tests/mock-locked-object.c | 88 +++++ pkcs11/gck/tests/mock-locked-object.h | 52 +++ pkcs11/gck/tests/test-module.c | 34 ++- pkcs11/gck/tests/test-module.h | 10 +- pkcs11/gck/tests/unit-test-attributes.c | 27 ++ pkcs11/gck/tests/unit-test-authenticator.c | 254 +++++++++++++++ pkcs11/gck/tests/unit-test-memory-store.c | 11 +- pkcs11/gck/tests/unit-test-object.c | 245 ++++++++++++++ pkcs11/gck/tests/unit-test-store.c | 6 + pkcs11/gck/tests/unit-test-timer.c | 8 +- pkcs11/pkcs11g.h | 25 +- pkcs11/plex-layer/gck-plex-layer.c | 123 ++------ pkcs11/roots-store/gck-roots-certificate.c | 24 ++- pkcs11/roots-store/gck-roots-certificate.h | 3 +- pkcs11/roots-store/gck-roots-module.c | 2 +- pkcs11/ssh-agent/gck-ssh-agent-ops.c | 32 ++- pkcs11/ssh-store/gck-ssh-module.c | 10 +- pkcs11/ssh-store/gck-ssh-private-key.c | 55 +++- pkcs11/ssh-store/gck-ssh-private-key.h | 3 +- pkcs11/ssh-store/gck-ssh-public-key.c | 9 +- pkcs11/ssh-store/gck-ssh-public-key.h | 3 +- pkcs11/ssh-store/tests/Makefile.am | 4 +- pkcs11/ssh-store/tests/test-ssh-module.c | 105 ++++++ pkcs11/ssh-store/tests/test-ssh-module.h | 43 +++ pkcs11/ssh-store/tests/unit-test-private-key.c | 97 ++++++ pkcs11/user-store/gck-user-module.c | 2 +- pkcs11/user-store/gck-user-private-key.c | 12 +- pkcs11/user-store/gck-user-public-key.c | 4 +- pkcs11/user-store/gck-user-storage.c | 49 ++- pkcs11/user-store/gck-user-storage.h | 2 +- pkcs11/user-store/tests/.gitignore | 2 + pkcs11/user-store/tests/Makefile.am | 7 + pkcs11/user-store/tests/p11-tests.conf | 2 + 64 files changed, 2289 insertions(+), 974 deletions(-) commit a957c0ed1fbf61d355e8d0eed8fdfac969ec4e37 Merge: 40a9f2a... 5baf704... Author: Stef Walter <stef@memberwebs.com> Date: Sun Jul 19 19:29:05 2009 +0000 Merge branch 'unlock-objects' commit 5baf7048a690e1d1e6d6b677f19bcd687c92c9ac Author: Stef Walter <stef@memberwebs.com> Date: Sun Jul 19 19:25:45 2009 +0000 [daemon, ssh-agent] Use authenticator objects instead of auth-cache. Now that we have authenticator objects in the actual pkcs#11 modules remove the auth cache for unlocking objects from the daemon, and use authenticator objects to lock objects from the ssh-agent. commit 71c0f037a3bbcf2ba4e1c46302834b345cf41847 Author: Stef Walter <stef@memberwebs.com> Date: Sun Jul 19 19:24:27 2009 +0000 [gck] Pass session to get/set attribute functions. This allows attributes to be different depending on which session or application they're accessed from. Implement CKA_ALWAYS_AUTHENTICATE as a session dependent attribute. commit 45c79443a280bcda6e4c88f79e5db2f3391ac938 Author: Stef Walter <stef@memberwebs.com> Date: Sun Jul 19 18:40:35 2009 +0000 [gck] Don't map object identifiers in plex layer. This conflicts with future plans for having object handles present in the attributes of other objects. commit d5f960e74da3c39c46b358f982e24407c77441b9 Author: Stef Walter <stef@memberwebs.com> Date: Sun Jul 19 17:39:56 2009 +0000 [gck] Tweak authenticator enumerator to return boolean. gck_session_for_each_authenticator() now returns a boolean value if any of the callbacks returned successfully. commit 6a6d07aa2265ff842b37900faaf13b096c0fdd5a Author: Stef Walter <stef@memberwebs.com> Date: Sun Jul 19 03:44:53 2009 +0000 [gck] Add authenticator objects for storing authenticated state. Authenticator objects are now used for PKCS#11 context specific logins. In the future we'll use them for unlocking stuff on a token, session, or timeout basis. commit 4508771f734590140051391b0a9222451f2aa453 Author: Stef Walter <stef@memberwebs.com> Date: Sat Jul 18 22:42:48 2009 +0000 [gck] Add support for transient PKCS#11 objects. Transient objects are not stored permanently. These may be token objects with token 'scope' but dissappear automatically at some point in the future. Auto-destructed objects are always token objects. commit 3f7704cd340e9e8ee88dd5180a52cc583bb9daf3 Author: Stef Walter <stef@memberwebs.com> Date: Sat Jul 18 22:42:19 2009 +0000 [gck] Add helper function to parse bool attribute. commit b98358d3c3e43792474ff2622c6d3397c3a9badf Author: Stef Walter <stef@memberwebs.com> Date: Sat Jul 18 20:13:18 2009 +0000 [gck] PKCS#11 objects now track the module they're in. Since multiple modules share the same address space, this is a necessary change for future robustness and changes. commit 508054a15c332a7fa3ba3dd509863ed81eeea188 Author: Stef Walter <stef@memberwebs.com> Date: Sat Jul 18 19:30:16 2009 +0000 Figure out CKA_TOKEN for a PKCS#11 object based on manager. Instead of manually setting CKA_TOKEN as the 'permanent' property on an object, we automatically infer it from the manager that the token object is on.
Wow, that is a huge patch. I've git-cloned the today's version and built it into my 2.26 gnome. I ran several tests that would crash the previous version and this one looks quite stable (at least in the parallel auth). Thanks a lot! P.S. If I run across any bugs, I will file them here. Thanks again!
Yeah, that was a lot of changes, but it was needed for stuff coming in future versions of gnome-keyring. The previous architecture was just plain wrong, as your testing and locking proved. A matter of personal preference, could you file new bugs for the new ones you find, unless this fix is absolutely wrong? Thanks!