Adding unnecessary zeros when sync between encrypted folders

Feature
1

Steps:

  1. Create 3 devices: dev1, dev2, dev3
  2. Create folder on dev1 with any 1 byte data file
  3. Share folder from dev1 to dev2 with password
  4. Share folder from dev2 to dev3(Receive encrypted)
  5. Compare files on dev2 and dev3, we see difference like on screenshot

Screenshot from 2022-09-27 01-24-10
Screenshot from 2022-09-27 01-24-101920Ă—1080 364 KB

2

Interesting. That should work, though I’m not sure if it’s been tested so far :slight_smile: The size does change from the plaintext to the encrypted side, perhaps there’s a bug somewhere that tries to account for the same size change when going encrypted → encrypted.

If you do it again, does it add another block of padding?

3

Yes, every encrypted->encrypted transfer add additional zeros

Screenshot from 2022-09-30 13-26-21
Screenshot from 2022-09-30 13-26-211920Ă—1080 249 KB

4

There seems to be an issue already on encryption, i.e. dev1 to dev2. The encrypted file should be minPaddedSize + blockOverhead = 1064 bytes, but is 1435 bytes.

The zero padding on dev3 starts exactly after 1064 bytes. Might be the request somehow gets messed up because of the unexpected length of the file on dev2. Or a completely independent bug.

Could you post the output of syncthing cli debug file <folder-id> <filename> from dev2 and dev3.

6

Note about difference: screenshots for 2 byte file, dumps for 1 byte file

dev2:

{
  "availability": [
    "XGBTOVD-EFMHVRR-4BEWVV4-ZRLKYVL-UZHLLDR-UEBYHJ4-7C4QNPN-TEUXJQ3",
    "7777777-777777N-7777777-777777N-7777777-777777N-7777777-77777Q4",
    "63D5LQ4-J2VJRH3-4MAWJZJ-EQJZKOV-6QNU5F6-KMZG3NJ-TWDHLYI-KRPD7AG"
  ],
  "global": {
    "deleted": false,
    "ignored": false,
    "invalid": false,
    "localFlags": 0,
    "modified": "2009-02-14T02:31:30+03:00",
    "modifiedBy": "",
    "mustRescan": false,
    "name": "K.syncthing-enc/2U/16J8IDHF6U740H3GJ7C36I9RJ8K4ALRC5DTG",
    "noPermissions": false,
    "numBlocks": 1,
    "permissions": "0644",
    "sequence": 1,
    "size": 1064,
    "type": "FILE_INFO_TYPE_FILE",
    "version": [
      "AAAAAAA:1664531802"
    ]
  },
  "globalVersions": "{{Version:{[{AAAAAAA 1664531802}]}, Deleted:false, Devices:{XGBTOVD, 7777777, 63D5LQ4}, Invalid:{}}}",
  "local": {
    "deleted": false,
    "ignored": false,
    "invalid": false,
    "localFlags": 0,
    "modified": "2009-02-14T02:31:30+03:00",
    "modifiedBy": "",
    "mustRescan": false,
    "name": "K.syncthing-enc/2U/16J8IDHF6U740H3GJ7C36I9RJ8K4ALRC5DTG",
    "noPermissions": false,
    "numBlocks": 1,
    "permissions": "0644",
    "sequence": 1,
    "size": 1438,
    "type": "FILE_INFO_TYPE_FILE",
    "version": [
      "AAAAAAA:1664531802"
    ]
  },
  "mtime": {
    "err": null,
    "value": {
      "real": "0001-01-01T00:00:00Z",
      "virtual": "0001-01-01T00:00:00Z"
    }
  }
}

dev3:

{
  "availability": [
    "CXXMLAW-TWEUJV7-3ZCK5RN-NUGMTE6-VGYEGNM-ZHAEZ2I-PGRQEAV-Y22J6QE",
    "7777777-777777N-7777777-777777N-7777777-777777N-7777777-77777Q4"
  ],
  "global": {
    "deleted": false,
    "ignored": false,
    "invalid": false,
    "localFlags": 0,
    "modified": "2009-02-14T02:31:30+03:00",
    "modifiedBy": "",
    "mustRescan": false,
    "name": "K.syncthing-enc/2U/16J8IDHF6U740H3GJ7C36I9RJ8K4ALRC5DTG",
    "noPermissions": false,
    "numBlocks": 1,
    "permissions": "0644",
    "sequence": 1,
    "size": 1438,
    "type": "FILE_INFO_TYPE_FILE",
    "version": [
      "AAAAAAA:1664531802"
    ]
  },
  "globalVersions": "{{Version:{[{AAAAAAA 1664531802}]}, Deleted:false, Devices:{CXXMLAW, 7777777}, Invalid:{}}}",
  "local": {
    "deleted": false,
    "ignored": false,
    "invalid": false,
    "localFlags": 0,
    "modified": "2009-02-14T02:31:30+03:00",
    "modifiedBy": "",
    "mustRescan": false,
    "name": "K.syncthing-enc/2U/16J8IDHF6U740H3GJ7C36I9RJ8K4ALRC5DTG",
    "noPermissions": false,
    "numBlocks": 1,
    "permissions": "0644",
    "sequence": 1,
    "size": 1812,
    "type": "FILE_INFO_TYPE_FILE",
    "version": [
      "AAAAAAA:1664531802"
    ]
  },
  "mtime": {
    "err": null,
    "value": {
      "real": "0001-01-01T00:00:00Z",
      "virtual": "0001-01-01T00:00:00Z"
    }
  }
}
1 Like
7

Note about difference between screenshot 2 byte file and dumps 1 byte file

dev2.txt (1.4 KB)

dev3.txt (1.3 KB)

1 Like
8

Ah forgot about the “encrypted-file-info-trailer”. So the extra size above 1064B is that one, and apparently it’s also added to the file-info. Which makes sense for scanning (larger size isn’t detected as a change), but breaks the global state: There are now two file-infos with the same, global version, but different sizes. Dev3 shows the bigger size as it’s known global, either because it doesn’t have a file info from a un-encrypted device or because of a race condition.

I’ll need to dig and think a bit more, but seems like updating the file-info size for the trailer is wrong. Instead we should teach the scanner about it, such that it checks that <size on disk> == <size on fileinfo> + <trailer size>.

There might also be an issue with data requests, i.e. ensuring that we never serve that trailer there. However with the fix above, that likely shouldn’t happen as the other device will never request a byte range going into the trailer. We don’t have any safeguards against it happening though.

And when pulling on dev3 we create a file of the larger size, but then only fill one block → 1064 random bytes + a bunch of zeros for the dev2 trailer size + the trailer on dev3. Again as in the request, we could also add general safeguards for that but at the same time the proposed fix will make this case impossible again.

3 Likes