base kmsauth token on bastion_user instead of remote_usernames

[djcrabhat]

The iconic bless client from the team that made kmsauth in the first place, https://github.com/lyft/python-blessclient, uses the same value for bastion_user and remote_usernames. Their clever use of IAM policy (https://github.com/lyft/python-blessclient#setup-a-kmsauth-key--policy-in-your-aws-account) to make sure a user can only generate certs for themselves works great if you use the same bastion_user and remote_usernames. They also try to pull the IAM username of the current credentials and default that for the bastion_user.

But my situation is a little different. I want to use BLESS in a similar way to this suggestion from Facebook, where my users will be logging on to root or some other sort of shared, privileged account. So I'll have a bastion_user that maps 1:1 to an AWS user, but the signed cert will now be good for a privileged local user on the BLESS-CA-trusting server I'm trying to login to.

For example, imagine a payload like this

{
    'bastion_user': 'djcrabhat', 
    'bastion_user_ip': my_bastion_ip,
    'remote_usernames': 'ubuntu', 
    'bastion_ips': bastion_ips,
    'command': '*',
    'public_key_to_sign': my_public_key
}

I could then send a kmsauth_token with an encryption context of djcrabhat, which directly links me to my IAM user, instead of having the encryption context of ubuntu.

(PS: and this shouldn't be disruptive to python-blessclient, as they'll still end up sending the same value for bastion_user and remote_usernames.)

[coveralls]

Coverage remained the same at 95.741% when pulling 073fdb4 on djcrabhat:kms-bastion-user-instead-of-remote into d5a1c1f on Netflix:master.

[djcrabhat]

tagging @Stype just to make sure he agrees that Blessclient would still work and that he agrees with the spirit of this PR

[Stype]

This would put us in an awkward situation since the certificate is based off of the remote_username. So with this patch I think user1 can monkey with with version of blessclient and get a certificate to login as user2, by setting different remote/bastion usernames. Since there isn't a way to enforce that in the Lambda currently.

1. I've thought it would be nice to add multiple remote_usernames from an approved list ('ubuntu', 'root', etc), and have the Lambda validate that the remote_usernames are either in the kmsauth token or on the approved list. I think that would work for your case, and would make the kmsauth path line up better with the non-kmsauth path.

2. The other option that would make this safe for us would be adding another Lambda config that requires the bastion and remote usernames to match.

I could probably get to 1 this week if needed (or happy to work with you on something like that). Unless Russell has strong feelings on it.

[djcrabhat]

Ooooh option #2 is a very interesting idea, I may take a stab at that.

How it currently works, KMS just will crap out, since you're signing with username https://github.com/lyft/python-blessclient/blob/master/blessclient/client.py#L462, (which by default, is the IAM user name of the current creds. https://github.com/lyft/python-blessclient/blob/master/blessclient/client.py#L368 ) and then you send the remote_username (which can already be a command delimitated list) and bastion_user as the same value. Bless is checking that your encryption context from is remote_usernames .https://github.com/Netflix/bless/blob/master/bless/aws_lambda/bless_lambda.py#L146 I think by default, if using KMS, I'll enforce that remote and bastion users are the same, to keep current functionality. Then there will be a shiny, new config value to allow those values to diverge.

The one thing I don't love about 1 is I lose that awesome ability to set the encryption context in the IAM that from={aws.username}. Instead, I'd have an IAM policy that let people sign from:ubuntu or from:root, which feels kinda awkward, tbh.

[russell-lewis]

@djcrabhat I don't think what @Stype was proposing for option 1 would require the change in IAM policy you were describing.

The way I'm reading that, you would still do the kmsauth token check against request.bastion_user, but precede that with a check to make sure that request. remote_usernames is either the same as request.bastion_user or in a whitelist of allowed remote_usernames that certs can be issued for.

On that whitelist, if you allowed prefixes, you could still implement per-instance certs (e.g. ubuntu-i-1234567890abcdef0 instead of just ubuntu) if you had your instances set up scoped AuthorizedPrincipals.

I should add that I'm not sure if either of y'all would want to go down the per-instance cert route, given that your bastionless ssh route. But if you did, you'd also need to generate keypairs for each ssh session, due to limits of how ssh finds/uses certificates associated with a given keypair.

[Stype]

The way I'm reading that, you would still do the kmsauth token check against request.bastion_user, but precede that with a check to make sure that request. remote_usernames is either the same as request.bastion_user or in a whitelist of allowed remote_usernames that certs can be issued for.

Yep, more like this. I wasn't thinking I would enforce which other names the user was approved to use in the kmsauth token. I would just make it a whitelist of other names that anyone with a valid kmsauth token could assume.

If you want to enforce with IAM policy that a group of users can login as a particular username there are probably ways you could do that, but I'm not sure if that's what you're looking for.

[djcrabhat]

Ooooh I gotcha guys now. So I agree, that 1 is a very nice solution.

And I see why my proposed solution would fix my problem, but also open the door to people logging in as anyone they wanted, as long as they signing the bastion_user as themselves.

So that's a great idea, some sorta logic in the bless lambda, that if you're doing this kmsauth verification 1) ensure the remote and bastion users are the same or 2) if not the same, check the config for some sorta whitelist of allowed remote users.

I'll take a stab at that approach on this branch this week. Thank you both so much for being so responsive on this PR, you guys came up with a much better solution.

[coveralls]

Coverage decreased (-1.4%) to 94.343% when pulling b3e4eab on djcrabhat:kms-bastion-user-instead-of-remote into d5a1c1f on Netflix:master.

[coveralls]

Coverage decreased (-0.1%) to 95.628% when pulling 7e8f30f on djcrabhat:kms-bastion-user-instead-of-remote into d5a1c1f on Netflix:master.

[coveralls]

Coverage increased (+0.07%) to 95.811% when pulling 5b02087 on djcrabhat:kms-bastion-user-instead-of-remote into d5a1c1f on Netflix:master.

[coveralls]

Coverage increased (+0.08%) to 95.818% when pulling c9e1e02 on djcrabhat:kms-bastion-user-instead-of-remote into d5a1c1f on Netflix:master.

[djcrabhat]

Alright, so I've added a config value for kmsauth_remote_usernames_allowed, where if you want to allow the bastion_user and remote_usernames to diverge, you can be explicit about which remote_usernames are allowed. By default, enforces the old behavior that bastion_user and remote_usernames are the same.

(Also added some tests to get coverage I mistakenly overlooked)

[djcrabhat]

Tagging @Stype and @russell-lewis .

[Stype]

I didn't test it, but it looks like that should work for us. +1 from me.