rustic-rs / rustic

rustic - fast, encrypted, and deduplicated backups powered by Rust
https://rustic.cli.rs
Apache License 2.0
1.92k stars 71 forks source link

Warn about missing permissions before attempting to back up to a repository #1128

Open Cookie04DE opened 6 months ago

Cookie04DE commented 6 months ago

I am frequently using rustic to back up to different types of file systems: ext4 and exFAT for example.

I have until now been to lazy to add the devices to the fstab, so I usually mount the devices manually.

With ext4 that causes no issues, but for file systems without permission data, if you forget to include uid=<your user id here>,gid=<your group id here> in the mount options the file system will get mounted with all files belonging to the root user.

Now this is typically caught pretty early. In the case of rustic however, the missing permission error occurs at the very end of the backup procedure. Thus, it has already spent quite a bit of time to gather all the data to be committed to the repository before it attempts to write them and fails with a permission denied error.

This is frustrating to the user, because it means they have to redo the whole thing, this time with the correct mount options and wait again until the backup procedure has gathered all the relevant data.

I propose to add a check at the very start of the backup procedure, that checks if writing to the repo is even possible.

aawsome commented 6 months ago

@Cookie04DE Thanks for opening this issue!

I have a question: How should rustic check if the repository is writable?

The point is, currently writing fails after rustic tries to write something to the repository, so we must write something to repository in order to check if it is writable. Now, this poses several problems:

When I read your issue I had two point in mind:

Cookie04DE commented 6 months ago

If compatible with current constraints, I would suggest to make rustic open the necessary files in write mode at the very start of the backup produce without writing anything to them yet. If that fails, it can fail fast; otherwise proceed, gather the backup data and actually attempt to write it.

Otherwise, rustic could query the file permissions of the repo and cross-check them with the current uid/gid (don't know how compatible that approach is with different backends; or if false positives could occur).

I'd be glad to provide you with the requested information. How can I obtain it? Is it just the regular output, or do I need to specify some environment variables or command line flags?

However, to provide actually realistic values I would wait until the next regular scheduled date (next Saturday probably), if that's ok with you.

Regarding your second point: Since I have now added my mounts to the fstab, I don't expect this to be a problem anymore. This having been a regular annoyance of mine can be chalked up to my laziness up to this point and I don't think I actually need a pre-backup hook or other custom checks for this specific case.

Apart from that, I want to express my gratitude towards you and the other maintainers of this project for donating their time. I use it quite often, and it's very useful to me.

Thanks!

aawsome commented 5 months ago

If compatible with current constraints, I would suggest to make rustic open the necessary files in write mode at the very start of the backup produce without writing anything to them yet. If that fails, it can fail fast; otherwise proceed, gather the backup data and actually attempt to write it.

Otherwise, rustic could query the file permissions of the repo and cross-check them with the current uid/gid (don't know how compatible that approach is with different backends; or if false positives could occur).

I fear this is totally incompatible with rustic's architecture and fundamentals. First: Newly written files are named by the SHA256 of their content, i.e. we can only open files once we know the content. Well at this time, rustic already starts to simply save that content. So a pre-open is not necessary or even possible.

About checking permissions: First, this would be a local-Backend-only thing or at least need very backend-specific implementations, which we don't like to have as we focus on backup and have very few self-written code in rustic_backend (in fact moving most work to supporting libraries like opendal). So I'd strictly vote against that for the unified-backend reason. But there is IMO another issue: if we do whatever check we do, there might be other cases where writing still fails. Just think about full disc or additional ACLs provided by some specific file system. We would get more and more requests about pre-checking things before we write - while the writing still could fail for another reason we are not yet checking. Moreover, we would have to be very precise as false positives in those checks would be even more annoying than a late and correct write-error.

In my opinion such extra checks are widely out of scope for rustic. TL;DR: Better report true errors instead of trying to guess which errors could occur.

I'm still very interested in your reasons for the long time until rustic tries to write something!

simonsan commented 5 months ago

I understand the issue and also what @aawsome is saying, I do wonder, though, if it could be sensible to use a "throw-away" file as an indicator if something is writable. E.g. touch .rustic-backup in the new snapshots root repository directory and then remove it again. Not sure, would that be enough?

aawsome commented 5 months ago

@simonsan See my first answer. I would strongly vote against such a "throw-away" or garbage file.

However we could add an an optional feature to mimic restic's locking (create a lock file in the beginning and removes it in the end) - which would also increase compatibility for people who want to use restic and rustic on the same repository. That said, if you look at the restic forum, there have been tons of issues just because of not correctly removed lock files. I am actually very happy that we don't have to mess with these things in rustic, currently. But yes - if someone comes up with a PR to optionally add this, I would support merging this.

Cookie04DE commented 5 months ago

Ok, I got the output. The device is a Raspberry Pi 4 with the data to back up being on HDD 1. The storage devices (as well as the source of the data: HDD 1) are connected via an USB hub connected to one of the USB3 ports. Not an ideal setup, I am very aware, but perhaps not the only reason for the slowness. The backups are run in serial not parallel and in the order presented below. There are three repos in total.

Repo on HDD 1:

using config ./rustic.toml
[INFO] repository local:.: password is correct.
[INFO] using cache at /home/user-redacted/.cache/rustic/77d1bd6f45dcea53e57f5b8cd0f165dd4756719357de347b15d1bd95b283649c
[00:00:00] reading index...               ████████████████████████████████████████         26/26
[INFO] using all backup sources from config file.
[INFO] merging source="/path/to/hdd1/source_files" section from config file
[00:00:00] getting latest snapshot...     ████████████████████████████████████████         10/10
[INFO] using parent 865ac3ce
[INFO] starting to backup "/path/to/hdd1/source_files"...
[00:14:28] backing up...                  ████████████████████████████████████████   9.84 GiB/9.84 GiB   11.60 MiB/s  (ETA 0s)
Files:       22712 new, 973 changed, 52989 unchanged
Dirs:        4653 new, 6559 changed, 22109 unchanged
Added to the repo: 341.8 MiB (raw: 760.8 MiB)
processed 76674 files, 9.8 GiB
snapshot fee7723c successfully saved.
[INFO] backup of "/path/to/hdd1/source_files" done.

real    14m30.809s
user    3m6.732s
sys 2m21.491s

Repo on HDD 2 (bad mount):

using config ./rustic.toml
[INFO] repository local:.: password is correct.
[INFO] using cache at /home/user-redacted/.cache/rustic/2d64775251dafa21db3235e10d784c1e7370b61051ce9f062bff0275ee22add6
[00:00:00] reading index...               ████████████████████████████████████████         16/16
[INFO] using all backup sources from config file.
[INFO] merging source="/path/to/hdd1/source_files" section from config file
[00:00:00] getting latest snapshot...     ████████████████████████████████████████          9/9
[INFO] using parent c323632d
[INFO] starting to backup "/path/to/hdd1/source_files"...
error: opening file failed: `Os { code: 13, kind: PermissionDenied, message: "Permission denied" }`

real    11m34.468s
user    3m31.606s
sys 2m17.654s

The bulk of the time was spent on the line after starting to backup : [hour:minute:second] backing up... <progress bar> X.XX GiB/Y.YY GiB ZZ.ZZ MiB/s (ETA Xm). This line gets removed by rustic in the final output.

Repo on HDD 2 (good mount):

using config ./rustic.toml
[INFO] repository local:.: password is correct.
[INFO] using cache at /home/user-redacted/.cache/rustic/2d64775251dafa21db3235e10d784c1e7370b61051ce9f062bff0275ee22add6
[00:00:07] reading index...               ████████████████████████████████████████         16/16
[INFO] using all backup sources from config file.
[INFO] merging source="/path/to/hdd1/source_files" section from config file
[00:00:00] getting latest snapshot...     ████████████████████████████████████████          9/9
[INFO] using parent c323632d
[INFO] starting to backup "/path/to/hdd1/source_files"...
[00:08:07] backing up...                  ████████████████████████████████████████   9.84 GiB/9.84 GiB   20.65 MiB/s  (ETA 0s)
Files:       22771 new, 997 changed, 52908 unchanged
Dirs:        4672 new, 6558 changed, 22091 unchanged
Added to the repo: 498.6 MiB (raw: 1.0 GiB)
processed 76676 files, 9.8 GiB
snapshot b850bcf9 successfully saved.
[INFO] backup of "/path/to/hdd1/source_files" done.

real    8m16.065s
user    3m29.459s
sys 2m24.230s

Repo on Flash drive (bad mount):

using config ./rustic.toml
enter repository password: [hidden]
[INFO] repository local:.: password is correct.
[INFO] using cache at /home/user-redacted/.cache/rustic/6ff3bb27f73b51710b778b290908f6029f0fa3eeb0ff9c68b578a4dcb8f8082f
[00:00:00] reading index...               ████████████████████████████████████████         19/19
[INFO] using all backup sources from config file.
[INFO] merging source="/path/to/hdd1/source_files" section from config file
[00:00:00] getting latest snapshot...     ████████████████████████████████████████         10/10
[INFO] using parent ce95988b
[INFO] starting to backup "/path/to/hdd1/source_files"...
error: opening file failed: `Os { code: 13, kind: PermissionDenied, message: "Permission denied" }`

real    8m45.108s
user    3m38.724s
sys 2m35.795s

Again most of the time spent on the process described previously.

Repo on Flash drive (good mount):

using config ./rustic.toml
enter repository password: [hidden]
[INFO] repository local:.: password is correct.
[INFO] using cache at /home/user-redacted/.cache/rustic/6ff3bb27f73b51710b778b290908f6029f0fa3eeb0ff9c68b578a4dcb8f8082f
[00:00:00] reading index...               ████████████████████████████████████████         19/19
[INFO] using all backup sources from config file.
[INFO] merging source="/path/to/hdd1/source_files" section from config file
[00:00:00] getting latest snapshot...     ████████████████████████████████████████         10/10
[INFO] using parent ce95988b
[INFO] starting to backup "/path/to/hdd1/source_files"...
[00:10:14] backing up...                  ████████████████████████████████████████   9.84 GiB/9.84 GiB   16.39 MiB/s  (ETA 0s)
Files:       22715 new, 975 changed, 52987 unchanged
Dirs:        4653 new, 6559 changed, 22109 unchanged
Added to the repo: 511.8 MiB (raw: 1.0 GiB)
processed 76677 files, 9.8 GiB
snapshot 96710723 successfully saved.
[INFO] backup of "/path/to/hdd1/source_files" done.

real    10m26.042s
user    3m35.584s
sys 2m40.390s
aawsome commented 5 months ago

@Cookie04DE Thanks for your results. So, yes, you have many new or changed files which explains why pretty much data is written. And I guess you have quite a large pack files size which is also good in principle, but of course rustic waits until a pack file is full or the backup ends before writing to the repository. So, altogether your behavior can be explained, but I fear an early write-test is only possible once someone implements optional locking for rustic. Currently, I can only suggest to run the backup (much) more often. For only few changed or new files, the backup is much faster and you'll get faster feedback.

intgr commented 1 month ago

It's possible to test for writability without actually creating any files. The Unix access() syscall is available in nix crate (already present in rustic_core with the fs feature). It considers filesystem ACLs correctly and understands special permissions of root user.

(Rust also has the builtin std::fs::metadata()?.permissions().readonly(), but it's a very naive implementation -- it ignores user groups, ACLs, root privileges, etc.)

aawsome commented 1 month ago

@intgr IMO it is not about whether it is possible to do the one or other check, I think we simply should not do such checks in rusitc, see my former post:

About checking permissions: First, this would be a local-Backend-only thing or at least need very backend-specific implementations, which we don't like to have as we focus on backup and have very few self-written code in rustic_backend (in fact moving most work to supporting libraries like opendal). So I'd strictly vote against that for the unified-backend reason. But there is IMO another issue: if we do whatever check we do, there might be other cases where writing still fails. Just think about full disc or additional ACLs provided by some specific file system. We would get more and more requests about pre-checking things before we write - while the writing still could fail for another reason we are not yet checking. Moreover, we would have to be very precise as false positives in those checks would be even more annoying than a late and correct write-error.

In my opinion such extra checks are widely out of scope for rustic. TL;DR: Better report true errors instead of trying to guess which errors could occur.

In addition, as we will add hooks (see https://github.com/rustic-rs/rustic/pull/1218) in near future, I think writing some check-script yourself (for the specific error cases which do matter for you) and add them as repository hook is the way to go here.

intgr commented 1 month ago

That's fair. A generic mechanism like hooks sounds like a better way to solve this.

Another example that access() fails to detect is read-only mounts.