2FA credential sharing for node-private repo

[gibfahn]

Not sure where this was discussed, but @rvagg mentioned in the standup (#939) that there was an issue giving the bot user access to the node private account, because there wasn't a way to share the information with everyone in the infra team.

So for 2FA the QR code you always see to scan is actually just a long hex string, which is basically the private key that generates the minute long ones. Usually with 2FA there's a "Can't read QR code" option that allows you to copy and paste the long string.

If we save that into the infra repo, then anyone who needs access just pastes the string into Google Authenticator and they can generate codes.

Alternately we could just gpg encrypt the QR code png, and people could decrypt and scan.

[rvagg]

removed the node-ci user from node-private and turned back on the 2fa requirement there but we're still trying to figure out how we can get node-test-commit-private and sub-jobs access to node-private when required, ideally permanently so it still may require 2fa support.

/cc @joaocgreis FYI

[rvagg]

@joaocgreis @MylesBorins @gibfahn here's some of the basics of the problems were dealing with:

• CI needs access to nodejs-private/node-private for use in node-test-commit-private
• All of the sub-jobs also need access with node-test-commit-private defers to them, ugh
• We have a "node-private" deploy key in CI that we can use to access nodejs-private/node-private
• git-rebase uses a different key, that's associated with a user (node-ci) so it can write to the node_binary_tmp repo, this is used for the fanned jobs
• You can't select which SSH key to use for any particular operation, if you add both keys to the SSH Agent then it'll just use the first one in there for git@ operations, unless we store the key on disk (where we could load it with GIT_SSH) I can't find a way to mess with this, so git-rebase can only use one key to do both its read from node-private and its write to node_binary_tmp

So, we could consolidate everything into the single node-ci user and give it access to write to both node_binary_tmp and read from node-private. We did that for this week's security release.

Unfortunately, when we give node-ci elevated permissions then we open up the ability for CI to read arbitrarily from node-private, potentially giving anyone with CI access (or who can get code submitted in to CI without it being picked up by a collaborator beforehand) to sensitive information we store there. e.g. you could do some git CLI work to fetch stuff and spit it to console which is public. It's awkward and you'd have to be very motivated to get anything actually useful, but the avenue exists.

So, here's a suggestion for solving it at a higher level than just figuring out how to propagate a sensitive SSH key from node-test-commit-private downward:

• Make it much easier to flip Jenkins into "protected" mode, which could just be nodejs/security and nodejs/jenkins-admins having access, everyone else locked out
• Any switch should also involve giving the node-ci user into the node-private repository, we'd have to add shared 2fa access to this user I think
• When you flip Jenkins out of protected mode, we also remove the node-ci user

Perhaps this could be automated, but we could at least document the process so it's not hard to follow. We could flip it on any time we need to do a bit of testing, not just the few days before a release (ideally).

A couple of other options:

• Figure out how to make the an SSH key propagate downward from node-test-commit-private only so that you don't have access to that key unless you're starting from that root job, then we have a new user, maybe node-private-ci, that can access both node-private and node_binary_tmp, it might make git-rebase awkward but not impossible (I think). I just don't know if this is even possible to do without making holes for it to be exploited people with general collaborator access.
• Set up an entirely new Jenkins, but that means new testing nodes and that's probably not practical
• Set up node-test-commit-private so that it doesn't use sub-jobs, remove the fanned stuff and do the compile/test directly on the windows & arm machines and just accept the extended build time, it also might mean removing some nodes that are a bit too complicated (fips maybe?)
• Set up our Release Jenkins to be able to run tests too and just do it all on there. We'd only have a limited set of machines we can test across but at least it'd be fully protected.

Anything we do has to deal with the fact that we don't do this kind of work often enough and so any config tends to get out of sync with our standard test setup. I noticed that node-test-commit-private doesn't currently have the full range of subjobs in it even now!

[mhdawson]

It would probably be good to open an issue in the build repo listing the sub-jobs which are not on node-test-commit-private so we can see if that's on purpose or by accident.

Your suggestion seems the most practical. It would be nice to be able to test on the release ci, but I doubt that is practical for arm/windows that require the fans. I assume other platforms would be ok.

[jasnell]

automating the generation of 2FA codes is quite trivial once you have the key and the cycle time. I've got an old (likely quite crappy) bit of code here that shows how it is done: https://github.com/jasnell/tfa. It should be rather trivial to work this into any auth workflow automation that required specifying a 2FA value.

[joaocgreis]

@rvagg @MylesBorins (and everyone else) there is a very important detail about the private jobs that you might be missing: only the private jobs are private, subjobs launched by them are not. For example, try to open the following links (while they are still available) in an incognito window (logged out of GH):

• https://ci.nodejs.org/view/All/job/node-test-commit-private/173/
• https://ci.nodejs.org/job/node-test-commit-freebsd/12615/

The second one was launched by the first and is perfectly accessible. There are even windows binaries available from https://ci.nodejs.org/job/node-compile-windows/12806/label=win-vs2015/ (there should be arm binaries as well, but the private job is out of sync and does not have arm-fanned).

Jenkins must always be locked for security work. Even if we manage to fix the issue above, I do not trust Jenkins (or our knowledge of it) to do security work without locking.

The private jobs might still have a reason to exist: to provide default values. But they should only pass the parameters to the regular jobs, not try to duplicate.

[MylesBorins]

With that in mind would it perhaps make sense to create new private jobs that simply wrap the main job with sensible defaults? I would like to see us so some research on having an approach that doesn't have to shut down CI. Having to rush the testing in to the last 24 hours is not only stressful, but a massive risk.

…

On Oct 26, 2017 11:10 AM, "João Reis" ***@***.***> wrote: @rvagg <https://github.com/rvagg> @MylesBorins <https://github.com/mylesborins> (and everyone else) there is a very important detail about the private jobs that you might be missing: only the private jobs are private, subjobs launched by them are not. For example, try to open the following links (while they are still available) in an incognito window (logged out of GH): - https://ci.nodejs.org/view/All/job/node-test-commit-private/173/ - https://ci.nodejs.org/job/node-test-commit-freebsd/12615/ The second one was launched by the first and is perfectly accessible. There are even windows binaries available from https://ci.nodejs.org/job/ node-compile-windows/12806/label=win-vs2015/ (there should be arm binaries as well, but the private job is out of sync and does not have arm-fanned). Jenkins must always be locked for security work. Even if we manage to fix the issue above, I do not trust Jenkins (or our knowledge of it) to do security work without locking. The private jobs might still have a reason to exist: to provide default values. But they should only pass the parameters to the regular jobs, not try to duplicate. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#943 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecVz1C94E1I2X4LP_0kVu_VTWkzdi4ks5swKCZgaJpZM4QFPsM> .

[Trott]

Closing due to long period of inactivity. Feel free to re-open if this is a thing. I'm just trying to close stuff that has been ignored for sufficiently long that it seems likely it's not something we're going to get to.