Closed
Bug 1246620
Opened 9 years ago
Closed 5 years ago
worker type secrets for storing secret keys instead of secret blobs
Categories
(Taskcluster :: Services, defect)
Taskcluster
Services
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: pmoore, Unassigned)
References
Details
When secrets were added to AWS provisioner worker types, we didn't have the taskcluster secrets service.
Now that we have a dedicated secrets service, we could migrate the secrets out of the worker types, into the secrets service. As a side-effect, this has the very nice property of making worker types non-sensitive, which would help with e.g. bug 1246191.
I would propose that the secrets section would remain, but instead of directly embedding the secret, instead the values would be keys to the secrets store. The advantage of this is that secrets can be stored once in the secret store, but multiple worker types that require the secret can point to it.
This is very useful for worker start-up, where we want a generic mechanism to load secrets onto the worker. Furthermore, we already have code for converting secrets into binary file-system secrets, such as in the worker type win2012r2. We could reuse this in taskcluster-worker to enable filesystem secrets, e.g. for releng secrets.
For example, the secret could be referred to in the aws worker type config with:
{
"description": "SSL key for livelog",
"path": "C:\\generic-worker\\livelog.key",
"secret": "taskcluster-worker/livelog/livelog.key",
"encoding": "base64",
"format": "file"
}
and then the secret "taskcluster-worker/livelog/livelog.key" would be retrieved from the secret store at start up.
On a linux/mac worker type, the `path` would be different, but the same secret containing the encoded file could be used.
I'm marking this as blocking the taskcluster-worker design, as if we go with this, it will affect how we implement secret loading in the taskcluster-worker.
Reporter | ||
Comment 1•9 years ago
|
||
Note, this doesn't relate only to file secrets, arbitrary secrets, such as taskcluster worker config, could also be stored via taskcluster secrets. The key point is that the worker type just refers to the secret key, not the secret value...
Reporter | ||
Updated•9 years ago
|
Comment 2•9 years ago
|
||
> but instead of directly embedding the secret, instead the values would be keys to the secrets store
This is no longer secret, so it should just be part of the public configuration options that
aws-provisioner hands out. See "userData" in current aws-provisioner.
> Note, this doesn't relate only to file secrets,
Good, because (unrelated to this) we shouldn't store secrets in files.
Memory is harder to steal, and in memory we can have a finalizer that zeros the bytes.
(just a random unrelated note)
Comment 3•9 years ago
|
||
Basically:
if we stop using the "data" property from provisioner.getSecret(token), then
we're only using the secret token to issue temporary credentials.
See: http://docs.taskcluster.net/aws-provisioner/api-docs/#getSecret
Updated•9 years ago
|
Assignee: nobody → dustin
Comment 4•9 years ago
|
||
So the plan here is to move everything in the `data` property into the secrets API, and fetch the secrets from there. This will require some modifications to the worker implementation. Somewhat longer-term, we'll want to get many of these secrets from the auth API, so that they are not fixed but generated dynamically for each worker instance.
How does the worker know what secret name to use? is this configurable? in the data property? We need to balance the complexity of setting up a new workerType against code complexity in docker-worker and against difficulty in rotating secrets.
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] from comment #4)
> How does the worker know what secret name to use? is this configurable? in
> the data property? We need to balance the complexity of setting up a new
> workerType against code complexity in docker-worker and against difficulty
> in rotating secrets.
I would envisage a worker-type definition like this:
{
....
....
"userData": {
"workerConfig": {
"plugins": {
"livelog": {
"sharedSecret": "@secret{project/taskcluster/livelog/sharedSecret}",
"subdomain": "taskcluster-worker.net",
"certificate": "@secret{project/taskcluster/livelog/certificate}",
"key": "@secret{project/taskcluster/livelog/key}"
},
....
},
"engine": {
....
},
"global": {
"engine": "win2012r2",
"enabledPlugins": [
"livelog",
"success",
....
]
}
},
"additionalEngineFiles": [
{
"description": "RelEng secrets",
"path": "C:\\builds",
"content": "@secret{project/releng/gecko-secrets}",
"encoding": "base64",
"format": "zip"
}
]
},
....
....
}
We'd need some pre-parser for translating the @secret{...} references, but I think it is worth doing. This way you get
* visibility of config settings, even if you don't know what values are
* public/private properties shown together (e.g. livelog.subdomain is not private, so does not need to be inside a secret)
* no change on AWS provisioner side - userData is still arbitrary json object to be interpreted by worker
* worker type definitions can be publicly viewed/audited
Of course, I'm open to a different format for the secret templating.
Reporter | ||
Comment 6•9 years ago
|
||
Note, also this way you can reuse the same AMI but different worker types can have different additional files, which can have either public or secret content. In this example I've added RelEng secrets, but this could also be for files that do not contain secrets - although ideally any public files would be downloaded/checked out as part of the task (not burned into the worker). It is more geared for secret files, such as RelEng secrets, that need to be available to a task, but without it being too easy to see what is in there (i.e. not checked in), and not easy to modify them, for example. I used such a scheme in the generic worker for getting the RelEng secrets onto the Windows environment, so that I didn't have to burn them into the AMI, but rather burned them into the worker type (which we would now be able to put into taskcluster-secrets with this bug).
Reporter | ||
Comment 7•9 years ago
|
||
Note, the provisioner would return the userData uninterpreted, i.e. with the secret template references still in it - it would be for the worker to resolve this user data it gets by retrieving the secrets it refers to.
Regarding the taskcluster credentials - I think these still need to be stored in provisioner, because of the chicken/egg problem of if credentials are stored as a secret, we don't have credentials to access them from the secret store.
Comment 8•9 years ago
|
||
That's awesome, thanks! I think the only change I'd make is to format the secret as {"secret": "project/releng/gecko-secrets"} instead (that is, as a JSON object rather than a string), thereby avoiding ambiguity and the need to parse strings.
The taskcluster credentials given to the worker will be temporary credentials, so dynamically generated by the provisioner and not embedded in the workerType (and, in fact, not in the userData -- they are fetched using the one-time secret token).
Updated•8 years ago
|
Summary: Discussion: worker type secrets for storing secret keys instead of secret blobs → worker type secrets for storing secret keys instead of secret blobs
Updated•8 years ago
|
Assignee: dustin → nobody
Comment 9•7 years ago
|
||
I agree that we should remove provisioner secrets. This is mostly blocked on workers migrating away from provisioner secrets. Once that happens, we can remove the secrets storage in the entities and stop returning them anywhere. This will also allow worker type configurations to hopefully become publicly viewable.
Reporter | ||
Updated•7 years ago
|
Comment 10•6 years ago
|
||
(In reply to John Ford [:jhford] CET/CEST Berlin Time from comment #9)
> I agree that we should remove provisioner secrets. This is mostly blocked
> on workers migrating away from provisioner secrets. Once that happens, we
> can remove the secrets storage in the entities and stop returning them
> anywhere. This will also allow worker type configurations to hopefully
> become publicly viewable.
We think we have the information we need from Amazon to proceed here now. John was going to do some exploratory work with OpenSSL to verify.
Comment 11•6 years ago
|
||
We are still blocked on the workers removing support for provisioner secrets.
Assignee | ||
Updated•6 years ago
|
Component: AWS-Provisioner → Services
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Comment 12•6 years ago
|
||
No longer blocks bug 1375155 since publicly viewable definitions are made possible via bug 1375195.
No longer blocks: 1375155
Reporter | ||
Updated•5 years ago
|
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•