About audited Directus version.
It has been cloned from suite repo.
Latest commit https://github.com/directus/directus/commit/1d151a9034514e3f2ec1c80001e7c5fffdef2d4e
Unauthenticated attackers can bypass the application's authentication enforcement and directly access internal components, effectively circumventing the authentication credentials validation phase.
An attacker can get an access to the possible sensitive files.
Forceful Browsing attacks (direct access to internal components) can be used to exploit incomplete authentication enforcement in the server-side application. The forceful browsing technique enables attackers to access internal sensitive components, without undergoing the application authentication credentials validation phase.
Through the following link it is possible to get an access to application images that should be accessible after the authentication.
https://xxxxxxxx/uploads/_/originals/photo2_fullsize.jpg

Fixes security hole.
Maybe, with help/guidance from Directus team.
Hmm, there is the option that you can enable/disable public access to the files collection but I guess it was forgotten to add these rules to the thumbnailer too.
Also, isnt this the same issue as in #986 ?
The app side loads these images using the thumbnailer which would be as described in the other issue.
Also, isnt this the same issue as in #986 ?
Yeah, kinda. I added this issue just because security department suggested different solutions("How do you think this should be implemented" section) from referenced one.
@benhaynes This issue is identical to #986, to resolve this we have to restrict files directly being accessed and implement a script in which files are restricted for logged in user only, and we have to place the script API side where the files are being accessed.
Makes sense. Will this effect caching, CDNs, latency, etc? Are there any drawbacks to this approach? If so, we should look into making it optional β especially since we'll be switching to UUID naming, which isn't "guessable" (part of the problem).
Any estimate on how long this will take to implement @itsmerhp?
ping @rijkvanzanten
@benhaynes Yes, It will effect on caching, CDNs, and latency.
According to approch provided by @itsmerhp
We should give an option for each image for the access permissions.
For example,
the logo doesn't need any access permission. So one should set the public permission for logo file.
If you set the private permission then it will require token to token access the file.
We also need to increase (or make customizable) the ttl time for access_token as currently, we have 5 min which is very small for CDN and other caching services.
We may think for a special token which only uses for accessing the image.
According to me, we should implement the UUID first,
UUID supported (give an option in the to use the UUID in the file name) @rijkvanzanten Let me know your thoughts. So we can finalize the approach.
Thanks @hemratna β that seems like a great approach. I agree that we should start with UUID since that partially solves the problem. After that we can write the permission gateway to secure files, which I think should be off by default (so files are public).
With the individual file permissions, files would still need to go through the permission gateway to check if each are public/private, right? Would we want a global option so we can completely ignore that check if the admin sets all files to public?
@benhaynes We can start with UUID for the file name, which is also configurable by giving the options in the admin panel.
Regarding the file permission, We will give a global option for access permission.
@benhaynes
As @hemratna suggested, we will have a toggle button in APP to ON / OFF UUID for the file name.
Question: What should be the default file naming option UUID or Original name?
Instead of a toggle, let's use a Dropdown of options for file name. That will be more future-proof if we want to add more options in the future.
It looks like UUID will be the file name convention... by a hair!!
https://twitter.com/directus/status/1139618994879696897
It will be partially fixed with the development of #337
@bjgajjar I think it fixes this fully actually π Having UUIDs for filenames fixes this problem. Changing the filenaming from UUID to a less-safe option is a user decision.
@rijkvanzanten But I think this will not resolve our private permission issue. Am I right?
Ah good point. I got confused by the other issue that references the "guessability" of the names
@benhaynes @hemratna
If we set global permission regarding file permission, then a user can either permit all the files to the public or none.
I have a few questions in it:
Question 1: Are we going to facilitate global permission user specific or it will be applicable for the entire system? For example: If a user has set global files permission to private then only that user is going to access, not even admins? Or if global permission set to private then all the users of that system can access all the files but not public?
Question 2: What if admin wants to give files permissions to the specific role's all users, but not public or other users?
Regarding file permission, we can give role-based permissions also, but as per me, there seem two problems in it.
Problem 1: All files are storing in the directus_files table and I think for core collections we don't have "mine" option and due to that we can't restrict that roles can see and modify their files only.
Problem 2: For example File interface has been used in a collection as a field and for that collection, a role has not permission but for File Library(directus_files) that role has permission, then in that case how to restrict the listing of files of that collection in file directory?
We could make this a global setting, but I think it make more sense to have this be a configuration option for the storage adapter. That way you could have all local files be private, but have all s3 file be public. That seems like a good amount of granularity... especially when we allow for _choosing_ the adapter per upload (later).
https://example.com/uploads/[project]/files/[id]/datauuid for the directus_files.filename` so it's different from the item's primary key. That way we can still refer to the File in the App/API by the primary key, but you can't guess its URL from that. https://example.com/uploads/[project]/originals/image.pngSet for the each storage adapter, regardless of the role/user. Either way permissions would decide if you can see the asset through the App... but if set to public then you could still view the asset after logging out (since you know the actual asset URL).
You can still do this for public and private files... but public files can still be accessed when the user is not authenticated (if they know the exact asset URL).
We should use the existing directus_files.uploaded_by field for this mine permission. This seems like a _different_ ticket... so let's wait on this. The focus here is allowing files to be private at the storage adapter level.
To keep things simple, I think we keep everything the same... but if you do not have access to Directus Files, then you're "Choose Existing Files" modal would be empty, and any relationships to files you don't have permission to would be NULL (or however this works for other collections/data now).
Does all this make sense?
TL;DR: We should add the ability to make files private at the storage adapter level, and those files get routed through a gateway that checks auth/permission. Any other features should be in a new ticket.
Global vs Storage Adapter
We could make this a global setting, but I think it make more sense to have this be a configuration option for the storage adapter. That way you could have all
localfiles be private, but have alls3file be public. That seems like a good amount of granularity... especially when we allow for _choosing_ the adapter per upload (later).
Just to make things more clear, We also need to give support for the multiple adapters of the same storage type.
For example, One can have two S3 bucket one used for public files and one for private files, or S3 bucket for private files and local for public files.
How this works...
- Private β If a storage adapter is set to private, then all asset requests are routed through a gateway that checks auth/permission and the true asset URL/UUID is never seen. Perhaps something that extends the API files endpoint:
Instead of writing gateway, can we use AWS S3 ACL
Benefits.
Writing our own gateway has some downsides,
Implementing the ONLY AWS S3 ACL will not support private file mode for local adapter.
Question 1
Set for the each storage adapter, regardless of the role/user. Either way permissions would decide if you can see the asset through the App... but if set to public then you could still view the asset after logging out (since you know the actual asset URL).
Agreed, In the future, we might require an option for role wise permission, For example, admin can see all the media, but user can only see their's media.
If the media is uploaded to storge which is set to private, and the current user has permission to access the file,
append_storage_information function https://github.com/directus/api/blob/master/src/helpers/file.php#L99 will generate S3 URL with append extra query params like X-Amz-Security-Token, X-Amz-Expires and other required information.
In case the user doesn't have permission this will return a blank string.
We also use the same concept for all the other file adapter which support inbuilt ACL like DigitalOcean Spaces, Google Cloud Storage, etc.
Thanks @hemratna! It seems like there are some great benefits to using the S3 ACL. Maybe we should have a core (local) gateway as a fallback, but progressive enhancements for other storage adapters (eg: S3)?
We could start with whichever is easier (S3 ACL or local gateway) and to the other later. The important thing is having support for private files somewhere.
@rijkvanzanten β thoughts on this approach?
We've discussed serving files _through_ the API instead of relying on the webserver before (in that case to figure out the CORS config). Doing that would also allow us to make files actually private by relying on the Directus API ACL.
Do you have a sense of time needed to integrate each of these options? @hemratna
Perhaps we should start with the global/local/fallback/default with an API endpoint that returns actual files (based on auth/acl)? Once we have that we could look into progressive enhancements for S3, etc.
Thoughts??
@itsmerhp Any thoughts on the timeline?
@directus/api-team
Hello @benhaynes @hemratna
As discussed in the above comments,
Please let me know if I am missing anything for initial implementation.
There will be two settings options for a storage adaptor
- Private: Only user who has uploaded file can only access the file, including logo image.
- Public: Files will be accessible publicly, not even login needed for it.
Can we also support role for private? So it's more in line with regular permissions none / mine / role / full
@itsmerhp
I like that @rijkvanzanten β but then would we also want some sort of none? Or does that not make sense for files?
Also, remember that this is a setting for the whole Storage Adapter! :)
That makes sense to me. A user role can have permission to create files, but not read them. This is the same with the ability to create new items in the CMS, but not read them (useful for user submissions)
Ahh, true! Forgot about those external apps. π
Hello @benhaynes
I agree with @rijkvanzanten, we should set role-wise permissions for files and existing permission logic should be used to allow/restrict file access.
If we use role-wise permission(App > Roles & Permissions) for files accessibility, then public/private can't be applied storage adaptor wise(for eg. can't set one local storage public and another private), permissions will be applied for all the files of all the local storage adaptors.
Approx Timeline: 40 hours
Perfect β thank you @itsmerhp!
This all sounds good, and the timeline works... but let's wait on starting this until the App is "stable". We have a much higher ticket count on the App and we should get most of those resolved before starting in on a bigger overhaul like this one.
In the meantime, if an external dev wants to give it a try we can help guide them and review the PR! But internally we need to triage the _whole_ platform first.
Sound good?
Currently, the AWS S3 ACL for the private file is also not working. Please consider adding the support for this.
Most helpful comment
Instead of a toggle, let's use a Dropdown of options for file name. That will be more future-proof if we want to add more options in the future.
It looks like
UUIDwill be the file name convention... by a hair!!https://twitter.com/directus/status/1139618994879696897