search

getkirby / ideas

This is the backlog of ideas and feature requests from the last two years. Use our new feedback platform to post your new ideas or vote on existing ideas.
https://feedback.getkirby.com
20 stars 0 forks source link

Disabling media generation for specific files #554

Open findthebug opened 4 years ago

findthebug commented 4 years ago

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.

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.

lukasbestle commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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").

findthebug commented 4 years ago

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?

lukasbestle commented 4 years ago

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:

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

findthebug commented 4 years ago

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.

lukasbestle commented 4 years ago

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 commented 4 years ago

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

texnixe commented 4 years ago

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.

lukasbestle commented 4 years ago

@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.

texnixe commented 4 years ago

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

Yes!

lukasbestle commented 4 years ago

Two of three parts are already done:

lukasbestle commented 4 years ago

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

findthebug commented 4 years ago

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

afbora commented 4 years ago

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