nubisproject / nubis-docs

Documentation for the Nubis project
Mozilla Public License 2.0
3 stars 16 forks source link

#adminuserapikeys - Administrative users have long lived API keys #125

Open tinnightcap opened 7 years ago

tinnightcap commented 7 years ago

From @gozer on August 24, 2015 18:4

From https://mana.mozilla.org/wiki/display/SECURITY/Nubis#Nubis-#adminuserapikeys

Administrative users have long lived API keys

The dev automation users which have local non-federated user accounts in the stage and prod nubis AWS accounts create long lived AWS API keys which they store on their local workstations. Since API keys can not use MFA, a compromise of any of these workstations would result in a full compromise of the nubis aws accounts.

Recommendations :

Copied from original issue: nubisproject/nubis-meta#22

tinnightcap commented 7 years ago

The solution is to create a script that does the STS dance and updates the local ~/.aws/credentials file.

The script will be installed as an alias to the aws command and will evaluate the current key to determine if it is still valid. If not it will initialize the STS dance and refresh the credentials. If the credential is not valid the script will attempt to use the in memory MFA key to get new temporary credentials. If the MFA credentials are invalid it will generate a dialog and trigger the token request for MFA creds that it will then store only in memory.

tinnightcap commented 7 years ago

From @gozer in #28

http://www.infoq.com/presentations/aws-cli-federation

It's pinterest's aeres, IIRC

tinnightcap commented 7 years ago

From @gene1wood on June 3, 2016 17:43

Nubis is planning to use a handful of technologies to improve their security posture. The goal is to remedy these risks :

The plan is described here : https://github.com/nubisproject/nubis-deploy/blob/master/SECURITY.md

The technologies are

User policy

https://github.com/nubisproject/nubis-deploy/blob/1219a41dcd9581bd90dbdccc2a5144db7c4f3fb7/modules/global/admins/mfa-policy.json.tmpl

Element Effect
"Condition": { "Null": { "aws:MultiFactorAuthAge": "true" } } The multi-factor auth key code is missing
"NotAction": "iam:*" The action attempted is something other than iam:*
"Resource": "*" The target of the action is anything

If the above 3 conditions are all met, then the action is denied.

I believe this means that the mfa-policy.json.tmpl policy does not allow the user to STS:AssumeRole

How can a user STS:AssumeRole to one of the AWS managed roles as it doesn't appear they're granted the STS:AssumeRole permission?

AWS Managed Roles

I'm unsure how these roles are assumed

aws-vault

The effect of using this tool from a security perspective is that the long lived API keys are encrypted at rest as opposed to stored in plain text if the tool were not used.

tinnightcap commented 7 years ago

From @gozer on June 3, 2016 17:56

First, a small point of clarification. There is one shared read-only role indeed, but for administrative actions, each user has his own admin policy, which is * : * and only him/her can assume, you can see that here:

https://github.com/nubisproject/nubis-deploy/blob/master/modules/global/admins/main.tf#L72

I suspect that might be what makes the STS assumption into the admin role work.

tinnightcap commented 7 years ago

From @gozer on June 3, 2016 17:57

AWS Managed Roles? They are never assumed.

The only 2 roles ever assumed by a user are the nubis created readonly one as well as the per-user admin role.

tinnightcap commented 7 years ago

From @gozer on June 3, 2016 17:58

@limed I see what gene says about being able to change a user's password without the MFA device...

And correct me if I am wrong, but that's the web Console password, isn't it?

If that's the case, I'd protect that under the MFA-only policy, thoughts?

tinnightcap commented 7 years ago

From @gene1wood on June 3, 2016 18:35

There is one shared read-only role indeed

I'm assuming that's this role

each user has his own admin policy

I'm assuming that this is the policy for the admin role

The challenge is this policy applies to a role, and the role can't grant the users sts:AssumeRole because they need that permission to assume the admin role (catch 22).

Would it be possible to see the entire collection of policies that apply to a user, roles which are created and policies which apply to those roles that these terraform scripts create?

AWS Managed Roles? They are never assumed.

Got it, this was based off of a guess by JD while you were out.

I see what gene says about being able to change a user's password without the MFA device

Oh I wasn't calling this out as a problem, just describing what the policies cause. Deciding to require MFA in order to do a password change may be a confusing user experience. The security benefits for requiring MFA on password change are

In the end I'd say the minor benefits aren't worth the potential user confusion.

tinnightcap commented 7 years ago

From @gozer on July 8, 2016 15:10

Would it be possible to see the entire collection of policies that apply to a user,

Each user is in an Administrator group, with no policies attached directly to any user.

The Administrator group has 2 policies attached:

roles which are created and

Each admin user gets his/her own role:

policies which apply to those roles

A single policy per role, you can see it here [https://github.com/nubisproject/nubis-deploy/blob/master/modules/global/admins/main.tf#L38]

tinnightcap commented 7 years ago

From @rollobancher on July 28, 2016 15:47

@gene1wood Just a reminder to review gozers last comment from 7/8. Thanks in advance!

tinnightcap commented 7 years ago

From @gene1wood on July 28, 2016 21:2

@gozer thanks for the clarification. Couple more questions

I'm unclear how the two policies you identified (mfa-policy and arn:aws:iam::aws:policy/AdministratorAccess) prevent an attacker with a users password but without their MFA token from disabling MFA on their account.

Here's a policy simulation of those two policies and the action iam:DeactivateMFADevice

# User without MFA tries to disable MFA
aws iam simulate-custom-policy --policy-input-list '{"Version":"2012-10-17","Statement":[{"Sid":"AllowAllUsersToListAccounts","Effect":"Allow","Action":["iam:ListUsers","iam:ListAccountAliases"],"Resource":["arn:aws:iam::656532927350:user/*"]},{"Sid":"AllowIndividualUserToSeeTheirAccountInformation","Effect":"Allow","Action":["iam:ChangePassword","iam:CreateLoginProfile","iam:DeleteLoginProfile","iam:GetAccountPasswordPolicy","iam:GetAccountSummary","iam:GetLoginProfile","iam:UpdateLoginProfile"],"Resource":["arn:aws:iam::656532927350:user/jdoe"]},{"Sid":"AllowIndividualUserToListTheirMFA","Effect":"Allow","Action":["iam:ListVirtualMFADevices","iam:ListMFADevices"],"Resource":"*"},{"Sid":"AllowIndividualUserToManageThierMFA","Effect":"Allow","Action":["iam:CreateVirtualMFADevice","iam:EnableMFADevice","iam:ResyncMFADevice"],"Resource":["arn:aws:iam::656532927350:mfa/jdoe","arn:aws:iam::656532927350:user/jdoe"]},{"Sid":"DoNotAllowAnythingOtherThanAboveUnlessMFAd","Effect":"Deny","NotAction":"iam:*","Resource":"*","Condition":{"Null":{"aws:MultiFactorAuthAge":"true"}}}]}' '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}' --caller-arn arn:aws:iam::656532927350:user/jdoe --action-names 'iam:DeactivateMFADevice'

which returns "EvalDecision": "allowed"

Second question : What is the purpose of the IAM Roles (arn:aws:iam::<account-id>:role/nubis/admin/<username>) which are created, one per user? It seems like each user, without assuming either a read only or an administrative role, would have rights to do administrative functions already.

Here's a user with the mfa-policy and arn:aws:iam::aws:policy/AdministratorAccess policy attempting an ec2:DeleteVpc

# User with MFA tries to delete VPC
aws iam simulate-custom-policy --policy-input-list '{"Version":"2012-10-17","Statement":[{"Sid":"AllowAllUsersToListAccounts","Effect":"Allow","Action":["iam:ListUsers","iam:ListAccountAliases"],"Resource":["arn:aws:iam::656532927350:user/*"]},{"Sid":"AllowIndividualUserToSeeTheirAccountInformation","Effect":"Allow","Action":["iam:ChangePassword","iam:CreateLoginProfile","iam:DeleteLoginProfile","iam:GetAccountPasswordPolicy","iam:GetAccountSummary","iam:GetLoginProfile","iam:UpdateLoginProfile"],"Resource":["arn:aws:iam::656532927350:user/jdoe"]},{"Sid":"AllowIndividualUserToListTheirMFA","Effect":"Allow","Action":["iam:ListVirtualMFADevices","iam:ListMFADevices"],"Resource":"*"},{"Sid":"AllowIndividualUserToManageThierMFA","Effect":"Allow","Action":["iam:CreateVirtualMFADevice","iam:EnableMFADevice","iam:ResyncMFADevice"],"Resource":["arn:aws:iam::656532927350:mfa/jdoe","arn:aws:iam::656532927350:user/jdoe"]},{"Sid":"DoNotAllowAnythingOtherThanAboveUnlessMFAd","Effect":"Deny","NotAction":"iam:*","Resource":"*","Condition":{"Null":{"aws:MultiFactorAuthAge":"true"}}}]}' '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}' --context-entries ContextKeyName=aws:MultiFactorAuthAge,ContextKeyValues=3600,ContextKeyType=numeric --caller-arn arn:aws:iam::656532927350:user/jdoe --action-names 'ec2:DeleteVpc'

which returns "EvalDecision": "allowed"

tinnightcap commented 7 years ago

From @gozer on August 4, 2016 18:29

The iam:DeactivateMFADevice case, I believe, is basically a bug fixed with this:

https://gist.github.com/cd560d3deff8cb343918d6310bbf2425

tinnightcap commented 7 years ago

From @gozer on August 4, 2016 18:31

What is the purpose of the IAM Roles

A good question, and yes the arn:aws:iam::aws:policy/AdministratorAccess would be good enough.

Unfortunately, we need a role that STS can use to assume, for things like logging into the web console and all. And the Amazon policies like the one above can't be used for that. Thus the existence of one specific admin role per user.

tinnightcap commented 7 years ago

From @gozer on August 4, 2016 18:32

Oh, and that also allows us to see who is logged into the web console, for instance, nubis/admin/gozer vs. nubis/admin/jd

And you'll notice the permissions only allow me to take the gozer admin role, not jd's

tinnightcap commented 7 years ago

From @gene1wood on August 5, 2016 16:44

The iam:DeactivateMFADevice case, I believe, is basically a bug fixed with this:

I don't think it is. If you make that change you break a users ability to setup MFA at all

# User without MFA tries to create an MFA device
aws iam simulate-custom-policy --policy-input-list '{"Version":"2012-10-17","Statement":[{"Sid":"AllowAllUsersToListAccounts","Effect":"Allow","Action":["iam:ListUsers","iam:ListAccountAliases"],"Resource":["arn:aws:iam::656532927350:user/*"]},{"Sid":"AllowIndividualUserToSeeTheirAccountInformation","Effect":"Allow","Action":["iam:ChangePassword","iam:CreateLoginProfile","iam:DeleteLoginProfile","iam:GetAccountPasswordPolicy","iam:GetAccountSummary","iam:GetLoginProfile","iam:UpdateLoginProfile"],"Resource":["arn:aws:iam::656532927350:user/jdoe"]},{"Sid":"AllowIndividualUserToListTheirMFA","Effect":"Allow","Action":["iam:ListVirtualMFADevices","iam:ListMFADevices"],"Resource":"*"},{"Sid":"AllowIndividualUserToManageThierMFA","Effect":"Allow","Action":["iam:CreateVirtualMFADevice","iam:EnableMFADevice","iam:ResyncMFADevice"],"Resource":["arn:aws:iam::656532927350:mfa/jdoe","arn:aws:iam::656532927350:user/jdoe"]},{"Sid":"DoNotAllowAnythingOtherThanAboveUnlessMFAd","Effect":"Deny","Action": "*","Resource":"*","Condition":{"Null":{"aws:MultiFactorAuthAge":"true"}}}]}' '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"*","Resource":"*"}]}' --caller-arn arn:aws:iam::656532927350:user/jdoe --action-names 'iam:CreateVirtualMFADevice'

It's probably a good idea to setup a suite of unit tests in the form of AWS policy simulation commands that show the behavior you want then run them against your policies to come up with something that works. Once you have that battery of policy simulations, add them to this ticket along with the new policy so I can see what the intended behavior is (from the tests) and how the policy simulation passes those tests.

we need a role that STS can use to assume, for things like logging into the web console and all

Why would an admin user who first logs in to the web console as themselves with their username password and MFA, assume a role when they already are in the web console and have administrative rights? When you say that you "need a role that STS can use to assume, for things like logging into the web console" I don't understand. Why does a user need a role for STS to assume? That role grants them no different permissions than they already have logged into the web console as their user.

tinnightcap commented 7 years ago

From @gozer on August 5, 2016 17:22

I don't think it is. If you make that change you break a users ability to setup MFA at all

You're right, I got the overriding logic wrong, but it should illustrate my point anyhow. The correct fix should be something more like : this gist

It's probably a good idea to setup a suite of unit tests in the form of AWS policy simulation commands

And generally a good idea to have tests for anything important, I agree. Unfortunately, at the moment, we don't exactly have the resources available to do any kind of testing, so it will have to wait

Why would an admin user who first logs in to the web console as themselves with their username password and MFA, assume a role when they already are in the web console and have administrative rights?

Ah, you must have missed the part where the IAM users we create for admins only have API credentials, no web console password at all. This way, we don't have that password to manage at all, but it's what requires the use of STS for getting into the web console

tinnightcap commented 7 years ago

@gene1wood I just want to clarify where we are. As I understand it, if @gozer applies the gist in his last comment (and it works) we will be all good here? - Thanks

tinnightcap commented 7 years ago

From @gene1wood on August 9, 2016 17:37

@tinnightcap I'm not sure. If you could add the current policies (with all the diffs applied) that you've tested and verified do what you're hoping they do, I can review them.

tinnightcap commented 7 years ago

@gene1wood gozer applied the patch, can you have a look?

tinnightcap commented 7 years ago

From @gene1wood on October 5, 2016 19:50

Yes, happy to. Could you point me to the current policy (with all the diffs applied) that's been tested, the ticket's a bit long and I'm not sure what patch you mean.

tinnightcap commented 7 years ago

@gene1wood well I am not exactly sure what you need at this point. I was referring to your previous comment and the fact that @gozer applied a diff as:

https://github.com/nubisproject/nubis-deploy/pull/109/files

that you can see directly before my last comment.

Actually I don't know where this review is at in any respect. I guess what I need from you is to know exactly what we (the Nubis team) need to do in order to get this review finished up. Can you tell me exactly what you need in order to close this out?

Thanks

tinnightcap commented 7 years ago

From @gozer on October 11, 2016 17:38

For instance, the 'gozer' admin policy looks like:

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Action": "*",
      "Effect": "Allow",
      "Resource": "*",
      "Sid": "admin"
    }
  ]
}

The trust relationship is:

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Sid": "gozer",
      "Effect": "Allow",
      "Principal": {
        "AWS": "arn:aws:iam::517826968395:user/nubis/admin/gozer"
      },
      "Action": "sts:AssumeRole"
    }
  ]
}

My user has 2 policies attached, the Amazon AdministratorAccess one, plus our own MFA enforcing policy:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "AllowAllUsersToListAccounts",
            "Effect": "Allow",
            "Action": [
                "iam:ListUsers",
                "iam:ListAccountAliases"
            ],
            "Resource": [
                "arn:aws:iam::517826968395:user/*"
            ]
        },
        {
            "Sid": "AllowIndividualUserToSeeTheirAccountInformation",
            "Effect": "Allow",
            "Action": [
                "iam:ChangePassword",
                "iam:CreateLoginProfile",
                "iam:DeleteLoginProfile",
                "iam:GetAccountPasswordPolicy",
                "iam:GetAccountSummary",
                "iam:GetLoginProfile",
                "iam:UpdateLoginProfile"
            ],
            "Resource": [
                "arn:aws:iam::517826968395:user/${aws:username}"
            ]
        },
        {
            "Sid": "AllowIndividualUserToListTheirMFA",
            "Effect": "Allow",
            "Action": [
                "iam:ListVirtualMFADevices",
                "iam:ListMFADevices"
            ],
            "Resource": "*"
        },
        {
            "Sid": "AllowIndividualUserToManageThierMFA",
            "Effect": "Allow",
            "Action": [
                "iam:CreateVirtualMFADevice",
                "iam:EnableMFADevice",
                "iam:ResyncMFADevice"
            ],
            "Resource": [
                "arn:aws:iam::517826968395:mfa/${aws:username}",
                "arn:aws:iam::517826968395:user/${aws:username}"
            ]
        },
        {
            "Sid": "DeactivatingMFANeedsMFA",
            "Effect": "Deny",
            "Action": [
                "iam:DeactivateMFADevice"
            ],
            "Resource": "*",
            "Condition": {
                "Bool": {
                    "aws:MultiFactorAuthPresent": "false"
                }
            }
        },
        {
            "Sid": "DoNotAllowAnythingOtherThanAboveUnlessMFAd",
            "Effect": "Deny",
            "NotAction": "iam:*",
            "Resource": "*",
            "Condition": {
                "Null": {
                    "aws:MultiFactorAuthAge": "true"
                }
            }
        }
    ]
}
tinnightcap commented 7 years ago

From @gozer on October 11, 2016 17:43

https://github.com/nubisproject/nubis-deploy/blob/master/modules/global/admins/main.tf#L147

tinnightcap commented 7 years ago

From @gene1wood on October 13, 2016 23:4

The remaining tasks to review this are :