GNOME Bugzilla – Bug 657234
gnome-keyring does not store password if file permissions are wrong
Last modified: 2015-11-30 08:55:22 UTC
We got a setup with CIFS mounted /home directories, where every file within ~ gets 700 permissions. gnome-keyring seems to expect 600 permissions on the keyring files in order to store passwords in it. This check should be a bit softened, so 700 permissions are accepted. too.
Which version is this about?
gnome-keyring-3.0.3 Here are some warnings I have found in the logs: couldn't create temporary file for: /home/user/.gnome2/keyrings/login_25.keyring: Operation not supported messages:Aug 24 12:29:01 host gnome-keyring-daemon[5586]: couldn't create login keyring: An error occurred on the device messages:Aug 24 12:29:01 host gnome-keyring-daemon[5586]: failed to unlock login keyring on startup messages:Aug 24 12:29:33 host gnome-keyring-daemon[5586]: keyring was in an invalid or unrecognized format: /home/user/.gnome2/keyrings/login_25.keyring messages:Aug 24 12:29:36 host gnome-keyring-daemon[5586]: couldn't create temporary file for: /home/user/.gnome2/keyrings/default.keyring: Operation not supported messages:Aug 24 12:29:36 host gnome-keyring-daemon[5586]: couldn't create collection: An error occurred on the device Could it perhaps be, that the keyring files are hardlinks? Because hardlinks are not supported on the CIFS shares.
gnome-keyring uses flock() for synchronization. If flock() is not supported on the file system, then gnome-keyring cannot (as currently implemented) store keyrings on that file system. If someone wishes to implement a way to fix this, I would help by reviewing such patches.
Afair flock() is neither supported by CIFS nor NFS. It is recommended to use fcntl, instead.
Okay, understood. Any one be interested in implementing a change to fcntl? I don't have time to fix this right now, but I'd certainly review patches right away.
There are some problems with POSIX locking. It can only be made reliable if we can be sure that the locking process never opens the file again or dups the file descriptor. This is not easy to guarantee. A solution to this problem is the use of a separate lock file on which fcntl locking works. It might however be easier to use a straight lockfile instead of fcntling a lock file. I am currently evaluating the pros and cons.
Perhaps (f)locking in a separate file in tmp could be an option?
You have remotely mounted directories and thus it is possible that other clients or the server may access the files. /tmp is not shared. Sure, sensitive data like keyrings should only be accessed by one user and better by one program. However, the user might be logged in to two boxes and wants to acces his keyrings. After all this is the reason why gnome-keyring locks these files.
Werner, I'm interested in you research once you figure out (In reply to comment #6) > A solution to this problem is the use of a separate lock file on which > fcntl locking works. It might however be easier to use a straight > lockfile instead of fcntling a lock file. I am currently evaluating > the pros and cons. Do you mean creating and destroying a lockfile and synchronizing on that? I'll be interested in your research. Especially where there are big performance issues. FWIW, sqlite also can't lock on NFS reliably. They say that fcntl() is broken on many NFS implementations: http://www.sqlite.org/faq.html#q5
Right. GnuPG works this way because its update procedure is to copy, modify and rename. Thus we can't have a lock on the original file. However there might be problems on NFS and SMBFS as well. What definitely not work is running GnuPG on the same home directory from a Windows and a Unix system. I plan to do some tests.
Meanwhile I updated the GnuPG code and made it buildable without the GnuPG framework. It still uses a link file but detects at runtime whether link works. Thus on a FAT fs it will use an O_EXCL style locking but on most others fs it will use the more reliable hardlink based locking. There is a simple test program common/t-dotlock.c which can be used to test the dotlock module. I ran successful tests on FAT and SMBFS (mounted from an XP instance). However I have currently no CIFS running, maybe someone can test this. Build instructions are at the end of t-dotlock.c. You need to get the files dotlock.c, dotlock.h and t-dotlock.c from http://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=tree;f=common
Regarding performance: The link(2) approach should be pretty fast as it merely updates a directory entry. fcntl locking might me faster but given all the NFS problems I doubt it makes sense unless you really have a need for fast record locking. The bottlenech will be the communication overhead of the remote FSs. I have not done any benchmarks, though.
Please disregard my comments on SMBFS. I actually used CIFS for testing; must have changed my configuration quite some time ago.
Dear Werner. I found some time to test t-dotlock and it works fine on our CIFS mounted shares which makes use of mitchell + french symlinks. Btw. gnupg itself has the same problems, but I guess this is a separate topic.
Created attachment 198431 [details] [review] Replace flock by dot file locking
Thanks for testing. I did a patch against gnome 3.2. However this is not tested; I have to setup a new VM for GNOME first. Stef: Am I right that there is no regression test case for the locking?
Sweeet. Thanks! I'll give a shot at make a regression test case. I agree we should have that. Additionally, dotfile.c should probably go alongside stuff like egg-mkdtemp.c and egg-gmtime.c in the egg/ directory.
A recent test showed that the file locking was not the original problem. It is a problem of course but it will only show up later. I had Stef's comment on flock in mind and jumped on that train. However the transaction stuff is the actual culprit for the errors reported in comment 2. I have not yet done any debugging but it uses link(2) and checks for the error code - as we know, this is not reliable on remote file systems. That may or may not be the cause for the errors.
Stef: Do you have any description of the transaction system as implemented in GKR? BTW: Are EGG, GCR and GCK acronyms?
Created attachment 198844 [details] [review] Moved the dotlock module to egg/ This is an additional patch to move the dotlock files to the egg directory. The files are not prefixed with an "egg-" prefix for easier update from the GnuPG repo. However all external symbols are prefixed with "egg_".
Hi, I cannot really reproduce this problem. When putting my home directory on a cifs the gconf-santiy check fails at startup and when i ignore this i get a lengthy error message from gconf when i start gnome applications: Quote: No database available to save your configuration: Unable to store a value at key '/apps/seahorse/listing/show_validity', as the configuration server has no writable databases. There are some common causes of this problem: 1) your configuration path file /etc/gconf/2/path doesn't contain any databases or wasn't found 2) somehow we mistakenly created two gconfd processes 3) your operating system is misconfigured so NFS file locking doesn't work in your home directory or 4) your NFS client machine crashed and didn't properly notify the server on reboot that file locks should be dropped. If you have two gconfd processes (or had two at the time the second was launched), logging out, killing all copies of gconfd, and logging back in may help. If you have stale locks, remove ~/.gconf*/*lock. Perhaps the problem is that you attempted to use GConf from two machines at once, and ORBit still has its default configuration that prevents remote CORBA connections - put "ORBIIOPIPv4=1" in /etc/orbitrc. As always, check the user.* syslog for details on problems gconfd encountered. There can only be one gconfd per home directory, and it must own a lockfile in ~/.gconfd and also lockfiles in individual storage locations such as ~/.gconf End Quote So it seems like gconf has probably the same locking problem. Each time i log in a new keyring is created. I can then unlock/lock it and store passwords in it with seahorse. But i get errors in /var/log/messages: Oct 12 16:44:33 localhost gnome-keyring-daemon[5008]: couldn't stat directory: /home/andre/.gnome2/keyrings: Value too large for defined data type Looking at the code it seems to me that this failue to stat the directory causes gnome-keyring not to "see" the available keyrings in that directory and thus does not even try to open them. I have mounted the home directory with the following options (output of mount command): //192.168.123.1/testordner on /home/andre type cifs (rw,relatime,sec=ntlm,unc=\\192.168.123.1\testordner,username=,uid=500,forceuid,gid=0,noforcegid,addr=192.168.123.1,file_mode=0700,dir_mode=0755,nounix,serverino,rsize=16384,wsize=131008,actimeo=1) Any advice on how to fix the stat error / get my system on cifs into a state where gnome-keyring actually tries to unlock existing keyrings on login?
Why do you mount with option "nounix"?
nounix was added by default. I've solved my problem by adding the option noserveriono as suggested on https://wiki.archlinux.org/index.php/Samba But now it just works correctly and unlocks the default keyring on login.
Our environment is of course a bit special. We have to rely on EMC storage which does not support server side Unix extensions. To allow symlinks, we make use of Mitchell and French Fake Symlinks. Here is the list of mount options that we use: nobrl,mfsymlinks,noserverino,file_mode=0700,dir_mode=0700,user=$USER,uid=$UIDNUM,cruid=$UIDNUM,gid=$GIDNUM,sec=krb5
As Werner stated, link is used to create a hardlink which is not supported on that kind of shares. A solution could be to use softlinks instead.
Do you say that EMC does not support hardlinks, a feature we have in Windows since XP?
(In reply to comment #19) > Stef: Do you have any description of the transaction system as > implemented in GKR? > > BTW: Are EGG, egg is a GNOMEism for code that copied between modules and does not yet live in a library. Should probably eventually end up in a library. > GCR Gcr is a crypto UI library. It's been recently split out of gnome-keyring, and now lives at git.gnome.org/gcr.git. Documentation: http://developer.gnome.org/gcr/unstable/ > GCK acronyms? Gck is a library of PKCS#11 GObject bindings. Lives with Gcr. Documentation: http://developer.gnome.org/gck/unstable/
(In reply to comment #19) > Stef: Do you have any description of the transaction system as > implemented in GKR? (sorry, responding in bits and pieces, between taking care of the son :) The transaction system basically needs to be able to roll back any changes that are made if a failure happens later in the transaction. The changes need to appear to take effect up until that point. Only one thread is in side a PKCS#11 module at any given time, so the transaction doesn't have to worry about multi threading. Implemented in pkcs11/gkm/gkm-transaction.[ch] and tested in the accompanying tests directory.
> Stef: Am I right that there is no regression test case for the locking? So I added a regression case for the locking, and found a few bugs along the way, which underscores the value of testing. FWIW, this gnome2-store code has some problems, isn't heavily used, and in fact will be phased out soon. But I would like to get the locking squared away, since new code will almost certainly have similar requirements. So I appreciate you help getting this locking right. I went on to apply your patch and fix up conflicts with gnome-keyring git master. But then I noticed that dotfile.c is LGPLv3+ (and GPLv2+). The code in the gnome2-store part of gnome-keyring is LGPLv2+, and including LGPLv3+ code would make it all LGPLv3+, unless I'm misunderstanding something. We haven't moved to LGPLv3 yet, so this poses a bit of an obstacle. Is this code written by you Werner, and would you be opposed to relicensing the dotfile stuff as LGPLv2+? I'm not personally against the GPLv3 per se, but I'm not especially interested in getting involved in the politics surrounding being the one of the first GNOME module to go GPLv3.
@Werner, hardlinks are a problem on mfsymlink mounted shares.
Stef, thanks for the explanations. Some smaller parts of GnuPG have always been under LGPL; we changed that to LGPLv3+ some time ago. This is a quite natural choice for GPLv3 software. These parts are not heavily used by other applications and thus the GPLv2only/LGPLv3+ incompatibiliy was not an issue. Now while I extracted the code to GKR I imagined that this GPLv2only incompatibility might be a problem and thus I dual-licensed it according the the GNU maintainer standards. I don't think that moving back to LGPLv2+ is a good idea for the JNLIB stuff in GnuPG. Given that getting locking right on remote file systems is a problem on all OSes and that I would like to do it once and for all OSes, it might be better to allow as an alternative a BSD license just for dotlock.{c,h}. This will be similar to gnupg/common/estream-printf.c. What do you think?
Marcus: In comment 14 you wrote that the t-dotlock test program worked fine for you. Did you noticed a locking for `t-dotlock.tmp.lock' done via O_EXCL message?
No, I did not notice such a message.
That means that hardlinks work on your system. The runtime test we use is pretty straightforward: if (stat (tname, &sb)) return -1; nlink = (unsigned int)sb.st_nlink; lname = jnlib_malloc (strlen (tname) + 1 + 1); if (!lname) return -1; strcpy (lname, tname); strcat (lname, "x"); link (tname, lname); if (stat (tname, &sb)) res = -1; /* Ooops. */ else if (sb.st_nlink == nlink + 1) res = 0; /* Yeah, hardlinks are supported. */ else res = 1; /* No hardlink support. */ The trick is not to look at the result of link(2) and ERRNO but to check that the NLINK field has the expected value. The transact code explicitly tests for certain ERRNO values of link(2).
Ah, sorry I was wrong. Just missed it as it was directly on top: locking for `t-dotlock.tmp.lock' done via O_EXCL t-dotlock[31496]: lock created t-dotlock[31496]: lock taken t-dotlock[31496]: lock released
(In reply to comment #31) > I don't think that moving back to LGPLv2+ is a good idea for the JNLIB > stuff in GnuPG. Given that getting locking right on remote file > systems is a problem on all OSes and that I would like to do it once > and for all OSes, it might be better to allow as an alternative a BSD > license just for dotlock.{c,h}. This will be similar to > gnupg/common/estream-printf.c. What do you think? That would be perfect. Thanks!
I did that. Please find them in the GnuPG master.
Merged with master, and did more tests. The code in the gnome2-store module isn't exactly pretty, so it took a bit of time to get it all right. :S
Is this fixed with this patch? If not, then please do reopen this bug, with further patch(es).
Meanwhile I looked at the transaction system and confirmed that it is the root cause of Marcus’ problem. To finally solve this buf, I created two patches: The first improves the tests for the transaction part and the second fixes the problem (surprise). Marcus tested that patch for me (on F17) and it seems it really does what it should do. The trick is to fallback to a file copy if link(2) fails with anything other than EEXISTS. Any objections to change the code to work on non-hardlink capable systems?
Created attachment 220660 [details] [review] improve transaction tests
Created attachment 220661 [details] [review] Fallback for systems without a working hardlink
Review of attachment 220660 [details] [review]: Looks good. Would prefer just one change below. ::: pkcs11/gkm/tests/test-transaction.c @@ +385,3 @@ + do_test_write_large_file (test, 512); + do_test_write_large_file (test, 513); + g_assert_cmpuint (n_data, ==, buffersize); Could you make these separate tests? You would call g_test_add() multiple times passing GUINT_TO_POINTER (N) as the (ususally NULL) tdata argument to g_test_add(). You could then use that value in the second argument to the test case.
Created attachment 220665 [details] [review] Provide fallback for file systems without working hardlinks Thanks for the patch! This patch looks ready to go in once the tests are updated (since it's good if they're committed in that order, obviously). I've updated it to fix the indentation (we try to use tabs, not spaces).
Comment on attachment 220665 [details] [review] Provide fallback for file systems without working hardlinks BTW, Werner do you have commit access to git.gnome.org?
(In reply to comment #45) > (From update of attachment 220665 [details] [review]) > BTW, Werner do you have commit access to git.gnome.org? No.
Created attachment 220763 [details] [review] improve transaction tests (2)
Comment on attachment 220665 [details] [review] Provide fallback for file systems without working hardlinks Attachment 220665 [details] pushed as 7f31eeb - Provide fallback for file systems without working hardlinks
Comment on attachment 220763 [details] [review] improve transaction tests (2) Attachment 220763 [details] pushed as c95d152 - Improve the gkm transaction tests
Thanks Werner. Pushed to git master. Will be part of gnome-keyring 3.5.6.
@Werner Koch, @Stef Walter: Hi guys, this commit caused a huge regression, creating thousand of temp files when SSHFS is used for $HOME: https://git.gnome.org/browse/gnome-keyring/commit/?id=7f31eebd35a84ed4c09970edb0b8d99a6a8af6f5 Provide fallback for file systems without working hardlinks The relevant bug report is in: https://bugzilla.gnome.org/show_bug.cgi?id=730587 Could you please comment on if you plan to fix or revert that regression? It's prohibiting thousands of installations (some of them shown in http://www.ltsp.org/stories/widget-map) from upgrading to newer Gnome versions (all distributions), so your input will be much appreciated. Thank you!