fluxcd / flux

Successor: https://github.com/fluxcd/flux2
https://fluxcd.io
Apache License 2.0
6.9k stars 1.08k forks source link

flux leaks https password in git error messages #2655

Closed dayglojesus closed 3 years ago

dayglojesus commented 4 years ago

Describe the bug

When configured to use Git over HTTPs, Flux will log errors containing the PAT.

To Reproduce

  1. setup Flux to use Git over HTTPS, rather than SSH
  2. disrupt the network such that Flux cannot reach the Git server
  3. observe logs

Expected behavior

Error logs containing URLS should be sanitized.

Logs

{
  "caller": "loop.go:101",
  "component": "sync-loop",
  "err": "git repo not ready: git clone --mirror: fatal: unable to access 'https://git-user:50ce089ca2e9634fd48201094f26e364ca4c5d2e@my.git.com/git-user/gitops-repo/': Could not resolve host: my.git.com, full output:\n Cloning into bare repository '/tmp/flux-gitclone708690917'...\nfatal: unable to access 'https://git-user:50ce089ca2e9634fd48201094f26e364ca4c5d2e@my.git.com/git-user/gitops-repo/': Could not resolve host: my.git.com\n",
  "ts": "2019-12-02T23:08:13.556931366Z"
}

Additional context

2opremio commented 4 years ago

There are some improvements for this in 1.16.0 (see https://github.com/fluxcd/flux/pull/2549 ), but I am afraid it doesn't cover this case.

I think we will need to go through all the potential error cases as well, which is going to be tough since, for instance the error above comes from an invocation of the git command whose format we don't fully control.

dayglojesus commented 4 years ago

Thanks @2opremio i missed that issue when searching. Glad a fix is on the books. Cheers.

2opremio commented 4 years ago

I have given some more thought to this but I don't think we can strive to remove the URL password from every possible error message.

Also, having the password in the Flux command-line arguments by itself is not a great idea. It could be considered a leak in the same way as printing them in the logs.

Thus, I would recommend using something like the Git Credential storage. I haven't tried it, but you should be able to change Flux's git configuration to use this, plus you could mount the credentials file from a Kubernetes secret.

dayglojesus commented 4 years ago

I have already done something very similar to this in my current HTTPS setup. You may be interested to know that the credentials supplied in a Git HTTPS url are easily persisted to a user's credential store. All that is required is for one to create a .gitconfig with the following example content:

[credential]
    helper = store --file /root/.git-credentials

Once this config is created, any credential used in a Git HTTPS operation will be persisted in the file referenced in the config above. Extrapolating, it should be possible for Flux to use the credentialed HTTPS url one-time (to persist the credentials) and thereafter, substitute a sanitized URL in future Git operations.

gecube commented 4 years ago

The same issue with leaking of PAT as topicstarter stated.

P.S. also it looks like that fluxd 1.17.0 effectively hides PAT and the issue is resolved.

dayglojesus commented 4 years ago

@2opremio I can confirm @gecube 's assertion -- my testing of 1.17.1 show no PAT leak. Thanks.

jeff-minard-ck commented 4 years ago

I've seen this in a different error message:

ts=2020-02-20T17:15:49.132981898Z caller=loop.go:107 component=sync-loop err="pushing tag to origin: failed to push some refs to 'https://user:password@ghe-host.com/org/reponame', full output:\n remote: [remote rejected] flux -> flux (cannot lock ref 'refs/tags/flux': is at 0d4da5701b74649504b99610be02ccab5e793fd3 but expected 1fd37a8e76afb6ee38f6e57bf09322fb48e6641f)\nerror: failed to push some refs to 'https://user:password@ghe-host.com/org/reponame'\n"

Would be open to setting up something else for https auth, but the above comments don't quite make it clear how this should be done.

Would git, in this context, respect a .netrc file?

$ cat ~/.netrc
machine ghe-host.com
login usernamehere
password tokengoeshere

On version 1.18.0

2opremio commented 4 years ago

I have never used .netrc files, but Flux executes the git command normally (as provided by the Flux container image). So whatever works when you invoke git manually, in principle should also work for Flux.

Just try it.

jeff-minard-ck commented 4 years ago

How about that, works perfectly:

apiVersion: v1
kind: Secret
metadata:
    name: flux-netrc
    namespace: flux
type: Opaque
stringData:
    netrc: |-
        machine ghe-host.com
        login usernamehere
        password tokengoeshere
# flux command:
      --git-url=https://ghe-host.com/org/repo

# under volumes:
      - name: flux-netrc
        secret:
          secretName: flux-netrc
          defaultMode: 0400

# under volumeMounts
        - name: flux-netrc
          mountPath: /root/.netrc
          subPath: netrc

Now logs all look slightly funny, but everything works with zero leak chance:

ts=2020-02-20T18:01:34.969940608Z caller=main.go:653 url=https://@ghe-host.com/org/reponame etc, etc
...
ts=2020-02-20T18:01:48.237757484Z caller=loop.go:133 component=sync-loop event=refreshed url=https://@ghe-host.com/org/reponame branch=master HEAD=31244e5fa4a8b130c23a351c47dc7a212ea1c0c0
kingdonb commented 3 years ago

Flux v1 is formally superseded since the GitOps Toolkit APIs have been declared stable:

https://fluxcd.io/docs/migration/timetable/

The repo will remain in maintenance for some time, but no new features can be accepted. Bugs can be addressed if they are critical and there is a PR to resolve it, but soon only CVEs can be addressed in Flux v1, and new users are all recommended to use Flux v2 for some time now.

I am not sure how to resolve this issue without a breaking change, but I also do not want to spend time on it unless there is an actual user who is actively being impacted by this. Please speak up if you still need help. My preference would be to work around rather than patch, until they are able to migrate to Flux v2 where this type of issue can be given priority if it is present. I do not think you will find this problem anymore in modern Flux releases.

Thanks for using Flux!