TempFile + rename results in wrong permissions on blobs

[FallenWarrior2k]

Output of rest-server --version

rest-server 0.10.0 compiled with go1.16.3 on linux/amd64

How did you run rest-server exactly?

/usr/bin/restic-rest-server --path /srv/nas/Backup/restic

Data directory: /srv/nas/Backup/restic
Authentication enabled
Private repositories disabled
Starting server on :8000

What backend/server/service did you use to store the repository?

Local file system

Expected behavior

rest-server should honor umask and default permissions when creating files.

Actual behavior

Blobs created by rest-server have 0600 permissions no matter what and ignore any default ACLs present on parent directories.

Steps to reproduce the behavior

1. Create a test directory.
2. Apply a default ACL of choice to it. I had g::rx in hopes of bypassing past issues of restic not honoring umask.
3. Run rest-server with a umask that would result in 0640 or more permissive, in my case 0027.
4. Perform a backup.

The resulting data blobs will not be readable by group, which in my case stopped the off-site sync process from copying them.

Do you have any idea what may have caused this?

Blobs are created by ioutil.TempFile and then renamed, but TempFile is known not to honor umask, which does make sense to an extent for a temporary file only intended for use by the current process, but becomes problematic for files intended to persist and potentially be accessed by other software.
I can only assume but have not verified that similar restrictions apply to default ACLs.

Do you have an idea how to solve the issue?

I'm not too experienced with Go, but I would assume creating the file "normally" using os.Create() should result in the correct permissions being applied.

Did rest-server help you today? Did it make you happy in any way?

It, especially the append-only mode, is very helpful for me to avoid lots of SFTP connections everywhere and permission chaos to retain least possible privilege.

[wojas]

You're right, it would make sense to just build the temp file name ourselves. No need to use OS specific temp file logic here.

[MichaelEischer]

@FallenWarrior2k Did you test rest-server 0.10 or the current master branch? TempFile + rename is only used on the master branch. From what I can tell there's been no change in behavior due to #142, previously rest-server was also hardwired to use 0600 as file permissions (in rest-server v0.10.0):

rest-server/handlers.go

Line 518 in 9313f19

tf, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0600)

The (new) FileMode option in repo.go hasn't been exposed somewhere as far as I can see.

I've opened PR #171 to use a custom tempFile method which honors the FileMode option. But that PR still won't expose the filemode option to users.

[FallenWarrior2k]

I did test 0.10 but forgot to switch to the 0.10 tag when I investigated before opening this issue, my bad.

In any case, I've just tested the exact call you posted above from a Python shell. It seems that explicitly providing a mode like that overrides corresponding default ACLs that would apply. It also appears to clear the mask, rendering any named entries in the ACL useless.

Guess I gotta keep using chmod -R g+r before syncing my backups for time being.

[MichaelEischer]

Duplicate of #189