Create .git-credentials to allow secure auth when cloning private repos #711

andrewjeffree · 2019-07-17T06:31:17Z

Currently when Atlantis is running in a docker container it cannot access private repositories without people either pushing ssh private keys into the container with commands at launch or in some cases forking off their own copy where they embed the ssh key/secrets in the image. I'm sure there are a myriad of other ways people are doing it also. None of these are particularly nice options at least personally as then you're providing two sets of secrets instead of one.

GitHub, GitLab and Bitbucket all support push/pull via HTTPS you just need to provide the respective token. git will and does prompt you for the username and password required to authenticate over HTTPS however it is not cached to disk.

Terraform itself doesn't support any sort of environment variables either which would allow it to take care of this. As such we need to use git-credentials-store to configure this as a once off when we run the container. We do this by writing out a .git-credentials file into the atlantis users home directory with the relevant information that is already provided to the container as environment variables when launched and then running git config --global credential.helper store as the atlantis user so that it's aware of the .git-credentials file.

This will only affect/work for those who use HTTPS sources.

I've conducted some basic testing on all three providers and didn't run into any issues.

Happy to enhance/tweak as required.

codecov · 2019-07-17T06:33:06Z

Codecov Report

Merging #711 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #711   +/-   ##
=======================================
  Coverage   72.45%   72.45%           
=======================================
  Files          61       61           
  Lines        4679     4679           
=======================================
  Hits         3390     3390           
  Misses       1039     1039           
  Partials      250      250

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8180f35...0e99363. Read the comment docs.

codecov · 2019-07-17T06:33:06Z

Codecov Report

Merging #711 into master will decrease coverage by 0.04%.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##           master     #711      +/-   ##
==========================================
- Coverage   72.37%   72.32%   -0.05%     
==========================================
  Files          61       62       +1     
  Lines        4695     4712      +17     
==========================================
+ Hits         3398     3408      +10     
- Misses       1044     1052       +8     
+ Partials      253      252       -1

Impacted Files	Coverage Δ
cmd/server.go	`79.42% <ø> (ø)`	⬆️
server/user_config.go	`100% <ø> (ø)`	⬆️
server/server.go	`63.87% <0%> (-3.02%)`	⬇️
server/events/git_cred_writer.go	`90% <90%> (ø)`
server/events/runtime/runtime.go	`78.94% <0%> (ø)`	⬆️
server/events/terraform/terraform_client.go	`79.03% <0%> (+0.97%)`	⬆️
server/events/runtime/run_step_runner.go	`100% <0%> (+10%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f9412b...6120974. Read the comment docs.

lkysow · 2019-07-17T10:45:05Z

Hi Andrew, thanks for the PR! I'm wondering if we should make this optional based on an environment variable because it's writing the credentials in plaintext to disk?

andrewjeffree · 2019-07-17T11:11:48Z

Hey. I can see the logic behind that, and am happy to implement that. Do you have any preferences around the variable name?

lkysow · 2019-07-17T14:40:30Z

Hmm, maybe ATLANTIS_GIT_CREDS=true?

lkysow · 2019-07-17T14:44:18Z

Hmm, I'm wondering if we should actually implement this within Atlantis itself on startup? Because this requires you use environment variables and our Docker container.

What about a flag --write-git-creds=true?

andrewjeffree · 2019-07-17T14:57:55Z

Implementing it into Atlantis itself is probably a better solution. I’m happy to give it a shot though I have next to no experience with go. I like --write-git-creds=true

lkysow · 2019-07-17T15:44:53Z

See Add flag for setting Terraform Enterprise hostname #706 for how to add a CLI flag.
I'd put the implementation into a new file called server/events/git_cred_writer.go
It can be a function without a struct/class, ex
```
func writeGitCreds(ghUser, ghToken, ghHostname, glUser ...)
```
should be similar in function to https://github.com/runatlantis/atlantis/blob/master/server/events/terraform/terraform_client.go#L401
Can be called in the NewServer() method of server/server.go
Will need tests, see the tests for generateRCFiles here https://github.com/runatlantis/atlantis/blob/master/server/events/terraform/terraform_client_internal_test.go#L17

andrewjeffree · 2019-07-18T03:16:26Z

So I'm just in the process of figuring out where to put the logic in the NewServer() method of server/server.go and I can see two ways of going about it.

I put something like the below in the respective GitHub, GitLab and Bitbucket blocks here

if userConfig.WriteGitCreds == true {
    events.writeGitCreds(userConfig.User, userConfig.Token, userConfig.Host)
 }

Or I split it out to its own logic just below it and duplicate the logic around determining which git provider is being used.

Do you have any preferences around this?

andrewjeffree · 2019-07-18T07:37:04Z

I think I've got the basic logic in place now. I have no idea why the testing is saying writeGitCreds is undefined I'm a bit out of my depth here, unfortunately. Would appreciate some pointers and/or assistance in getting this working.

andrewjeffree · 2019-07-18T10:18:37Z

Well, I figured it out and it's passing now after some swearing on my end. I've squashed it to make it easier to review/merge as desired. Let me know if you want adjustments made.

andrewjeffree · 2019-07-19T13:54:25Z

@lkysow I think it's ready for review. I'm not sure how to fix the coverage with server.go though.

lkysow · 2019-07-19T14:45:51Z

Nice! Don't worry about the server.go coverage. I'll try to get to the review asap but I've got a lot on my plate so you might need to wait a while.

lkysow

This is awesome! Final feedback:

some small changes requested
please squash your commits

Not in this PR but if you're down to do another one I'd like to:

Get ssh private cloning working as well. All you need to do is write an additional line to the .gitconfig:
```
[url "https://TOKEN@github.com"]
   insteadOf = ssh://git@github.com
```
Get this working if using more than one VCS provider. Right now if you're using github and gitlab then it will fail because it tries to overwrite itself.

server/events/git_cred_writer.go

--write-git-creds will create a .git-credentials file and configure git to use it. To allow authentication with your git remotes over https. To access private repos.

andrewjeffree · 2019-08-02T00:17:43Z

* some small changes requested

* please squash your commits

Changes made and commits squashed.

Not in this PR but if you're down to do another one I'd like to:

1. Get ssh private cloning working as well. All you need to do is write an additional line to the .gitconfig:
   ```
   [url "https://TOKEN@github.com"]
      insteadOf = ssh://git@github.com
   ```

2. Get this working if using more than one VCS provider. Right now if you're using github and gitlab then it will fail because it tries to overwrite itself.

Sure. If you create an issue and assign it to me I'll give it a shot.

chadasapp · 2019-08-05T10:57:05Z

Get ssh private cloning working as well. All you need to do is write an additional line to the .gitconfig:
[url "https://TOKEN@github.com"]
   insteadOf = ssh://git@github.com

I wouldn't suggest doing that, actually. We shouldn't assume that all repos available over SSH are accessible to this user with their token provided. The Atlantis user is only required to have read access to the repository/repositories being acted upon. Other git operations as a consequence (I'm looking at you, modules), are not necessarily accessible to that Github user.

I could see a scenario in which the Atlantis user doesn't even have an SSH key, but various modules that need accessing are given deploy keys that are loaded into the ssh-agent that Atlantis uses. Compartmentalizing access isn't a bad idea, after all :)

I'd stick to just HTTPS with this patch and leave SSH as-is.

Get this working if using more than one VCS provider. Right now if you're using github and gitlab then it will fail because it tries to overwrite itself.

Github Enterprise is being forgotten here. Please vary all config based on ATLANTIS_GH_HOSTNAME where appropriate.

chadasapp · 2019-08-05T11:13:35Z

Generally speaking, I think we can do a little better here to help avoid the "malicious PRs could exfiltrate credentials" issue.

Don't worry about keeping the username private, its already known to the people submitting PRs anyway. So you can get away with: git config --global credential.https://github.saobby.my.eu.org.username $GIT_USER (vary on ATLANTIS_GH_HOSTNAME, as in my prior comment, please)
Write the token to a bash script on disk when git clone/fetch operations happen, then kill it afterwords. Use GIT_ASKPASS to point to said script. That way the token doesn't have to exist on-disk when Terraform operations are happening.

Another approach could include messing with the credential cache times -- basically set it to infinity so it's in memory and so after first authentication we can kill the on-disk credential entirely.

andrewjeffree · 2019-08-06T00:49:25Z

wouldn't suggest doing that, actually. We shouldn't assume that all repos available over SSH are accessible to this user with their token provided. The Atlantis user is only required to have read access to the repository/repositories being acted upon. Other git operations as a consequence (I'm looking at you, modules), are not necessarily accessible to that Github user.

How do you expect Terraform to be able to checkout modules via terraform init without the user itself having access to those modules? All my modules are private git repos. If the Atlantis user doesn't have access to the modules I'm unsure how you'd actually be making effective use of Atlantis. Perhaps if you could share more detail on your workflow?

I could see a scenario in which the Atlantis user doesn't even have an SSH key, but various modules that need accessing are given deploy keys that are loaded into the ssh-agent that Atlantis uses. Compartmentalizing access isn't a bad idea, after all :)

But then you're storing secrets (ssh keys) in your terraform repo/git? Perhaps I'm misunderstanding your intended workflow here.

I'd stick to just HTTPS with this patch and leave SSH as-is.

I believe the request was to look at doing SSH in a separate PR. I certainly have no intention of doing anything apart from HTTPS in this PR at this point in time. Perhaps in a future PR if/when SSH is looked at it could be added as another flag to alivieate your concerns here.

Github Enterprise is being forgotten here. Please vary all config based on ATLANTIS_GH_HOSTNAME where appropriate.

The code should work fine with Github Enterprise and already makes use of the ATLANTIS_GH_HOSTNAME value. We pass this value to the function on every usage when GitHub user is actually set.

https://github.com/ImperialXT/atlantis/blob/master/server/server.go#L161-L165

By default, the Github hostname value is github.com unless overridden via the CLI fags or environment variable ATLANTIS_GH_HOSTNAME.
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L125-L127
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L80

Write the token to a bash script on disk when git clone/fetch operations happen, then kill it afterwords. Use GIT_ASKPASS to point to said script. That way the token doesn't have to exist on-disk when Terraform operations are happening.

That could work. I think the only place it'd need to be called would be in server/events/runtime/init_step_runner.go @lkysow What are your thoughts on implementing it this way instead?

chadasapp · 2019-08-06T01:17:51Z

wouldn't suggest doing that, actually. We shouldn't assume that all repos available over SSH are accessible to this user with their token provided. The Atlantis user is only required to have read access to the repository/repositories being acted upon. Other git operations as a consequence (I'm looking at you, modules), are not necessarily accessible to that Github user.

How do you expect Terraform to be able to checkout modules via terraform init without the user itself having access to those modules? All my modules are private git repos. If the Atlantis user doesn't have access to the modules I'm unsure how you'd actually be making effective use of Atlantis. Perhaps if you could share more detail on your workflow?

If the repos are accessed over SSH instead of HTTPS, they could be done with deploy keys as I say below, which are injected into the ssh-agent. The Atlantis Github user doesn't have any relation to these keys at all, hence why rewriting SSH to HTTPS would break things.

This isn't actually my workflow, but it doesn't seem implausible and rewriting protocols just struck me as likely to cause issues for someone so I was just playing hypotheticals.

I could see a scenario in which the Atlantis user doesn't even have an SSH key, but various modules that need accessing are given deploy keys that are loaded into the ssh-agent that Atlantis uses. Compartmentalizing access isn't a bad idea, after all :)

But then you're storing secrets (ssh keys) in your terraform repo/git? Perhaps I'm misunderstanding your intended workflow here.

The public key would be a deploy key on the repo on Github, the private key would be injected into the ssh-agent where Atlantis is running (be that Docker container or otherwise). At my last job, we actually wrote some software to allow for SSH keys to be used without exposing the private key to the caller, so this isn't entirely implausible either.

I'd stick to just HTTPS with this patch and leave SSH as-is.

I believe the request was to look at doing SSH in a separate PR. I certainly have no intention of doing anything apart from HTTPS in this PR at this point in time. Perhaps in a future PR if/when SSH is looked at it could be added as another flag to alivieate your concerns here.

I misunderstood! Carry on then :)

Github Enterprise is being forgotten here. Please vary all config based on ATLANTIS_GH_HOSTNAME where appropriate.

The code should work fine with Github Enterprise and already makes use of the ATLANTIS_GH_HOSTNAME value. We pass this value to the function on every usage when GitHub user is actually set.

https://github.com/ImperialXT/atlantis/blob/master/server/server.go#L161-L165

By default, the Github hostname value is github.com unless overridden via the CLI fags or environment variable ATLANTIS_GH_HOSTNAME.
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L125-L127
https://github.com/ImperialXT/atlantis/blob/master/cmd/server.go#L80

I must've misread something or misunderstood the comments, I guess! Again, carry on!

Write the token to a bash script on disk when git clone/fetch operations happen, then kill it afterwords. Use GIT_ASKPASS to point to said script. That way the token doesn't have to exist on-disk when Terraform operations are happening.

That could work. I think the only place it'd need to be called would be in server/events/runtime/init_step_runner.go @lkysow What are your thoughts on implementing it this way instead?

Oh cool, I wasn't sure if that'd be feasible or not. That'd be a great route and would alleviate a lot of the concerns with exfiltration we see right now.

lkysow

Thanks for the great PR!

I'd be happy to take a look at a PR that implements the ASK_PASS suggestions but this is a nice improvement for now and is optional for users that don't want to enable.

andrewjeffree changed the base branch from master to tfe-remote-backend July 18, 2019 11:21

andrewjeffree changed the base branch from tfe-remote-backend to master July 18, 2019 11:21

andrewjeffree changed the title ~~Create .git-credentials in docker to allow secure auth when cloning private repos~~ Create .git-credentials to allow secure auth when cloning private repos Jul 19, 2019

lkysow self-requested a review August 1, 2019 18:27

lkysow self-assigned this Aug 1, 2019

lkysow reviewed Aug 1, 2019

View reviewed changes

server/events/git_cred_writer.go Outdated Show resolved Hide resolved

server/events/git_cred_writer.go Outdated Show resolved Hide resolved

server/events/git_cred_writer.go Show resolved Hide resolved

Add flag for configuring git-credentials-store

e140823

--write-git-creds will create a .git-credentials file and configure git to use it. To allow authentication with your git remotes over https. To access private repos.

Update server-configuration.md

6120974

lkysow approved these changes Aug 6, 2019

View reviewed changes

lkysow merged commit 3ebebe5 into runatlantis:master Aug 6, 2019

chaseconey mentioned this pull request Aug 29, 2019

authenticating for a module with a private github.com repo source #281

Closed

andrewjeffree mentioned this pull request Oct 2, 2019

Configure git to use https instead of ssh. #797

Closed

andrewjeffree mentioned this pull request Oct 3, 2019

Configure git to use https instead of ssh. #799

Merged

frank-bee mentioned this pull request May 31, 2023

Allow accessing private terraform modules (github) diggerhq/digger#296

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create .git-credentials to allow secure auth when cloning private repos #711

Create .git-credentials to allow secure auth when cloning private repos #711

andrewjeffree commented Jul 17, 2019 •

edited

Loading

codecov bot commented Jul 17, 2019

codecov bot commented Jul 17, 2019 •

edited

Loading

lkysow commented Jul 17, 2019

andrewjeffree commented Jul 17, 2019

lkysow commented Jul 17, 2019

lkysow commented Jul 17, 2019

andrewjeffree commented Jul 17, 2019

lkysow commented Jul 17, 2019

andrewjeffree commented Jul 18, 2019 •

edited

Loading

andrewjeffree commented Jul 18, 2019

andrewjeffree commented Jul 18, 2019

andrewjeffree commented Jul 19, 2019

lkysow commented Jul 19, 2019

lkysow left a comment

andrewjeffree commented Aug 2, 2019

chadasapp commented Aug 5, 2019

chadasapp commented Aug 5, 2019 •

edited

Loading

andrewjeffree commented Aug 6, 2019

chadasapp commented Aug 6, 2019

lkysow left a comment

Create .git-credentials to allow secure auth when cloning private repos #711

Create .git-credentials to allow secure auth when cloning private repos #711

Conversation

andrewjeffree commented Jul 17, 2019 • edited Loading

codecov bot commented Jul 17, 2019

Codecov Report

codecov bot commented Jul 17, 2019 • edited Loading

Codecov Report

lkysow commented Jul 17, 2019

andrewjeffree commented Jul 17, 2019

lkysow commented Jul 17, 2019

lkysow commented Jul 17, 2019

andrewjeffree commented Jul 17, 2019

lkysow commented Jul 17, 2019

andrewjeffree commented Jul 18, 2019 • edited Loading

andrewjeffree commented Jul 18, 2019

andrewjeffree commented Jul 18, 2019

andrewjeffree commented Jul 19, 2019

lkysow commented Jul 19, 2019

lkysow left a comment

Choose a reason for hiding this comment

andrewjeffree commented Aug 2, 2019

chadasapp commented Aug 5, 2019

chadasapp commented Aug 5, 2019 • edited Loading

andrewjeffree commented Aug 6, 2019

chadasapp commented Aug 6, 2019

lkysow left a comment

Choose a reason for hiding this comment

andrewjeffree commented Jul 17, 2019 •

edited

Loading

codecov bot commented Jul 17, 2019 •

edited

Loading

andrewjeffree commented Jul 18, 2019 •

edited

Loading

chadasapp commented Aug 5, 2019 •

edited

Loading