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

Manage sshd_config file #725

Closed
wants to merge 3 commits into from
Closed

Conversation

aneeshusa
Copy link
Contributor

@aneeshusa aneeshusa commented Sep 30, 2017

This is based on Mozilla's Modern sshd_config from:
https://wiki.mozilla.org/Security/Guidelines/OpenSSH

Note that root login is still allowed,
because we have not yet set up per-user accounts.

Add a test to ensure the sshd_config file is properly parsed and
validated by the OpenSSH version on the machine to help guard
against this behavior.

Additionally, our new 10.11.6 macOS machines have Xcode 8.2 installed.
Use xcode8 on Travis, which maps to Xcode 8gm and macOS 10.11.
This is also important to help ensure tests like the sshd config test
match the behavior we'll see in the field.


This change is Reviewable

@aneeshusa
Copy link
Contributor Author

This is based on part of #628; @edunham, could you please take a look?

Do not merge this yet - we need to figure out how to bring the versions on servo-mac1 inline with the other Macs, mostly due to the mismatch in the sshd_config location.
IMO, it would be nice to update all the Macs to OS X 10.11 and Xcode 8:

  • We can use a modern sshd_config on all machines
  • servo-mac1 has been running this and builds are green
  • This matches one of the possible Travis configurations for better test/prod parity

@aneeshusa
Copy link
Contributor Author

Also cc @larsbergstrom and @metajack about getting our versions to all match. I don't know how this works with MacStadium:

  • Can we pick what version of macOS to have when provisioning a new machine, or is it standard?
  • Are we supposed to handle macOS version upgrades, or is that a MacStadium support ticket item?

Another option is using the OpenSSH in Homebrew, but I'm a little wary of messing with the default ssh.plist file and getting locked out.

@larsbergstrom
Copy link
Contributor

  1. We can pick the version of macOS at provisioning time and they will image the machine with it.
  2. We can manually upgrade the macOS version upgrades but have to be ready for potential recovery steps - in my experience it will occasionally shut off both ssh and VNC and then you need to open a support ticket.

I'd support a move to 10.11.

@metajack
Copy link
Contributor

metajack commented Oct 8, 2017

macOS is at 10.13 now. I think we should move to 10.12 if we are going to upgrade. 10.13 is really new, and some things may be getting ironed out still, but 10.12 should be plenty stable.

@aneeshusa
Copy link
Contributor Author

I'm +1 on standardizing on 10.12 (thanks for the suggestion @metajack), but I'd like to move just one builder to 10.12 first and let it run for a bit to make sure it won't cause any build breakage. It's probably easiest to upgrade servo-mac1 to 10.12, but we could also add a new servo-mac10 for a bit more safety.

It would also be nice to standardize the XCode version; I think one of the 8.x versions is probably good for now.

@aneeshusa
Copy link
Contributor Author

With the resolution of #731, 5/9 Mac builders now have macOS 10.11.6, so I think for now it'll be easiest to get all the Macs on 10.11.6 and think about upgrading to 10.12 later. Macs that still need to be upgraded:

  • servo-mac2
  • servo-mac3
  • servo-mac4
  • servo-mac7

I also looked at the sw_vers output and it looks like most of the 10.11.6 Macs are on Build Version 15G1611. I'm not sure if this matters, but extra consistency here can't hurt.

@larsbergstrom or @edunham Can you put in tickets with MacStadium to upgrade the remaining Macs as well?

@aneeshusa
Copy link
Contributor Author

Updated and simplified to assume all macOS machines are on macOS 10.11.

@larsbergstrom
Copy link
Contributor

I think that @edunham has gotten all the machines to 10.11 now,so this should theoretically be good to review and land :-)

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #807) made this pull request unmergeable. Please resolve the merge conflicts.

This is based on Mozilla's Modern sshd_config from:
https://wiki.mozilla.org/Security/Guidelines/OpenSSH

Note that root login is still allowed,
because we have not yet set up per-user accounts.

Add a test to ensure the sshd_config file is properly parsed and
validated by the OpenSSH version on the machine to help guard
against this behavior.
Our new 10.11.6 macOS machines have Xcode 8.2 installed.
Use xcode8 on Travis, which maps to Xcode 8gm and macOS 10.11.

This is also important to help ensure tests like the sshd config test
match the behavior we'll see in the field.
@aneeshusa
Copy link
Contributor Author

@jdm I'm still interested in this PR and have rebased it, but we ought to do #811 before merging this, hence the lack of updates so far.

I also chose the Xcode version specifically to be xcode8 instead of xcode8.3 per the second commit message, which looks like got lost in between #807/#808. Was there a reason for that? I'd like to match our Travis builders to the actual machines in prod as closely as possible.

We still have production machines on macOS 10.10.5 (and Xcode 7.2);
this is the closest fit that Travis offers,
which macOS 10.10, albeit an older Xcode version of 6.4.
@aneeshusa
Copy link
Contributor Author

I just pushed a commit to add more Travis builders for 10.10 (so we test both 10.10 and 10.11 on Travis); if that looks OK we may be able to merge this without waiting for #811.

@jdm
Copy link
Member

jdm commented Mar 6, 2018

We can revert to xcode8 if it works. I updated to xcode8.3 to make #807 work, but I didn't test anything in between.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #969) made this pull request unmergeable. Please resolve the merge conflicts.

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

6 participants