Syncthing runs into encfs file name length limit #3338

Closed
afiskon opened this Issue Jun 24, 2016 · 19 comments

Projects

None yet

4 participants

@afiskon
afiskon commented Jun 24, 2016 edited

Version Information

Syncthing Version: v0.13.7
OS Version: Ubuntu 14.04 (also affects Windows, MacOS, etc)

Steps to reproduce the problem

This bug can be reproduced in different environments. Please don't get an impression that only Linux users are affected.

Here is an example how to reproduce an issue:

  1. Install Ununtu 14.04 LTS, enable home directory encryption using EncFS. EncFS has a serious limit on maximum file name length which will help us to reproduce an issue.
  2. Install Syncthing
  3. Create a file with long enough name (close to FS limits) in Sync directory, for instance: /home/eax/Sync/books/by_topic/papers-redbook/0032 - ARIES- A Transaction Recovery Method Supporting Fine-Granularity Locking and Partial Rollbacks Using Write-Ahead Logging.pdf
  4. Sync with second machine that doesn't use EncFS
  5. Delete Syncthing, clear Sync directory
  6. Re-Install Syncthing, try to sync with second machine

What you expected to happen

All files sync successfully.

What actually happened

Syncthing doesn't sync some files. In log there are multiple error messages like this:

[TQISY] 10:29:46 INFO: Puller: final: open /home/eax/Sync/books/by_topic/papers-redbook/.syncthing.032 - ARIES- A Transaction Recovery Method Supporting Fine-Granularity Locking and Partial Rollbacks Using Write-Ahead Logging.pdf.tmp: file name too long 

Suggestion

Temp file name is longer than original file name and thus doesn't satisfy filesystem limits. Perhaps instead of .syncthing.(full file name).tmp you could use .syncthing.(sha256 of file name).tmp.

@canton7
Member
canton7 commented Jun 24, 2016

That won't solve the case where the folder names being too long is the problem: with long folder names it will make the issue worse, as a sha256 filename is significantly longer than most...

@afiskon
afiskon commented Jun 24, 2016

@canton7 You are right. I wonder whether there is a reason to create a temporary file at all...

@calmh
Member
calmh commented Jun 24, 2016

Syncthing is aware of typical Linux and Windows file system limits and uses hashing of the file name when necessary. This is encfs brokenness as far as I understand.

@afiskon
afiskon commented Jun 24, 2016 edited

@calmh "the problem is not on our side", I understand :) I'm using Ubuntu for like 5 years now. So far Syncthing is the only application I know that assumes it can take any file name then add a few characters to it's name and file with a resulting name suppose to create successfully. It's just not true on any file system.

@calmh
Member
calmh commented Jun 25, 2016

Google for "encfs filename too long". You'll get literally hundreds of reports of similar issues for all kinds of programs. The issue here is encfs arbitrarily reducing the allowed file name length.

@calmh calmh closed this Jun 25, 2016
@afiskon
afiskon commented Jun 25, 2016 edited

@calmh google "buffer overflow" (or "null pointer exception") you will find literally thousands of programs with such an issue. It doesn't mean that buffer overflows are OK or that this is an issue on Intel architecture side and developers have to do nothing about it.

@afiskon
afiskon commented Jun 25, 2016

@calmh Also I realize that Syncthing is an open source project and perhaps it's developed by a few people in their free time (I really don't know - is there any company behind Syncthing?). I'm a software developer too, I understand. Perhaps a donation to some PayPal account could change an attitude to this issue?

@AudriusButkevicius
Member

A pull request fixing whatever you are unhappy with can change the attitude of the issue.

@afiskon
afiskon commented Jun 25, 2016

@AudriusButkevicius true, but I'm afraid I personally can't do this. I have no expertise in Go and this project in particular. Not mentioning lack of free time. Is there anyone who would like to fix this for, I don't know, say 300 USD?

@AudriusButkevicius
Member

So what's the filename length limit in encfs?

From what I found online, it's 179 bytes (vgough/encfs#7), which even with all of our prefixes appended and prepended to the file you've pasted in question, does not exceed their limit, hence does not explain the problem.

@calmh calmh changed the title from EPIC: Syncthing 0.13 doesn't sync _some_ files to Syncthing runs into encfs file name length limit Jun 25, 2016
@calmh
Member
calmh commented Jun 25, 2016 edited

Feel free to extract this constant (240) to a named constant and tweak the value of it:

https://github.com/syncthing/syncthing/blob/master/lib/model/tempname.go#L38

It should probably be something along the lines of

const maxFilenameLength = 255 - len(".syncthing.") - len(".tmp")

or so, with 255 being some other number (that is the formula behind the current 240). If there is a documented, sane value that we should use I'd be fine with changing it. We should probably s/md5/sha1/ as well while we're on it.

@afiskon
afiskon commented Jun 27, 2016

Maximum file name supported by EncFS is documented in man encfs:

   One such limitation is filename length.  If your underlying filesystem
   limits you to N characters in a filename, then EncFS will limit you to
   **approximately 3*(N-2)/4**.  For example if the host filesystem limits to
   256 characters, then EncFS will be limited to 190 character filenames.
   This is because encrypted filenames are always longer then plaintext
   filenames.

Since underlying ext4 filesystem supports file names up to 255 maximum file name length for EncFS in theory should be "approximately" 189 bytes. In practice it's exactly 188 bytes. Perhaps one byte is reserved for trailing \0, I don't know. 188 - len(".syncthing.") - len(".tmp") = 173.

Not sure about:

We should probably s/md5/sha1/ as well while we're on it.

Both are crypto hash function designed to make it difficult to intentionally find two values with the same hash value. I don't think it's a case. I would say that even first 16 characters of MD5 (e.g. a244c98f25a26784) is more then enough.

@st-review st-review pushed a commit that referenced this issue Jun 27, 2016
@calmh calmh lib/model: Decrease max temp filename length (fixes #3338, fixes #3355)
GitHub-Pull-Request: #3356
a165838
@afiskon
afiskon commented Jun 27, 2016

@calmh Thanks a lot for a165838! I made a donation to Syncthing project as promised.

Fun fact. Last time I reported a problem regarding file loss to Dropbox developers I got a response like "well, we provide you this service for free so if something doesn't work it's not really our problem, especially if your are Linux user". And Syncthing team fixed a similar problem literally in 3 days (including weekends). You are the best! :)

@AudriusButkevicius
Member

I would still rather you follow up with encfs for them to either fix the docs or for them to explain why your shorter filename is failing

@calmh
Member
calmh commented Jun 27, 2016

@afiskon Thank you, much appreciated, but certainly not why we made the change. :)

@calmh calmh added a commit to calmh/syncthing that referenced this issue Jun 27, 2016
@calmh calmh Merge branch 'master' into protobufs
* master:
  lib/events: Introduce per-subscription event IDs (fixes #3335)
  lib/model: Decrease max temp filename length (fixes #3338, fixes #3355)
  gui: Suggest lower case only folder ID (fixes #3128)
  lib/events: Add logging/receiving benchmark
  lib/db: Fix alignment crash on 32 bit platforms
  gui, man: Update docs & translations
  gui: Add addresses for disconnected devices (fixes #3340)
  lib/config: Retain slash at end of path after expanding ~ (fixes #2782)
  lib/nat: Avoid concurrent reset of NAT timer (fixes #3337)
  lib/model: Refactor CheckFolderHealth into separate methods
77834d0
@afiskon
afiskon commented Jul 8, 2016 edited

Bug still reproduces on latest 0.13.10 build. Does this build include a165838?

@calmh
Member
calmh commented Jul 9, 2016

Yes.

@afiskon
afiskon commented Jul 9, 2016

After all too strict limitations are perhaps really an EncFS problem. Thanks anyway!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment