Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ingest access legacy migration #2987

Closed
bcmdarroch opened this issue Feb 28, 2020 · 4 comments
Closed

Ingest access legacy migration #2987

bcmdarroch opened this issue Feb 28, 2020 · 4 comments
Assignees
Labels
auth-team anything that needs to be on the auth team board emergent v2 force upgrade branch

Comments

@bcmdarroch
Copy link
Contributor

bcmdarroch commented Feb 28, 2020

Current Solution

On v1, we had some default policies: one granting all tokens access to ingest endpoints (like /data-collector/v0) and a few others granting all users access to cfgmgmt things (GET client runs, etc)

		constants.IngestWildcardPolicyID: {
			ID:       ids[constants.IngestWildcardPolicyID],
			Subjects: []string{"token:*"},
			Resource: "ingest:*",
			Action:   "*",
			Effect:   "allow",
			Version:  1,
		},

		constants.CfgmgmtStatsWildcardPolicyID: {
			ID:       ids[constants.CfgmgmtStatsWildcardPolicyID],
			Subjects: []string{"user:*"},
			Resource: "cfgmgmt:stats:*",
			Action:   "*",
			Effect:   "allow",
			Version:  1,
		},
		constants.CfgmgmtNodesWildcardPolicyID: {
			ID:       ids[constants.CfgmgmtNodesWildcardPolicyID],
			Subjects: []string{"user:*"},
			Resource: "cfgmgmt:nodes:*",
			Action:   "*",
			Effect:   "allow",
			Version:  1,
		},
		constants.CfgmgmtNodesContainerPolicyID: {
			ID:       ids[constants.CfgmgmtNodesContainerPolicyID],
			Subjects: []string{"user:*"},
			Resource: "cfgmgmt:nodes",
			Action:   "*",
			Effect:   "allow",
			Version:  1,
		},

On v2, the ingest endpoints moved under the infra:* namespace (aka the new name for cfgmgmt). This meant users got access the formerly client-only endpoints when they got infra:* permissions. To minimize security risks, we decided to add a DENY statement so users could not access infra:ingest:* endpoints (like /data-collector/v0)

    {
      "name": "[Legacy] Ingest Access",
      "id": "ingest-access-legacy",
      "type": "CUSTOM",
      "members": [
        "token:*"
      ],
      "statements": [
        {
          "effect": "ALLOW",
          "actions": [
            "infra:ingest:*"
          ],
          "role": "",
          "resources": [
            "*"
          ],
          "projects": [
            "*"
          ]
        }
      ],
      "projects": []
    }
{
      "name": "[Legacy] Infrastructure Automation Access",
      "id": "infrastructure-automation-access-legacy",
      "type": "CUSTOM",
      "members": [
        "user:*"
      ],
      "statements": [
        {
          "effect": "ALLOW",
          "actions": [
            "infra:*"
          ],
          "role": "",
          "resources": [
            "*"
          ],
          "projects": [
            "*"
          ]
        },
        {
          "effect": "DENY",
          "actions": [
            "infra:ingest:*"
          ],
          "role": "",
          "resources": [
            "*"
          ],
          "projects": [
            "*"
          ]
        }
      ],
      "projects": []
    }

This causes problems however because it blocks admins from using infra:ingest endpoints. Admins should be able to do anything that won't break the system (like deleting themselves)

Option 1

We move https://github.com/chef/automate/blob/master/api/external/ingest/chef.proto and https://github.com/chef/automate/blob/master/components/automate-gateway/api/legacy/legacy.proto endpoints into their own top-level ingest:* namespace. Then the v1 default policies can be migrated to v2 as:

 {
      "name": "[Legacy] Ingest Access",
      "id": "ingest-access-legacy",
      "type": "CUSTOM",
      "members": [
        "token:*"
      ],
      "statements": [
        {
          "effect": "ALLOW",
          "actions": [
            "ingest:*"
          ],
          "role": "",
          "resources": [
            "*"
          ],
          "projects": [
            "*"
          ]
        }
      ],
      "projects": []
   }
 {
      "name": "[Legacy] Infrastructure Automation Access",
      "id": "infrastructure-automation-access-legacy",
      "type": "CUSTOM",
      "members": [
        "user:*"
      ],
      "statements": [
        {
          "effect": "ALLOW",
          "actions": [
            "infra:*"
          ],
          "role": "",
          "resources": [
            "*"
          ],
          "projects": [
            "*"
          ]
        } 
      ],
      "projects": []
    }

This will take longer and might require some migrations (if customers made new policies with infra:ingest:*, but in the end might represent the ingest actions more accurately.

Option 2

We remove the DENY statement and only grant users access to non infra:ingest:* actions

 {
      "name": "[Legacy] Ingest Access",
      "id": "ingest-access-legacy",
      "type": "CUSTOM",
      "members": [
        "token:*"
      ],
      "statements": [
        {
          "effect": "ALLOW",
          "actions": [
            "ingest:*"
          ],
          "role": "",
          "resources": [
            "*"
          ],
          "projects": [
            "*"
          ]
        }
      ],
      "projects": []
   }
 {
      "name": "[Legacy] Infrastructure Automation Access",
      "id": "infrastructure-automation-access-legacy",
      "type": "CUSTOM",
      "members": [
        "user:*"
      ],
      "statements": [
        {
          "effect": "ALLOW",
          "actions": [
            "infra:nodes:*", "infra:servers:*", "infra:serverOrgCookbooks:*", "infra:nodeManagers:*"
          ],
          "role": "",
          "resources": [
            "*"
          ],
          "projects": [
            "*"
          ]
        } 
      ],
      "projects": []
    }

This is the quickest option

@bcmdarroch bcmdarroch added auth-team anything that needs to be on the auth team board needs-triage v2 force upgrade branch labels Feb 28, 2020
@lancewf
Copy link
Contributor

lancewf commented Feb 28, 2020

I like option one better because it does not give the user the impression that they can only ingest chef infra data.

@vjeffrey
Copy link

+1 on the first option

@bcmdarroch
Copy link
Contributor Author

Option 1 it is!

@susanev
Copy link
Contributor

susanev commented Feb 29, 2020

this will also require a change to the editor, viewer, project owner, and ingest roles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth-team anything that needs to be on the auth team board emergent v2 force upgrade branch
Projects
None yet
Development

No branches or pull requests

6 participants