1261212 - Upgrade hgssh to CentOS 7

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect)

Description

[Gregory Szorc [:gps]]

We need to move to the future.

Comment 1

[Gregory Szorc [:gps]]

Attachment: MozReview Request: docker: add base ansible image for CentOS 7 (bug 1261212); r=fubar

In preparation for switching the HG machines from RHEL6 to CentOS 7 in
production.

The base image is from commit 7fccb94631c03fb30109e960c28d23cbdb4a45a3
from the github.com:CentOS/sig-cloud-instance-images.git Git repo.

For now, the Ansible builder is a copy of the CentOS 6 one. Although,
we'll likely need to make changes to support systemd.

Review commit: https://reviewboard.mozilla.org/r/43659/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43659/

Comment 2

[Gregory Szorc [:gps]]

Attachment: MozReview Request: docker: add Docker builder to produce Python 2.7 tarball

Review commit: https://reviewboard.mozilla.org/r/43661/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43661/

Comment 3

[Gregory Szorc [:gps]]

Attachment: MozReview Request: docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r=fubar

Because we're about to add a container to build RPMs in CentOS 7.

Review commit: https://reviewboard.mozilla.org/r/43663/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43663/

Comment 4

[Gregory Szorc [:gps]]

Attachment: MozReview Request: docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r=fubar

This is mostly a copy of the existing CentOS 6 builder.

Review commit: https://reviewboard.mozilla.org/r/43665/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43665/

Comment 5

[Gregory Szorc [:gps]]

Attachment: MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

THIS IS STILL A BIT HACKY. THE TEST ENVIRIONMENT IS STILL COMPLETELY
BROKEN. BE GENTLE DURING REVIEW.

As part of the switch, the sshd config has changed. We no longer user
the custom openssh package with the LPK patches. Instead, we use stock
openssh, which has an AuthorizedKeysCommand which can look up SSH keys
in LDAP. The openssh-ldap package provided by CentOS provides a helper
executable to perform the LDAP lookups, which we use.

Review commit: https://reviewboard.mozilla.org/r/43667/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43667/

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43667/diff/1-2/

Comment 7

[Gregory Szorc [:gps]]

Attachment: MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang

Modern versions of OpenSSH support calling a program that will print SSH
public keys for a user. This script added by this patch is intended to be
used for this purpose.

We go beyond simple key lookup by filtering out DSA keys because they
are insecure and shouldn't be allowed. We stop short of filtering out
RSA keys with a short modulus. However, I don't want to introduce a
BER decoder just yet (we would likely import paramiko's).

Review commit: https://reviewboard.mozilla.org/r/43697/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43697/

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8737025 [details]
MozReview Request: docker: add base ansible image for CentOS 7 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43659/diff/1-2/

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8737026 [details]
MozReview Request: docker: add Docker builder to produce Python 2.7 tarball

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43661/diff/1-2/

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8737027 [details]
MozReview Request: docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43663/diff/1-2/

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8737028 [details]
MozReview Request: docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43665/diff/1-2/

Comment 12

[Gregory Szorc [:gps]]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43667/diff/2-3/

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8737064 [details]
MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43697/diff/1-2/

Comment 14

[Gregory Szorc [:gps]]

Comment on attachment 8737025 [details]
MozReview Request: docker: add base ansible image for CentOS 7 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43659/diff/2-3/

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8737026 [details]
MozReview Request: docker: add Docker builder to produce Python 2.7 tarball

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43661/diff/2-3/

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8737027 [details]
MozReview Request: docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43663/diff/2-3/

Comment 17

[Gregory Szorc [:gps]]

Comment on attachment 8737028 [details]
MozReview Request: docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43665/diff/2-3/

Comment 18

[Gregory Szorc [:gps]]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43667/diff/3-4/

Comment 19

[Kendall Libby [:fubar] (he/him)]

Comment on attachment 8737025 [details]
MozReview Request: docker: add base ansible image for CentOS 7 (bug 1261212); r=fubar

https://reviewboard.mozilla.org/r/43659/#review40333

Comment 20

[Kendall Libby [:fubar] (he/him)]

Comment on attachment 8737027 [details]
MozReview Request: docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r=fubar

https://reviewboard.mozilla.org/r/43663/#review40347

Comment 21

[Kendall Libby [:fubar] (he/him)]

Comment on attachment 8737028 [details]
MozReview Request: docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r=fubar

https://reviewboard.mozilla.org/r/43665/#review40351

Comment 22

[Kendall Libby [:fubar] (he/him)]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

https://reviewboard.mozilla.org/r/43667/#review40359

excellent start!

::: ansible/roles/docker-hg-ssh/files/entrypoint.py:70
(Diff revision 4)
>              line = 'hosts = %s\n' % ', '.join(kafka_servers)
>  
>          fh.write(line)
>  
> -subprocess.check_call(['/sbin/service', 'rsyslog', 'start'])
> +# TODO CentOS 7 uses systemd
> +#subprocess.check_call(['/sbin/service', 'rsyslog', 'start'])

I think this becomes:

subprocess.check_call(['/usr/bin/systemctl', 'start', 'rsyslog.service'])

::: ansible/roles/hg-ssh/files/sshd_config:1
(Diff revision 4)
> +SyslogFacility AUTHPRIV

Since we're going to have multiple ssh daemons, can we get a header comment indicating that there are multiple daemons, and which service it is?

::: ansible/roles/hg-ssh/templates/nslcd.conf.j2:15
(Diff revision 4)
> +map passwd uid {{ uid_attribute | mandatory }}
> +map passwd homeDirectory {{ home_attribute | mandatory }}
> +
> +uid nslcd
> +gid ldap
> +ssl no

Should we perhaps be requiring ssl, even though we're in the data center? That might be extra pain with the docker env, maybe, but I think I'd like to see ssl where possible.

Comment 23

[Kendall Libby [:fubar] (he/him)]

Comment on attachment 8737064 [details]
MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang

https://reviewboard.mozilla.org/r/43697/#review40369

I have very vague reservations about snarfing keys directly from ldap, but I think that it's due to paranoia and lack of sleep. I would like to see that we're doing queries/binds via ssl, though, fwiw (which needs acl changes. argh.)

Comment 24

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/43697/#review40369

What I have implemented in this series is effecitvely what openssh-lpk is doing today. What is different is we're doing it outside the sshd process.

Yes, we're doing plaintext today (see the sshd_config settings). Yes, that's wrong. We should try to fix that as part of the rollout. Should be easy enough - hopefully it's just a matter of using ldaps:// instead of ldap:// in the ldap.json config file, which all the Ansible foo is derived from.

Comment 25

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/43667/#review40359

> Since we're going to have multiple ssh daemons, can we get a header comment indicating that there are multiple daemons, and which service it is?

sshd_config only seems to offer SyslogFacility, so not sure how we'd change logging params other than by changing the facility :/

From the man page, the options are:

DAEMON, USER, AUTH, AUTHPRIV, LOCAL0, LOCAL1, LOCAL2, LOCAL3, LOCAL4, LOCAL5, LOCAL6, LOCAL7.  The default is AUTH

Perhaps we should use `AUTHPRIV` for system:22 and `AUTH` for hg:222?

> Should we perhaps be requiring ssl, even though we're in the data center? That might be extra pain with the docker env, maybe, but I think I'd like to see ssl where possible.

I can look into this. We already have conditional code elsewhere in Docker land to do plaintext in Docker and SSL elsewhere.

Are you OK with deferring to a follow-up since this is how things are today?

Comment 26

[Kendall Libby [:fubar] (he/him)]

https://reviewboard.mozilla.org/r/43667/#review40359

> sshd_config only seems to offer SyslogFacility, so not sure how we'd change logging params other than by changing the facility :/
> 
> From the man page, the options are:
> 
> DAEMON, USER, AUTH, AUTHPRIV, LOCAL0, LOCAL1, LOCAL2, LOCAL3, LOCAL4, LOCAL5, LOCAL6, LOCAL7.  The default is AUTH
> 
> Perhaps we should use `AUTHPRIV` for system:22 and `AUTH` for hg:222?

Sorry; I meant, can we have a comment in the file that notes we have multiple daemons and which one the config applies to? I'm having difficulty keeping them straight, so I'm willing to bet that someone else down the road will get confused (/me eyes hal warily). It *would* be nice if logs would show the difference, but we can worry about that as an improvement.

> I can look into this. We already have conditional code elsewhere in Docker land to do plaintext in Docker and SSL elsewhere.
> 
> Are you OK with deferring to a follow-up since this is how things are today?

I am ok with that.

Comment 30

[Gregory Szorc [:gps]]

Comment on attachment 8737025 [details]
MozReview Request: docker: add base ansible image for CentOS 7 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43659/diff/3-4/

Comment 31

[Gregory Szorc [:gps]]

Comment on attachment 8737027 [details]
MozReview Request: docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43663/diff/3-4/

Comment 32

[Gregory Szorc [:gps]]

Comment on attachment 8737028 [details]
MozReview Request: docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r=fubar

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43665/diff/3-4/

Comment 33

[Gregory Szorc [:gps]]

Comment on attachment 8737064 [details]
MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43697/diff/2-3/

Comment 34

[Gregory Szorc [:gps]]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43667/diff/4-5/

Comment 35

[Gregory Szorc [:gps]]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43667/diff/5-6/

Comment 36

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8737064 [details]
MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang

https://reviewboard.mozilla.org/r/43697/#review40525

Comment 37

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

https://reviewboard.mozilla.org/r/43667/#review40527

::: ansible/roles/hg-ssh/files/sshd_config:8
(Diff revision 6)
> +
> +SyslogFacility AUTHPRIV
> +LogLevel VERBOSE
> +PermitRootLogin no
> +PasswordAuthentication no
> +ChallengeResponseAuthentication no

we'll want Duo2FA for admins here

Comment 38

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/43667/#review40527

> we'll want Duo2FA for admins here

2FA is tracked in bug 1259231. I'll add an inline comment/todo and we'll address this later (not on a Friday).

Comment 39

[Gregory Szorc [:gps]]

Comment on attachment 8737064 [details]
MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43697/diff/3-4/

Comment 40

[Gregory Szorc [:gps]]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43667/diff/6-7/

Comment 41

[Kendall Libby [:fubar] (he/him)]

Comment on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

https://reviewboard.mozilla.org/r/43667/#review40663

Comment 42

[Kendall Libby [:fubar] (he/him)]

(In reply to Guillaume Destuynder [:kang] from comment #37)
> Comment on attachment 8737029 [details]
> MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212);
> r?fubar, kang
> 
> https://reviewboard.mozilla.org/r/43667/#review40527
> 
> ::: ansible/roles/hg-ssh/files/sshd_config:8
> (Diff revision 6)
> > +
> > +SyslogFacility AUTHPRIV
> > +LogLevel VERBOSE
> > +PermitRootLogin no
> > +PasswordAuthentication no
> > +ChallengeResponseAuthentication no
> 
> we'll want Duo2FA for admins here

So... if system sshd (i.e. NOT hg.mozilla.org) access is only via the VPN, what do we actually gain by using the same MFA system for both the VPN and the system login? 

Something else that we're missing - we should drastically cut back on who has system level access, full stop.

Comment 43

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/7e9a579ee528e15b6c4fcf717d9ba8be570ce9ef
docker: add base ansible image for CentOS 7 (bug 1261212); r=fubar

https://hg.mozilla.org/hgcustom/version-control-tools/rev/716b60a2e0691e335e9ee626102a9e1fba1006e3
docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r=fubar

https://hg.mozilla.org/hgcustom/version-control-tools/rev/94f53821e6678083f638d11d1697c102ae884c53
docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r=fubar

https://hg.mozilla.org/hgcustom/version-control-tools/rev/f32bc8e372a32a1eda15b779c9cafbbed07ad2e5
scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang

https://hg.mozilla.org/hgcustom/version-control-tools/rev/8d3f3d98b826c50a2ed3e1517e7cee65f672d46a
ansible: switch hgmaster to CentOS 7 (bug 1261212); r=fubar, kang

Comment 44

[Gregory Szorc [:gps]]

Attachment: MozReview Request: ansible/hg-ssh: add legacy MACs to support ancient OpenSSH clients (bug 1261212); r?kang

Review commit: https://reviewboard.mozilla.org/r/44143/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44143/

Comment 45

[Guillaume Destuynder [:kang] [this account is no longer active]]

Comment on attachment 8737845 [details]
MozReview Request: ansible/hg-ssh: add legacy MACs to support ancient OpenSSH clients (bug 1261212); r?kang

https://reviewboard.mozilla.org/r/44143/#review40739

looks good! - hope we can revert to the standard modern config in a not-too-long-time-period though :)

Comment 46

[Gregory Szorc [:gps]]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/0fb00c1de92723693fa654e0e3ec280a7ffc347c
ansible/hg-ssh: add legacy MACs to support ancient OpenSSH clients (bug 1261212); r=kang

Comment 47

[Kendall Libby [:fubar] (he/him)]

https://hg.mozilla.org/hgcustom/version-control-tools/rev/70a22c8dfde920bab564f72303acbcb053bbc3d4
scripts: re-enable DSA SSH keys (bug 1261212); r=me

Comment 48

[Guillaume Destuynder [:kang] [this account is no longer active]]

(In reply to Kendall Libby [:fubar] from comment #42)
> (In reply to Guillaume Destuynder [:kang] from comment #37)
> > Comment on attachment 8737029 [details]
> > MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212);
> > r?fubar, kang
> > 
> > https://reviewboard.mozilla.org/r/43667/#review40527
> > 
> > ::: ansible/roles/hg-ssh/files/sshd_config:8
> > (Diff revision 6)
> > > +
> > > +SyslogFacility AUTHPRIV
> > > +LogLevel VERBOSE
> > > +PermitRootLogin no
> > > +PasswordAuthentication no
> > > +ChallengeResponseAuthentication no
> > 
> > we'll want Duo2FA for admins here
> 
> So... if system sshd (i.e. NOT hg.mozilla.org) access is only via the VPN,
> what do we actually gain by using the same MFA system for both the VPN and
> the system login? 
> 
> Something else that we're missing - we should drastically cut back on who
> has system level access, full stop.

you can access directly without VPN - if you change that, thats ok - currently duo will automatically whitelist the VPN anyway. If you do not do that, it means it depends on the mechanism you use to ensure only the vpn has access.
Also, in all cases, if IPs are internally spoofed it is possible to bypass MFA when the authorization is done via IP addresses.

Comment 49

[Kendall Libby [:fubar] (he/him)]

(In reply to Guillaume Destuynder [:kang] from comment #48)

> you can access directly without VPN - if you change that, thats ok -

hg.m.o, yes, but we're not duo'ing that. and that won't get you shell.

> currently duo will automatically whitelist the VPN anyway. If you do not do
> that, it means it depends on the mechanism you use to ensure only the vpn
> has access.
> Also, in all cases, if IPs are internally spoofed it is possible to bypass
> MFA when the authorization is done via IP addresses.

cool, thanks. I mostly asked because I saw someone making the same case elsewhere and thought this would be a good way to get an actual answer. :-D

Comment 50

[Gregory Szorc [:gps]]

No activity on this bug in 22 days. I don't see value in keeping this bug open. We can file follow-ups as needed, including converting hgssh2 to CentOS 7.