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

Patch Openssh for Roundup #15 #21500

Merged
merged 4 commits into from Dec 29, 2016
Merged

Patch Openssh for Roundup #15 #21500

merged 4 commits into from Dec 29, 2016

Conversation

grahamc
Copy link
Member

@grahamc grahamc commented Dec 29, 2016

Motivation for this change

#21457 (comment)

cc @vcunat @aneeshusa @LnL7

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@grahamc grahamc added 1.severity: security 9.needs: port to stable A PR needs a backport to the stable release. labels Dec 29, 2016
@mention-bot
Copy link

@grahamc, thanks for your PR! By analyzing the history of the files in this pull request, we identified @vcunat, @edolstra and @peti to be potential reviewers.

Copy link
Member

@LnL7 LnL7 left a comment

Choose a reason for hiding this comment

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

looks good to me

@grahamc
Copy link
Member Author

grahamc commented Dec 29, 2016

I'm feeling a bit inclined to wait for @vcunat to take a look. What do y'all think?

@vcunat
Copy link
Member

vcunat commented Dec 29, 2016

No need for that, really. If you had a look and are convinced it will work...

@aneeshusa
Copy link
Contributor

Just woke up, but the reason I haven't updated openssh yet is I've been waiting for updated GSSAPI patches from upstream (Debian). I ran some quick builds and I couldn't get this to build with GSSAPI enabled, hence why I hadn't made a PR. Can you get this to build with GSSAPI?

If not, we'll need to weigh leaving open CVEs w/ an old version of openssh, or breaking GSSAPI support.

I haven't looked at the systemd stuff yet.

@vcunat
Copy link
Member

vcunat commented Dec 29, 2016

GSSAPI hpn version is marked as broken. It didn't build before update and doesn't build after.

@aneeshusa
Copy link
Contributor

I only remember hpnSupport being broken, and I remember testing GSSAPI as working when I did #17493; let me double-check...

@vcunat
Copy link
Member

vcunat commented Dec 29, 2016

I was wrong (edited above immediately). I haven't tested anything that has no top-level attribute :-)

@vcunat
Copy link
Member

vcunat commented Dec 29, 2016

Yeah, it builds on master and this PR breaks it.

@aneeshusa
Copy link
Contributor

I think Debian has updated the GSSAPI patch, looking at the git log for the master branch at https://anonscm.debian.org/cgit/pkg-ssh/openssh.git, if we want to give that a try.

@aneeshusa
Copy link
Contributor

aneeshusa commented Dec 29, 2016

@vcunat
Copy link
Member

vcunat commented Dec 29, 2016

Yes, applies and I'm testing the build already.

@vcunat
Copy link
Member

vcunat commented Dec 29, 2016

@grahamc: second-order PR submitted at grahamc#5 ;-)

@aneeshusa
Copy link
Contributor

Changes around daemonization and testing LGTM as well. Building + testing this locally now.

@grahamc
Copy link
Member Author

grahamc commented Dec 29, 2016

LMK how it goes @aneeshusa and I'll merge / backport straight away. :)

@grahamc
Copy link
Member Author

grahamc commented Dec 29, 2016

Also -- I really appreciate all the help on this one.

Copy link
Contributor

@aneeshusa aneeshusa left a comment

Choose a reason for hiding this comment

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

Built and tested, everything looks good. Thanks for keeping me in the loop!

@grahamc grahamc merged commit b500024 into NixOS:master Dec 29, 2016
@vcunat
Copy link
Member

vcunat commented Dec 29, 2016

Nice teamwork :-)

@samueldr samueldr removed the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants