Opened 16 years ago

Closed 16 years ago

#1873 closed New Feature (fixed)

Add option to change permissions applied with chmod commmand

Reported by: Wiktor Walc Owned by: Wiktor Walc
Priority: Normal Milestone: FCKeditor 2.6
Component: Server : Python Version:
Keywords: Confirmed Review+ Cc:

Description

For more info please take a look at: #950

Attachments (3)

1873.patch (1.7 KB) - added by Wiktor Walc 16 years ago.
1873_2.patch (2.3 KB) - added by Wiktor Walc 16 years ago.
1873_3.patch (2.5 KB) - added by Wiktor Walc 16 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 16 years ago by Wojciech Olchawa

Keywords: Confirmed added

Changed 16 years ago by Wiktor Walc

Attachment: 1873.patch added

comment:2 Changed 16 years ago by Wiktor Walc

Keywords: Review? added

A few notes abouth this patch:

perl and PHP connectors by default changed the file permissions to 0777. However python connector changed them to more restrictive 0755. So to maintain backwards compatibility, 0755 is still used in this connector.

comment:3 Changed 16 years ago by Alfonso Martínez de Lizarrondo

I know very little about permissions, but if 0755 is safer, could we assume that the previous 0777 was a bug and now set all the connectors by default to 0755 ?

If someone needs the more relaxed 0777 he can change it now with this set of patches

comment:4 Changed 16 years ago by Wiktor Walc

Thanks Alfonso for your comment. I also thought about it and I'm still not quite sure what's the best approach. 755 is still not 100% safe, because anyone can still read your documents... it depends on your system configuration what's the best setting.

Before we change 777 to 755, we should consider that the connector is disabled by default. So if anyone enables it, he agrees that he understood all settings. If we leave 777, we are sure that anyone will be able to upload files straight after enabling the connector and won't be confused that something doesn't work as expected.

comment:5 Changed 16 years ago by Alfonso Martínez de Lizarrondo

I always thought that the idea of putting something on a web server was to allow the people to see your content :-)

I don't really understand the risks of using 777, I've used very little Linux servers, and you can bet that many people won't know that either (think that there are just too many people that will use it without almost any knowledge about Linux or PHP), so if I enable the connector in a server I would leave it as is. Just think how many people does just set enabled = true in the connector without realizing that it means that it's possible that way for anyone to use it.

comment:6 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The same configuration should be used for the folder creation. So, it should also have a generic name, like "Chmod", with a generic comment explaining it.

comment:7 Changed 16 years ago by Wiktor Walc

The most commonly used permissions are 755 for folders and 644 for files, so making it configurable with just one variable may be a wrong idea.

I think we could use something similar to that in #1872 ($CHMOD_ON_FOLDER_CREATE).

Rethinking the problem with 755/777: if I were to decide, I would make all that stuff disabled by default (ChmodOnUpload and ChmodOnFolderCreate set to 0). This is the most universal solution. Currently, we set permissions to 777 to help people that have apache running as nobody and want to delete that file via FTP. But most of the users that live on a shared hosts don't need that and for them this is simply a bad idea.

"I always thought that the idea of putting something on a web server was to allow the people to see your content" - this is almost true. Somethimes you want only authorized users to be allowed to see your files. If you're on a shared host and set permissions to 755, anyone else that is on your server can access those files.

So to summarize, I would vote for setting the default values to*:

ChmodOnUpload = 0;
ChmodOnFolderCreate = 0;

Changed 16 years ago by Wiktor Walc

Attachment: 1873_2.patch added

comment:8 Changed 16 years ago by Wiktor Walc

Keywords: Review? added; Review- removed

Yet another comment: note that we are using "umask" (in each connector) every time we create directory and I didn't change that. I strongly believe that this approach is incorrect and when ChmodOnFolderCreate is set to 0, we should't use umask at all.

comment:9 Changed 16 years ago by Wiktor Walc

Keywords: Review? removed

Changed 16 years ago by Wiktor Walc

Attachment: 1873_3.patch added

comment:10 Changed 16 years ago by Wiktor Walc

Keywords: Review? added

Ok, this is the final solution for creating folders... if ChmodOnFolderCreate is set to anything other than 0, we use umask to disable the default user mask. If ChmodOnFolderCreate is undefined, we use the same default behaviour as in 2.5.1.

Here's the change: if ChmodOnFolderCreate is set to 0, we use the mkdir/makedirs calls without extra parameters, which theoretically means 0777... but user mask restrictions will be still applied, so most of the times it will be something less than 0777.

If anyone agree with that proposal, I'll include it also in PHP/Perl.

comment:11 in reply to:  10 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Replying to wwalc:

Here's the change: if ChmodOnFolderCreate is set to 0, we use the mkdir/makedirs calls without extra parameters, which theoretically means 0777... but user mask restrictions will be still applied, so most of the times it will be something less than 0777. If anyone agree with that proposal, I'll include it also in PHP/Perl.

That sounds good for me.

Please add the changelog on commit.

comment:12 Changed 16 years ago by Wiktor Walc

Resolution: fixed
Status: newclosed

Fixed with [1621] and [1622].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy