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 646389 - keyring should use g_get_user_runtime_dir
keyring should use g_get_user_runtime_dir
Status: RESOLVED FIXED
Product: gnome-keyring
Classification: Core
Component: general
2.91.x
Other Linux
: Normal normal
: ---
Assigned To: GNOME keyring maintainer(s)
GNOME keyring maintainer(s)
: 646492 (view as bug list)
Depends on:
Blocks: 523057
 
 
Reported: 2011-03-31 22:20 UTC by William Jon McCann
Modified: 2012-04-02 15:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use runtime dir instead of tmp for sockets (4.73 KB, patch)
2011-04-01 20:47 UTC, William Jon McCann
needs-work Details | Review

Description William Jon McCann 2011-03-31 22:20:02 UTC
Would be good to use the XDG runtime directory provided by g_get_runtime_dir() instead of /tmp for sockets etc.
Comment 1 William Jon McCann 2011-03-31 22:37:03 UTC
oops I meant g_get_user_runtime_dir()
Comment 2 Lennart Poettering 2011-03-31 22:39:24 UTC
yes, please!
Comment 3 William Jon McCann 2011-04-01 20:47:55 UTC
Created attachment 184903 [details] [review]
Use runtime dir instead of tmp for sockets
Comment 4 Stef Walter 2011-04-02 18:22:17 UTC
*** Bug 646492 has been marked as a duplicate of this bug. ***
Comment 5 Stef Walter 2011-04-03 05:09:32 UTC
Review of attachment 184903 [details] [review]:

Cool. Thanks! Just needs a few changes before this can go in.

::: daemon/gkd-util.c
@@ +113,3 @@
 	/* Generate a new directory */
 	if (!valid) {
+		master_directory = g_build_filename (g_get_user_runtime_dir (), "keyring", NULL);

I think the string here needs have the "-XXXXXX" ending, since its passed to egg_mkdtemp().

::: daemon/gpg-agent/gkd-gpg-agent-standalone.c
@@ +104,3 @@
 		return 1;
 
+	sock = gkd_gpg_agent_startup (g_get_user_runtime_dir ());

Don't need to make this change, as its just a example program.

::: daemon/ssh-agent/gkd-ssh-agent-standalone.c
@@ +106,3 @@
 		return 1;
 
+	sock = gkd_ssh_agent_startup (g_get_user_runtime_dir ());

Don't need to make this change, as its just a example program.

::: pkcs11/rpc-layer/Makefile.am
@@ +32,2 @@
 	$(GLIB_CFLAGS)
 

Did this accidentally get included in this patch?

@@ +65,3 @@
+
+gkm_rpc_daemon_standalone_CFLAGS = \
+	$(GLIB_CFLAGS)

Good fix, but maybe this should be split off to another commit. You can push this right away.

::: pkcs11/rpc-layer/gkm-rpc-daemon-standalone.c
@@ +115,1 @@
 	if (sock == -1)

This is just a test/example/devel program, so no need to make this change.
Comment 6 Josh Triplett 2011-04-04 05:24:07 UTC
(In reply to comment #5)
> Review of attachment 184903 [details] [review]:
> 
> Cool. Thanks! Just needs a few changes before this can go in.
> 
> ::: daemon/gkd-util.c
> @@ +113,3 @@
>      /* Generate a new directory */
>      if (!valid) {
> +        master_directory = g_build_filename (g_get_user_runtime_dir (),
> "keyring", NULL);
> 
> I think the string here needs have the "-XXXXXX" ending, since its passed to
> egg_mkdtemp().

The subdirectory in the user runtime dir doesn't need to use a secure name, since it will go in a user-private directory.  It doesn't hurt to do so, though.
Comment 7 Stef Walter 2011-04-04 06:27:47 UTC
Josh, if keyring daemon is run for multiple login sessions, each will need its own subdirectory of XDG_RUNTIME_DIR.

Hmmm, on another topic, there's also this strangeness in the spec:

    Files in this directory MAY be subjected to periodic clean-up. To
    ensure that your files are not removed, they should have their
    access time timestamp modified at least once every 6 hours of
    monotonic time or the 'sticky' bit should be set on the file.

That's not an annoying feature from the perspective of a daemon writer. But I guess we need to either update the time every so often. or set the sticky bit. Are there security ramifications to setting the sticky bit?

One thing I forgot, William, is that the glib dependency needs to be updated to account for the use of the new g_get_user_runtime_dir() function.
Comment 8 Stef Walter 2012-03-15 12:34:45 UTC
Split this patch into two, and pushed to git. Skipped on changes to the example programs.

Thanks.
Comment 9 Stef Walter 2012-04-02 15:06:18 UTC
This patch doesn't work in practice :S

XDG_RUNTIME_DIR is not yet set when gnome-keyring-daemon is started by PAM.

Unlike other bits of initialization, gnome-keyring-daemon needs to create this directory immediately, so that it get initialization further by the session startup.

When XDG_RUNTIME_DIR is not set, Glib defaults to /data/.cache as the return value from g_get_user_runtime_dir(). And it's easy to have this be a directory that doesn't work with sockets (NFS, etc.)
Comment 10 Stef Walter 2012-04-02 15:55:55 UTC
This is caused by ordering issues in PAM configuration. In particular pam_gnome_keyring.so 'session' directive needs to be after pam_systemd.so

Posted heads up for packagers to desktop-devel-list about this.
Comment 11 Stef Walter 2012-04-02 15:56:35 UTC
Related fix (which will go into gnome-keyring 3.4.1):

commit cc6e1136fb9c98f26d60afee492a60865b0b2b93
Author: Stef Walter <stefw@gnome.org>
Date:   Mon Apr 2 16:22:48 2012 +0200

    Use correct XDG_RUNTIME_DIR when started from PAM
    
     * Accept the XDG_RUNTIME_DIR environment variable from
       the session when initializing gnome-keyring-daemon