1175085 - Disable check_permissions for S3 hosting

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[Nick Thomas [:nthomas] (UTC+12)]

check_permissions
Issue: ssh to stage to verify file and directory permissions
Code: http://hg.mozilla.org/build/tools/file/default/scripts/release/stage-tasks.py#l68
Option 1: work with Cloud Services to ensure appropriate ACL on files
SWAG: 1 week
Option 2: Get LIST of files, and use http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGETacl.html. Requires parsing policy xml, too fragile ?
SWAG: 1 week

oremj, are we getting close to having ACL policies for buckets ? If they're simple we might deprecate this job, but if they're complicated it might pay for us to verify each time we do a release.

Comment 1

[Jeremy Orem [:oremj]]

For public buckets, I was hoping we could make them world readable. Does that work for you?

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

It's more the write/PUT access. If only the ffxbld key has that on a candidates bucket (eg contrib being elsewhere) then I'm not too worried, but if it's more complex it may be worth having regular checks.

Comment 3

[[e-mail] (:bhearsum)]

If the permissions are bucket-wide, I wonder if it even makes sense to do these checks as part of the release process. Maybe a nagios-style check is more appropriate?

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

Richard, I'm looking for some advice on how to verify some access permissions to an S3 bucket. The scenario is that CloudOps are implementing the buckets and access policies, and we would like to be thorough (see paranoid) and independently verify. Example use case - ensure only the IAM user responsible for firefox releases has access to firefox release files (leaving aside root-like access of the people running the systems).

The AWS Policy Simulator doesn't seem quite right, it looks like a front end to the internal engine that is used on each request. ie supply a user, set of policies, and optionally a resource, then test access to some set of permissions. I'm more interested in another angle of attack - given a resource, what users have a permission like s3:PutObject. Do you know if that is available ?

Comment 5

[Richard Weiss [:r2]]

Nick, I'm not aware of any automated tools that would do this, but it is likely there are some pen-test type tools out there. The standard practice for locking down a bucket they way you seem to want to do it is to:
1) Create an IAM role that has permissions for that bucket. (It can have other permissions as well - it doesn't matter.)
2) Assign the role to the person to whom you want to grant access.
3) Create a bucket policy granting access only to that role.

You need to create the role first because you need to put the arn of the role in the bucket policy. If you use this method you need to make sure that permissions to change IAM are locked down, otherwise someone with IAM permissions could assign themselves the role and gain access to the bucket. I suppose you could also create a bucket policy that would grant access to a particular IAM user and that might be more difficult to bypass.

There are other secure methods using cross account access, but these might not meet your needs if the user is in the same account.

I'd be happy to take a look at your policies and see if they have any obvious exposures.

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

oremj, how are permissions setup on buckets ? Is it a policy on the IAM role you created in bug 1190309, like Richard describes ? Or perhaps a policy on the bucket that we might be able to retrieve with boto.bucket.get_policy() ?

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

rail, any thoughts on what we should do here ? There's the 'should we still verify this' question, which seems like yes to me, but we're not in a strong position to do so. Ben suggests we may be better testing elsewhere (nagios), or perhaps InfoSec has global access to the policies and could do automated verification.

Comment 8

[Rail Aliiev [:rail]]

IIRC, our current check verifies:
1) all files are publicly accessible
2) the "contrib" directory has enough permissions to be written by 3rd party
3) other directories are writeable by us only

In S3 2) and 3) are not that relevant. In other words there is no need to check them every time, because they are global policies (I think). We still need to verify these from time to time. Maybe something like ansible rules (or something like that), which would report if the rules have been changed since last run?

To check 1) we can just try to HEAD (or GET with short a small range) all files. This way we may not need to grant any access to the checker.

Comment 9

[Jeremy Orem [:oremj]]

Each bucket does have a bucket policy, to grant public reads. The rest of the permissions are are IAM policies on various groups and users.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

https://aws.amazon.com/blogs/aws/aws-config-rules-dynamic-compliance-checking-for-cloud-resources/ may provide a way to check for permissions we know we need, triggered by config changes. Doesn't look it can prove the opposite though.

Comment 11

[Nick Thomas [:nthomas] (UTC+12)]

Never mind actually, no support for S3 yet.

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: [buildbotcustom] Disable permision checks by default

We don't have a solution here, so lets disable the old-style automated checks so that they don't fail.

Comment 13

[Rail Aliiev [:rail]]

Comment on attachment 8672568 [details] [diff] [review]
[buildbotcustom] Disable permision checks by default

Review of attachment 8672568 [details] [diff] [review]:
-----------------------------------------------------------------

::: process/release.py
@@ +1397,5 @@
>          ))
>          post_signing_builders.append(builderPrefix('%s_%s_updates' % (releaseConfig['productName'], releaseChannel)))
>  
>  
> +    if releaseConfig.get('enablePermissionCheck'):

Can you also remove all appearances of "disablePermissionCheck" in the configs?

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

Status - ready to go
To land at transition: patch for buildbotcustom; removing disablePermissionCheck from buildbot-configs is rolled into bug 1165405

Comment 15

[Rail Aliiev [:rail]]

Attachment: perm.diff

One more tweak to make push to mirrors work automatically (I had to force it in 42.0b9)

Comment 16

[Rail Aliiev [:rail]]

Comment on attachment 8678181 [details] [diff] [review]
perm.diff

https://hg.mozilla.org/build/buildbotcustom/rev/3d4e1078bcd9