ideas icon indicating copy to clipboard operation
ideas copied to clipboard

Disabling media generation for specific files

Open findthebug opened this issue 4 years ago • 15 comments

Problem: We run into an issue because of the the core function of generating media objects from content files. In our case, we do store sensitive data in an .json file in a page content directory. The File should be hidden and not reachable over the frontend or any URL. We do not print the file in any way in the frontend, it's just for backend-download only.

  • If you never open the file in the frontend via url-> no media object will be created.
  • If you open the file via the panel -> media file will be created.

There is also a talk about this in the forum, but this is not an optimal solution, because it's just for the frontend and in my case not save for sensitive data. https://forum.getkirby.com/t/disabling-media-generation-for-specific-files/13010/2

Proposal: A clean solution would be to introduce some options:

Page option: (Prevent creation of media objects for the given template)

createMedia: true/false

Field option: (Prevent creation of media objects for given file-type, you can have 2 different file type in a page, so one is for secure data, the other for public)

createMedia: true/false

Site option: (Prevent creation of media objects via config)

  'media' => [
     'pages' => [
         'createMedia' => true
     ],
    'ignore' => [
         'template' => ['secret','download']
     ],
  ],

I see this is a big change, but i think is important to have a kind of safe harbor for storing sensitive data in a proper way.

findthebug avatar May 26 '20 08:05 findthebug

I agree that protection for files in the content directory is something we should have. However I'm not sure about the all-or-nothing configuration.

What about this: If the URL to a file is accessed from the site code (in a template, controller, snippet etc.), Kirby creates a "job file" in the media directory (inspired by how our thumbs work). When the file gets accessed over HTTP, Kirby checks if a job file exists, otherwise access is blocked. This way, files are only accessible if the specific file was whitelisted. Also it would be a completely automatic process without any need for configuration.

The only issue is: The Panel relies on the media folder for previews and for downloading files from the Panel. The Panel should however always be treated differently than the frontend if we take this seriously (meaning: accessing files in the Panel should be based on the user's permissions while accessing them from the frontend should be based on the job file whitelist). The only way this would be possible is serving the files and thumbs dynamically from the Panel and not storing them in the media folder. However that's a major change from how it works at the moment, so I'm not sure if it's viable.

@bastianallgeier @distantnative?

lukasbestle avatar Jun 01 '20 14:06 lukasbestle

Another alternative, inspired by https://github.com/getkirby/kirby/pull/2612: The URLs could include a generated token that can't be guessed without the configured or generated salt. Which would mean: As long as you don't actively output the URL in your templates, access to the files wouldn't be possible. As the Panel will be able to generate that token as well, there's nothing special we would need to do for the Panel.

lukasbestle avatar Jun 02 '20 15:06 lukasbestle

@findthebug Would you be fine with my second proposal (the non-guessable token in the media URL)? It would mean that the file would still be accessible via URL in theory, but not in practice because the URL can't be guessed (unless directory listing is enabled on the server, which should always be disabled anyway).

The advantages of this would be better performance (media requests don't have to be handled by the router), compatibility with CDNs (which wouldn't work at all when some files are protected by the router) and ease of use ("it just works").

lukasbestle avatar Jun 14 '20 09:06 lukasbestle

right now we have also blocked index listening and we do uuid filenames so in theory the same protection i think. if this comes native im satisfied and for most use cases this would be fine.

nevertheless: im not sure this solution will pass a proper security or pen test? maybe im a little skeptical about public files anyways. in theory and maybe also in practice this solutions is fine for most use case but think about this like, "would you protect your bank account data" with this solution?

findthebug avatar Jun 14 '20 10:06 findthebug

The question is: What is the alternative?

Sending all media requests through Kirby's router would IMO be the only fully secure implementation, but then people will use that together with a CDN and be confused why the CDN suddenly serves the private files anyway because it has cached the file that should only have been accessible by certain users.

but think about this like, "would you protect your bank account data" with this solution?

I agree – you wouldn't. But doesn't this mean that you shouldn't save your bank account data in files inside the content directory?

OK, maybe that's an extreme case, so maybe we should indeed have two layers of security for all data that is sensitive, but not as sensitive as bank account data:

  • Kirby should protect the URLs by default with the token solution.
  • And if there are files that should absolutely never be public, there should be an option in the file blueprint (I don't think this belongs in the page blueprint, the field config or the site config). If the option is enabled, the Panel will serve the backend downloads over a special route that is protected according to permissions (without using the media folder).

That would basically be what you originally proposed with the added bonus that all other files are also somewhat protected.

lukasbestle avatar Jun 14 '20 10:06 lukasbestle

bank account was just an example to point out how good a protection should work, but at the end it doesn't matter what kind of data you try to hide. as you mention, the token solution should be default in the first place.

I agree – you wouldn't. But doesn't this mean that you shouldn't save your bank account data in files inside the content directory?

true but i think we have to think about the concept of the content folder in general. e.g. the .txt files also not visible in the media folder but technically the txt's are also part of the content so we do mix here visibility anyways.

just thinking here without any tech in mind. whats about a second content folder like content_private? maybe there should be a option for files like:

privateMedia: true/false

so instead of putting the files to the public content folder, it's in the content_private one. you would have a better separation from a structure point of view, and you can just access the folder with given permission and bypassing media complete.

findthebug avatar Jun 14 '20 11:06 findthebug

The content folder is already private since Kirby 3, files inside it only become public once they are copied to the media folder. And that is something that could be hard-disabled using an option. So I think your createMedia suggestion solves this pretty well.

Or would there be another benefit of a second private content folder? Keep in mind that this would increase the system complexity by a lot.

lukasbestle avatar Jun 14 '20 14:06 lukasbestle

@bastianallgeier @distantnative Also looking forward to your opinion on this.

lukasbestle avatar Jun 14 '20 14:06 lukasbestle

The content folder is already private since Kirby 3

I thought that was only the case if you put the folder out of the web root? So you have an option to make it private, but not out of the box. Files in a publicly accessible content folder are still reachable via their URL, e.g. mydomain.de/content/1_photography/1_trees/monster-trees-in-the-fog.jpg.

texnixe avatar Jun 14 '20 14:06 texnixe

@texnixe You are right – I thought we already had a rule for that in the default .htaccess, but we still only block access to the text files by default. Because of the media folder in v3, users should however already be able to block access to all files inside content and we should probably make this the default in the .htaccess we ship with our kits.

lukasbestle avatar Jun 14 '20 14:06 lukasbestle

and we should probably make this the default in the .htaccess we ship with our kits.

Yes!

texnixe avatar Jun 14 '20 14:06 texnixe

Two of three parts are already done:

  • ✅ Block access to the full content folder in our default .htaccess https://github.com/getkirby/starterkit/pull/8
  • ✅ Use a salted content token for media directories by default https://github.com/getkirby/kirby/pull/2641
  • New createMedia and protect options for file blueprints to disable copying files to the media folder (useful for large files that shouldn't be copied) and to restrict file access to users with a files.read permission respectively

lukasbestle avatar Jun 15 '20 20:06 lukasbestle

The remaining three PRs that are required for the third feature we discussed are now also ready:

  • ✅ New files.read permission https://github.com/getkirby/kirby/pull/2644
  • $props param for Response::file()/download() https://github.com/getkirby/kirby/pull/2645
  • createMedia and protect options for file blueprints https://github.com/getkirby/kirby/pull/2646

lukasbestle avatar Jun 17 '20 19:06 lukasbestle

Looks like createMedia and protected are not in the 3.4.2 right?

findthebug avatar Aug 31 '20 13:08 findthebug

@findthebug Not yet, planned on 3.5.0 release https://github.com/getkirby/kirby/pull/2646

afbora avatar Aug 31 '20 21:08 afbora