Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

openssl: add x509 cert chain handling patches #38897

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 23 additions & 17 deletions Library/Formula/openssl.rb
Expand Up @@ -21,12 +21,20 @@ class Openssl < Formula
keg_only :provided_by_osx,
"Apple has deprecated use of OpenSSL in favor of its own TLS and crypto libraries"

# This is a workaround for Apple removing the Equifax Secure CA root from the System in 10.10.3
# Their doing so has broken certificate verification and consquently secure connection for dependants.
# Scope this to Yosemite and remove immediately once Apple have fixed the issue.
resource "Equifax_CA" do
url "https://www.geotrust.com/resources/root_certificates/certificates/Equifax_Secure_Certificate_Authority.pem"
sha256 "f24e19fb93983b4fd0a377335613305f330c699892c789356eb216449804d0e9"
# Remove both patches with the 1.0.2b release.
# They fix:
# https://github.com/Homebrew/homebrew/pull/38495
# https://github.com/Homebrew/homebrew/issues/38491
# Upstream discussions:
# https://www.mail-archive.com/openssl-dev@openssl.org/msg38674.html
patch do
url "https://github.com/openssl/openssl/commit/6281abc796234.diff"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit pick: If you use .patch, github will return a git am'able format that applies cleanly with patch (as used by brew) and is self-explanatory. Not sure what brew's policy on this is.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't use .patch because the git version is appended and it can break if upgraded or something like that. @MikeMcQuaid explains it better/clearer/more accurately/more eloquently than I do.

Copy link
Member

Choose a reason for hiding this comment

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

What @DomT4 said.

Copy link
Member Author

Choose a reason for hiding this comment

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

😸

sha256 "f8b94201ac2cd7dcdee3b07fb3cd77a2de6b81ea67da9ae075cf06fb0ba73cea"
Copy link
Contributor

Choose a reason for hiding this comment

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

In addition, this one has become relevant and should be added: https://github.com/openssl/openssl/commit/e5991ec528b1c339062440811e2641f5ea2b328b.patch

Copy link
Member Author

Choose a reason for hiding this comment

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

Upstream explicitly told me not to use this one. Something change?

You don't need this one. I'm about to revert it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I missed that part. Nevermind!

end

patch do
url "https://github.com/openssl/openssl/commit/dfd3322d72a2.diff"
sha256 "0602eef6e38368c7b34994deb9b49be1a54037de5e8b814748d55882bfba4eac"
end

def arch_args
Expand All @@ -37,13 +45,13 @@ def arch_args
end

def configure_args; %W[
--prefix=#{prefix}
--openssldir=#{openssldir}
no-ssl2
zlib-dynamic
shared
enable-cms
]
--prefix=#{prefix}
--openssldir=#{openssldir}
no-ssl2
zlib-dynamic
shared
enable-cms
]
end

def install
Expand Down Expand Up @@ -120,10 +128,8 @@ def post_install
openssldir.mkpath
(openssldir/"cert.pem").atomic_write `security find-certificate -a -p #{keychains.join(" ")}`

if MacOS.version == :yosemite
(openssldir/"certs").install resource("Equifax_CA")
system bin/"c_rehash"
end
# Remove this once 1.0.2b lands.
rm_f openssldir/"certs/Equifax_CA" if MacOS.version == :yosemite
Copy link
Contributor

Choose a reason for hiding this comment

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

c_rehash generates a copy of the cert, doesn't it? Do we need to remove that as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

It generates a symlink rather than a copy. If you kill the actual cert, the symlink becomes ineffective. If the symlink is named 578d5c04.0 on your machine as well, I'll add removing it to the formula.

Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

The file was named 578d5c04.0 on my system as well, although I was using the mozilla ca bundle from the curl website, so I think it's not safe to simply remove the symlink. A better solution could be to run system bin/"c_rehash" after the file is removed, because it will clean up orphaned symlinks.

end

def caveats; <<-EOS.undent
Expand Down