qubes-issues icon indicating copy to clipboard operation
qubes-issues copied to clipboard

Less restricted file copy qrexec service

Open Nurmagoz opened this issue 2 years ago • 44 comments

Qubes OS release

4.2

Brief summary

moving/coping files was working ok in 4.1 regardless their name, now its rejected due to the name of the file used.

Steps to reproduce

Try to move/copy a file in none latin letters

Expected behavior

To move/copy file regardless what characters used to name it.

Actual behavior

nonenglishcharct

Tasks

More detailed design in https://github.com/QubesOS/qubes-issues/issues/8332#issuecomment-1912403043

  • [ ] Extend qfile-unpacker to have extra options for disabling various restrictions (independently): file names validation, symlink target restriction (refusing absolute symlinks or pointing outside of target directory)
  • [ ] Add a new qubes.UnsafeFilecopy qrexec service with file names validation disabled (but symlinks validation enabled)
  • [ ] Modify qvm-copy to select the service based on chosen files (see https://github.com/QubesOS/qubes-issues/issues/8332#issuecomment-1912403043)
  • [ ] Make qubes-builderv2 disable both restrictions when copying files into dispvm

Nurmagoz avatar Jul 07 '23 20:07 Nurmagoz

Indeed R4.2 introduces stricter filename validation, it needs to be (among other things) proper UTF-8 string. Maybe you use some different encoding for your file names?

marmarek avatar Jul 07 '23 20:07 marmarek

Maybe you use some different encoding for your file names?

I was using Debian 11 as a standalone backup, which I had shifted from Qubes 4.1 using Qubes backup. After restoring my backup, I now want to create a new standalone based on Debian 12. I successfully created the new Debian 12 standalone and attempted to transfer my files to it. However, I encountered an issue where only files with non-English characters became stuck during the transfer process.

Workaround: you can change the file names to English before transferring them. Once the files have reached the destination, you can revert the changes by renaming them back to their original names.

Nurmagoz avatar Jul 07 '23 20:07 Nurmagoz

Workaround: you can change the file names to English before transferring them. Once the files have reached the destination, you can revert the changes by renaming them back to their original names.

I recommend archiving them all into a single archive file so that you can keep the original names in the destination.

B

brendanhoar avatar Jul 07 '23 23:07 brendanhoar

Workaround: you can change the file names to English before transferring them. Once the files have reached the destination, you can revert the changes by renaming them back to their original names.

I recommend archiving them all into a single archive file so that you can keep the original names in the destination.

B

Note that this loses the protection provided by qfile-unpacker’s filtering.

DemiMarie avatar Jul 08 '23 00:07 DemiMarie

The error message (“invalid or incomplete multibyte or wide character”) is horrible, not least because it provides no information as to which character was disallowed. At a minimum, there should be separate errors for “name is not valid UTF-8” and “name contains a forbidden UTF-8 character”, and the latter must state explicitly which character was forbidden.

DemiMarie avatar Jul 08 '23 00:07 DemiMarie

I recommend archiving them all into a single archive file so that you can keep the original names in the destination.

Yes, and it destroys the whole concept of security improvements that qvm-copy now should provide. The whole strict policy change makes no sense in this case, only affects user experience.

I personally think, that not being able to copy symlinks and files with non-utf-8 chars in name anymore is a major design mistake. The user data should be preserved at all costs. Ext2/3/4 allows non-utf8 chars in path and that is how it is. Only '\0' is not allowed, it is the only exception according to FS specs.

@marmarek @DemiMarie What about adding some flags to qvm-copy to make it work as it was prior to R4.2, as expected by all current users? Is it possible to add it before R4.2 release, that will break users experience and force users to pack everything in tar (like @brendanhoar suggested and I will also have to do) just due to the fear that qvm-copy will silently fail with the copy process, skip something or break intentionally?

jamke avatar Dec 01 '23 05:12 jamke

Moving my messages here:

Let's consider a directory with 100 000 files. It has 1 absolute symlink somewhere in the depth, 1 relative symlink targeting above this directory, and several files that are fine except their filenames have byte sequences that cannot be interpreted as utf-8 string.

User wants to copy it to the other qube (maybe much less secure) the same way as EVERY other copy tool works: like cp and including qvm-copy that existed for many years up to R4.1.

What should the user do to achieve it with the new R4.2 approach?

jamke avatar Dec 02 '23 05:12 jamke

There are 2 cases: from terminal and from GUI (Nautilus or Dolphin). What the user should do in both cases to copy files and avoid user's data loss?

jamke avatar Dec 02 '23 05:12 jamke

Would it make sense to have a "qvm-copy --insecure" tool/option (including in the file manager)?

I think it definitely would! I understand what you are saying, I just think consequences can be worse, than what's gained, at least in my use cases. Because qvm-copy is already "secure", and the filtering is like adding additional layer. Like: why not check copied files with antivirus, or block copying shell/ELF files or something, it all looks as the same approach that copy tool should do to be called secure.

For me secure is more like:

  • for any directory/file it does not cause any code execution itself because of the file naming or data (like buffer overflow or bad parsing code),
  • for any directory/file it should not cause user data losses (or increase changes of it); other meaning of secure as in safe.

The R4.1 meets these criteria completely.


My main point: After these changes the user will never be sure if Qubes OS copy tool will work as they expect:

  • User moves a directory with a big size, goes to drink tea or even sleep, comes back and sees that there is an error about symlink or filename that happened in the beginning of the process.
  • After that user can remove this symlink or tar it and start again, once again not being sure that it will not stop in a minute, or an hour.
  • If user is able to auto-skip such files, than they will almost certainly loose some files eventually. At least I would.

jamke avatar Dec 02 '23 05:12 jamke

Best to model the Linux coreutils cp command?

adrelanos avatar Dec 02 '23 07:12 adrelanos

@marmarta what do you think? I don’t know enough about UX to be able to make an informed statement here.

@jamke So the problem with symlinks is:

echo > ~/QubesIncoming/x/a
# oops, overwrote ~/.bashrc (or some other important file)
cat ~/QubesIncoming/x/b
# oops, just read e.g. ~/.ssh/id_ed25519 (or some other secret key)

DemiMarie avatar Dec 03 '23 02:12 DemiMarie

As far as I can tell, symlinks are off-topic in this issue.

andrewdavidwong avatar Dec 03 '23 03:12 andrewdavidwong

@DemiMarie

@jamke So the problem with symlinks is

  1. It is not a problem, it is what symlinks are. It is the same in any GNU/Linux distro.

  2. Why not also fall with error on copying shell scripts? ELF binary files? Or exe files (they run on double-click in wine/mono even without x permission!)? Why not add Antivirus checks on copying? I have an answer: because all this is not required from secure and reliable copy tool, even if it adds some additional security, the same goes for current breaking and unasked changes.

  3. And what is the attack scenario that can be exploited this way (forcing user to run such commands)? User blindly running some commands from the internet? And who can copy these malicious symlinks from one qube to another in the first place? Only user can do it, selecting target qube manually, not some malicious application.


I can give up convincing that all this changes (breaking symlinks, failing on valid ext4 filenames) are a mistake that brings more problems to users than security (it also makes the qvm-copy tool and scripts that depend on it less reliable).

Just, please, make it possible to run qrexec-client-vm with some option which will make it copy files as expected: binary perfect without unasked checks and errors. As cp, qvm-copy in R4.1 and any other copy tool does.

jamke avatar Dec 03 '23 14:12 jamke

@jamke This would need to be a separate service (qubes.FileCopyUnsafe?) so that the prompt presented to the user indicates that this is a more dangerous action. @marmarta thoughts on the name?

DemiMarie avatar Dec 03 '23 14:12 DemiMarie

This would need to be a separate service (qubes.FileCopyUnsafe?) so that the prompt presented to the user indicates that this is a more dangerous action.

@DemiMarie Sounds interesting, I like the idea.

jamke avatar Dec 03 '23 14:12 jamke

This would need to be a separate service (qubes.FileCopyUnsafe?) so that the prompt presented to the user indicates that this is a more dangerous action.

@DemiMarie Sounds interesting, I like the idea.

I think this is something that could be considered. This would still preserve less metadata than tar or cp -a, because the latter two preserve e.g. file ownership, SELinux contexts, and possibly hard links as well.

DemiMarie avatar Dec 03 '23 15:12 DemiMarie

I think this is something that could be considered. This would still preserve less metadata than tar or cp -a, because the latter two preserve e.g. file ownership, SELinux contexts, and possibly hard links as well.

I agree.

  • time data (like mtime) filenames, filecontent should be preserved as is,
  • the ownership can be dropped, as user IDs can be different in general case,
  • the rights (chmod) can be preserved if it is OK,
  • symlinks should not be affected,
  • and hardlinks should be dropped for sure, as it is not a copy process inside one drive by design,
  • other metadata (like SELinux contexts) can be dropped to.

The whole process can be considered similar to copying between 2 air-gapped PCs with a flash-drive formatted to ext4. At least it sounds logical and fits architecture of qubes as separated PCs.

jamke avatar Dec 03 '23 18:12 jamke

Absolutely agree that there should be a "copy_unsafe" option; ran into this issue now several times and it is extremely annoying to deal with...even when a file name has only Latin characters it sometimes crops up, not even to mention non-Latin ones.

I'll also add that UX is still pretty bad with the error message as the target qube is asked with a pop up dialogue, but the error / failure message is just an echo in the terminal; the last time I did qvm-copy and it failed I actually lost the data, because I assumed that the error would be another pop up dialogue, which didn't happen, and the filename was only Latin characters (and some apparently non-standard apostrophes) and then I shut down the disposable thinking everything had been transferred when it wasn't (it was easy to recreate that data - this time -, but still...super bad UX).

UndeadDevel avatar Dec 28 '23 19:12 UndeadDevel

I assumed that the error would be another pop up dialogue, which didn't happen

That should be fixed by https://github.com/QubesOS/updates-status/issues/4240 + https://github.com/QubesOS/updates-status/issues/4233

Edit: Those are for GUI file copy

rustybird avatar Dec 28 '23 20:12 rustybird

(Since the change in behavior from 4.1 to 4.2 was intentional, instead of closing this as "not a bug," I've converted this issue into an enhancement request to allow the desired behavior in some way.)

andrewdavidwong avatar Dec 28 '23 21:12 andrewdavidwong

Absolutely agree that there should be a "copy_unsafe" option; ran into this issue now several times and it is extremely annoying to deal with...even when a file name has only Latin characters it sometimes crops up, not even to mention non-Latin ones.

Is there a list of known Unicode grapheme clusters? Right now, combining characters are rejected to block Zalgo text.

DemiMarie avatar Dec 29 '23 01:12 DemiMarie

Is there a list of known Unicode grapheme clusters? Right now, combining characters are rejected to block Zalgo text.

No idea, but the file name that was refused to be copied simply contained: and ’ | as the problematic portions (when I replaced these with just spaces qvm-copy worked), so nothing that looked weird or made me suspect that qvm-copy would fail...so technically the problem was not Latin characters (the rest of the file name was Latin characters, these were just making the long file name more readable), but just an everyday file name without weird stuff that refuses to be copied.

UndeadDevel avatar Dec 29 '23 02:12 UndeadDevel

Is there a list of known Unicode grapheme clusters? Right now, combining characters are rejected to block Zalgo text.

No idea, but the file name that was refused to be copied simply contained: and ’ | as the problematic portions (when I replaced these with just spaces qvm-copy worked), so nothing that looked weird or made me suspect that qvm-copy would fail...so technically the problem was not Latin characters (the rest of the file name was Latin characters, these were just making the long file name more readable), but just an everyday file name without weird stuff that refuses to be copied.

I just checked and (U+2018) and (U+2019) are (correctly) accepted. If it would not reveal anything sensitive, would you mind filing a bug report with the specific file name? To avoid ambiguity, it might be better to use C escape syntax for the non-ASCII bytes.

DemiMarie avatar Dec 29 '23 04:12 DemiMarie

the last time I did qvm-copy and it failed I actually lost the data

Sorry to hear that.

So, let's note that the are case(s) of user's data loss due to this changes, as I would expect, and no real-life cases when user was "saved" because of this unnecessary filtering and copy fails. It is the case when possible security improvement makes the tool less reliable.

Moreover, during Qubes OS R1.0-R4.1 era the copy tools were working properly with no limitations on filenames and symlinks (which is now completely broken). They copied files and file trees as cp does, and during all that time I have never seen any reports from users that somebody was infected, affected, or that somebody was asking much that copy will be burdened with additional filtering and would be unreliable.

The huge problem, in my opinion, is that currently when user starts qvm-copy on 1 TiB directory, they NEVER can be sure that it:

  1. will be copied properly at all (as cp would)
  2. won't stop in a minute due to some file/symlink
  3. won't stop in an 3 hours due to some file/symlink

In all cases user will be required to make many additional steps manually and start the copy process again. Almost for nothing.

jamke avatar Dec 29 '23 04:12 jamke

@DemiMarie I did some more testing and it turns out it was the "vertical bar" character that caused the issue; opened #8807 now.

UndeadDevel avatar Dec 29 '23 19:12 UndeadDevel

@UndeadDevel QubesOS/qubes-linux-utils#109 should add that to the safe list.

DemiMarie avatar Dec 30 '23 01:12 DemiMarie

As I understand it the motivation for this (according to this PR) is the following:

There is currently no restriction on allowed file names. This means that Trojan Source-style attacks might be possible.

However, the docs already hint at this possibility:

However, one should keep in mind that performing a data transfer from less trusted to more trusted qubes is always potentially insecure if the data will be parsed in the target qube. This is because the data that we copy could try to exploit some hypothetical bug in software running in the target qube.

So unless I am misunderstanding something, I don't think qvm-copy should prevent the user from shooting themselves in the foot.

Meta question: is qvm-copy supposed to be sanitizing?

To elaborate more on this, I never understood qvm-copy as a fundamental sanitizing operation. It should match user's expectations from the cp command generally. (I get the symlink exclusion part, so let's ignore that)

To me personally, qvm-copy should not give any chance to filenames to interfere with the qfile-agent in the receiving end. If another program (or user) can be exploited or tricked, that's beyond the responsibility of qvm-copy and thus it should not make judgements (blocking copies).

User Impact

As some have highlighted, the user impact is severe. I've had this on an innocent looking music file, but if we're talking about a journalist, who receives potentially untrusted documents from sources and wants to copy to another qube, this is a major regression. The workflow would be severely hampered since if there's any non-perfect UTF character in the name the whole source archive it won't be movable from the source qube at all, except with the use of trickery (like zipping).

Given the high impact and (at least to me) relatively little explanation of what practical threats this can mitigate against, my recommendation would be to reconsider this or provide a workaround (preferably GUI-based), or perhaps make the "unsafe" (read: 4.1-like) the default and perhaps a non-default qubes.FileCopySanitizeFilenames policy.

deeplow avatar Jan 26 '24 12:01 deeplow

To elaborate more on this, I never understood qvm-copy as a fundamental sanitizing operation. It should match user's expectations from the cp command generally.

I completely agree with @deeplow! I would press like button multiple times if it were possible.


The workflow would be severely hampered since if there's any non-perfect UTF character in the name the whole source archive it won't be movable from the source qube at all, except with the use of trickery (like zipping).

True, the current breaking change for both ext4-valid filenames and valid symlinks is an mistake and a regression. BTW, ext4 is char-set agnostic and allows any bytes as filenames except '\0' by design.

This sanitizing idea is only making things worse. No examples of infection of Qubes OS users has ever happened this way, but the copy (like in cp) logic is already broken in R4.2 for everybody. And users have to pack and unpack files, I have not seen anybody actually rooting for this breaking change. Not like some people among Qubes OS users were hoping for this change at this point.


Given the high impact and (at least to me) relatively little explanation of what practical threats this can mitigate against, my recommendation would be to reconsider this or provide a workaround (preferably GUI-based), or perhaps make the "unsafe" (read: 4.1-like) the default and perhaps a non-default qubes.FileCopySanitizeFilenames policy.

I agree with this.

  1. The first step: to revert default logic, so less users wound be affected by it and notice problems. Users used the copy tool for 10+ years in Qubes OS with NO complains.
  2. Second step: consider a way to make a user choice for optional copying with sanitizing if user really needs it.

@marmarek can this really be reconsidered? Is it possible?

jamke avatar Jan 26 '24 15:01 jamke

  • The first step: to revert default logic, so less users wound be affected by it and notice problems. Users used the copy tool for 10+ years in Qubes OS with NO complains.

  • Second step: consider a way to make a user choice for optional copying with sanitizing if user really needs it.

I agree completely with both of those points. Mostly with the first, but if we really need the second, it can be like a qvm-copy with an --extra-paranoid option or something...

marmarta avatar Jan 26 '24 16:01 marmarta

After some discussion on Matrix (thanks @xaki23 and @DemiMarie for the ideas!), here is a proposal:

Keep filename filtering in qubes.Filecopy, but introduce new service qubes.UnsafeFilecopy without filename filtering (but with symlinks checks, at least for now). The default policy for the "unsafe" one will (by default) be the same as for the filtered one - "ask", but can be changed. Additionally, qvm-copy will gain a new --sanitize option, that will replace forbidden characters with _ or similar allowed character.

Then, qvm-copy can check for forbidden chars, if none found, will use qubes.Filecopy service. But if there are some, we have two possibilities:

  • no other option is given: use the new qubes.UnsafeFilecopy - depending on the policy, it will either result in a similar prompt (in default setup), but one can configure it also to deny without any prompt. If it doesn't exist in the target (for example updates not installed), it will fail, but so will the current qubes.Filecopy
  • --sanitize option is given: use qubes.Filecopy (filtered one), but replace forbidden chars with _ or such, so the copy will still succeeded, just with sanitized file names

The above proposal, with all updates installed (in the source qube, target qube and dom0) should allow unfiltered filenames by default, but should still allow somebody to enforce filtering if they want to. There are a few corner cases if not all is updated (like only source qube but not the target one), but those will affect copying with "unsafe" filenames only, which are currently rejected anyway (so, technically it isn't worse than the current situation).

marmarek avatar Jan 26 '24 17:01 marmarek