m-lab / ndt-server / 38729695820

77%

Build Type

push

github

Committed by web-flow

Commit Message

feat(ndt5, ndt7): MkdirAll w/ 0775 perms (#440)

This patch modifies ndt5 to use 0775 (it used to be 0777) and ndt7
to use 0775 (it used to be 0755) for MkdirAll.

The permissions with which we create directories are influenced
by the umask under which we are running. A typical umask is
`0o022` (disable write for group and others).

The original motivation for this patch is that `0o755 & ~0o022`
yields `0o755`, which prevents the group from writing. But,
in the context of `byos-debian`, I am working to separate the
privileges of `ndt-server` and of core services. However, if
we run `ndt-server` and `jostler` under different users, we need
`jostler` to delete uploaded files, which, in turn, requires
sharing the group and using `0o775 & ~0o022` to allow the
common group to write (delete) directory entries. The file
permission of `0o644` is fine because `jostler` does not need
to write the files: it suffices to read them.

While working on this patch, however, I noticed that ndt5
was using `0777`, which strikes me as "too much". I do not
think there is any situation in which, regardless of the
umask, anyone not in the proper groups should write the
directories containing the result.

Now, let me elaborate on why I think this change is safe:

1. in `m-lab/k8s-support`, I cannot see any umask override in
any jsonnet file, which suggests that the default is used

2. we use `runc` to run containers AFAICT

3. it seems to me that `runc` uses `0o022` as the default
https://github.com/opencontainers/runc/blob/6659c4a/libcontainer/standard_init_linux.go#L121

4. in `m-lab/autonode`, I also do not see any umask override
but the umask should be `0o022`: `docker run` has `--umask`
and documents that `0o022` (the `runc` default) is used
otherwise; `docker compose` does not seem to have any way
in which to specify the umask in its YAML file

5. likewise in `m-lab/byos-debian` there's no umask override in
the systemd units (default `0o022`; see `systemd.exec(5)`)

FWI... (continued)

Coverage Stats

2 of 2 new or added lines in 2 files covered. (100.0%)

3 existing lines in 1 file now uncovered.

1833 of 2390 relevant lines covered (76.69%)

1.61 hits per line