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

opensaml 2.6.1 & shibboleth-sp 2.6.1 #20723

Closed
wants to merge 2 commits into from
Closed

opensaml 2.6.1 & shibboleth-sp 2.6.1 #20723

wants to merge 2 commits into from

Conversation

DomT4
Copy link
Member

@DomT4 DomT4 commented Nov 16, 2017

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

So, umm, I tried to build this one from source locally, and... I literally can't. Homebrew has an interesting failure point going on here.

==> Installing opensaml dependency: curl
/usr/bin/sandbox-exec -f /tmp/homebrew20171116-30544-1rh8v9b.sb nice /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/build.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/curl.rb --verbose --with-libssh2 --with-c-ares --with-gssapi --with-libmetalink --with-nghttp2 --with-openssl
==> Downloading https://curl.haxx.se/download/curl-7.56.1.tar.bz2
/usr/bin/curl --show-error --user-agent Homebrew/1.3.7-4-g2953493 (Macintosh; Intel Mac OS X 10.13.2) curl/7.54.0 --fail --location --remote-time --continue-at - --output /usr/local/var/homebrewcache/curl-7.56.1.tar.bz2.incomplete https://curl.haxx.se/download/curl-7.56.1.tar.bz2 --connect-timeout 5
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 2758k  100 2758k    0     0  1382k      0  0:00:01  0:00:01 --:--:-- 1382k

==> Summary
🍺  /usr/local/Cellar/curl/7.56.1: 414 files, 3.2MB, built in 2 minutes 7 seconds
==> Installing opensaml dependency: xml-tooling-c
==> Installing dependencies for xml-tooling-c: curl

~> brew install opensaml -s -v
==> Installing dependencies for opensaml: curl, xml-tooling-c
==> Installing opensaml dependency: curl
/usr/bin/sandbox-exec -f /tmp/homebrew20171116-43275-1sybd8f.sb nice /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/build.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/curl.rb --verbose --with-libssh2 --with-c-ares --with-gssapi --with-libmetalink --with-nghttp2 --with-openssl

Rinse and repeat.

@DomT4
Copy link
Member Author

DomT4 commented Nov 16, 2017

The opensaml release fixes CVE-2017-16853 & the shibboleth-sp release fixes CVE-2017-16852.

I feel like people are going to start dreading my PRs because I most frequently file them in response to CVE disclosures these days 😅.

@DomT4 DomT4 changed the title opensaml 2.6.1 opensaml 2.6.1 & shibboleth-sp 2.6.1 Nov 16, 2017
@ilovezfs
Copy link
Contributor

What's the failure?

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

It reinstalls curl endlessly lol.

@ilovezfs
Copy link
Contributor

Doesn't seem to happen on test-bot …

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

And note here:

==> Installing opensaml dependency: xml-tooling-c
==> Installing dependencies for xml-tooling-c: curl

I didn't abort. It just stopped.

Doesn't seem to happen on test-bot …

No. I already have curl --with-openssl (amongst other options) installed locally so I'm slightly puzzled as to why it's happening at all. The dependency should be considered satisfied.

@ilovezfs
Copy link
Contributor

Have you tried uninstalling curl first?

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Yeah, that works fine if you do it that way. But if I remove curl and then install it fresh with my previously used options (--with-libssh2 --with-c-ares --with-gssapi --with-libmetalink --with-nghttp2 --with-openssl) we're back in the failure loop.

@ilovezfs
Copy link
Contributor

What if you also pass --build-bottle to the curl install with the options?

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

It could be simply that curl technically has no --with-openssl option on >10.8 when building with nghttp2?

  # HTTP/2 support requires OpenSSL 1.0.2+ or LibreSSL 2.1.3+ for ALPN Support
  # which is currently not supported by Secure Transport (DarwinSSL).
  if MacOS.version < :mountain_lion || build.with?("nghttp2")
    depends_on "openssl"
  else
    option "with-openssl", "Build with OpenSSL instead of Secure Transport"
    depends_on "openssl" => :optional
  end

What if you also pass --build-bottle to the curl install with the options?

I'll check. One sec.

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

I'll check. One sec.

No change:

==> Summary
🍺  /usr/local/Cellar/curl/7.56.1: 420 files, 6.3MB, built in 1 minute 50 seconds
~> brew install -s -v opensaml
==> Installing dependencies for opensaml: curl
==> Installing opensaml dependency: curl
/usr/bin/sandbox-exec -f /tmp/homebrew20171117-84185-xq3icn.sb nice /System/Library/Frameworks/Ruby.framework/Versions/2.3/usr/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/build.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/curl.rb --verbose --with-libssh2 --with-c-ares --with-gssapi --with-libmetalink --with-nghttp2 --with-openssl
==> Downloading https://curl.haxx.se/download/curl-7.56.1.tar.bz2
Already downloaded: /usr/local/var/homebrewcache/curl-7.56.1.tar.bz2
==> Verifying curl-7.56.1.tar.bz2 checksum
tar xjf /usr/local/var/homebrewcache/curl-7.56.1.tar.bz2
==> ./configure --disable-debug --disable-dependency-tracking --disable-silent-rules --prefix=/usr/local/Cellar/curl/7.56.1 --with-ssl=/usr/local/opt/openssl --with-ca-bundle=/usr/local/etc/openssl/cert.pem --with-ca-path=/usr/local/etc/openssl/certs --with-libssh2 --with-libmetalink --with-gssapi --without-librtmp --enable-ares=/usr/local/opt/c-ares

@ilovezfs
Copy link
Contributor

Does it go away if you remove --with-nghttp2 ?

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Seems nghttp2 is the trigger.

~> brew install curl --with-libssh2 --with-c-ares --with-gssapi --with-libmetalink --with-openssl

followed by brew install opensaml does not reproduce the failure.

Does it go away if you remove --with-nghttp2 ?

Heh, didn't see this before my comment. We had the same thought, apparently.

@ilovezfs
Copy link
Contributor

Try

  option "with-openssl", "Build with OpenSSL instead of Secure Transport"
  depends_on "openssl" => :optional

  if MacOS.version < :mountain_lion || build.with?("nghttp2")
      depends_on "openssl"
  end

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

I basically did a lazier/less complete version of your suggestion locally, heh. It looks like you do have to explicitly pass --with-openssl to brew install curl --with-nghttp2 for my change at least:

diff --git a/Formula/curl.rb b/Formula/curl.rb
index dd78b45d27..c1639a9eaa 100644
--- a/Formula/curl.rb
+++ b/Formula/curl.rb
@@ -28,6 +28,7 @@ class Curl < Formula
   option "with-gssapi", "Build with GSSAPI/Kerberos authentication support."
   option "with-libmetalink", "Build with libmetalink support."
   option "with-nghttp2", "Build with HTTP/2 support (requires OpenSSL)"
+  option "with-openssl", "Build with OpenSSL instead of Secure Transport" if MacOS.version > :mountain_lion
 
   deprecated_option "with-rtmp" => "with-rtmpdump"
   deprecated_option "with-ssh" => "with-libssh2"
@@ -35,10 +36,9 @@ class Curl < Formula
 
   # HTTP/2 support requires OpenSSL 1.0.2+ or LibreSSL 2.1.3+ for ALPN Support
   # which is currently not supported by Secure Transport (DarwinSSL).
-  if MacOS.version < :mountain_lion || build.with?("nghttp2")
+  if MacOS.version < :mountain_lion || build.with?("nghttp2") || build.with?("openssl")
     depends_on "openssl"
   else
-    option "with-openssl", "Build with OpenSSL instead of Secure Transport"
     depends_on "openssl" => :optional
   end

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Yeah, unsurprisingly given they are effectively the same idea, your tweak also requires an explicit passing of ~> brew install curl --with-nghttp2 --with-openssl to resolve the issue. Simply passing ~> brew install curl --with-nghttp2 gets us back in the failure loop.

@ilovezfs
Copy link
Contributor

My guess is if you only pass --with-nghttp2 the depends_on "curl" => "with-openssl" will still trigger a recompile but no endless loop.

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Will check that explicitly now. Fun evening for my CPU 🙈.

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Your guess is correct.

@ilovezfs
Copy link
Contributor

I assume it's the same behavior for yours and mine?

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Aye.

@ilovezfs
Copy link
Contributor

I'm thinking no need to make any changes at this point since #13133 already covers this horror show.

@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Yeah, personally I'd be happy here to merge this & see how it goes on user reports. There's ~78k IOR analytic hits for "vanilla" curl & only ~8k IOR hits for curl --with-nghttp2. Neither opensaml or shibboleth-sp even make the list.

@ilovezfs
Copy link
Contributor

🙈 🙉 🤕 🔒

@ilovezfs ilovezfs closed this in e8131a6 Nov 17, 2017
@DomT4 DomT4 deleted the opensaml branch November 17, 2017 17:10
@DomT4
Copy link
Member Author

DomT4 commented Nov 17, 2017

Ta ❤️. What fun this was to troubleshoot, heh.

@ilovezfs
Copy link
Contributor

depends_on "foo" => "with-bar"

is the 😈

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants