After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 580068 - gnome-keyring-daemon doesn't work with many simultaneius connections
gnome-keyring-daemon doesn't work with many simultaneius connections
Status: RESOLVED FIXED
Product: gnome-keyring
Classification: Core
Component: general
2.26.x
Other All
: Normal normal
: ---
Assigned To: GNOME keyring maintainer(s)
GNOME keyring maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2009-04-24 03:37 UTC by Pavel Pushkarev
Modified: 2009-07-27 15:55 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26



Description Pavel Pushkarev 2009-04-24 03:37:38 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:
Comment 1 Pavel Pushkarev 2009-05-20 00:02:57 UTC
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.
Comment 2 Pavel Pushkarev 2009-05-20 00:22:01 UTC
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);
Comment 3 Stef Walter 2009-06-26 16:30:55 UTC
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.
Comment 4 Stef Walter 2009-07-19 19:32:58 UTC
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.
Comment 5 Pavel Pushkarev 2009-07-27 15:36:00 UTC
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!
Comment 6 Stef Walter 2009-07-27 15:55:01 UTC
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!