1137740 - Auth: Stricter scope policy when auth.taskcluster.net issues temporary credentials to <anyone>@mozilla.com

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Description

[Rail Aliiev [:rail]]

Anyone with @mozilla.com account can create credentials with "*" scope. This is not ideal for some sensitive tasks, like signing.

It would be great if we can limit this and revoke all * scopes.

Comment 1

[Jonas Finnemann Jensen (:jonasfj)]

We've been talking about this for a while. The scopes issued to <anyone>@mozilla.com is an
obvious problem. But there is a lot of other places we can and should avoid handing out the '*' scope.

I've filed bug 1137827 as meta-bug for tracking all scope reduction work.

Comment 2

[Jonas Finnemann Jensen (:jonasfj)]

Attachment: github PR

Should be a quick review... Sorry the diff is a bit messed up, indentation...

Note, self defined "auth_clientIdForTempCreds" before you merge and/or deploy this...

Comment 3

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

Comment on attachment 8593679 [details] [review]
github PR

This looks good to me!  I'll let you merge it as I don't want to break anything whist deploying.

Comment 4

[Jonas Finnemann Jensen (:jonasfj)]

Temporary credentials are now issued using the "yHLBn3GaTY-SYhTnKw3X-Q" user.
called "temporary-credentials" we can now restrict which scopes it has in the UI.

Comment 5

[Pete Moore [:pmoore][:pete]]

Resetting Version and Target Milestone that accidentally got changed...

Comment 6

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

(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #4)
> Temporary credentials are now issued using the "yHLBn3GaTY-SYhTnKw3X-Q" user.
> called "temporary-credentials" we can now restrict which scopes it has in
> the UI.

..and yet we didn't do that, so this isn't really resolved!

Comment 7

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

When bug 1203659 lands, this will no longer be the case, and the credential in comment 4 should be deleted.

Comment 8

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

Looking forward to a time when taskcluster-login is deployed, I'm going to start creating some group-related roles and assuming those roles from this credential.

Comment 9

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

I added the following as a rough draft:

    'client-id:yHLBn3GaTY-SYhTnKw3X-Q': [  # temporary credentials
        'assume:moz-ldap-group:scm_level_3',
        'assume:moz-ldap-group:team_moco',
        '*', # XXX
    ],  
    'moz-ldap-group:releng': [
        'scheduler:*',
        'queue:*',
        'docker-worker:*',
        'buildbot-bridge:*',
        'signing:*',
        'releng:*',
    ],  
    'moz-ldap-group:scm_level_1': [
        'assume:moz-tree:level:1',
    ],  
    'moz-ldap-group:scm_level_2': [
        'assume:moz-tree:level:2',
    ],  
    'moz-ldap-group:scm_level_3': [
        'assume:moz-tree:level:3',
    ],  
    'moz-ldap-group:team_moco': [
        'auth:create-role:*',
        'auth:update-role:*',
    ],  
    'moz-ldap-group:team_taskcluster': [
        'auth:create-client:*',
        'auth:create-role:*',
        'auth:delete-client:*',
        'auth:delete-role:*',
        'auth:update-client:*',
        'auth:update-role:*',
        '*',
    ],  
    'moz-ldap-group:team_relops': [
        'assume:moz-ldap-group:releng',
    ],  

I'm worried that removing `*` will invalidate any existing temp creds.  I'll verify that it does, then ponder how to roll this out.

Comment 10

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

Yes, after removing `*`, anything that requires scopes results in "ext.issuer does not have sufficient scopes" or something like that.  Which isn't such a big deal -- most day-to-day activities (task inspector, task-graph-inspector) don't require scopes so they continue to work fine.

That said, we very much want to invalidate these temporary credentials, since they are good for 1000 years (literally).

Also, the temp credentials are granted using `{scopes: client.expandScopes()}` which means that the temp credentials don't automatically adjust as roles are modified.  Instead, if a role shrinks (e.g., we remove `auth:create-task:<some unused worker>` from a role), all temp credentials with that role become completely invalid.  If a role expands, the temporary credentials do not satisfy the newly added scopes.

I'll propose a small modification to tc-auth to handle this case, even though tc-login will result in removing this code entirely.  Once the change has landed, we can remove the `*` scope above.

Comment 11

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

https://github.com/taskcluster/taskcluster-auth/pull/25

Comment 12

[Jonas Finnemann Jensen (:jonasfj)]

@dustin, there were never good for 1000 years... The date is in ms.
they are currently good for 31 days.

And no server will accept temp creds with expiry over 31 days.
We check this on the server for every request:
https://github.com/taskcluster/taskcluster-base/blob/5883272d75cd7f508b9542a147e6af589b4115cc/api.js#L790-L792

Comment 13

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

Haha, yes, you're right.  I've forgotten JS's affinity for ms more times than I care to discuss.

Comment 14

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

OK, we're agreed that the plan is:

 * land and deploy #25
 * modify client-id:yHLBn3GaTY-SYhTnKw3X-Q to remove '*'
   * this will immediately invalidate all existing temp creds, so people will re-auth

After this, the temp creds will have scopes `["assume:client-id:yHLBn3GaTY-SYhTnKw3X-Q"]`, so whatever scopes that clientId has will be immediately available or unavailable to all temp creds.

Comment 15

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

Done and done.  I verified I can still create a new task with a new temp cred.