m-lab / autojoin

72%

LAST BUILD ON BRANCH main

branch: SELECT

CHANGE BRANCH

x

• No branch selected
• dependabot/github_actions/actions-3a1c37d3b7
• dependabot/go_modules/github.com/googleapis/gax-go-2.0.2incompatible
• dependabot/go_modules/golang.org/x/crypto-0.45.0
• dependabot/go_modules/gomod-minor-patch-4a58ba8e14
• dependabot/go_modules/gomod-minor-patch-4b732bdfc4
• dependabot/go_modules/gomod-minor-patch-7678b51ea3
• dependabot/go_modules/gomod-minor-patch-84a25a23f2
• dependabot/go_modules/gomod-minor-patch-8c811e0a14
• dependabot/go_modules/gomod-minor-patch-9998116421
• dependabot/go_modules/gomod-minor-patch-fb4d939785
• dependabot/go_modules/gomod-minor-patch-fcfb1189d2
• fix-register-healthcheck-exit
• fix/autofix-workflow-permissions
• fix/key_perms
• main
• sandbox-kinkade
• sandbox-roberto-consolidate
• sandbox-roberto-dependabot
• sandbox-roberto-fix
• sandbox-roberto-fix-apikey
• sandbox-roberto-fix-delete-auth
• sandbox-roberto-fix-probability
• sandbox-roberto-gc-error-metrics
• sandbox-roberto-jwt
• sandbox-roberto-key-verifier
• sandbox-roberto-mandatory-ipv4
• sandbox-roberto-minver
• sandbox-roberto-probability
• sandbox-roberto-security-fixes
• sandbox-roberto-update-app-yaml
• sandbox-roberto-update-iata-csv
• sandbox-roberto-update-orgadm

Build # 36022004223

Build Type

push

github

Committed by web-flow

Commit Message

fix(cmd/register): service account key permissions (#103)

The service account key is written with 0644 permissions. Because
it is a secret, it should instead be written with 0600.

I noticed this issue when analyzing `byos-debian` where the key is
written to `/var/lib/mlab/node`. The directory itself is `02750` and
this prevents users that are not in the `mlab-node` group from reading
the file. However, in that context, services such as `ndt-server` are
able to read the secret even though they do not need to. This patch
doesn't address that, but I am working on another set of patches
to privilege-separate core services and measurement services.

I did not spend time focusing on whether this was a security issue
for autonode. As regards autonode, my main focus has been trying
to understand whether changing the permissions could break it, and
I concluded that this is not the case (see below).

To address the permissions, this patch does the following:

1. call `os.WriteFile` with `0600`, which covers new files only
while existing files are just truncated w/o a `chmod`

2. `os.Chmod` with `0600` thus covering already existing files

Following the Boy Scout rule, this patch also changes the wording
and spelling of some comments and adds minor style tweaks.

Regarding why I am confident that autonode containers (and namely
`jostler` and `uuid-annotator`) run as root, this is why:

1. the three `Dockefile`s do not specify `USER`

2. there is no `user:` in the `docker-compose.yml`

As such, this change is safe. Even if the file was owned by
another user and had `0600`, root holds `CAP_DAC_OVERRIDE` so
it is able to read the service account key anyway.

Coverage Stats

0 of 22 new or added lines in 1 file covered. (0.0%)

1 existing line in 1 file now uncovered.

1254 of 1753 relevant lines covered (71.53%)

0.79 hits per line