Page MenuHomePhabricator

audit labs versus production ssh keys
Closed, ResolvedPublic

Description

Recently, I added a new user, and then found out afterwards that they had mistakenly used the same ssh key in labs and production. Our L3 document clearly states that no one should do that, but mistakes happen.

I recall @ArielGlenn conducting an audit over a year ago (and finding quite a few back then and getting them fixed.) We're overdue for checking this again.

Event Timeline

RobH created this task.Aug 5 2015, 6:36 PM
RobH claimed this task.
RobH raised the priority of this task from to Medium.
RobH updated the task description. (Show Details)
RobH added projects: acl*sre-team, Cloud-Services.
RobH added subscribers: RobH, ArielGlenn.
Restricted Application added subscribers: Matanya, Aklapper. · View Herald TranscriptAug 5 2015, 6:36 PM

According to my script, both @Mvolz and @Jdouglas have labs keys in production.

Mvolz added a comment.Aug 5 2015, 8:45 PM

Whoops. You can revoke that key, I haven't needed production access in a
while. https://gerrit.wikimedia.org/r/190405 was the change.

Change 229562 had a related patch set uploaded (by Dzahn):
admin: deactivate shell for user mvolz

https://gerrit.wikimedia.org/r/229562

Per request, here's the script. Stick it in modules/ldap/files/scripts (operations/puppet clone) on a machine which is connected to labs LDAP, and run it from that directory.

import ldapsupportlib, ldap, yaml

ldapSupportLib = ldapsupportlib.LDAPSupportLib()
ds = ldapSupportLib.connect()
base = "ou=people," + ldapSupportLib.getBase()

keys = set()
for entry in ds.search_s(base, ldap.SCOPE_SUBTREE, "(objectclass=inetOrgPerson)"):
    for k, v in entry[1].items():
        if k == "sshPublicKey":
            for v2 in v:
                key = ' '.join(v2.split(' ')[:2])
                keys.update([key])

prodData = yaml.load(open('../../../admin/data/data.yaml'))
for userName, userData in prodData['users'].items():
    badKeys = list(set(userData['ssh_keys']).intersection(keys))
    if len(badKeys) > 0:
        print userName, badKeys

ds.unbind()

Change 229562 abandoned by Dzahn:
admin: deactivate shell for user mvolz

https://gerrit.wikimedia.org/r/229562

@RobH: Any reason for this to remain open?

RobH closed this task as Resolved.Sep 4 2015, 9:35 PM

Nope! both sub-tasks resolved, resolving task.

@RobH: Do you think we should turn this into some sort of regular check? Not sure icinga necessarily would be appropriate, but...

Dzahn added a subscriber: Dzahn.Sep 9 2015, 12:48 AM

@Krenair I think in a perfect world it would be detected by jenkins. If somebody touches a file in admin and there is a key in it it would have to compare it to all existing labs keys though and vote -1 if it finds a match. How unrealistic is that?

@Krenair I think in a perfect world it would be detected by jenkins. If somebody touches a file in admin and there is a key in it it would have to compare it to all existing labs keys though and vote -1 if it finds a match. How unrealistic is that?

That would be an annoying check to have fail when trying to do something completely separate. Good idea for new keys being added, but a production key can be added as a labs one at any time.

Krenair added a comment.EditedSep 9 2015, 12:52 AM

That said, I think jenkins might be less annoying in operations/puppet than it is where I'm used to it - i.e. VE-MW (where you can't merge anything without either being jenkins or l10n-bot.)

Dzahn added a comment.Feb 12 2016, 9:12 PM

maybe we can get this merged and ask CI team to add it as a non-voting check on operations/puppet and see how it works. then if we like it , just change non-voting to voting

We are all agreed that jenkins failing on this would be pretty annoying, since it could block unrelated changes. BUT surely we can do some sort of regular audit.

Yes. Let's open a separate task (maybe a subtask of T142815) for that?

RobH removed RobH as the assignee of this task.Dec 12 2016, 1:43 AM