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

document how to create a full backup without running restic as root #1483

Merged
merged 1 commit into from
Jan 16, 2018

Conversation

dstosberg
Copy link
Contributor

What is the purpose of this change? What does it change?

Add an example section to the documentation that explains how one can use restic to create a full backup without having to run it as root.

Was the change discussed in an issue or in the forum before?

The idea occurred in discussing issue #1473.

Checklist

  • I have read the Contribution Guidelines
  • I have added tests for all changes in this PR
  • I have added documentation for the changes (in the manual)
  • There's an entry in the CHANGELOG.md file that describe the changes for our users
  • I have run gofmt on the code in all commits
  • All commit messages are formatted in the same style as the other commits in the repo
  • I'm done, this Pull Request is ready for review

Copy link
Member

@fawick fawick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is a great idea and very helpful, thanks for contributing to the documentation! I have some questions below.


.. code-block:: console

root@a3e580b6369d:/# useradd -m restic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If sudo is always used, would it make sense to restrict the login shell (-s /bin/false), too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would not be a bad idea to do that. But personally, I find it very convenient that I can run a shell as user restic and use it examine or restore backups.

There are a lot of things that you could do to harden or optimize the setup. But I think that is out of the scope of this chapter.


.. code-block:: console

root@a3e580b6369d:/# sudo -u restic -H -s
Copy link
Member

@fawick fawick Dec 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be a good reason for it which I cannot see, but why two commands instead of, e.g. sudo -u restic ~restic/bin/restic -r /tmp backup /

Likewise, what is the rationale for -x? I'd assume backing up the whole system may require to access multiple file systems, may it not? In case you really want to have -x it needs to be after backup as it is not a global flag.

Should --exclude /tmp as the repository be added here? Or how about adding --exclude={"/dev/*","/proc/*","/sys/*","/tmp/*","/run/*","/lost+found"}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we could easily put sudo and the call to restic a single command.

The intention for the -x was to prevent a clueless reader from backing up his /proc or /sys which might never actually terminate. But you are right, a proper generic exclude list, that works for most people, is a better fit. And -x might exclude /home, which is quite commonly on a separate filesystem.

@codecov-io
Copy link

codecov-io commented Dec 14, 2017

Codecov Report

Merging #1483 into master will decrease coverage by 5.25%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1483      +/-   ##
==========================================
- Coverage   51.55%   46.29%   -5.26%     
==========================================
  Files         138      138              
  Lines       11080    11080              
==========================================
- Hits         5712     5130     -582     
- Misses       4503     5139     +636     
+ Partials      865      811      -54
Impacted Files Coverage Δ
internal/backend/b2/b2.go 0% <0%> (-78.84%) ⬇️
internal/backend/azure/azure.go 0% <0%> (-77.85%) ⬇️
internal/backend/swift/swift.go 0% <0%> (-76.03%) ⬇️
internal/backend/gs/gs.go 0% <0%> (-72%) ⬇️
internal/backend/swift/config.go 36.95% <0%> (-54.35%) ⬇️
internal/backend/s3/s3.go 65.76% <0%> (+1.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99f0fce...60a7a9d. Read the comment docs.


.. code-block:: console

root@a3e580b6369d:/# setcap cap_dac_read_search=+ep ~restic/bin/restic
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I agree, that restic ideally shouldn't be run as root, assigning cap_dac_read_search to the binary directly is dangerous as this would allow now anyone able to execute restic to also ready any system file.

I'd rather handle this by letting systemd control all those aspects.

My WIP service instance unit for restic: /etc/systemd/system/restic@.service

[Unit]
Description=Backup %I using 'restic'
ConditionFileNotEmpty=/etc/restic/%i/restic.env
ConditionPathExists=/etc/restic/%i/backup.exclude

[Service]
ExecStart=/usr/bin/restic backup --cache-dir=/var/cache/restic/%i --one-file-system --exclude-file=/etc/restic/%i/backup.exclude --exclude=/var/cache/* --exclude=/tmp/* --exclude=/var/tmp/* --exclude=/home/*/.cache/* --exclude=/run/* --quiet /
Type=oneshot
EnvironmentFile=/etc/restic/%i/restic.env
DynamicUser=yes
PrivateDevices=yes
CacheDirectory=restic/%i
CacheDirectoryMode=0700
ConfigurationDirectory=restic/%i
ConfigurationDirectoryMode=0700
#SystemCallFilter=@basic-io @file-system @network-io @io-event @ipc @cpu-emulation
#SystemCallErrorNumber=EPERM
AmbientCapabilities=CAP_DAC_READ_SEARCH

# TODO
# fix SystemCallFilter= usage, which currently fails, although the required tls syscall should be included in @default:
#   -- cannot set up thread-local storage: cannot set %fs base address for thread-local storage
# - does restic support sd_notify()? if not, create an RFC or PR
# - ensure systemd's ProtectSystem= (+ related options) is utilized to the fullest
# - set low io class

Copy link
Member

@fd0 fd0 Dec 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I agree, that restic ideally shouldn't be run as root, assigning cap_dac_read_search to the binary directly is dangerous as this would allow now anyone able to execute restic to also ready any system file.

Indeed, and that's why the permissions are set like this:

root@a3e580b6369d:/# chown root:restic ~restic/bin/restic
root@a3e580b6369d:/# chmod 750 ~restic/bin/restic

Did you see that?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed that - sorry about the noise.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries :)

@fd0 fd0 added state: need feedback waiting for feedback, e.g. from the submitter PR:Feedback needed and removed state: need feedback waiting for feedback, e.g. from the submitter labels Dec 19, 2017
@fd0
Copy link
Member

fd0 commented Jan 7, 2018

ping @dstosberg, are you still interested in continuing the work here, or should someone else take over?

Copy link
Member

@fd0 fd0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the issues in the comments, thanks!

Copy link
Member

@fd0 fd0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fd0 fd0 merged commit 60a7a9d into restic:master Jan 16, 2018
fd0 added a commit that referenced this pull request Jan 16, 2018
document how to create a full backup without running restic as root
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants