Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply policy resource restrictions for file extensions #2842

Merged
merged 5 commits into from
Jun 15, 2023

Conversation

prakashsvmx
Copy link
Member

@prakashsvmx prakashsvmx commented May 30, 2023

Fixes #2838

Sample policy to test:
( Allow and deny both actions are used)

Policy 1
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "s3:ListBucket",
                "s3:DeleteObject",
                "s3:GetObject",
                "s3:GetObjectVersion",
                "s3:GetObjectVersionTagging",
                "s3:ListAllMyBuckets"
            ],
            "Resource": [
                "arn:aws:s3:::mybucket/*"
            ]
        },
        {
            "Effect": "Allow",
            "Action": [
                "s3:PutObject"
            ],
            "Resource": [
                "arn:aws:s3:::mybucket/*.jpg",
                "arn:aws:s3:::mybucket/anything/*",
                "arn:aws:s3:::mybucket/jpg/*.jpg",
                "arn:aws:s3:::mybucket/l1/l2/*.txt",
                "arn:aws:s3:::mybucket/png/*.png",
                "arn:aws:s3:::mybucket/txt/*.doc",
                "arn:aws:s3:::mybucket/txt/*.txt"
            ]
        }
    ]
}
Policy 2
{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "s3:DeleteObject",
                "s3:GetObject",
                "s3:GetObjectVersion",
                "s3:GetObjectVersionTagging",
                "s3:ListAllMyBuckets",
                "s3:ListBucket"
            ],
            "Resource": [
                "arn:aws:s3:::mybucket/*"
            ]
        },
        {
            "Effect": "Allow",
            "Action": [
                "s3:PutObject"
            ],
            "Resource": [
                "arn:aws:s3:::mybucket/txt/*.txt",
                "arn:aws:s3:::mybucket/*.jpg",
                "arn:aws:s3:::mybucket/anything/*",
                "arn:aws:s3:::mybucket/jpg/*.jpg",
                "arn:aws:s3:::mybucket/l1/l2/*.txt",
                "arn:aws:s3:::mybucket/png/*.png",
                "arn:aws:s3:::mybucket/txt/*.doc"
            ]
        },
        {
            "Effect": "Deny",
            "Action": [
                "s3:PutObject"
            ],
            "Resource": [
                "arn:aws:s3:::mybucket/txt/*.doc",
                "arn:aws:s3:::mybucket/txt/*.txt"
            ]
        }
    ]
}

Testing:

CI=true MINIO_ROOT_USER=minio MINIO_ROOT_PASSWORD=minio123 minio server /tmp/site1{1...4} --address ":22000" --console-address ":9025"

CONSOLE_DEV_MODE=on CONSOLE_MINIO_SERVER=http://localhost:22000 ./console server

  • Allow/Deny file extensions on a root level,
  • Allow/Deny at prefix level
  • Drag and Drop individual files
  • Drag and drop folders ( only allowed extensions are uploaded )

@prakashsvmx prakashsvmx marked this pull request as draft May 30, 2023 10:04
@prakashsvmx prakashsvmx marked this pull request as ready for review May 30, 2023 13:08
@bexsoft bexsoft changed the title apply policy resource restrictions for file extensions Apply policy resource restrictions for file extensions Jun 2, 2023
Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an issue if I try to upload a file using drag and drop:

2023-06-02 12-36-01 2023-06-02 12_38_09

@prakashsvmx
Copy link
Member Author

Thank you @bexsoft 👍 . I have added logs and improved the dnd handling to accept one or more files.

@dvaldivia
Copy link
Collaborator

there's a warning @prakashsvmx

@prakashsvmx
Copy link
Member Author

@dvaldivia thank you, i have updated the type reference.

Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I try to upload a file with the extension .JPG (Uppercase), an error is thrown

Screenshot 2023-06-14 at 19 17 59

Trying to do drag and drop, I see the following issue in any case:

Screenshot 2023-06-14 at 19 18 47

If I try to create a new subpath inside anything folder, button is not enabled
Screenshot 2023-06-14 at 19 26 28

@prakashsvmx
Copy link
Member Author

@bexsoft

  1. If I try to upload a file with the extension .JPG (Uppercase), an error is thrown

it is working as expected. test with mc

  1. Trying to do drag and drop, I see the following issue in any case:

Fixed

  1. If I try to create a new subpath inside anything folder, button is not enabled

it is working as per master/release. ( in master/release as well it is disabled)

➜ mc cp 1.png fxt/mybucket/png                 
...-file-types/1.png: 4 B / 4 B ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 501 B/s 0s 

# Copy PNG ( uppercase extension)

 ➜ mc cp 2.PNG fxt/mybucket/png/       
mc: <ERROR> Failed to copy `/home/prakash/Downloads/test-all-file-types/2.PNG`. Insufficient permissions to access this path `http://localhost:22000/mybucket/png/2.PNG`

 ➜ mc ls fxt/mybucket/png/          
[2023-06-15 09:48:22 IST]     4B STANDARD 1.png
[2023-06-15 08:50:22 IST] 9.5KiB STANDARD image (4).png

Copy link
Collaborator

@bexsoft bexsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewing

@cesnietor
Copy link
Collaborator

cesnietor commented Jun 15, 2023

@prakashsvmx I was also expecing a similar thing as @bexsoft.
And I think we should solve the following:
Since JPG (uppercase) is not allowed, it shouldn't allow you to select the file in the browser, similar to how others are not allowed (to keep consistency), otherwise it's letting me know that it's allowed but then the app fails. This however can be an enhancement.

Screenshot 2023-06-15 at 10 03 57 AM

Copy link
Collaborator

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cesnietor cesnietor merged commit 864cf7a into minio:master Jun 15, 2023
@cesnietor
Copy link
Collaborator

@prakashsvmx I was also expecing a similar thing as @bexsoft. And I think we should solve the following: Since JPG (uppercase) is not allowed, it shouldn't allow you to select the file in the browser, similar to how others are not allowed (to keep consistency), otherwise it's letting me know that it's allowed but then the app fails. This however can be an enhancement.

So yes I was verifying also with AWS and they do allow to select all types of files but then it would only be failing at server level once it actually tries to upload it.
Screenshot 2023-06-15 at 12 29 22 PM
Screenshot 2023-06-15 at 11 15 03 AM

@prakashsvmx
Copy link
Member Author

Thank you @cesnietor 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restricting s3:PutObject in IAM policy doesn't work in UI
5 participants