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

Partition and restrict S3 paths across SFTP users #9

Merged
merged 6 commits into from
Aug 9, 2021

Conversation

bradj
Copy link
Contributor

@bradj bradj commented Aug 7, 2021

what

  • SFTP users should have their own folder in S3 bucket
  • SFTP users should not be able to move outside of their home directory
  • Each SFTP user gets their own IAM role and policy which only gives access to their S3 home directory

why

  • Easily distinguish who uploaded what
  • More security; prevents users from peering into what others have uploaded

references

@bradj bradj requested review from a team as code owners August 7, 2021 15:37
@bradj bradj requested a review from a team as a code owner August 7, 2021 15:37
@bradj bradj requested review from srhopkins and brcnblc August 7, 2021 15:37
variables.tf Outdated
@@ -14,6 +14,12 @@ variable "sftp_users" {
description = "List of SFTP usernames and public keys"
}

variable "sftp_restricted" {
Copy link
Member

Choose a reason for hiding this comment

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

Let's call it restricted home

Copy link
Member

Choose a reason for hiding this comment

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

home_directory_type would keep it consistent with the argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm leaning towards restricted_home because home_directory_type on the aws_transfer_user resource doesn't entirely control whether or not the user will be restricted to their home directory.

main.tf Outdated

name = module.iam_label.id
policy = join("", data.aws_iam_policy_document.allows_s3[*].json)
name = "${module.iam_label.id}-${each.value}"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this use a separate label module reference in case someone changes the delimiter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! oops

Copy link
Contributor

Choose a reason for hiding this comment

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

It would also be OK to replace the - with ${module.iam_label.delimiter}, but a new label module is better because it will also handle case conversion and forbidden character removal on each.value.

main.tf Show resolved Hide resolved
main.tf Show resolved Hide resolved
@bradj bradj merged commit b17ec48 into master Aug 9, 2021
@bradj bradj deleted the feat/restricted-s3-paths branch August 9, 2021 17:29
Nuru added a commit that referenced this pull request Jul 29, 2022
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.

5 participants