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

[Security] netrc support passes original credentials on HTTP redirects #9327

Closed
keith opened this issue Sep 4, 2019 · 26 comments
Closed

[Security] netrc support passes original credentials on HTTP redirects #9327

keith opened this issue Sep 4, 2019 · 26 comments
Assignees
Labels
P0 This is an emergency and more important than other current work. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.

Comments

@keith
Copy link
Member

keith commented Sep 4, 2019

Description of the problem / feature request:

When you have a ~/.netrc file with these contents:

machine github.com
  login something
  password something

Downloads of GitHub tarballs fail with this error:

skylib.0.8.0.tar.gz: GET returned 400 Bad Request
ERROR: no such package '@bazel_skylib//': java.io.IOException: Error downloading [https://github.com/bazelbuild/bazel-skylib/releases/download/0.8.0/bazel-skylib.0.8.0.tar.gz] to 

If you copy the curl request of what bazel is doing, you can see why:

% curl -H 'User-Agent: Bazel/release 0.29.0' -H 'Authorization: Basic somebase64' -H 'Host: github.com' -H 'Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2' 'http://github.com/bazelbuild/bazel-skylib/releases/download/0.8.0/bazel-skylib.0.8.0.tar.gz' -L
<?xml version="1.0" encoding="UTF-8"?>
<Error><Code>InvalidArgument</Code><Message>Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified</Message><ArgumentName>Authorization</ArgumentName><ArgumentValue>token anything</ArgumentValue><RequestId>8A1908B7B4F61B0B</RequestId><HostId>G3CxaBga4CoVWwqf1fu4WMrr2LjLJ28s2H8+Kn90PFFFygw8KKQ6+JINoeUTipnpbtbcxmKhr9A=</HostId></Error>

Removing the Authorization header from the request fixes this issue. Switching the Basic auth to bearer or token and providing that directly doesn't solve this issue.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

bazel build //... --repository_cache=

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

release 0.29.0

Have you found anything relevant by searching the web?

#7978

@keith
Copy link
Member Author

keith commented Sep 4, 2019

cc @genrym

@jin
Copy link
Member

jin commented Sep 5, 2019

cc @aehlig

@jin jin added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. untriaged labels Sep 5, 2019
@dslomov
Copy link
Contributor

dslomov commented Sep 5, 2019

cc @laurentlb this looks like a release blocker for 0.29 /cc #9293

@katre
Copy link
Member

katre commented Sep 5, 2019

Do we know what Bazel commit caused this to stop? Does it work with 0.28.1?

@genrym
Copy link

genrym commented Sep 5, 2019

@keith the PR referenced there is not the final implementation, and it was scrapped. The final implementation is pretty different.

Just to clarify, the same configuration works well with bazel 0.28.1?

@dslomov
Copy link
Contributor

dslomov commented Sep 5, 2019

The original functionality went in in c26e339, so should already be present in 0.28. Does this work with 0.28 for you @keith?

@laurentlb
Copy link
Contributor

I think c26e339 was released only in 0.29 (it has only the 0.29 tag on GitHub; the baseline of 0.28 was from July 1st, so a few days before).

@katre
Copy link
Member

katre commented Sep 5, 2019

Is the solution to roll back c26e339, then?

@dslomov
Copy link
Contributor

dslomov commented Sep 5, 2019

The rollback might not be that easy... Can we investigate an actual fix?

@katre
Copy link
Member

katre commented Sep 5, 2019

I am afraid I don't understand the error. What is wrong with the "Authorization" header that Bazel is generating in the curl command?

@dslomov
Copy link
Contributor

dslomov commented Sep 5, 2019

I can reproduce the problem with 0.29.0 (with Bazel not with curl). I assume some versions of curl remove duplicate Authorization: headers and some don't.

@dslomov dslomov added release blocker P0 This is an emergency and more important than other current work. (Assignee required) labels Sep 5, 2019
@dslomov
Copy link
Contributor

dslomov commented Sep 5, 2019

can reproduce the problem with 0.29.0 (with Bazel not with curl). I assume some versions of curl remove duplicate Authorization: headers and some don't.

(meaning one header comes from .netrc and another provided on the command line)

@katre
Copy link
Member

katre commented Sep 5, 2019

Oh, is curl also reading .netrc? I was not clear why there are two Authentication headers

@keith
Copy link
Member Author

keith commented Sep 5, 2019

I can confirm this isn't a problem with 0.28.1 for me. If curl is reading the netrc that's happening with the default macOS curl:

% curl --version
curl 7.54.0 (x86_64-apple-darwin18.0) libcurl/7.54.0 LibreSSL/2.6.5 zlib/1.2.11 nghttp2/1.24.1
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IPv6 Largefile GSS-API Kerberos SPNEGO NTLM NTLM_WB SSL libz HTTP2 UnixSockets HTTPS-proxy

@davido
Copy link
Contributor

davido commented Sep 5, 2019

Gerrit users started to report build breakage already, so that we urgently need a fix here. Unfortunately we bumped .bazelversion in gerrit repository to 0.29.0.

katre added a commit to katre/bazel that referenced this issue Sep 5, 2019
@katre
Copy link
Member

katre commented Sep 5, 2019

Sent #9341 as a rollback of the offending commit, since no one seems to know how to fix it. I can't reproduce, can either @keith or @davido please test and verify?

@keith
Copy link
Member Author

keith commented Sep 5, 2019

This would definitely fix the issue, to actually fix this @artem-zinnatullin brought up that the issue is probably bazel passing authorization headers through to redirects, which is also likely a security problem, but some services might be depending on this.

@davido
Copy link
Contributor

davido commented Sep 5, 2019

@katre Thanks for reverting the offending commit.

I am able to reproduce the problem on Gerrit repository, stable-3.0 branch:

  1. clone gerrit repository recursively:
  $ git clone --recurse-submodules https://gerrit.googlesource.com/gerrit
  1. Add this line to you ~/.netrc file:
machine github.com login foo password bar
  1. Try build gerrit with:
  $ bazel build headless
  INFO: Invocation ID: 69fc22f0-4090-481a-888f-d68c765dbb79
INFO: Call stack for the definition of repository 'bazel_skylib' which is a http_archive (rule definition at /home/davido/.cache/bazel/_bazel_davido/27a001f4182820ef315d8d2d4f1edafe/external/bazel_tools/tools/build_defs/repo/http.bzl:292:16):
 - /home/davido/projects/gerrit/WORKSPACE:43:1
WARNING: Download from https://github.com/bazelbuild/bazel-skylib/archive/0.8.0.tar.gz failed: class com.google.devtools.build.lib.bazel.repository.downloader.UnrecoverableHttpException GET returned 401 Unauthorized
ERROR: An error occurred during the fetch of repository 'bazel_skylib':
   java.io.IOException: Error downloading [https://github.com/bazelbuild/bazel-skylib/archive/0.8.0.tar.gz] to /home/davido/.cache/bazel/_bazel_davido/27a001f4182820ef315d8d2d4f1edafe/external/bazel_skylib/0.8.0.tar.gz: GET returned 401 Unauthorized
ERROR: no such package '@bazel_skylib//lib': java.io.IOException: Error downloading [https://github.com/bazelbuild/bazel-skylib/archive/0.8.0.tar.gz] to /home/davido/.cache/bazel/_bazel_davido/27a001f4182820ef315d8d2d4f1edafe/external/bazel_skylib/0.8.0.tar.gz: GET returned 401 Unauthorized
ERROR: no such package '@bazel_skylib//lib': java.io.IOException: Error downloading [https://github.com/bazelbuild/bazel-skylib/archive/0.8.0.tar.gz] to /home/davido/.cache/bazel/_bazel_davido/27a001f4182820ef315d8d2d4f1edafe/external/bazel_skylib/0.8.0.tar.gz: GET returned 401 Unauthorized
INFO: Elapsed time: 0.372s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)

My Bazel version is:

$ bazel version
Starting local Bazel server and connecting to it...
Build label: 0.29.1rc1
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Tue Sep 3 13:49:03 2019 (1567518543)
Build timestamp: 1567518543
Build timestamp as int: 1567518543

@artem-zinnatullin
Copy link
Contributor

We've spent more time with @keith looking at it, it seems that Bazel passes Auth headers along when server issues 3xx redirects even if target host changes (!) and that's why download from GitHub fails — it redirects to AWS S3 with AWS auth headers but Bazel also adds GitHub basic auth header on top and then S3 bails out because multiple auth methods are passed.

This sounds like a pretty scary security vulnerability and reverting ~/.netrc support doesn't fix the root cause — which is passing auth headers on redirects

@katre
Copy link
Member

katre commented Sep 5, 2019

I'm just the release manager: if there are security implications then @dslomov needs to weigh in.

@artem-zinnatullin
Copy link
Contributor

My 2 cents:

  • First: From release perspective, I think reverting ~/.netrc support and releasing 0.29.1 ASAP is the right thing to do, this will stop exposing credentials listed in ~/.netrc to final destination servers like AWS S3 in this case.
  • Then:
    • A carefully crafted, tested and reviewed PR needs to be made to stop passing auth headers for redirects when the host changes
    • ~/.netrc support should be merged back
    • New Bazel release should be issued

@katre
Copy link
Member

katre commented Sep 5, 2019

That sounds like a reasonable plan.

@dslomov
Copy link
Contributor

dslomov commented Sep 6, 2019

We've spent more time with @keith looking at it, it seems that Bazel passes Auth headers along when server issues 3xx redirects even if target host changes (!) and that's why download from GitHub fails — it redirects to AWS S3 with AWS auth headers but Bazel also adds GitHub basic auth header on top and then S3 bails out because multiple auth methods are passed.

This sounds like a pretty scary security vulnerability and reverting ~/.netrc support doesn't fix the root cause — which is passing auth headers on redirects

Ouch, thanks for taking a look!

First: From release perspective, I think reverting ~/.netrc support and releasing 0.29.1 ASAP is the right thing to do, this will stop exposing credentials listed in ~/.netrc to final destination servers like AWS S3 in this case.

Reverting .netrc is not enough: the underlying API - auth parameter to download and download_and_extract - is a security vulnerability. Thankfully it only exists since 0.28.

I'll prepare a patch that makes auth a no-op and removes .netrc parsing. The patch needs to be cherry-picked into 0.29.1 and 1.0.0
We will need to design this more carefully and ship in in 1.1

@dslomov dslomov removed the untriaged label Sep 6, 2019
@dslomov dslomov self-assigned this Sep 6, 2019
@dslomov dslomov changed the title netrc support breaks downloads of github archives [Security] netrc support passes original credentials on HTTP redirects Sep 6, 2019
dslomov added a commit that referenced this issue Sep 6, 2019
Fixes #9327

Closes #9343.

PiperOrigin-RevId: 267568221
katre pushed a commit that referenced this issue Sep 6, 2019
Fixes #9327

Closes #9343.

PiperOrigin-RevId: 267568221
qtprojectorg pushed a commit to qtqa/gerrit that referenced this issue Sep 10, 2019
Given this severe security vulnerability in Bazel 0.29: [1] we prefer
to upgrade to 1.0.0rc2, even though it is a release candidate and not
officially released Bazel version.

This effectively forces users to either download this release
candidate manually from this loation: [2], or use bazelisk if they are
not already, since this RC version is not included in regular package
managers yet.

[1] bazelbuild/bazel#9327
[2] https://releases.bazel.build/1.0.0/rc2/index.html

Change-Id: I7d4121a145450f0d8687b239db3afa14f2dc0785
dslomov added a commit that referenced this issue Sep 16, 2019
Fixes #9327

Closes #9343.

PiperOrigin-RevId: 267568221
bazel-io pushed a commit that referenced this issue Sep 16, 2019
…ders

Fix the signature of HttpConnector.connect. As the connect method
handles redirects, we cannot have a fixed set of headers, once we
support authentication, as the authentication headers depend on
the host and that might change in a redirect.

This change fixes the header generation in the HttpDownloader,
but does not yet re-add the usage of the auth parmeter in the
Starlark-provided download functions.

Related #9327

Change-Id: Id12aa6a9a790fac6133a4da64baff58f02d36ef4
PiperOrigin-RevId: 269300472
aehlig added a commit to aehlig/bazel that referenced this issue Sep 16, 2019
Now that the root cause of bazelbuild#9327 is fixed in e38d838.

This reverts commit a0e3bb2.
bazel-io pushed a commit that referenced this issue Sep 20, 2019
Now that the root cause of #9327 is fixed in e38d838.

This reverts commit a0e3bb2.

Closes #9388.

PiperOrigin-RevId: 270263985
dslomov added a commit that referenced this issue Sep 30, 2019
Fixes #9327

Closes #9343.

PiperOrigin-RevId: 267568221
dslomov added a commit that referenced this issue Oct 2, 2019
Fixes #9327

Closes #9343.

PiperOrigin-RevId: 267568221
@orgads
Copy link

orgads commented Dec 24, 2019

This still happens to me with bazel 1.2.0.

@orgads
Copy link

orgads commented Dec 24, 2019

I had a cached version of http.bzl. I cleared the cache and it was solved.

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@orgads
Copy link

orgads commented Aug 19, 2020

I just tried to build on a new machine, and ran into this issue again with 3.4.1. Possibly related #10998.

@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 This is an emergency and more important than other current work. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants