YunoHost-Apps / borg_ynh

An experimental Borg implementation for YunoHost
https://www.borgbackup.org/
GNU Affero General Public License v3.0
19 stars 22 forks source link

The backup-with-borg script shouldnt be called with sudo ... #134

Open alexAubin opened 1 year ago

alexAubin commented 1 year ago

Problem

Borg is expected to add a sudoer file allowing borg to run a few specific command as root ...

But instead the entire script is apparently run as root so there's a false sense of "security" ...

Solution

Don't run the script as root ... It's supposed to work without running it as root because it calls sudo on commands allowed in the sudoer file ...

PR Status

Automatic tests

Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)

zamentur commented 1 year ago

Need to be tested on the battle field to be sure that nothing fail...

zamentur commented 1 year ago

What's the worse ?

No backup because the damned script use a new forbidden command (missing in sudoer) OR to give sudo permission on all the script ...

On my side, i consider we should prefer to be sure to have backup (at least until we have a way to do CI test on specific action like trigger a backup).

alexAubin commented 1 year ago

Wokay but that's just a false dilemna ... like "Do you prefer something that works and is insecure, or that is secure but doesnt work" ... Like, yeah, actually we can have both, this just need to be carefully tested, and the cron checks are supposed to notify when backups are not properly created x_x ...

zamentur commented 1 year ago

and the cron checks are supposed to notify when backups are not properly created x_x ...

borgserver cron only check that the repo has changed, but some archive could be incomplete or missing.

The cron task on borg_ynh send a report of what succeed and what failed (and diagnosis too cause borg.service is marked as failed), but it's still easy to have temporary issues triggering alert that mask a big problem.

You also know the issue of people that never consult their self-hosted admin email...

As florent said we miss something to check backup (in an ideal world that archives contains sql dumps with a lot of data).

something that works and is insecure I don't think putting sudo on this script enlarge so much the attack area...

Remember that i added the borg user only cause linter was crying, but the truth is this app need to access on all sensible data to backup it. Keep in mind that the yunohost backup borg implementation run as root too.