Backend Security Model #111
Comments
|
Hi @dereks. Seems you are busy with your contract, anyhow I would like to start implementing a new security model. I agree with your conclusions and wish to clarify some points:
|
|
@servocoder Hi, thanks for following up with me, and for moving this forward. Yesterday I worked on RichFilemanager (RFM) for several hours, and today I'm putting in a few hours on the duplicate configuration options. I expect to have Pull Requests for this work some time this week. I apologize for the delay. In regards to your questions:
Yes, the on each listed directory and file name. This type of basic function is necessary to protect common web-sensitive files, such as First, I think file glob wild cards should be supported, e.g., something similar to this Apache config for all This could be done via regular expressions, or via PHP's Second, I think we need an Auth API that is more flexible than just a single hard-coded list (or two hard-coded lists, If you can give me another week, I will do some research and propose a more formal "Auth" plugin API to allow users to configure external authentication and authorization, so that RFM could be easily used with pre-existing WordPress, Drupal, or corporate LDAP/ActiveDirectory user databases. But
CRUD and BREAD are not perfect models for a file manager. They apply more to database tables. For example, anyone with write permission can Create, Update, or Delete a file on a filesystem. And I don't think we will offer users the ability to Update files with partial content, by using However, I think those models are closer to what the backend should emulate than the existing
First, consider existing file system permissions:
I am using RFM with suPHP, so that the web server is running as the logged-in user. That allows me to fall back to the Owner-Group-Other permissions on my Linux filesystem for security. It also allows me to use complex ACLs, if I wanted to. Furthermore, I want to share my RFM root folder with other server protocols, such as Samba, WebDAV, and SSH/SFTP, so the files can also be accessed from desktop file managers and from Android devices. This requirement means I absolutely must depend on the underlying file system access control, without relying on RFM to protect files. But my configuration is not for everyone -- we should support deployment to a simple web host DocumentRoot where all files are owned by user apache, group apache. Also, we should support backend file systems that don't support the Unix permissions model. In summary:
So I propose:
//
// Set this to FALSE to allow file uploads, renames, moves, deletes:
//
['read_only'] = TRUE;
//
// These file globs are hidden and protected by RichFilemanager:
//
['always_exclude']['policy'] = ALLOW_ALL; // or DISALLOW_ALL
['always_exclude']['list'] => [
".htaccess",
"web.config",
"_thumbs",
".CDN_ACCESS_LOGS",
"*.php",
];
// Auth Plugin
//
// Configure RFM to use a 3rd-party callback for file access on a per-user basis
//
//TODO: Let me do some research this week. (There are APIs that do this already.)To answer your questions specifically:
Yes, in the core RFM server code, it should only need the single It should also check the underlying system for read/write access (like it does now), so that if the web server process can't access a particular file, that file is greyed out in the user interface.
No, trying to implement a full CRUD/BREAD/POSIX/ACL model is just asking for trouble. It requires a backend database for users, groups, passwords, and permissions, along with several admin pages. Let's just create an Auth Plugin API, with some kind of
All RFM server actions should check the Any of the write methods: create, upload, move/rename, delete should check All RFM server actions should use the Auth plugin callback, if configured, to authorize each individual action for each individual user. I will post a proposal for what that would look like within the next week or so. (I saw another PHP-based file manager that already had such an API, I need to find it and check it out.)
Sure, please let me know your questions! Also, everything I wrote here is just a proposal, please feel free to disagree or offer alternative ideas. Thanks again, I look forward to seeing these changes. |
|
Thank you for detailed answer and for keep working on this. I appreciate your efforts.
Supporting glob wild cards is a nice thing, however I think it should be considered as one of 2 lists which define permissions, as you suggested in the first post:
We also keep ['policy'] for both options:
Hm, at the moment we have
I definitely don't want to make thing too complex to support POSIX permissions or ACLs, the things must remain simple for end users and for maintainers. But I'm not sure that
That why I don't like idea to reduce
Replace is specific action, which is delete+create(upload). Actually this is too specific action and it can be removed. User can easily make delete + create in two steps. Not a big overhead for end user I believe.
General idea of my question is that I don't understand the idea behind grouping / renaming the existing methods according to the table that you provided in the first post (download, readfile, getimage -> read_file_contents and others). We can leave methods as is even if we decide to use new security model. Isn't it? |
Yes, we can. Please ignore that table. I was confused by the word "actions" and thought that also meant server "methods", and was trying to collapse the two into a single list of actions/methods for an easier to understand security model.
Ah, okay, I think I better understand what you are looking for. I have considered RFM as a pure file manager, functionally equivalent to Ubuntu's Nautilus, or Mac's Finder, or Android's ES File Explorer Pro, or Unix's shell commands (cp, mv, rm, etc.). Those file managers do not have features like "create/add, but no delete" and "modify/move but no create and no delete". In fact, I've never seen a file manager on any platform that offered those features. I think they are specific to a web-based file sharing application, where you want to easily share files with other people (or allow them to easily upload files to you). Consider that if your file manager only had "create/add, but no delete" then eventually your hard drive will fill up and you won't be able to use it anymore. You would need another "real" file manager to go in and delete old files. So for these features, RFM is more like a web collaboration tool than a traditional file manager.
For these kinds of collaboration-specific features, yes, I think you will need to define a "specific model" of configuration options for RFM. But this new "specific model" will apply only to the user interface features of RFM, not to the server's data security. Data security must boil down to a read or write permission. For example, consider the first use case you mentioned:
This is the original problem that started this thread. It conflates user interface (buttons) with data security. If you allow replace, you have automatically allowed delete, because (as you said):
So you can create a config option that will tell the Javascript to hide the "Delete" button, and show the "Replace" button, but at the end of the day the user still has write permission on the file(s) and he can destroy your data. As another example, if the user can move files, then they have write permission (and your files are not secure). Here I will destroy the organization of a file system and erase all file names from the data store, using only the move command: derek@toshiba:~/MyRootFolder$ ls -1
important_file1.doc
important_file2.pdf
junk_file.jpg
derek@toshiba:~/MyRootFolder$ mv important_file1.doc `uuidgen`
derek@toshiba:~/MyRootFolder$ mv important_file2.pdf `uuidgen`
derek@toshiba:~/MyRootFolder$ mv important_file3.jpg `uuidgen`
derek@toshiba:~/MyRootFolder$ ls -1
0124987e-0c1f-48f6-988d-005a64c3da80
3e8a9a37-4056-400d-aa47-a6ae829a7dfb
b7e74ed0-c34e-47e2-9571-6a8e4010319a
derek@toshiba:~/MyRootFolder$This is possible because I have write permission. However, if I only have read permission, I can't do any damage: derek@toshiba:~/MyRootFolder$ chmod -w ./
derek@toshiba:~/MyRootFolder$ mv important_file1.doc temp
mv: cannot move ‘important_file1.doc’ to ‘temp’: Permission denied
derek@toshiba:~/MyRootFolder$
Well, once again, if you can edit a file, you can automatically replace it with any bytes that you want, including just a zero-byte string. Either way, when it comes to data security, an attacker would have the write permission necessary to erase your data. Also, if you can copy a file, then (by definition) you have the ability to create a new file. By combining copy plus edit as you propose, the attacker can achieve the same result as a file upload. derek@toshiba:~/MyRootFolder$ cp important_file1.doc bad_file.doc
derek@toshiba:~/MyRootFolder$ vi bad_file.doc # Same result as just uploading bad_file.docYou can create an RFM config option that tells the Javascript to hide the "Replace", "Rename", and "Upload" buttons, but show the "Edit" button. This would discourage team collaborators from creating new files in your team's work flow. But at the end of the day the user still has write permission on the file(s), and an attacker can hand-craft a series of Ajax requests that achieves the upload, replace, or create action that you were trying to prevent.
Since both of your example use cases include copy and move, the files must be writable, and your data would be vulnerable to a user with bad intentions. If you want to restrict the user interface, to (for example) hide the Upload button but not the Copy button, then that would be an application-specific configuration setting. But that has nothing to do with data security. When it comes to security, it doesn't matter if I "Delete" a file instead of "Edit" it to have zero bytes content. It doesn't matter if I "Copy" and "Edit" dangerous file contents instead of just "Upload" those bytes. In my opinion, the backend connector server code must boil each server method down to a read or write permission. This could be done via a simple-minded global ['read_only'] = TRUE;or, alternatively, it could be done by something more complicated, like an SQL database lookup, or by checking the client's IP address. If you want to create a more collobarative user interface experience, then create some new client-side Javascript config options that will hide specific buttons (like the "Rename" or "Upload" buttons). |
Correct. RFM provides few more layers to manage files/folder than a pure file manager.
Sure, but since RFM is "web collaboration tool", as you said above, there can be some "admin" user which observe such cases and manage folders for "regular" users. There are lot of cases how it can be done and that is why I wish to make it flexible enough.
I can't disagree with the examples that your have described. They all are possible scenarios. So that I have decided to agree with you and add That would be a good compromise. What do you think? Please let me know if you are going to continue implement this stuff, so that we don't do the same work. I'm appreciate your intention to make RFM better in any case. |
Yes, confirmed. We hit a big milestone on my other project, and RFM has become my next priority. I am aiming to have a PR for the config options to you by Thursday, and a PR for harder security by this weekend.
Thank you. I appreciate your willingness to hear my ideas. You are an excellent Open Source project leader, and I think RFM will be very popular because of it.
Agreed, but this is getting into the realm of users, groups, passwords, permissions, and a backend database to store it all. Plus "admin" pages for adding or removing users, and setting their access level. Users will also need a "profile" page for changing passwords and updating their email address. And once you define all of this, RFM becomes much less flexible. For me, a major feature of RFM is specifically that it does not try to force any such schema onto my environment.
Rather than implement specific permissions (and users, and auth levels), I think there should be an API layer that makes it easy to integrate RFM with pre-existing user/permission databases. I would like to see RFM used with Wordpress, Drupal, corporate LDAP, Samba, PAM, Apache's auth modules, etc. Those environments each have their own definitions of users, groups, and permissions. The current auth() function does provide a global hook, but that simple function does not make it easy to answer the question, "Does this user have the read or write permission to do this request on this particular file?". The current auth() requires manually parsing out all GET request variables, and knowing what each server method does, to know whether or not this user has the desired permission. I think a proper auth API would clearly define each possible server method, along with all necessary arguments (e.g. a target path, or source and destination paths). By the way, this is why I think of "actions" and "server methods" as being the same thing -- from a security point of view, the actual server methods are all that matter, and any external auth library will need to know exactly what each method does.
Respectfully, I disagree. This is exactly what I am arguing against. The "actions" you listed are not really server-side "actions". They are User Interface workflows. There is no way "Upload" will ever be compatible with "Copy & Edit", as far as data security goes. They both require write permission -- it is a law of physics. And if an attacker has write permission, they can bypass your "actions" list by hand-crafting Ajax calls to ruin your data. Instead of thinking of these User Interface "actions" as "permissions", think of them as "buttons" -- because that is all they are. Trying to implement permissions-by-buttons (or more correctly, button visibility), will never work. I recommend adding new Javascript (client-side only) config options to enable or disable these buttons individually. That will give you the collaborative features you want (like preventing an Upload through the GUI, or preventing the renaming of files through the GUI), without giving the server admin a false (and meaningless) sense of security.
Ok, sounds good. I'm not particularly excited about the I will follow up later with a more detailed proposal for an Auth API that would allow tying into existing user/group/permission databases to decide read or write permission for each request. Thanks! |
Great! I'm going start review it on weekend in this case.
Absolutely not. We will keep things simple. I have confused you when said about "admin" user, I just meant some technical specialist who observe free space at the background, reveive alerts and can extend disk space. That is it.
I'm not aware of specific permissions for a particulat platforms. If you think that it may be useful and won't complicate things too much then I don't mind if you create another PR and well documented wiki article for this. The best way is to have it as a side/optional feature which is weakly integrated with existing code so that it won't become headache for a regular RFM users. The only thing I want to ask you is to be consistent and implement the features that we already discussed at first.
Technically you are right. In a perfect world it would be the same )
Ok, ok. You have persuaded me. Let's try and look how it goes.
We already have this
Perhaps you will come to some better solution during implementation, but it's ok for now.
Ok, go ahead. I am intrigued ) |
|
Just a quick update. I plan to split this work into two PRs. The first one will implement the The next PR will be to refactor the backend into read and write permissions, as discussed above. This will be a more major change to the backend. That will include a well-documented API that will define how an "Auth Plugin" can decide whether or not the current user can read or write the particular file paths being requested. There will be no definition of a "User", or "Group", or "Access Level". All of that will be left to the Plugin author. However, there will be a very clear definition of the method (upload, download, move, delete, etc.) and what the arguments are. (Usually there will just be a single file path argument, or sometimes two paths for "source" and "destination". Maybe "move" will allow for a list of multiple files into a single destination.) As part of that work, I hope to reduce the backend API into a shorter list of methods. The list of server methods will be the core of the new Auth API -- basically, the Auth API will be the same as the list of HTTP server methods, but with a documented list of arguments (in the form of function arguments with a defined position for each argument, instead of HTTP querystring variables). The API will also document how the backend filesystem will be modified, so that Auth Plugin authors know the meaning of each method. I think the new server method list will be shorter than the current list of methods. There are currently five different ways to read/download the contents of a file, and because of that, there is some duplicate code in the server. These redundant methods will complicate the Auth API and make it take more work to implement. I still need to review the Javascript to see if this is plausible. For example, do we really the need the ability to download file contents as Ajax, for in-browser "edit", and also as a regular file contents download? Do we really need a special download method for images, or can they be treated the same as any other file download? Clearly, if the server methods change, there will be work on the Javascript side too. I will document my proposed changes here before beginning coding work. Currently I am finishing the duplicate configs (almost done!), and then new global excludes lists. As always, feedback/comments welcome. Thanks! |
Please, be very careful with that. I know that some methods looks similar and some code is duplicated, however there are specific differences for each method, so explore them thoroughly before take a decision to merge / remove of any.
Can you see another way to get "renderable"/"editable" file contents? I definitely don't want to get content upon files listing. There may by tons of such files and will result in lags on listing. But, as always, I'm happy to hear your suggestions.
"download" is not the same as "readFile", at least in terms of headers. "editFile" is close to the "readFile" but they differ in the way of content output. Also each of above includes specific validation and security checks. Again, be attentive to details when optimizing those methods.
Great, looking forward for updates. |
|
Regarding the But now that I am working with it, I think they are terrible names. I don't know if that means "ALL in the list, or "ALL except the list"? (I know it's the latter one, but only because I've looked it up in the documentation.) The word "ALL" is not meaningful here, and my first instinct is that it means the opposite of what it actually means. As a security option, this definition should be obvious and intrinsic from the name. We should make it hard for the admin to make a bad assumption. Therefor, I suggest new names: ALLOW_LIST or DISALLOW_LIST. These names make it obvious because there is no question about what is being allowed or disallowed. This would apply to both the As always, comments/feedback welcome. Thanks! |
|
I have no objections. These are clear names. |
|
Re: new Glob is basically shell wildcards, and is easier to use. Any command-line user (and practically all server admins) have been exposed to globs. Regex is more powerful, and allows for more complex pattern-matching. However, only those familiar with scripting (like Perl, Python, or PHP programmers) have used regex, and it is a fairly complicated subject. I think globs are a better option for the /**
* Files excluded from listing, using REGEX.
*/
"excluded_files_REGEXP" => "/^\\./",
/**
* Folders excluded from listing, using REGEX.
*/
"excluded_dirs_REGEXP" => "/^\\./",It would be easy to argue that moving from regex to glob is a reduction in functionality, since regex is more powerful. I still think glob is the better choice, but I just want to document here that we are making a conscious decision to replace these regex lists with glob lists. We're not just blindly making RFP "less functional" by moving to glob. It is possible to offer a second set of lists (similar to these upload Finally, we could offer another config flag that would switch the For now, I am moving forward with the glob patterns (as discussed). I would be happy to work on some form of regex support as well, but I think we should wait for a user to request the feature. |
|
Glob is ok. I haven't changed my mind. All thoughts that I posted in this answer #111 (comment) are still actual. I just want to make sure that you are going to implement 2 lists: extension-based and glob-based, as we discussed above, are you? |
Yes, confirmed, I originally wanted only a glob list, with |
Do you think this really make sense? I can assume that it may be useful for someone, but for others it may turns into headache. I think this should be optional as it is in Git:: the case sensitivity of |
|
Hi @dereks, how it's going? |
|
@servocoder Hello! Yes, I've working on the source code. I'm sorry I didn't get it completed last weekend. This week is very busy (I am travelling), so I expect to have a PR sometime this weekend (April 1 or 2). I apologize for the delay. After reviewing the source code, I have revised the "model" and new Auth Plugin API. It will be much simpler than I originally described above. Instead of having an Auth Plugin callback for each server method -- which requires the plugin to know what methods there are, what arguments they take, and what permission is necessary -- there will just be a simple boolean The implementation will be very similar to the server's existing functions It will be up to the plugin author to verify any PHP session variables, or usernames, or groups, or SQL tables, etc. The only thing RFM needs is whether or not the current HTTP request has the desired read or write permission. Originally I thought each server method would have an Auth API plugin callback. But as I compared that to a shell prompt, I realized it did not make sense. On most computers, if I have shell access, I can use the file management commands cp, mv, rm, vi, etc. Those commands require read or write on the target files, but not special permissions to run one command or another. (Yes, there is the POSIX execute bit, but that is not generally used to restrict file management commands on a per-user basis. Generally speaking, servers do not use a model that says "this user has permission to do mv and cp, but not rm, dd, or vi".) This simpler model allows us to add, remove, rename, or consolidate server methods internally without affecting plugins. I am still hoping to consolidate some of the file read methods (I think editfile can eventually go, for example) but that consolidation will be completely independent of the security model changes. Finally, these new security checks will be wrapped in a utility function that also checks the new The code for this is not very complicated, but it does require a security audit of all the existing server code. I think I can have it done by Sunday. Thanks! |
|
Splendid idea! This approach is clear enough, I like when things are getting simple.
Sure, these things are not related to each other. It could be another PR, it will be up to you.
Great news! Looking forward for updates. |
|
@dereks I have started to implement new backend security model. Let me know when you are back and we will discuss the further steps. |
|
@servocoder I am working on this today and am scheduled to finish it. I expect to have a PR ready by end of day today, however, if you are also working on it then let's coordinate. Are you available via Google Hangouts, IRC, or some other instant messaging system and/or conference system? |
|
This is the audit list I am using to walk through the code. These are all the places the new permissions checks must be implemented:
|
|
Have you pulled last changes, released yesterday?
Not today, but feel free to pull your PR and I will merge the changes. |
|
BTW, Telegram is also insecure, although it is less malicious (they have been hacked through incompetent encryption, as opposed to Microsoft's snooping-by-design). http://gizmodo.com/why-you-should-stop-using-telegram-right-now-1782557415
https://eprint.iacr.org/2015/1177.pdf
https://www.wired.com/2016/08/hack-brief-hackers-breach-ultra-secure-messaging-app-telegram-iran/
All they had to do was use HTTPS. Instead they decided that they were smarter than 27 years of human experience (across billions of users), and rolled their own encryption. Google Hangouts is also not secure, because it does not do end-to-end encryption. So when I use Hangouts, I am really just choosing to trust one company (Google/Alphabet) over another (Microsoft). But I make that decision based on their published track record -- only one of those companies has pushed the HTTPS Everywhere initiative for several years now, and only one of them has anonymized server logs since 2007. For mostly-secure chat, one must run a private IRC server over HTTPS, or maybe use a public Off-The-Record (OTR) server. But that stuff doesn't come pre-installed with your new Android phone, and just try explaining the setup process to grandma. Even then, data like IP addresses and traffic patterns can be monitored by Internet service providers (and nation-states -- not a theoretical issue in my country). OK, sorry for the off-topic posts... back to work :) |
|
A quick note: For directories under Unix, the read
For now, I am checking if a dir is readable only by calling PHP's
I am just posting this comment here for now; after some testing I will decide if we also need to check |
|
Ok, thnaks for note. I want to add that ALLOW_LIST or DISALLOW_LIST should affect only Let me know, when you are going to make release. |
|
I had to add the protected function has_system_write_permission($filepath)
{
// In order to create an entry in a POSIX dir, it must have
// both `-w-` write and `--x` execute permissions.
//
// NOTE: Windows PHP doesn't support standard POSIX permissions.
if (is_dir($filepath) && !($this->php_os_is_windows())) {
return (is_writable($filepath) && is_executable($filepath));
}
return is_writable($filepath);
}
Hmm, I am confused by this... I currently have two separate ALLOW_LIST / DISALLOW_LIST settings, one for each list.
I agree. That is why I had two different config settings for Does this work? Here is the code that checks those two variables (noting that this has not really been tested yet, I am still coding): public function is_unrestricted($filepath) {
// First, check the extension:
$extension = pathinfo($filepath, PATHINFO_EXTENSION);
if($this->config['extensions']['policy'] == 'ALLOW_LIST') {
if(!in_array($extension, $this->config['extensions']['restrictions'])) {
// Not in the allowed list, so it's restricted.
return false;
}
}
else if($this->config['extensions']['policy'] == 'DISALLOW_LIST') {
if(in_array($extension, $this->config['extensions']['restrictions'])) {
// It's in the disallowed list, so it's restricted.
return false;
}
}
else {
// Invalid config option for 'policy'. Deny everything for safety.
return false;
}
// Next, check the filename against the glob patterns:
$basename = pathinfo($filepath, PATHINFO_BASENAME);
// (check for a match before applying the restriction logic)
$match_was_found = false;
foreach ($this->config['patterns']['restrictions'] as $pattern) {
if (fnmatch($pattern, $basename, FNM_CASEFOLD)) {
$match_was_found = true;
break; // Done.
}
}
if($this->config['patterns']['policy'] == 'ALLOW_LIST') {
if($match_was_found) {
// The $basename matched the allowed pattern list, so it's not restricted:
return true;
}
}
else if($this->config['patterns']['policy'] == 'DISALLOW_LIST') {
if($match_was_found) {
// The $basename matched the disallowed pattern list, so it's restricted:
return false;
}
}
else {
// Invalid config option for 'policy'. Deny everything for safety.
return false;
}
return false; // Should never get here.
}Note that I have not yet added support for the
My goal is sometime today, within the next 4 hours or so. |
|
@servocoder Ok, phew, I finally submitted a PR. This turned out to be much more work than I thought it would be. This has only been lightly tested so far. I wanted to get it posted for review as quickly as possible. In particular, I haven't tested image thumbnails. I will do that this weekend. In the meantime, please take a look at the architecture and general design, to see if it's what you had in mind. I submitted a PR to the dev branch, but honestly I recommend putting it into a standalone branch first. (I couldn't figure out how to request that using the GitHub GUI.) I expect there will be several bugfixes coming as I test this more over the coming days. I have only done light testing (but it seems to work pretty well so far). I still have additional fixes I'd like to make:
But I expect those will all come in separate PRs over the coming weeks. Let's keep the current discussion focused on the new backend security model. Comments/feedback welcome! Thanks! |
That's ok. It was a referring to this #111 (comment). Perhaps I misunderstood you, because I thought you had suggested to one policy for both lists.
Great! It's an importand milestone. Thnak you.
I have merged you PR to the "security" branch, use it for all further related commits.
Good idea, I have to review PR that you have commited at this weekend and, perhaps, merge code with yours (since I performed some security model modifications along with yours, before you let me know that you are working on it). Please, review, test and commit PR's with bugfixes and other stuff related to the security model for today and tomorrow, before I will review it. I understand that there may be fixes after review, it's ok. Also I would ask you to pay attention to the issue #140, it's the most important thing after security. I would appreciate if you help with server-side implementaion of Collabora Online for WIndows and Linux platforms. I will take care of client-side. |
Yeah, it's better to create new issue for each so we can discuss and release it with a separate PR(s)
I have planned to do this for a long time. However, let's postpone a comprehensive refactoring until all other backend stuff is implemented. Big thanks for your contribution! |
|
Hi @dereks !
I created new branch "refactored" and merged your changes into it. The work it progress, I'm going to complete to the next weekend. If you are going to make any commit that affect PHP connector, use "refactored" branch, please. Some features I have implemented:
If you have time and wilingness to help, I would ask to take a lead on implementation of Collabora online. In this case we will not interfere with each other working on the same code I hope. It's rather likely since I'm in phase of active refactoring. |
|
@servocoder Hi! I have been offline for a while, so I wanted to check in. There are two RFM features that are a high priority for me. I will try to get these implemented as soon as possible, but I am working on other things at the same time. Please be patient with me.
Number 1 is an important security protection. A simple HTML email with an IMG tag is enough to trigger loss of data until this is implemented. Number 2 is because I need RFM to be friendly for smart phones. The basic UI design is that the left-side tree panel will "auto-hide" if using a narrow phone screen. But can be un-hidden by clicking a small button on the upper-left. This can be seen in Android's "Material Design" guidelines. This will the user to toggle between tree-view and files view, like many other phone apps. This will be some user-interface javascript and should not affect the server. The second part of this is making the tree widget touchscreen friendly. JsTree has this feature built in, so I'd like to complete my work on JsTree first. (It is ~80% done.) Finally, JsTree supports a feature of auto-selecting all child elements when a directory is selected. This allows the user to easily move entire sub-directories to a different parent folder (instead of just files). I need this feature, as it allows users to organize the directory hierarchy. But this needs some extra support on the server PHP, so that the "move" action will support a directory move. So I need to get that working server-side first. These are my RFM priorities. Once these things are working I will need to do a deep-dive on a separate research project, so it will be a few weeks, or months, before I can look at the Collabora integration. If that is a critical feature then I recommend getting someone else to work on it. Hopefully in the fall I will be able to make some big contributions to RFM again. I will file Issue Tickets for the above items, and start to work on them this weekend. Thank you! |
|
Hi @dereks, I'm glad you are back. I have completed refactoring at the refactored branch. Note that PHP connector was moved to the separate repository: https://github.com/servocoder/RichFilemanager-PHP The installation should be performed via composer. In addition to the update notes, that I listed in the previous post, I have to note that I completely removed "replace" action. I have also included all of your security and configuration updates to the refactored branch and going to release it this weekend. The only reason why I have not merged it to the master branch yet is that I hadn't enough time to update wiki docs. The rest stuff is ready to be released. Please use the refactored branch as the base for all further PR's. It will be great if you make a review of the code to make sure that I didn't miss anything.
Good idea, security is in priority.
Ok, will see what it will result in. Just be sure that all current capabilities will be migrated in full.
Moving directories is already implemented at the server-side for PHP connector. It works great in the right panel, so it will require modifications at the client side solely. Again, use refactored branch since there were some updates related to items selection.
Your contribution is very valuable and always welcome. Either way you already did a lot. |
|
New backend security model is released in v2.4.0 PHP connector is moved to https://github.com/servocoder/RichFilemanager-PHP repository.
@dereks You can write articles with examples and I will add them to Wiki. |
Problem Statement
The backend security model has a few holes. It conflates permissions for User Interface actions, like "edit", with data storage actions, like "update". This makes it possible to bypass the intended restrictions configured by the admin.
Here is the list of (potentially) allowed actions from
BaseFilemanager.php:But this list of actions does not correlate to a persistent storage model, like CRUD, or BREAD. This makes it possible to bypass file restrictions just by using a different User Interface experience.
For example, the
editRestrictionsoption prevents me from editing a file in the browser with CodeMirror, but it does nothing to prevent me from downloading it, editing it locally, and uploading it back.As another example, downloading a file and uploading it with a new filename bypasses the
allowChangeExtensionsrestriction. Conversely, uploading a file and then renaming it with a new extension bypasses the['upload']['restrictions']list. And theallowNoExtensionoption is only checked in the server's rename code, not the server's upload code.This also impacts the User Interface. I want to display text files in the CodeMirror editor, as part of the read-only file preview (even if the file is not writable). CodeMirror has a
readOnlyoption, so I could do this. But the server's "edit" action, which is used to instantiate CodeMirror, requires that the user has write permission.Proposed Fix
I propose refactoring the backend action list, and security model. Here is a first draft of what I had in mind.
(Note, some of the new actions are simply proposed for a consistent naming convention.)
To clean up the security settings, I propose replacing
excluded_filesandexcluded_dirswith new options:"Editable" files, and images, would not be treated any differently on the server; if the user can read a file's contents, then the whether or not CodeMirror will display that file should be purely a matter of User Interface settings. If it's readable, it's readable, period, regardless of what method the user uses to access the file.
Impact and Priority
This would be a major, backwards-incompatible change that would affect both backend and frontend code. I'm willing to work on it and submit pull requests, however, it would not be compatible with any other branches, and it would become almost impossible to bring in updates from the other branches.
If the maintainers agree that this is priority, then I think we should put the effort into creating a formal design document. Once the backend API and security model has been hashed out in a document, then we could coordinate the development.
Thoughts or feedback?
The text was updated successfully, but these errors were encountered: