s3tools / s3cmd

Official s3cmd repo -- Command line tool for managing S3 compatible storage services (including Amazon S3 and CloudFront).
https://s3tools.org/s3cmd
GNU General Public License v2.0
4.58k stars 906 forks source link

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

Closed twistedpair closed 12 years ago

twistedpair commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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

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

thejoshwolfe commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

@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 commented 4 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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