1217088 - Grant workers roles, rather than having workerTypeDef.scopes

Status: RESOLVED FIXED

(Taskcluster :: Services, defect)

Description

[Jonas Finnemann Jensen (:jonasfj)]

This bug has a few stages, but basically how I want things to work is as follows:

A) aws-provisioner gets clientId: tc-aws-provisioner
B) role "client-id:tc-aws-provisioner" -> [assume:worker-type:aws-provisioner/*]
C) when aws-provisioner creates temporary credentials  for a worker, it grants
   the scope: assume:worker-type:aws-provisioner/<workerType>

Notice that this works out very nicely as the queue, already requires the
  assume:worker-type:aws-provisioner/<workerType>
A nice effect of this is that all scopes issued to workerType will be available
in the list of roles. So we can get an easy overview, and run independent tests
on these things (plus have tools for figuring out who can get what scopes).

Down side is that we have to declare the scopes for workerTypes, by creating
a role... I don't think this will be a huge issue as we'll create one role:
  assume:worker-type:aws-provisioner/* -> [common scopes]
This role will then issues scopes that all workerTypes have in common, there is
probably a few. In fact I suspect most workerTypes don't have any special scopes
outside of these common scopes.

-----
Implementation stages:
 1) aws-provisioner gets clientId: tc-aws-provisioner
 2) role "client-id:tc-aws-provisioner" -> [*]
 3) for each workerType definition we change workerTypeDef.scopes = [
      assume:worker-type:aws-provisioner/<workerType>
    ]
 4) We now update aws-provisioner to not have the workerTypeDef.scopes
    property anymore but instead give workers the scope:
      assume:worker-type:aws-provisioner/<workerType>
 5) Update role "client-id:tc-aws-provisioner" such that:
    "client-id:tc-aws-provisioner" -> [assume:worker-type:aws-provisioner/*]

---
Note: I think there is bonus points in changing scopes required to modify a
workerType into:
  aws-provisioner:manage-worker-type:<workerType>
  assume:worker-type:aws-provisioner/<workerType>
or maybe:
  aws-provisioner:create-worker-type (one per operation)
  assume:worker-type:aws-provisioner/<workerType>
This way there is no risk of scope escalation, by creating a workerType
in order to get credentials with the scope:
  assume:worker-type:aws-provisioner/<workerType>

Comment 1

[Jonas Finnemann Jensen (:jonasfj)]

@jhford, garndt,
As provisioner and worker maintainer/configurator do you have any concerns with this?

If not, I say we get it done... steps (1) to (3) are purely configuration changes.
It's only step (4) that requires a minor code change in the provisioner, everything else is just config.

Comment 2

[Greg Arndt [:garndt]]

This sounds good to me.  Spoke to Jonas on IRC/vidyo to understand it more.  All of these are changes that don't affect the worker code directly.  The role given to all worker types could be the same scopes they currently get and then over time we can restrict these down better with temp creds when claiming tasks.

Comment 3

[John Ford [:jhford] CET/CEST Berlin Time]

so the idea is to remove the ability to set up arbitrary scopes in the worker type definition?  What will we do when we want to generate temporary credentials for other things?  Will someone have to generate them manually and insert them every 30 days?

Comment 4

[Jonas Finnemann Jensen (:jonasfj)]

> so the idea is to remove the ability to set up arbitrary scopes in the worker type definition?
Yes, instead of setting arbitrary scopes in workerType definition, a role matching the workertype
is created on auth... And provisioner grants this role to worker.

> What will we do when we want to generate temporary credentials for other things?
> Will someone have to generate them manually and insert them every 30 days?
I don't follow... I think you misunderstand we're not going to insert temp creds in workerType secrets.
Workers can still have arbitrary

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

To finish that thought, the temporary creds will be generated with the role as in comment 0 item C.  What that role means is configured in the roles.

This is part of an overall effort to remove lists of scopes from components and centralize them in roles, where they can be easily modified and audited.

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

We're at step 4.  Those roles should be:

  assume:worker-type:aws-provisioner-v1/<workerType>
  assume:worker-id:<region>/<instanceId>

(the assume-worker-id doesn't do much but allow queue operations)

Comment 11

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: pr

Jonas for the provisioner side review, Greg for ensuring that this wouldn't break the worker.  In my local testing, I got credentials that looked like this:

{
  "data": {},
  "scopes": [],
  "credentials": {
    "clientId": "snip",
    "accessToken": "snip",
    "certificate": "{\"version\":1,\"scopes\":[\"assume:worker-type:aws-provisioner2-dev/a\",\"assume:worker-id:*\"],\"start\":snip,\"expiry\":snip,\"seed\":\"snip"}"
  }

based on a secret that looked like this:

{
  "workerType": "a",
  "secrets": {},
  "scopes": [],
  "token": "snip",
  "expiration": "snip"
}

Comment 12

[Greg Arndt [:garndt]]

Comment on attachment 8750786 [details] [review]
pr

I do not see anything obvious that should break.  These should be the same temp creds that the provisioner otherwise would have given out anyways.  I do not think we override those scopes in any of our worker types.

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

This was deployed.

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

The assume:worker-id:.. bit is being carried on in bug 1272643.

Comment 15

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8750786 [details] [review]
pr

r+'ed on the PR I think...