Opened 9 years ago

Closed 9 years ago

#1873 closed New Feature (fixed)

Add option to change permissions applied with chmod commmand

Reported by: wwalc Owned by: wwalc
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 wwalc 9 years ago.
1873_2.patch (2.3 KB) - added by wwalc 9 years ago.
1873_3.patch (2.5 KB) - added by wwalc 9 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by w.olchawa

  • Keywords Confirmed added

Changed 9 years ago by wwalc

comment:2 Changed 9 years ago by wwalc

  • 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 9 years ago by alfonsoml

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 9 years ago by wwalc

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 9 years ago by alfonsoml

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 9 years ago by fredck

  • 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 9 years ago by wwalc

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 9 years ago by wwalc

comment:8 Changed 9 years ago by wwalc

  • 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 9 years ago by wwalc

  • Keywords Review? removed

Changed 9 years ago by wwalc

comment:10 follow-up: Changed 9 years ago by wwalc

  • 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 9 years ago by fredck

  • 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 9 years ago by wwalc

  • Resolution set to fixed
  • Status changed from new to closed

Fixed with [1621] and [1622].

Note: See TracTickets for help on using tickets.
© 2003 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy