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

Increased file size when sharing between encrypted devices #8556

Closed
Ratio2 opened this issue Sep 26, 2022 · 5 comments · Fixed by #8563
Closed

Increased file size when sharing between encrypted devices #8556

Ratio2 opened this issue Sep 26, 2022 · 5 comments · Fixed by #8563
Assignees
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Milestone

Comments

@Ratio2
Copy link
Contributor

Ratio2 commented Sep 26, 2022

v1.21.0, Linux (64-bit Intel/AMD)

Host1 normal: 2 bytes
Host1 normal -> host2 encrypted: 1435 byte
Host2 encrypted -> host3 encrypted: 1806 byte
Host3 encrypted -> host4 encrypted: 2177 byte

Seems just add 371 bytes of zero data

Data can be decrypted, but have some problems:

  1. Data not interchangeable between different encrypted hosts (seems file size stored in database)
  2. Data comparison problematic for check same state

All files for investigation: data.zip

@Ratio2 Ratio2 added bug A problem with current functionality, as opposed to missing functionality (enhancement) needs-triage New issues needed to be validated labels Sep 26, 2022
@AudriusButkevicius
Copy link
Member

AudriusButkevicius commented Sep 26, 2022

Data is not supposed to be interchangeable nor comparable, and this is done on purpose, as it's encryption after all.
Otherwise you potentially open up oracles and known cipher text attacks.

i.e, if I know that I have an encrypted file that is top secret government document, and encryption was interchangeable and comparable, I could just find all other people that have the same encrypted file, and expose them.

"Adds xyz zero data" - yes, encryption requires padding and adds non-trivial overheads.

If you wish to discuss this further, please take this to the forum.

@Ratio2
Copy link
Contributor Author

Ratio2 commented Sep 26, 2022

Discussion on forum problem for me, just some notes:

  1. Encrypted data same, only zeroes added on another encrypted device (see screenshot)
  2. We can add padding on data transmission, no reason to keep zeroes on disk (we can trim zeroes and get same data)

Screenshot from 2022-09-27 01-24-10

@imsodin
Copy link
Member

imsodin commented Sep 29, 2022

I don't think the OP was a complaint about added data/bytes when encrypting, but about unexpected behaviour of size when sharing with multiple encrypted devices. And the zeros look weird, data should be all random for encrypted files. Anyway it's not clear how the sharing exactly works. @Ratio2 Could you please explain with steps what you did to arrive to those file sizes, as suggest in a topic on https://forum.syncthing.net/ please.

@Ratio2
Copy link
Contributor Author

Ratio2 commented Sep 30, 2022

https://forum.syncthing.net/t/adding-unnecessary-zeros-when-sync-between-encrypted-folders/19028

@imsodin
Copy link
Member

imsodin commented Sep 30, 2022

With the additional information from the thread above, it's clear there is a bug:
The problem lies in the trailer added on receive-encrypted folders containing the encrypted file-info (for offline decryption): On device 2, the size on the global file is 1064 as expected (minPaddedSize + blockOverhead). The local file is 1435 bytes, as the trailer size is added to the fileinfo. dev3 has those 1435 bytes as global, and adds its own trailer -> 1812 bytes.

@imsodin imsodin reopened this Sep 30, 2022
@imsodin imsodin changed the title Increase size of data when sync encrypted folder from one device to encrypted folder on other device Increased file size when sharing between encrypted devices Sep 30, 2022
@imsodin imsodin self-assigned this Sep 30, 2022
@imsodin imsodin removed the needs-triage New issues needed to be validated label Sep 30, 2022
imsodin added a commit to imsodin/syncthing that referenced this issue Sep 30, 2022
@acolomb acolomb linked a pull request Nov 7, 2022 that will close this issue
@calmh calmh closed this as completed in da72df6 Mar 10, 2023
calmh added a commit to calmh/syncthing that referenced this issue Mar 10, 2023
* main:
  gui: Add Persian (fa) translation template (syncthing#8822)
  lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563)
  gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539)
  build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804)
  build: Update dependencies (syncthing#8821)
calmh added a commit to calmh/syncthing that referenced this issue Mar 10, 2023
* main:
  gui: Add Persian (fa) translation template (syncthing#8822)
  lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563)
  gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539)
  build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804)
  build: Update dependencies (syncthing#8821)
calmh added a commit to calmh/syncthing that referenced this issue Mar 12, 2023
* main:
  lib/protocol: Cache expensive key operations (fixes syncthing#8599) (syncthing#8820)
  gui: Add Persian (fa) translation template (syncthing#8822)
  lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563)
  gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539)
  build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804)
  build: Update dependencies (syncthing#8821)
@calmh calmh added this to the v1.23.3 milestone Mar 14, 2023
calmh added a commit to tomasz1986/syncthing that referenced this issue Mar 18, 2023
* main: (424 commits)
  gui, man, authors: Update docs, translations, and contributors
  lib/protocol: Cache expensive key operations (fixes syncthing#8599) (syncthing#8820)
  gui: Add Persian (fa) translation template (syncthing#8822)
  lib: Correctly handle encrypted trailer size (fixes syncthing#8556) (syncthing#8563)
  gui: Disable Restore Versions filters when no versioned files exist (fixes syncthing#5408) (syncthing#8539)
  build(deps): bump github.com/chmduquesne/rollinghash from 0.0.0-20180912150627-a60f8e7142b5 to 4.0.0+incompatible (syncthing#8804)
  build: Update dependencies (syncthing#8821)
  lib/api: Expose `blocksHash` in file info (syncthing#8810)
  gui, man, authors: Update docs, translations, and contributors
  lib/discover: Don't leak relay-tokens to discovery (syncthing#8762)
  gui, man, authors: Update docs, translations, and contributors
  gui: Add Croatian (hr) translation template (syncthing#8801)
  build(deps): bump github.com/quic-go/quic-go from 0.32.0 to 0.33.0 (syncthing#8800)
  cmd/stupgrades: Cache should apply to HEAD as well as GET
  build: Add more GitHub Actions
  gui: Remove non-existent customicons.css file reference (fixes syncthing#8797) (syncthing#8798)
  Only fail after chmod error if permissions differ (e.g. on config file) (syncthing#8771)
  gui, man, authors: Update docs, translations, and contributors
  build: Use Go 1.19.6 for Windows
  build: Update dependencies
  ...
imsodin added a commit to imsodin/syncthing that referenced this issue Mar 28, 2023
…ng#8556)

In the original fix in syncthing#8563 I simply forgot this. Which meant syncthing#8556
wasn't actually fixed, as the trialer size would have been 0 (default),
and thus we would have still sent the inflated size to encrypted peers.
imsodin added a commit to imsodin/syncthing that referenced this issue Mar 28, 2023
…ng#8556)

In the original fix in syncthing#8563 I simply forgot this. Which meant syncthing#8556
wasn't actually fixed, as the trialer size would have been 0 (default),
and thus we would have still sent the inflated size to encrypted peers.
calmh pushed a commit that referenced this issue Mar 28, 2023
In the original fix in #8563 I simply forgot this. Which meant #8556
wasn't actually fixed, as the trialer size would have been 0 (default),
and thus we would have still sent the inflated size to encrypted peers.
calmh added a commit to calmh/syncthing that referenced this issue Apr 14, 2023
* main: (28 commits)
  build: Update dependencies (syncthing#8869)
  lib/model: Improve path generation for auto accepted folders (fixes syncthing#8859) (syncthing#8860)
  docs: fix typo (syncthing#8857)
  gui, man, authors: Update docs, translations, and contributors
  lib/syncthing: Handle successful global migration (fixes syncthing#8851) (syncthing#8852)
  gui, man, authors: Update docs, translations, and contributors
  lib/model: Set enc. trailer size on pull (ref syncthing#8563, syncthing#8556) (syncthing#8839)
  lib/model: Fix file size inconsistency due to enc. trailer (syncthing#8840)
  gui, man, authors: Update docs, translations, and contributors
  cmd/stdiscorv: Fix database test (fixes syncthing#8828)
  lib/ur: Fix custom releases URL comparison
  gui: Remove untranslated strings from JSON files (syncthing#8836)
  all: Fix typos found by codespell (syncthing#8833)
  gui: Hide download progress legend when download progress is disabled
  gui, man, authors: Update docs, translations, and contributors
  lib/protocol: Handle encrypted requests without encrypted hash (fixes syncthing#8277) (syncthing#8827)
  build(deps): bump github.com/hashicorp/golang-lru/v2 from 2.0.1 to 2.0.2 (syncthing#8826)
  lib/config: Allow sub-second watcher delay (fixes syncthing#7859) (syncthing#7864)
  gui, man, authors: Update docs, translations, and contributors
  lib/protocol: Cache expensive key operations (fixes syncthing#8599) (syncthing#8820)
  ...
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Mar 9, 2024
@syncthing syncthing locked and limited conversation to collaborators Mar 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug A problem with current functionality, as opposed to missing functionality (enhancement) frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants