Homebrew / homebrew-core

🍻 Default formulae for the missing package manager for macOS (or Linux)
https://brew.sh
BSD 2-Clause "Simplified" License
13.72k stars 12.41k forks source link

[OpenSSH] OpenSSH v7.5p1, Homebrew `revision` 1 Fails to Build on OS X 'El Capitan' v10.11.6 #13132

Closed RandomDSdevel closed 7 years ago

RandomDSdevel commented 7 years ago

Please always follow these steps:


I suspect that the cause for this breakage was either the formula revision or a change in Homebrew itself between at least v1.1.11 (hopefully only v1.1.13, though, which is what I had installed prior to running brew update today) and v1.2.0.

DomT4 commented 7 years ago

Oh, it's because ldns is built against openssl@1.1. Homebrew tries fairly forcefully to ensure there's only one crypto library linked throughout the tree to avoid, and forgive the technical term here, "weirdness".

ilovezfs commented 7 years ago

Yeah @JCount and I were talking about this earlier and figure we should probably just drop the option.

ilovezfs commented 7 years ago

@DomT4 do you know the status of openssh with respect to building with openssl 1.1?

DomT4 commented 7 years ago

At last check it had issues. Debian has a super-handy list of things they are having active trouble moving over to the 1.1.x branch of OpenSSL here.

Debian's FAQ also has this interesting nugget on mixing OpenSSL versions in the same tree:

According to a post to debian-devel, it's ok to link to libraries which in turn use different versions of OpenSSL, as long as they don't exchange OpenSSL data structures: "The linking is fine, I believe even any eventual globals (if any) will be correctly handled in Debian nowadays. What causes extremely nasty issues is layering violations causing openssl data structures to leak from something linked to one version of the library, to something else linked to another version of the library. If, at any point, internal data structures from openssl are exposed, or OpenSSL contextes are passed between two libraries, or a library and an application, they must all be from the same openssl. This is not something the linker or dlopen/dlsym can enforce. And you need to manually inspect the code to be sure... usually. So, if Qt ever exposes its use of openssl anywere in its APIs, it might not be safe. If it doesn't (i.e. at most you have a qt flag that says "use SSL", etc), then it should be fine."

There's an upstream PR on the portable branch basically sat pending here.

DomT4 commented 7 years ago

There's also some discussion here.

The skinny of the situation seems to be summed up nicely here in that there's not really a ton for OpenSSH to gain by patching in newer OpenSSL support because it still builds fine with the 1.0.2 LTS branch & also LibreSSL, which is the default on OpenBSD.

ilovezfs commented 7 years ago

So when does OpenSSL announce that they were just kidding with 1.1 after all?

DomT4 commented 7 years ago

You think the current situation is fun. Wait till the @1.1 release with TLS 1.3 support (whether draft or final, who knows at this point) is available and projects start nudging users to build against an OpenSSL with TLS 1.3 available.

ilovezfs commented 7 years ago

That might improve the situation actually.

DomT4 commented 7 years ago

I suspect at some point in the not too distant future Homebrew is going to end up going big or going home on which OpenSSL it wants to mainstream 😓.

This softly softly dance I started a while back hasn't gone anywhere quickly; it's getting to the point where if some formulae options have to die so things can move over to openssl@1.1 maybe that's not awful.

ilovezfs commented 7 years ago

I'm concerned about the prospect of mass patching with the Debian patches if upstreams haven't themselves switched over. What could possibly go wrong?

DomT4 commented 7 years ago

To be fair to Debian they are pretty 💯 on submitting patches upstream where those upstreams are actually alive, so in a lot of cases it's "simply" waiting on new releases. I think in a fair few cases they punted stuff over to gnutls instead as well, rather than trying to maintain two major versions of OpenSSL for the next 2+ years.

RandomDSdevel commented 7 years ago

@DomT4 w. r. t. the conversation you and @ilovezfs have had in this thread starting with this post of yours:

Oh, it's because ldns is built against openssl@1.1. Homebrew tries fairly forcefully to ensure there's only one crypto library linked throughout the tree to avoid, and forgive the technical term here, "weirdness".

:man_facepalming: Of course! I should have seen that coming, shouldn't I? Guess I'll just have to wait and see what you guys decide on doing when it comes to OpenSSL and OpenSSH's --with-ldns option, then…or should I rebuild OpenSSH without that option in the meantime if you guys are just going to get rid of that option in the end? I suppose I'll just keep watching you guys hash out the direction you want to go in resolving Homebrew's OpenSSL versioning woes.

DomT4 commented 7 years ago

@RandomDSdevel If you're happy to live without ldns for now I'd recommend just rebuilding without that locally. I suspect at least in the short-term the ldns option will be killed here, with a view to it potentially returning when there's more harmonisation in terms of which OpenSSL is the mainstream dependency.

DomT4 commented 7 years ago

IMO openssh should really be building with libressl given that is both the upstream default & the default on macOS these days, but that wouldn't solve your issue here unless ldns was bundled into the formula & I don't think there's any desire to bust the crypto choice discussion open again from literally anyone 😄.

ilovezfs commented 7 years ago

I don't think there's any desire to bust the crypto choice discussion open again from literally anyone

Oh, please do! :popcorn:

DomT4 commented 7 years ago

🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐 🤐

RandomDSdevel commented 7 years ago

@DomT4:

@RandomDSdevel If you're happy to live without ldns for now I'd recommend just rebuilding without that locally. I suspect at least in the short-term the ldns option will be killed here, with a view to it potentially returning when there's more harmonisation in terms of which OpenSSL is the mainstream dependency.

TBH, I wasn't entirely sure what the benefits of building with it were in the first place (other than gaining more functionality since I like doing that despite not knowing what said extra functionality is. Maybe I should go look that up…)

RandomDSdevel commented 7 years ago

@DomT4: Just thought I'd comment that I just ran into a problem I've seen before when trying to brew reinstall a formula — i. e.: that Homebrew forgets to clear its cache of the options one has used previously when building a formula's package (probably a bug, but I've other things to attend to at the moment along with the resolution of this issue.) brew uninstall followed by brew install does the trick, though, so I'm good down here on my end.

ilovezfs commented 7 years ago

That is an intentional feature not a bug. The same applies to brew upgrade.

RandomDSdevel commented 7 years ago

@ilovezfs:

That is an intentional feature not a bug. The same applies to brew upgrade.

     Be that as it may, it still feels like a counterintuitive misfeature to me. It makes sense for brew upgrade to honor options you've previously brew installed a formula's associated software package with since such caching is a necessarily expected convenience in that case, but the fact that brew reinstall does it as well often throws me for a bit of a loop, especially when I'm trying to reinstall something without an option I've previously used (in this case, OpenSSH without ldns support.) What would make more sense to me is if brew reinstall behaved exactly like a fresh brew install of a currently uninstalled formula package with respect to option queueing and parsing unless one passed some kind of option saying 'use the set of options I last installed this formula package with' to the former command. That being said, this concern of mine is really only tangential to this issue, so I suppose I should probably let us get back to the subject at hand (unless, of course, you want to discuss this somewhere else, like in another, new issue — except in Homebrew/brew, as would be appropriate —, immediately,) eh?

vszakats commented 7 years ago

/off I'd be real cool to have a 'forget custom options' option for brew reinstall.

RandomDSdevel commented 7 years ago

@vszakats: Or if you had to pass an option to it to use your old options in the first place, but, yeah, that's off-topic. I'm not really annoyed enough by it at the moment to open a separate issue asking for this, though.

vszakats commented 7 years ago

@RandomDSdevel: The default is something up to debate, initially I also expected it to reset options, though I guess once you know how it works, it's fine either way. As for dropping options, I only needed it once (for wine, but the manual uninstall and reinstall was painful and error-prone due to the long list of dependencies.)

RandomDSdevel commented 7 years ago

@vszakats: True, but one would expect defaults to obey the principle of least surprise. I've actually had to deal with this more than once in the past, albeit only a handful of times, so maybe I'll open a new issue or homebrew-discuss mailing list thread contesting this default the next time it bites me. It hasn't been a problem for me lately, though, so I won't bother with that for now. At the very least, the default should be configurably overridable! Manual brew uninstall and brew install shouldn't be that much of a pain with respect to dependencies, though, unless you uninstall all of those in the process, too. Dependents, on the other hand…; those might get tricky. Either way, though, there's always the --ignore-dependencies option to brew uninstall (don't think you have to use that when dealing with packages downstream from the one you're messing with on the dependency tree including it, but I could be wrong.)

vszakats commented 7 years ago

I meant dependents :)

RandomDSdevel commented 7 years ago

@vszakats: Ah; no worries, then.

MikeMcQuaid commented 7 years ago

True, but one would expect defaults to obey the principle of least surprise.

I appreciate it wasn't intended that way but linking to that is a bit patronising. The feature does what it does not despite the principle of least surprise but because several people believed the principle of least surprise isn't present when your options are ignored. Note, the output of brew reinstall also always tells you what options it builds with.

At the very least, the default should be configurably overridable!

Down this way lies madness. We cannot conceivably add on/off to change defaults for every feature in Homebrew people some don't like. Already we do too much and allow too much customisation to be able to provide a consistently high-quality, reproducible experience.

RandomDSdevel commented 7 years ago

@MikeMcQuaid:

I appreciate it wasn't intended that way…

Thank you, but see below as well.

…but linking to that is a bit patronizing. …

You have my apologies, then. The statement including that link wasn't originally addressed directly to you, as I only intended to deliver the sentiment that catching anybody by surprise with unexpected behavior usually isn't a good experience, especially for the end user.

…The feature does what it does not despite the principle of least surprise but because several people believed the principle of least surprise isn't present when your options are ignored. …

That makes some sense, but, as I told @ilovezfs here above, the way brew reinstall digs into internal Homebrew package-management state and picks up the options you've previously used to brew install something smacks me like it's a computer-scientific analogue of Einstein's infamous 'spooky action at a distance' because it's inconsistent with how previously used options are dropped and ignored when you brew uninstall a formula's associated package and then brew install it again. In my opinion, brew reinstall should behave consistently with respect to that, not how brew upgrade works. That being said, this has all been rather longer of a tangent than I originally managed to take on this thread, so perhaps we should stop derailing things here…

…Note, the output of brew reinstall also always tells you what options it builds with.

True, but that information can flash by quite quickly at times (I probably exacerbate this by compensating for Homebrew's lack of build and installation progress feedback by enabling --verbose and --debug output to see how far down the filesystem's quasi-alphabetical sort order things have gotten modulo threading and build dependency order, but I already know I'm one of the odd ones out on this front.)

Down this way lies madness. We cannot conceivably add on/off to change defaults for every feature in Homebrew [some people] don't like. …

Indeed, but that presumes major scope creep far beyond what has already occurred here!

…Already we do too much and allow too much customization to be able to provide a consistently high-quality, reproducible experience.

I've actually been quite happy with the level of customization Homebrew provides, but I might not be using all of it, to be honest. There's probably some configuration knob I'm overlooking buried somewhere so deep in the codebase that supporting it is a maintenance nightmare due to a case of dependency hell even though I can't currently come up with what you think/know it is off the top of my head, so I can sympathize with you there. Discussing that would open a whole other can of worms, though, and I think we can both agree that conversation here has gone on long enough as it is!

MikeMcQuaid commented 7 years ago

You have my apologies, then. The statement including that link wasn't originally addressed directly to you, as I only intended to deliver the sentiment that catching anybody by surprise with unexpected behavior usually isn't a good experience, especially for the end user.

Thanks, I appreciate the apology.

In my opinion, brew reinstall should behave consistently with respect to that, not how brew upgrade works.

Sure. You can see how this is somewhat arbitrary though (as is it behaving as it does now, to be clear). As a result, it would be more confusing to current users to change the current behaviour again.

I probably exacerbate this

Yes 😉

so I can sympathize with you there.

Thanks!

so perhaps we should stop derailing things here…

Yup! Will stop now 👍

RandomDSdevel commented 7 years ago

Last reply here, @MikeMcQuaid, then I'll shut up:

Thanks, I appreciate the apology.

You're very welcome.

Sure. You can see how this is somewhat arbitrary though (as is it behaving as it does now, to be clear). As a result, it would be more confusing to current users to change the current behaviour again.

Yeah, I agree that having the defaults for some random instance of behavior in a piece of software to flip between several different values between its releases would start to annoy users quite a bit! Cross-release compatibility could, at least for that functionality, fly out the window depending on how far the results of changing a default setting were to propagate through the codebase.

Thanks!

Again, you're welcome.

Yup! Will stop now 👍

My lips are sealed. 😜

SunSparc commented 6 years ago

I am wondering if it has been "temporary" for long enough to put the ldns option back in?

I tried to force openssh to install with ldns by specifying that it use openssl@1.1 but it laughed at me: checking OpenSSL library version... configure: error: OpenSSL >= 1.1.0 is not yet supported (have "1010007f (OpenSSL 1.1.0g 2 Nov 2017)")

Is there an unsupported branch or some way to allow the use of unsupported versions?

I guess my next option would be to ditch any homebrew versions and build openssh from source...

Or there is this version that is built against getdns instead of ldns : https://github.com/phicoh/openssh-getdns/tree/github-getdns-7.5

RandomDSdevel commented 6 years ago

@SunSparc: Nope, OpenSSH is still listed on the page that @DomT4 referenced earlier. I'd check to see what progress, if any, upstream might currently have made on this as of this writing by trawling its bug/issue tracker for clues, but OpenSSH's host servers seem to be down right now.

DomT4 commented 6 years ago

https://github.com/openssh/openssh-portable/pull/48 is still pending upstream. If you really wanted to, I guess you could try & apply that as a patch in a custom openssh formula & then inreplace out the configure check... but at that point I think you start heading towards territory where you're sticking your hands in a fire.

RandomDSdevel commented 6 years ago

@SunSparc: Risk being @DomT4's guinea pig if you feel up to it; I think I'll wait for official upstream support.


@DomT4: Thanks for the reference. I would have gotten around to look OpenSSH-portable issues after I fished through OpenBSD OpenSSH's issues, but I never got through that due to the latter's hosting web site's server issues and was paying more attention to some issues I'm having local to Homebrew Core, so I forgot

DomT4 commented 6 years ago

Thanks for the reference.

No worries! I just remembered that thread & wondered whether it ever got merged.

I think I'll wait for official upstream support.

This would absolutely be my recommendation.

SunSparc commented 6 years ago

Would it help if everyone went over to the Make it build using OpenSSL 1.1.0 pull request and upvoted? :)

RandomDSdevel commented 6 years ago

@SunSparc: Nah, actual work on getting that merged would probably serve us all here better.

RandomDSdevel commented 6 years ago

@DomT4, @ilovezfs, and/or @MikeMcQuaid: The merger of #23284 into master likely means we can revert #13488 now, doesn't it?

ilovezfs commented 6 years ago

Yup

RandomDSdevel commented 6 years ago

@ilovezfs: I'm now creating a PR staging branch that implements the suggestion I made above and which you approved and will formally create a PR out of it soon.

RandomDSdevel commented 6 years ago

@DomT4, @ilovezfs, and/or @MikeMcQuaid:

     In the process of preparing my PR staging branch's contents for submission, I was running brew test -vd openssh, and it crashed with permissions errors, saying:

/usr/local/Homebrew/Library/Homebrew/brew.rb (Formulary::FormulaLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/openssh.rb
Testing openssh
/usr/bin/sandbox-exec -f /tmp/homebrew20180202-8911-1aeaues.sb /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/test.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/openssh.rb -vd
/usr/local/Homebrew/Library/Homebrew/test.rb (Formulary::FromPathLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/openssh.rb
==> /usr/local/Cellar/openssh/7.6p1/bin/ssh -V 2>&1
/usr/local/etc/ssh/sshd_config line 110: Deprecated option UsePrivilegeSeparation
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0644 for '/usr/local/etc/ssh/ssh_host_rsa_key' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
key_load_private: bad permissions
Could not load host key: /usr/local/etc/ssh/ssh_host_rsa_key
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0644 for '/usr/local/etc/ssh/ssh_host_dsa_key' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
key_load_private: bad permissions
Could not load host key: /usr/local/etc/ssh/ssh_host_dsa_key
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0644 for '/usr/local/etc/ssh/ssh_host_ecdsa_key' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
key_load_private: bad permissions
Could not load host key: /usr/local/etc/ssh/ssh_host_ecdsa_key
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@         WARNING: UNPROTECTED PRIVATE KEY FILE!          @
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
Permissions 0644 for '/usr/local/etc/ssh/ssh_host_ed25519_key' are too open.
It is required that your private key files are NOT accessible by others.
This private key will be ignored.
key_load_private: bad permissions
Could not load host key: /usr/local/etc/ssh/ssh_host_ed25519_key
sshd: no hostkeys available -- exiting.
==> lsof -i :8022
/usr/local/Homebrew/Library/Homebrew/debrew.rb:11:in `raise'
Test::Unit::AssertionFailedError: <0> expected but was
<1>.
1. raise
2. ignore
3. backtrace
4. irb
5. shell
Choose an action: 1
==> Kept temporary files
Temporary files retained at /tmp/openssh-test-20180202-8912-16wd6i
Error: openssh: failed
undefined class/module Debrew::
/usr/local/Homebrew/Library/Homebrew/utils/fork.rb:42:in `load'
/usr/local/Homebrew/Library/Homebrew/utils/fork.rb:42:in `block (3 levels) in safe_fork'
/usr/local/Homebrew/Library/Homebrew/utils.rb:396:in `ignore_interrupts'
/usr/local/Homebrew/Library/Homebrew/utils/fork.rb:26:in `block (2 levels) in safe_fork'
/usr/local/Homebrew/Library/Homebrew/utils/fork.rb:7:in `open'
/usr/local/Homebrew/Library/Homebrew/utils/fork.rb:7:in `block in safe_fork'
/usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3/lib/ruby/2.3.0/tmpdir.rb:89:in `mktmpdir'
/usr/local/Homebrew/Library/Homebrew/utils/fork.rb:6:in `safe_fork'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/test.rb:68:in `block in test'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/test.rb:29:in `each'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/test.rb:29:in `test'
/usr/local/Homebrew/Library/Homebrew/brew.rb:100:in `<main>'

What should those permissions actually be changed to for this to work? (I suspect the relevant permissions problems are from adjustments required to run Homebrew before the core/formula split.)

DomT4 commented 6 years ago

What should those permissions actually be changed to for this to work?

0600, IIRC.

RandomDSdevel commented 6 years ago

Thanks, @DomT4; that did the trick. PR incoming…