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

api: ignore HTTPS errors if required certs aren't installed #15895

Merged
merged 1 commit into from Aug 23, 2023

Conversation

EricFromCanada
Copy link
Member

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

A side effect of API installs being enabled on older macOS versions is that the JSON files are downloaded by the initial brew update when it attempts to brew install ca-certificates, which itself requires updated certificates. Adding --insecure to the curl call just in this instance avoids "curl: (60) SSL certificate problem: certificate has expired" when downloading https://formulae.brew.sh/api/formula.jws.json for the first time.

@EricFromCanada EricFromCanada added the install from api Relates to API installs label Aug 22, 2023
@@ -57,6 +57,8 @@ def self.fetch_json_api_file(endpoint, target: HOMEBREW_CACHE_API/endpoint,
curl_args << "--progress-bar" unless Context.current.verbose?
curl_args << "--verbose" if Homebrew::EnvConfig.curl_verbose?
curl_args << "--silent" if !$stdout.tty? || Context.current.quiet?
curl_args << "--insecure" if ENV["HOMEBREW_SYSTEM_CA_CERTIFICATES_TOO_OLD"].present? &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think it may be worth a big loud opoo for this case so users know the configuration is less secure here (and anywhere else we're doing --insecure like this).

CC @Bo98 @woodruffw for thoughts.

Copy link
Member

@Bo98 Bo98 Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already do for the other place we do, so we could do here.

In any case though, the integrity of the download is still verified by checking the signature so isn't that much more insecure on that front.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think it may be worth a big loud opoo for this case so users know the configuration is less secure here (and anywhere else we're doing --insecure like this).

Yeah, I think this is a good idea.


Making sure I understand: this happens when brew is in a freshly installed state without its own ca-certificates installed yet, correct? In which case, the following flow happens:

  1. brew install ... is triggered
  2. It attempts to retrieve the latest JSON over the new API via HTTPS, which then fails because there's no root of trust yet
  3. So we pass --insecure instead and rely on the subsequent JWS signature verification to ensure integrity/authenticity here

Is that understanding correct? If so, that seems mostly fine to me, with a few caveats:

  1. By trusting dropping cert validation on the HTTPS connection, you're enabling a MITM attacker to serve a valid (in terms of signature) but stale JSON API response, meaning that they could force a user to install an older (and therefore possibly vulnerable) version of the package they're trying to brew install
  2. Along with an opoo, it would probably be good to exit this less secure state ASAP -- ideally by installing ca-certificates as soon as possible, possibly right after the JSON API response is downloaded and verified.

Entirely separately, there may be some alternatives available to you:

Alternative 1: Is CURL_SSL_BACKEND=secure-transport an option here, at least for macOS? That would allow curl to use the system trust store, which should be recent enough on most installs to allow a successful chain construction here. It's possible that this is the exact problem being avoided here, however, so please ignore if this isn't an option!

Alternative 2: Given that Homebrew is already distributing its own signing certificate (homebrew-1.pem) as an in-tree asset, it might be worth doing the same for the brew.sh root of trust. Specifically, you could embed the ISRG Root X1 root CA and then leverage the fact that brew.sh sends its intermediate (Let's Encrypt R3) as part of the TLS connection to construct a valid chain without needing either the full ca-certificates or --insecure. This would require Homebrew to rotate the root CA every decade or so (the current ISRG root expires in 2035), at least for as long as this bootstrapping process is necessary, but that's a relatively small operational burden (and could be alerted on).

Does either of these alternatives seem reasonable? If so, I think Alternative 2 is probably the "best" in terms of security here: it avoids any amount of unverified TLS connections, at the cost of one additional .pem embedded in the Homebrew source 🙂.

Copy link
Member

@Bo98 Bo98 Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify here: this logic only applies (currently) to macOS < 10.15.6. It maybe should also cover when the user has explicitly opted into HOMEBREW_FORCE_BREWED_CA_CERTIFICATES (mainly for older Linuxes where we can't really detect where it's necessary).

Along with an opoo, it would probably be good to exit this less secure state ASAP -- ideally by installing ca-certificates as soon as possible, possibly right after the JSON API response is downloaded and verified.

ca-certificates will always be installed first on systems where that is necessary, yes. In fact brew will do it before updating anything:

brew install ca-certificates
.

What we could also do here is mark the API JSON mtime as 1970 so that further brew operations will immediately update it. That largely will address your other point. ca-certificates itself still could have an old copy (though we don't force ca-certificates to be up-to-date for existing installs either). It will however be newer than the system trust store on those systems.


Alternative 1: Is CURL_SSL_BACKEND=secure-transport an option here, at least for macOS?

Not all older macOS have that, no. Plus some have really old system trust stores - some of the really old macOS only use SecureTransport anyway and still don't work.

Alternative 2

This could work if GitHub Pages is guaranteed to always use that particular chain trust. We still need to install ca-certificates itself too at some point which, at the moment, comes from curl.se and will also need --insecure. But this is checksummed in the formula file and the formula file itself is checksummed by the JSON (assuming we are not using HOMEBREW_NO_INSTALL_FROM_API).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What we could also do here is mark the API JSON mtime as 1970 so that further brew operations will immediately update it. That largely will address your other point.

Yeah, I think that's a good compromise! Documenting that explicitly would be good, but otherwise IMO that solves the problem nicely here -- with that, the worst an attacker could do is temporarily serve an old ca-certificates, if I'm understanding correctly.


This could work if GitHub Pages is guaranteed to always use that particular chain trust.

Yeah, that's a potentially risky assumption. I think your suggestion above will probably be more reliable.

@EricFromCanada
Copy link
Member Author

I believe I've implemented the requested changes. In my testing, a new install works on a fresh High Sierra system. With debug turned on for the installer, I can see that formula.jws.json is downloaded & the warning is shown, then during the ca-certificates installation it's downloaded again just before post-install, before finally being securely downloaded by update.sh.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks @EricFromCanada @Bo98 @woodruffw!

@MikeMcQuaid MikeMcQuaid merged commit bc5fce2 into Homebrew:master Aug 23, 2023
24 checks passed
@EricFromCanada EricFromCanada deleted the api-old-certs branch August 23, 2023 13:35
@github-actions github-actions bot added the outdated PR was locked due to age label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
install from api Relates to API installs outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants