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

Don't store username in x-amz-meta-s3cmd-attrs header #67

Closed
twistedpair opened this issue Jul 11, 2012 · 12 comments
Closed

Don't store username in x-amz-meta-s3cmd-attrs header #67

twistedpair opened this issue Jul 11, 2012 · 12 comments

Comments

@twistedpair
Copy link

I'm a fan of not declaring the credentials my application is running as, however I cannot help but notice that it is placed in x-amz-meta-s3cmd-attrs.

This means anyone getting said files from CloudFront now has an edge on hacking my servers. Is there a reason this value is needed?

From S3:

x-amz-meta-s3cmd-attrs :
uid:1000/gname:marsupial/uname:wombat/gid:1000/mode:33204/mtime:1341974363/atime:1341974363/ctime:1341974525

Thanks.

@mludvig
Copy link
Contributor

mludvig commented Jul 11, 2012

Use --no-preserve to prevent storing of these informations. See s3cmd --help:

-p, --preserve Preserve filesystem attributes (mode, ownership, timestamps). Default for [sync] command.
--no-preserve Don't store FS attributes

Hope that helps.

M.

@twistedpair
Copy link
Author

Shame on me for not finding that. RTFM put nicely.

Keep up the great work! This utility makes my life so much easier. :)

@thejoshwolfe
Copy link

I am shocked that publishing my username to the world is the default with this utility. If I hadn't carefully poured over each entry in the headers, I would never have even noticed this was happening. This is a violation of secure by default. RTFM is not an acceptable solution to this issue.

@mastef
Copy link

mastef commented Oct 9, 2018

@fviard @mludvig this seems like a really bad default behaviour. If you google for x-amz-meta-s3cmd-attrs you'll see a lot of surprised users and security dev bounties to solve this.

s3cmd should not by default add such sensitive system meta data, unless the user wishes to do so.

@fviard
Copy link
Contributor

fviard commented Oct 9, 2018

@mastef By default, s3cmd backup your files to aws s3 in "private" not as "public" or as a "cloudfront" website.
If you are in one of these cases, it means that you did something for it.
In my opinion, the not "sane" default is more aws that share all the headers with a cloud front http get request.

I'm not sure of what can be done to improve this without reducing the default behavior that is useful to some. Maybe at least this can be added to the documentation i guess.

@mastef
Copy link

mastef commented Oct 9, 2018

@fviard as @thejoshwolfe mentioned this does kind of break "secure by default". A default use-case is to upload files to S3 with a certain ACL, and then have CloudFront serve them. That includes meta tags, since you need the Content-Type etc. This is a standard use-case of the system we're all working with here.

So although I understand your point that the S3/CF defaults could be better, we have to abide by/work with those defaults when creating tools for that system.

The s3cmd meta tags are extra information, and in this case contain privileged details. This fact is kind of hidden.

With hidden I mean it's not mentioned in the sync command description:

Synchronize a directory tree to S3 (checks files freshness using 
size and md5 checksum, unless overridden by options, see below)
s3cmd sync LOCAL_DIR s3://BUCKET[/PREFIX] or s3://BUCKET[/PREFIX] LOCAL_DIR

and it's not mentioned in the "how to sync" guide.

The --preserve documentation also doesn't mention that these file system attributes would be saved in the meta data.

  -p, --preserve        Preserve filesystem attributes (mode, ownership,
                        timestamps). Default for [sync] command.

So, this is a dangerous default and a bad surprise ( as you can see when people google for this ).

If somebody needs this behaviour, they would actively look for it in the documentation, and then add --preserve to the command. But as a default setting I would expect ownership information not to be saved in meta data.

@fviard
Copy link
Contributor

fviard commented Oct 9, 2018

Sorry, but the main usage of "sync" is to synchronise storage like for a backup.
If you push and then decide to "share", nothing assume that you don't want to share attributes like file modes.
And if you put open with cloudfront to host a website, then you are doing something particular.

Also to be noted that, even if we can consider it to be sensitive, the only annoying thing is the name of user and group, and that is not in itself enough to cause a breach to a system.
If you take a little bit of care, you might find it and some path in program / assets that are compiled/built, in pdf or image generated.

Good idea that "preserve default behavior" could also be added to the "sync" command description.

But I don't agree with you on:

The --preserve documentation also doesn't mention that these file system attributes would be saved in the meta data.

It is clearly written:

Preserve filesystem attributes (mode, ownership, timestamps)

Maybe you are not familiar with linux/unix and good improvemnt could be to replace that by "owner id, group id"?

@mastef
Copy link

mastef commented Oct 9, 2018

@fviard Please read my comment carefully. The documentation doesn't mention that it's stored in the meta data.

Uid, username and group knowledge combined with another exploit like heartbleed are a vulnerability.

If you scroll up you can see how people feel about this - if you google it you can also see that people classify is as a vulnerability. Everything that needed to be said has been said, and it's up to you to make a decision.

Security first, or convenience for a small user-base that needs this particular feature? Your decision.

@Skyfold
Copy link

Skyfold commented Aug 22, 2020

8 years later and developers like me are still appalled by this default. Say what you like, but sharing your local username and file permissions publicly is a terrible default. If the user is using the --acl-public there is no justification for exposing their local filesystem attributes. Its just bad security practice.

rvanlaar added a commit to rvanlaar/s3cmd that referenced this issue May 15, 2021
Sync by default preserves the file permissions, uid, gid and username in the `x-amz-meta-s3cmd-attrs` meta data field. 
This change documents that. Especially since a lot of people are surprised by this default. 

There was some discussion about this in: s3tools#67
@rvanlaar
Copy link
Contributor

The --preserve default surprised me.

It resulted in write errors when syncing the s3 bucket to another host.
I hope a small change in the documentation can make more people aware of it.

Could there also be information added on how to remove the x-amz-meta-s3cmd-attrs attribute?

@hmt
Copy link

hmt commented May 15, 2021

By accident I stumbled over this too. It should be more prominent in the docs to warn users. Since I'm using s3 with an nginx reverse proxy I added this line to keep the meta data but not send it over the wire:

proxy_hide_header x-amz-meta-s3cmd-attrs

@joeaguy
Copy link

joeaguy commented Aug 30, 2022

I find the --preserve option really helpful, but I wish it was more fine grained.

The storing and setting of the user and group name causes errors when syncing to hosts that don't have the same users. I still want to keep the other attributes, like modified date and permissions, but not sync the user and group names.

Since the default behavior of --preserve syncing everything would be a breaking change, providing a bunch of --no-preserve-* options might be a more acceptable approach to the maintainers.

--no-preserve-user
--no-preserve-group
--no-preserve-modified
--no-preserve-perms

...etc

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

No branches or pull requests

9 participants