Use random filenames or hashes for blobstorage #4309

link2xt · 2023-04-05T10:09:55Z

Recently there was an accident with a chatbot that replaced its avatar set from the command line with an unrelated avatar of a contact. Both the selfavatar setting and the contact avatar i param pointed to $BLOBDIR/avatar.png at the time it was detected. How this happened is unclear, but it is possible that avatar.png was removed, unmounted or otherwise not detected by the core, and the core stored avatar received from the contact as avatar.png, while selfavatar config still pointed to $BLOBDIR/avatar.png.

Such bugs are unavoidable even if the core itself has no bugs as we cannot rely on blobdir not reside on the faulty network filesystem, being incorrectly backed up and restored etc., so we should assume that files may be randomly removed. Then there may be dangling $BLOBDIR/... references in the database which may accidentally point to unrelated files, could even be an avatar.png file sent to the bot in private.

To prevent such bugs, we should choose a random filename for the blobdir objects or as a hash of the contents.

Related issue: #3817

The text was updated successfully, but these errors were encountered:

link2xt · 2023-07-19T03:27:45Z

There is an attempt to fix at #4555, but it does not consider the discussion at #3817, the issue closed in favor of this one.

As a first step I suggest adding Param::Filename in addition to Param::File, to store the original filename. This will be backwards-compatible because if Param::Filename is missing, we can still extract the filename from Param::File which stores the filesystem path to the blob. This Param::Filename should be used as an attachment filename in sent emails so random suffix is not sent over the network.

As a second step we need new APIs to avoid leaking the blob path to the UI, I have drafted them at #3817 (comment)

Once blob paths are not visible in the non-deprecated API anymore and are not sent over the network, we can merge #4555 or even use completely random filenames.

dumblob · 2023-07-19T21:36:58Z

...This Param::Filename should be used as an attachment filename in sent emails so random suffix is not sent over the network.

Will the attachment filename be also encrypted to not leak the file name?

hpk42 · 2023-07-20T09:45:22Z

On Wed, Jul 19, 2023 at 14:37 -0700, dumblob wrote: Will the attachment filename be also encrypted to not leak the file name?

yes, typically the mime attachment is fully contained in the encrypted part, including the filename.

#4309) - Deprecate dc_msg_get_file(). It's kept for compatibility and now creates a temporary directory inside the blobdir, saves a file copy with unmangled filename, and returns its path. In the housekeeping we regularly clear these directories. - Add dc_msg_save_file() which saves file copy at the provided path and fails if file already exists. The UI should open the file saving dialog, defaulting to Downloads and original filename, when asked to save the file. After confirmation it should call dc_msg_save_file(). - Make dc_msg_get_filename() return the original filename for the use as a default in the file saving dialog. For this purpose add Param::Filename in addition to Param::File, to store the original filename. Param::Filename is used as an attachment filename in sent emails. - Use random filename suffixes for blobstorage. Thanks to the added Param::Filename these random suffixes is not sent over the network.

link2xt · 2023-07-26T12:17:59Z

This will also allow us to get rid of the sanitize-filename dependency.

…ame (#4309) It can be used e.g. as a default in the file saving dialog. Also display the original filename in the message info. For these purposes add Param::Filename in addition to Param::File and use it as an attachment filename in sent emails.

#4309) ... and fails if file already exists. The UI should open the file saving dialog, defaulting to Downloads and original filename, when asked to save the file. After confirmation it should call dc_msg_save_file().

Thanks to the added Param::Filename these random suffixes aren't sent over the network.

#4309) ... and fails if file already exists. The UI should open the file saving dialog, defaulting to Downloads and original filename, when asked to save the file. After confirmation it should call dc_msg_save_file().

Thanks to the added Param::Filename these random suffixes aren't sent over the network.

#4309) ... and fails if file already exists. The UI should open the file saving dialog, defaulting to Downloads and original filename, when asked to save the file. After confirmation it should call dc_msg_save_file().

Thanks to the added Param::Filename these random suffixes aren't sent over the network.

This way filenames in the blobstorage are just random hex numbers. This also allows us to get rid of the `sanitize-filename` dependency.

) This way filenames in the blobstorage are just random hex numbers. This also allows us to get rid of the `sanitize-filename` dependency.

#4309) ... and fails if file already exists. The UI should open the file saving dialog, defaulting to Downloads and original filename, when asked to save the file. After confirmation it should call dc_msg_save_file().

Thanks to the added Param::Filename these random suffixes aren't sent over the network.

) This way filenames in the blobstorage are just random hex numbers. This also allows us to get rid of the `sanitize-filename` dependency. This also requires `Param::Filename` to be set to "debug_logging*.xdc" for messages containing logging webxdc-s, otherwise they are not detected properly. This is done in "fix: Message::set_file_from_bytes(): Set Param::Filename", so don't forget to update senders as well.

#4309) ... and fails if file already exists. The UI should open the file saving dialog, defaulting to Downloads and original filename, when asked to save the file. After confirmation it should call dc_msg_save_file().

Recently there was an accident with a chatbot that replaced its avatar set from the command line with an unrelated avatar of a contact. Both the `selfavatar` setting and the contact avatar `i` param pointed to `$BLOBDIR/avatar.png` at the time it was detected. How this happened is unclear, but it is possible that `avatar.png` was removed, unmounted or otherwise not detected by the core, and the core stored avatar received from the contact as `avatar.png`, while `selfavatar` config still pointed to `$BLOBDIR/avatar.png`. Such bugs are unavoidable even if the core itself has no bugs as we cannot rely on blobdir not reside on the faulty network filesystem, being incorrectly backed up and restored etc., so we should assume that files may be randomly removed. Then there may be dangling `$BLOBDIR/...` references in the database which may accidentally point to unrelated files, could even be an `avatar.png` file sent to the bot in private. To prevent such bugs, we add random filename suffixes for the blobdir objects. Thanks to the added Param::Filename these random suffixes aren't sent over the network.

iequidoo · 2024-03-09T20:22:03Z

The next steps are:

Merge feat: Use random filenames for blobstorage (#4309) #4583, it adds dc_msg_save_file() + random filename suffixes in the blobstorage.
Change dc_msg_get_file(), see Mangled attachment/blob filenames #3817 (comment):

dc_msg_get_file API should create a temporary directory inside the blobdir, save a copy with unmangled filename, and return its path. In the housekeeping we regularly clear these directories. This API is kept for compatibility, but deprecated.
Add some new API like dc_msg_get_filedata_path() that avoids unnecessary copying if an app needs just to open a file.
Save files in the blobstorage with completely random filenames.
Switch the apps to these new APIs.

We already have dc_msg_get_filename() so UIs can use it for the "Save as" dialog.

EDIT: Currently everything is done in the mentioned PR except "Switch the apps to these new APIs.".

#4309) ... and fails if file already exists. The UI should open the file saving dialog, defaulting to Downloads and original filename, when asked to save the file. After confirmation it should call dc_msg_save_file().

Recently there was an accident with a chatbot that replaced its avatar set from the command line with an unrelated avatar of a contact. Both the `selfavatar` setting and the contact avatar `i` param pointed to `$BLOBDIR/avatar.png` at the time it was detected. How this happened is unclear, but it is possible that `avatar.png` was removed, unmounted or otherwise not detected by the core, and the core stored avatar received from the contact as `avatar.png`, while `selfavatar` config still pointed to `$BLOBDIR/avatar.png`. Such bugs are unavoidable even if the core itself has no bugs as we cannot rely on blobdir not reside on the faulty network filesystem, being incorrectly backed up and restored etc., so we should assume that files may be randomly removed. Then there may be dangling `$BLOBDIR/...` references in the database which may accidentally point to unrelated files, could even be an `avatar.png` file sent to the bot in private. To prevent such bugs, we add random filename suffixes for the blobdir objects. Thanks to the added Param::Filename these random suffixes aren't sent over the network.

) `dc_msg_get_file()` should create a temporary directory inside the blobdir, save a copy with unmangled filename, and return its path. In the housekeeping we regularly clear these directories. This API is kept for compatibility, but deprecated. Instead, add `dc_msg_get_filedata_path()` that avoids unnecessary copying if an app needs just to open a file and its name doesn't matter.

`dc_msg_get_file()` should create a temporary directory inside the blobdir, save a copy with unmangled filename, and return its path. In the housekeeping we regularly clear these directories. This API is kept for compatibility, but deprecated. Instead, add `dc_msg_get_filedata_path()` that avoids unnecessary copying if an app needs just to open a file and its name doesn't matter.