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 minimum curl version isn't installed #16078

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

EricFromCanada
Copy link
Member

@EricFromCanada EricFromCanada commented Oct 4, 2023

  • 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?

Similar to the situation addressed in #15895, a system with a built-in curl that's too old to handle some certificates (e.g. OS X 10.11) won't be able to fetch the dependencies for compiling a newer curl, which is run by update.sh as part of the initial install process. That's been partially resolved by #16079 which fixes mirror support, and this PR finishes the job by:

  • in development_tools.rb, adding two functions to wrap the task of checking if the system CA certificates or curl need replacing; each just checks if they're a) too old and b) haven't been replaced yet. Also added is a function for generating the insecure download warning, called by api.rb and download_strategy.rb.
  • in api.rb, setting the flag to use --insecure when fetching the API files if either the system CA certificates or curl need replacing. Both need to be checked because brew.sh is written to specify that either of them is too old, but not both.
  • in resource.rb, modifying the line that allows the ca-certificates package to be downloaded with --insecure if either the system CA certificates or curl need replacing.

On a fresh OS X 10.11 system this results in a working installation.

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.

Thanks @EricFromCanada! Some mild refactoring suggestions here.

Library/Homebrew/api.rb Outdated Show resolved Hide resolved
Library/Homebrew/api.rb Outdated Show resolved Hide resolved
Library/Homebrew/resource.rb Outdated Show resolved Hide resolved
@Bo98
Copy link
Member

Bo98 commented Oct 4, 2023

This shoudn't be necessary as we provide HTTP mirrors, and have audits demanding that. That'll explain how this used to work before.

However our mirror support was completely broken a few months back. I'll fix that.

@EricFromCanada EricFromCanada changed the title Use --insecure to download curl source & dependencies where necessary api: ignore HTTPS errors if minimum curl version isn't installed Oct 4, 2023
@EricFromCanada EricFromCanada added the install from api Relates to API installs label Oct 4, 2023
@EricFromCanada
Copy link
Member Author

EricFromCanada commented Oct 4, 2023

That does help, but the api.rb changes are still needed because it needs to pass --insecure for downloading ca-certificates also if HOMEBREW_SYSTEM_CURL_TOO_OLD is set. I've updated the PR description accordingly.

Copy link
Member

@Bo98 Bo98 left a comment

Choose a reason for hiding this comment

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

Ok, make sense. Thanks!

@MikeMcQuaid
Copy link
Member

Thanks @EricFromCanada!

@MikeMcQuaid MikeMcQuaid merged commit 8c91152 into Homebrew:master Oct 5, 2023
27 checks passed
@EricFromCanada EricFromCanada deleted the curl-insecure branch October 7, 2023 23:55
@github-actions github-actions bot added the outdated PR was locked due to age label Nov 7, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 7, 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.

3 participants