Secure Uploads

[rmbolger (Ryan Bolger)]

I’m not crazy pants for thinking it’s weird that post text is protected but attachments are not though, right?

@pfaffman

If your users see sharing links to your assets they could just as well be downloading them to share them. It doesn’t seem like a big problem.

There’s a big difference between intentional and accidental sharing though. Obviously, there’s no real way to prevent the former. But the latter can happen right now simply because people don’t understand the underlying tech. Consider email notifications for posts that include links to attached images getting innocently forwarded to non-members. An option to redact attachments in emails that’s not tied to the secure media functionality might be a good alternative there.

Even when people do intentionally share attachment links, they might have simply forgotten that it was from a protected category. But in an ideal world, the attachment link should be worthless to anyone without access to that category.

A current or future bug in one of the S3 implementations could also potentially allow for anonymous enumeration of the resources in a bucket or the ability to guess and brute force object URLs. In an ideal world, I’d want my on-prem S3 interface to be firewalled off from the internet at large so only the Discourse server can reach it directly.

[sam (Sam Saffron)]

@rmbolger

I’m not crazy pants for thinking it’s weird that post text is protected but attachments are not though, right?

Not crazy, implementing secure uploads for non s3 setups is certainly not something I am against building. It is just that we have no urgency or push to build the feature and the change is non trivial.

We occasionally have interns and audition projects, this is certainly a type of project that could fit into one of those buckets.

[rmbolger (Ryan Bolger)]

@rmbolger

Would it be difficult to support a third bucket for secure uploads? Public uploads go in the public bucket, secure uploads go in the secure bucket, and no individual object ACLS are needed. And if the security status of an object changes, it could just be moved between buckets?

Any thoughts on the feasibility of this as an easier path for secure media on not-AWS-S3 storage?

[pfaffman (Jay Pfaffman)]

I’ve got a quick hack that I think would provide secure uploads for login-required sites.

Basically, you set up NGINX Docs | Authentication Based on Subrequest Result for uploads. The only issue is that I can’t find a URL that returns a 403/401 when login-required is turned on, so accessing an upload when you’re not logged in gives a 500 error. This would only happen if someone had an upload URL and tried to access it when they weren’t logged in, so it doesn’t seem that bad.

It’s something like this:

# JP
    location = /auth {
        internal;
        proxy_pass http://discourse/categories;
        proxy_pass_request_body off;
        proxy_set_header Content-Length "";
        proxy_set_header X-Original-URI $request_uri;
    } 
    # END JP
    location ~ ^/uploads/ {

      auth_request /auth; #$JP
      # NOTE: it is really annoying that we can't just define headers
      # at the top level and inherit.
      #

[martin (Martin Brennan)]

We have no plans to make distinct bucket functionality for secure uploads, nor do we have plans to make secure uploads work with non-S3 setups or setups without ACLs. We may look at doing this in the future if there is sufficient demand for this work to a point where it makes sense for us to devote time and resources to this effort.

[HashHackers]

why its using https://hashhackersforum.s3.us-east-2.amazonaws.com/optimized/2X/3/3204e85df407adfce19e105308248aee8b3b3f57_2_690x424.png?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAU5WBGG5Z7FNHQEIS%2F20210415%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20210415T025617Z&X-Amz-Expires=300&X-Amz-SignedHeaders=host&X-Amz-Signature=15c9118929ccd9c24a9594ab02a47e900269b9d921d41679a317e29d6174c2bc
instead of https://forum.cdn.hashhackers.com/optimized/2X/3/3204e85df407adfce19e105308248aee8b3b3f57_2_690x424.png?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAU5WBGG5Z7FNHQEIS%2F20210415%2Fus-east-2%2Fs3%2Faws4_request&X-Amz-Date=20210415T025617Z&X-Amz-Expires=300&X-Amz-SignedHeaders=host&X-Amz-Signature=15c9118929ccd9c24a9594ab02a47e900269b9d921d41679a317e29d6174c2bc

while i’ve entered CDN URL correctly.

[Falco]

It won’t use a CDN if you enable “Secure Media Uploads”.

[bigfudge (Ben)]

One thing I’ve noticed is that the admin section warns that

The server is configured to upload files to S3, but there is no S3 CDN configured. This can lead to expensive S3 costs and slower site performance. See “Using Object Storage for Uploads” to learn more.

I don’t mind ignoring it, but we have secure media enabled which means it’s not possible to use a CDN. This message probably shouldn’t be shown on sites with secure_media enabled.

[Falco]

@bigfudge

I don’t mind ignoring it, but we have secure media enabled which means it’s not possible to use a CDN.

A CDN can still be used for all the JS assets, making your site a lot faster to render for users around the world.

[bigfudge (Ben)]

That’s cool. It wasn’t clear to me that discourse would treat the static assets differently. I think I need to read all the threads here again.

[danilogit (Danilo L.)]

After I upgraded to 2.8.0.beta1 version, my badges using authenticated S3 Urls are not working. It was OK before the upgrade.

I checked the table uploads and the value for column secure is true.

Even after uploading a file using the option New Badge the image preview was kept empty.

No problems with other files using S3 url so far.

Any help? Thank you!

[martin (Martin Brennan)]

Ah…yet another place that is making secure uploads that shouldn’t be. Badges should not be marked secure because they are essentially public images, like avatars, category logos, and other things. I will need to do a fix to make sure that badge images are not marked secure and a migration script that fixes ones that are already secure.

I am surprised this was working for you at all, nothing in 2.8.0 should have affected this.

[danilogit (Danilo L.)]

Hello @martin

Thank you for your reply. This is my first time using Ruby and I am delighted how clear this language can be. After some hours debugging I believe I found where to look. I guess I did the opposite you said I put some lines in the Badge model and now it can load the images. I also see that we have a flag for_site_setting there. I believe it relies on this information to adjust the ACL for objects on S3, and set false for that column.

app/models/badge.rb

  def image_url
    if image_upload_id.present?
      return upload_cdn_path(image_upload.url) if !image_upload.url.include?(SiteSetting.Upload.absolute_base_url)
      uri = URI.parse(image_upload.url)
      Rails.application.routes.url_for(
        controller: "uploads",
        action: "show_secure",
        path: uri.path[1..-1],
        only_path: true
      )
    end
  end

I will have a look what will change in the next upgrade to learn more about that.
Could you tell me what is the best version to use in production ?

Thank you!

I hope I can contribute more in the code base in the future.

[martin (Martin Brennan)]

Hey @danilogit,

The change you made to the badge model will certainly work, and it’s great that you were able to put that together the first time you used ruby! Though I will still go ahead and fix the core issue where badges shouldn’t be marked as secure.

@danilogit

I will have a look what will change in the next upgrade to learn more about that.
Could you tell me what is the best version to use in production ?

Our tests-passed branch is the best version to use in production because that will get all the latest fixes as soon as they are available, though some people may prefer to stay on the beta branch which gets fixes and changes every few weeks or so.

I’ll let you know when I have the fix for badges being marked as secure done.

[martin (Martin Brennan)]

I finished a fix for this issue today:

FIX: Do not mark badge image uploads as secure

cc @danilogit

[bigfudge (Ben)]

I just posted this,

S3 Errors Uploading Profile Photos

Can confirm that when I try to upload an avatar the progress bar updates, and gets to 100%, but only then says “Error: Access Denied”. Looking in the js console, I get the error: https://discourse.psy.plymouth.ac.uk/uploads.json?client_id=0a2569993a6b43d6b5f8c60fdd2c913e Failed to load resource: the server responded with a status of 422 () And if I follow that url: {"errors":["The requested URL or resource could not be found."],"error_type":"not_found"} Checking the S3 bucket, nothing has m…

…but wonder if the issue is related to your comment here?

Is it supported to have users upload avatars when secure media uploads is turned on? At present I’m getting an error, but wonder if this is because avatars are uploaded to the same bucket where public access is not allowed…

[martin (Martin Brennan)]

I recently renamed “secure media” and related settings to “secure uploads” since it doesn’t just apply to images/video etc., and everyone generally refers to it as secure uploads anyway. The relevant core commit is here:

DEV: Rename secure_media to secure_uploads

This OP of this topic has now been updated to reflect this.