Upgrade hgssh to CentOS 7

RESOLVED FIXED

Status

Developer Services
Mercurial: hg.mozilla.org
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

(Blocks: 2 bugs)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(6 attachments, 1 obsolete attachment)

(Assignee)

Description

2 years ago
We need to move to the future.
(Assignee)

Comment 1

2 years ago
Created attachment 8737025 [details]
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/
Attachment #8737025 - Flags: review?(klibby)
Attachment #8737027 - Flags: review?(klibby)
Attachment #8737028 - Flags: review?(klibby)
Attachment #8737029 - Flags: review?(klibby)
Attachment #8737029 - Flags: review?(gdestuynder)
(Assignee)

Comment 2

2 years ago
Created attachment 8737026 [details]
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/
(Assignee)

Comment 3

2 years ago
Created attachment 8737027 [details]
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/
(Assignee)

Comment 4

2 years ago
Created attachment 8737028 [details]
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/
(Assignee)

Comment 5

2 years ago
Created attachment 8737029 [details]
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/
(Assignee)

Comment 6

2 years ago
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/
(Assignee)

Comment 7

2 years ago
Created attachment 8737064 [details]
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/
Attachment #8737064 - Flags: review?(klibby)
Attachment #8737064 - Flags: review?(gdestuynder)
(Assignee)

Comment 8

2 years ago
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/
(Assignee)

Comment 9

2 years ago
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/
(Assignee)

Comment 10

2 years ago
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/
(Assignee)

Comment 11

2 years ago
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/
(Assignee)

Comment 12

2 years ago
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/
(Assignee)

Comment 13

2 years ago
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/
(Assignee)

Comment 14

2 years ago
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/
(Assignee)

Comment 15

2 years ago
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/
(Assignee)

Comment 16

2 years ago
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/
(Assignee)

Comment 17

2 years ago
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/
(Assignee)

Comment 18

2 years ago
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 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
Attachment #8737025 - Flags: review?(klibby) → review+
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
Attachment #8737027 - Flags: review?(klibby) → review+

Updated

2 years ago
Attachment #8737028 - Flags: review?(klibby) → review+
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

Updated

2 years ago
Attachment #8737029 - Flags: review?(klibby)
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 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.)
Attachment #8737064 - Flags: review?(klibby) → review+
(Assignee)

Comment 24

2 years ago
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.
(Assignee)

Comment 25

2 years ago
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?
(Assignee)

Updated

2 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
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.
(Assignee)

Updated

2 years ago
Blocks: 1261478
(Assignee)

Updated

2 years ago
Blocks: 1261481
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1191554
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1217112
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1259247
(Assignee)

Comment 30

2 years ago
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/
Attachment #8737025 - Attachment description: MozReview Request: docker: add base ansible image for CentOS 7 (bug 1261212); r?fubar → MozReview Request: docker: add base ansible image for CentOS 7 (bug 1261212); r=fubar
Attachment #8737027 - Attachment description: MozReview Request: docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r?fubar → MozReview Request: docker: rename hgrpm builder to hgrpm-centos6 (bug 1261212); r=fubar
Attachment #8737028 - Attachment description: MozReview Request: docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r?fubar → MozReview Request: docker: support building Mercurial RPMs for CentOS 7 (bug 1261212); r=fubar
Attachment #8737029 - Flags: review?(klibby)
(Assignee)

Comment 31

2 years ago
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/
(Assignee)

Comment 32

2 years ago
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/
(Assignee)

Comment 33

2 years ago
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/
(Assignee)

Comment 34

2 years ago
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/
(Assignee)

Updated

2 years ago
Attachment #8737026 - Attachment is obsolete: true
(Assignee)

Comment 35

2 years ago
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 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
Attachment #8737064 - Flags: review?(gdestuynder) → review+
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
Attachment #8737029 - Flags: review?(gdestuynder) → review+
(Assignee)

Comment 38

2 years ago
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).
(Assignee)

Comment 39

2 years ago
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/
Attachment #8737064 - Attachment description: MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r?fubar, kang → MozReview Request: scripts: add a standalone script for performing SSH key lookup from LDAP (bug 1261212); r=fubar, kang
(Assignee)

Comment 40

2 years ago
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 on attachment 8737029 [details]
MozReview Request: ansible: switch hgmaster to CentOS 7 (bug 1261212); r?fubar, kang

https://reviewboard.mozilla.org/r/43667/#review40663
Attachment #8737029 - Flags: review?(klibby) → review+
(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.
Flags: needinfo?(gdestuynder)
(Assignee)

Comment 44

2 years ago
Created attachment 8737845 [details]
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/
Attachment #8737845 - Flags: review?(gdestuynder)
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 :)
Attachment #8737845 - Flags: review?(gdestuynder) → review+
(Assignee)

Comment 46

2 years ago
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
(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.
Flags: needinfo?(gdestuynder)
(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
(Assignee)

Comment 50

2 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.