Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External Changes on Samba Share not recognized on Linux #2536

Closed
jhit opened this issue Dec 4, 2018 · 11 comments · Fixed by #3612
Closed

External Changes on Samba Share not recognized on Linux #2536

jhit opened this issue Dec 4, 2018 · 11 comments · Fixed by #3612

Comments

@jhit
Copy link

jhit commented Dec 4, 2018

We work in a shared network environment with four users using one OSX, two Windows 10 and one Ubuntu 18.10 (me). Keepass databases are stored on a NAS that is accessible via Samba shares. We verified, that the KeepassXC instances on the Windows 10 machines properly detect external changes of the KeePass database (changes were made on Windows and Linux) while my instance on Ubuntu Linux does not. I played with the settings, disabled safe saving and auto-saving, but my KeepassXC never updated and always overwrites external changes without prompting or merging.

I tested this with current stable release 2.3.4 and with a custom compile from the develop branch. Both builds behave exactly the same. Fix from #1799 does not fix this issue on Samba shares.

Expected Behavior

Like on Windows 10 systems, I expect KeepassXC to detect external changes of the database and do a reload of the database.

Current Behavior

External changes are not detected when the database is located on a Samba share

Steps to Reproduce

  1. Put a kdbx file on a Samba share
  2. Open the kdbx File on a Linux workstation and a Windows 10 Workstation
  3. Change an entry on the Windows workstation. (notice the Linux workstation does not update)
  4. Change a different entry on the Linux workstation and safe the kdbx file.
  5. KeepassXC on the Windows workstation will reload the file and the changes there will disappear.

Debug Info

KeePassXC - Version 2.3.4
Revision: 6fe821c

Libraries:

  • Qt 5.11.1
  • libgcrypt 1.8.3

Operating system: Ubuntu 18.10
CPU architecture: x86_64
Kernel: linux 4.18.0-11-generic

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • Legacy Browser Integration (KeePassHTTP)
  • SSH Agent
  • YubiKey
@jhit jhit changed the title External changes on Samba share not recognized on Linux External Changes on Samba Share not recognized on Linux Dec 4, 2018
@jhit
Copy link
Author

jhit commented Dec 10, 2018

I did some background research on this Issue and tested if changing Samba protocol version in the fstab entry from ver. 2.1 to 1.0 would change anything - but no it does not. (See. https://askubuntu.com/a/995142/593177)
My mount options now are:
//server.local/work on /mnt/work type cifs (rw,relatime,vers=1.0,cache=strict,username=myuser,domain=workgroup,uid=1000,forceuid,gid=1000,forcegid,addr=192.168.1.20,nocase,soft,unix,posixpaths,serverino,mapposix,acl,rsize=1048576,wsize=1048576,echo_interval=60,actimeo=1,x-gvfs-show)
Seems that inotify with cifs is totally broken on Linux: https://stackoverflow.com/questions/8124617/getting-file-create-notifications-for-cifs-mount-in-linux/10107889#10107889.

So I guess going the inotify way here is not possible. Not for Samba/cifs and not for NFS

@jhit
Copy link
Author

jhit commented Dec 10, 2018

I checked if smbnetfs over FUSE can do the trick but unfortunately the aswer is NO.

@droidmonkey
Copy link
Member

The only real possibility I see is a timed poll of the file and performing a hash of the file on each poll.

@jhit
Copy link
Author

jhit commented Dec 10, 2018

Why not simply check the timestamp? If It was changed just do the reload. If the file did not change one reload more or less will not hurt, but continuously hash the database would probably be a waste of CPU cycles.
Or maybe store a hash as a unencrypted file attribute for the kdbx file when it gets written and check that file attribute?

@droidmonkey
Copy link
Member

Checking modified time would work although it can be unreliable. It is a good trigger to perform additional checks. I would have to check on storing the file hash in the header, I believe we can do that in KDBX4 because you can put arbitrary information in the header. KDBX3 is another story.

@jhit
Copy link
Author

jhit commented Dec 10, 2018

I'm fine with a KDBX4 only solution. I know Timestamp can be unreliable, but here it is only used to initiate a reload. I think for this it would be ok.

@jhit
Copy link
Author

jhit commented Jan 11, 2019

This is related to issue #2389 and might be solved together.

@JulienPalard
Copy link

It may be tightly related, or not, but I tried over sshfs and external changes are not spotted neither (the second user was overwriting the passwords entered by the first one, without any notice).

@wohali
Copy link

wohali commented May 2, 2019

Just ran into this bug myself, which caused loss of data :(

Any ETA for resolution? Switching to e.g. NFS for mounting isn't really an option; NFS mounts over Wi-Fi are generally asking for trouble.

@droidmonkey droidmonkey added this to the v2.5.0 milestone May 2, 2019
@droidmonkey droidmonkey self-assigned this May 2, 2019
@dssouza-ti
Copy link

Hi everyone. Is it possible that this issue could also affect KeeShare? I tried to share a group between a few Linux and Windows PCs, and stored the file in a samba share, but I was unable to make it work in the linux PC, only the Windows PC could detect changes made and update the shared group.

@droidmonkey
Copy link
Member

Yes keeshare uses the same change detection algorithm.

droidmonkey added a commit that referenced this issue Oct 10, 2019
* Fix #3506
* Fix #2389
* Fix #2536
* Fix #2230

Every database that has been opened now watch's it's own file. This allows the database class to manage file changes and detect fail conditions during saving. Additionally, all stakeholders of the database can listen for the database file changed notification and respond accordingly.

Performed significant cleanup of the autoreload code within DatabaseWidget. Fixed several issues with handling changes due to merging, not merging, and other scenarios while reloading.

Prevent database saves to the same file if there are changes on disk that have not been merged with the open database.
droidmonkey added a commit that referenced this issue Oct 14, 2019
* Fix #3506
* Fix #2389
* Fix #2536
* Fix #2230

Every database that has been opened now watch's it's own file. This allows the database class to manage file changes and detect fail conditions during saving. Additionally, all stakeholders of the database can listen for the database file changed notification and respond accordingly.

Performed significant cleanup of the autoreload code within DatabaseWidget. Fixed several issues with handling changes due to merging, not merging, and other scenarios while reloading.

Prevent database saves to the same file if there are changes on disk that have not been merged with the open database.
droidmonkey added a commit that referenced this issue Oct 20, 2019
* Fix #3506
* Fix #2389
* Fix #2536
* Fix #2230

Every database that has been opened now watch's it's own file. This allows the database class to manage file changes and detect fail conditions during saving. Additionally, all stakeholders of the database can listen for the database file changed notification and respond accordingly.

Performed significant cleanup of the autoreload code within DatabaseWidget. Fixed several issues with handling changes due to merging, not merging, and other scenarios while reloading.

Prevent database saves to the same file if there are changes on disk that have not been merged with the open database.
droidmonkey added a commit that referenced this issue Oct 20, 2019
* Fix #3506
* Fix #2389
* Fix #2536
* Fix #2230

Every database that has been opened now watch's it's own file. This allows the database class to manage file changes and detect fail conditions during saving. Additionally, all stakeholders of the database can listen for the database file changed notification and respond accordingly.

Performed significant cleanup of the autoreload code within DatabaseWidget. Fixed several issues with handling changes due to merging, not merging, and other scenarios while reloading.

Prevent database saves to the same file if there are changes on disk that have not been merged with the open database.
droidmonkey added a commit that referenced this issue Oct 20, 2019
* Fix #3506
* Fix #2389
* Fix #2536
* Fix #2230

Every database that has been opened now watch's it's own file. This allows the database class to manage file changes and detect fail conditions during saving. Additionally, all stakeholders of the database can listen for the database file changed notification and respond accordingly.

Performed significant cleanup of the autoreload code within DatabaseWidget. Fixed several issues with handling changes due to merging, not merging, and other scenarios while reloading.

Prevent database saves to the same file if there are changes on disk that have not been merged with the open database.
scoroi pushed a commit to scoroi/keepassxc that referenced this issue Nov 10, 2019
* Fix keepassxreboot#3506
* Fix keepassxreboot#2389
* Fix keepassxreboot#2536
* Fix keepassxreboot#2230

Every database that has been opened now watch's it's own file. This allows the database class to manage file changes and detect fail conditions during saving. Additionally, all stakeholders of the database can listen for the database file changed notification and respond accordingly.

Performed significant cleanup of the autoreload code within DatabaseWidget. Fixed several issues with handling changes due to merging, not merging, and other scenarios while reloading.

Prevent database saves to the same file if there are changes on disk that have not been merged with the open database.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants