Closed Bug 1531457 Opened 5 years ago Closed 5 years ago

generic-worker: don't replace existing generic-worker.config file on startup

Categories

(Taskcluster :: Workers, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: pmoore, Assigned: pmoore)

References

Details

Attachments

(1 file)

When generic-worker is run from an AWS Provisioner spawned instance, the worker builds its config by communicating with the AWS EC2 metadata service, the taskcluster AWS Provisioner, and the taskcluster secrets service.

If it didn't persist its config to the file system, the worker config wouldn't be available after a reboot, since, for example, we intentionally delete AWS credentials from the AWS Provisioner after fetching them, so that a malicious task doesn't have access to them via a secret key available from the instance's user data.

However, on workers where the config file is not fetched over the network but is provided locally on the file system, it is not ideal to rewrite the config file to disk after loading it. The original motivation for replacing the config file on startup was that:

  1. We (pre version 13 of generic-worker) allowed a hybrid configuration to be part provided by configuration providers (--configure-for-aws, --configure-for-gcp) and part provided by a local configuration file. This meant you could e.g. generate config at machine start up, before running the worker, if static values from e.g. the worker type definition, were not appropriate, and merge those with the values you got from the AWS Provisioner.

  2. It seemed useful to normalise the config file so that properties were sorted, and optional config settings were included, so that e.g. diffing config files was easier. It seemed useful for aiding troubleshooting, so regardless of what the original config file looked like, a troubleshooter could see what the effective config looked like.

We stopped providing support for a hybrid configuration in version 13, since it had never been used and made config management more complicated. An all-or-nothing approach of get-your-config-from-a-local-file or get-for-config-from-a-config-provider seemed to be the best approach, and kept things simple. This removed motivation 1).

When rolling out generic-worker on workers outside of AWS (mac, linux) we hit an issue where they could not write to their config file on startup since it was owned by root. Keeping it owned by root was a good thing, since tasks run as a less privileged user that can read the config file, but at least not write to it. It became clear it did not make sense to replace the config file if it already existed, and that the benefits of motivation 2) were already addressed by the fact that the worker logs its config with secrets obfuscated on start up, so there is no need to also write it to disk.

Therefore I have I decided to introduce the simplification that the worker config file is only written to disk if it doesn't exist (i.e. when the config is negotiated from a content provider such as via --configure-for-aws and --configure-for-gcp run options). If it exists already, we don't touch it.

If the tasks run as dedicated OS users, we continue to take ownership of the generic-worker config file so that we can block both read and write access of the generic-worker config file to all task users.

If the tasks run as the same OS user as the generic-worker process (currently what we do on linux/mac) then we don't attempt to "secure" the generic-worker config file, but leave it with its existing file access permissions. In order to take ownership of the file, we would need to have write permission, and we actually don't want to have write permission, as that would entitle a task also to be able to modify the config file, and then it could compromise a worker. Therefore we leave the permissions untouched. However, running tasks as the current user is inherently insecure, so we aren't too concerned with this use case, and we only use it for tests (not builds).

Assignee: nobody → pmoore
Attachment #9047492 - Flags: review?(jhford)

Released in generic-worker 13.0.3.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Attachment #9047492 - Flags: review?(jhford)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: