Question backup & restore - required rights

SebS

Hello there,

I’ve got a question / I don’t understand why that part exists in mailcow/mailcow-dockerizedblob/master/helper-scripts/backup_and_restore.sh

(I’ve found a discussion that solved to problem for someone, but still no explanation as to why)

if [[ ${1} == "backup" ]] && [[ -z $(echo $(stat -Lc %a ${BACKUP_LOCATION}) | grep -oE '[0-9][0-9][5-7]') ]]; then echo "${BACKUP_LOCATION} is not write-able for others, that's required for a backup." exit 1 fi

this does always require “775” on the target directory even if I execute the backup script as root user, and I do not understand why that is. shouldn’t it just test if e.g. a file can be written instead of checking for rights that aren’t actually required? Unless I am overlooking something I do not see where any part of the backup would be executed as a different user than the one calling?

I got it working now (problem was that I’m using this with a target directory that had a remote share mounted with sshfs and the allow-other option wasn’t enough for the check this script does) but I’d still like to understand the rationale.

thanks in advance,
S.

DocFraggle

SebS ${BACKUP_LOCATION} is not write-able for others, that’s required for a backup.

I think the echo output is wrong, the above check doesn’t even check for this, For others to write to the location it has to be XX7, XX5 is not enough, that’s just read and execute. So the grep command should be

grep -oE '[0-9][0-9][6-7]

to ensure that others can write.

My location (sshfs as well) is 755, and it’s working fine (as the above grep matches 755)

SebS

Yeah now that you say it …

Should I report that as a bug? I am new to mailcow, if you tell me that it will be ignored I won’t waste my time but it might save others some trouble if this gets reworked.

I still don’t get why it checks for “broader” rights than what’s required - why would anyone but the involved account (or accounts in case there’s more at any point now or in the future) need access to that directory anyways? If anything it is a potential security risk, I’d say?

DocFraggle

To be honest, I don’t have the slightest idea about the intentions of this check. You should run the backup as root anyways, so I agree that root is the only user who MUST have access. so even 700 would be sufficient.

And vice versa, the check currently would succeed even for 005 😃😃

Finally, the script would, by itself, create a directory with 755 if it’s not present 😃

mailcow/mailcow-dockerizedblob/master/helper-scripts/backup_and_restore.sh#L42

This would be a sufficient solution:

if [[ ${1} == "backup" ]] && [[ -w ${BACKUP_LOCATION}) ]]

This just checks if the backup dir is writable for the user running the script. Job done.

SebS

DocFraggle

Finally, the script would, by itself, create a directory with 755 if it’s not present 😃

That’s another surprising thing, after all the documentation (https://docs.mailcow.email/backup_restore/b_n_r-backup/#cronjob) says

You can run the backup script regularly via cronjob. Make sure BACKUP_LOCATION exists:

which should rather be “exists or that the executing user has rights to create it” (which should of course be the case running the scipt as root) or something along the lines.

I don’t have the time to check the commits at the moment, but both things combined lead me to believe that it didn’t always work as it does today and both the check as well as the line in the documentation are partially relics from the past.