Secure Media Uploads

Great! I’ll try it in a bit and post the results.

Thanks @mjrbrennan!

1 Like

As a heads up if you are intending on making your attachments secure as well you probably want to wait until this PR is merged FIX: Use full URL for secure attachments when secure media enabled by martin-brennan · Pull Request #9037 · discourse/discourse · GitHub

4 Likes

Ok will wait then, was about to run the script just now.

Thanks!

4 Likes

I have secure_media enabled with s3. Posting images in topics works fine, but category images, banners and logos give access denied when trying to view them. I looked into running the task you mentioned, uploads:secure_upload_analyse_and_update, but I don’t seem to have it as an available task nor uploads:sync_s3_acls.

these are the only e rack tasks
rails emails:import
rails emails:test[email]
rails emoji:test
rails emoji:update
rails enqueue_digest_emails
rails export:categories[category_ids]
rails export:category_structure[include_group_users,file_name]

I am on the latest version, 2.5.0.beta2

1 Like

I am not sure what is going on with your list of tasks there, if I run bundle exec rake -T -A I get a huge list of tasks including the uploads-related tasks. It is definitely there.

Were these existing images or were they uploaded after secure media was enabled? If the answer is the former, did you run the uploads:ensure_correct_acl task before enabling secure media and turning your S3 bucket off of Public?

6 Likes

Hey @mjrbrennan

I ran the script but the task is getting killed and never completes.

image

Do you have idea what the issue could be? Our machine has 2gb of ram with 2gb of swap

Thanks!

1 Like

Check dmesg | less and press G then scroll up, I highly suspect you will see some lines from our friend the oom-killer.

7 Likes

Sorry I’m new to ruby. bundle exec rake -T -A worked for me. I used bundle exec rake -T initially. I also attempted to run the command in rails, rails c, terminal which didn’t work - noob ruby mistake.

After that ran into a ActiveRecord::NoDatabaseError: FATAL: Peer authentication failed for user "discourse" error, but found docs explaining it was a issue with using root, which seems to be a ruby thing. Also setting the RAILS_ENV=production also seemed to be required.
src

Lastly, I have a Bootsnap::LoadPathCache::FallbackScan. Hopefully, I can just rebuild to resolve this issue.
src

Also for context, I thought they were put in place when “secure_media” was enabled, but I was fiddling around as it didn’t seem to be working. I think it’s a me problem though.

Update: I recall what I did. I was receiving access denied with trying to upload and I disabled secure media and it worked to allow uploading for the banners. This is what I see when it’s enabled.

1 Like

There should probably be a lot of warnings around this feature @mjrbrennan as it is an :warning: ADVANCED thing, not for the faint of :heart:, and there is a limit to how much we will support it outside our enterprise tier. Bring your own expertise…

8 Likes

Ok after running this on a bigger instance (4GB ram + 8GB swap) I was able to run this task successfully. Here’s what I did before running the task.

  • Enabled Read only mode.
  • Excluded task from oom-killer, finding the PID of the task and running sudo bash -c "echo '-16' | tee /proc/$PID/oom_adj".
  • Ran the task using screen.

This rake task is consuming way too much memory, possibly related to not using batches when looping through uploads, and keeping all records in memory while we run the task.

Peak memory for this task after finishing analyzing records was 10GB between real memory and swap, once analyzing records was completed, memory increase wasn’t noticeable. Here’s how memory consumption growth with the number of records.

image

Here’s the log

# bundle exec rake uploads:secure_upload_analyse_and_update
Analyzing security for uploads in default...

There are 53216 upload(s) that could be marked secure.

Marking posts as secure in the next step because login_required is false.

Analysing which of 53216 uploads need to be marked secure and be rebaked.

Analysis complete!                52668 / 53216 ( 99.0%)
Marking 50566 uploads as secure because UploadSecurity determined them to be secure.
Marking 489 uploads as not secure because UploadSecurity determined them to be not secure.
Finished updating upload security.

Rebaking 556 posts with affected uploads.

Rebaking complete!                  556 / 556 (100.0%)

Updating ACL for 51055 uploads.

Updaing ACLs complete!            51055 / 51055 (100.0%)

Done!

If increasing memory didn’t work I would give it a look at the code, but it did work :sweat_smile:. I made some tests and everything looks good.

Thanks!

8 Likes

Orlando, thank you so much for your detailed analysis. I will take a look at the code to see what I can do to decrease memory usage. I thought I was being careful, I guess not!

9 Likes

Well… Ruby and “memory use” are not exactly the best of buddies historically … ask poor @udan11 about his 50mb strings :rofl:

7 Likes

I agree with you there, but 10GB is maybe just a little greedy :sweat_smile:

4 Likes

You deserve all 10 gigabizzedys of that memory, Martin! You’re worth it!!

7 Likes

Check out the import scripts for some tricks with record streaming & dropping already-processed items from memory.

4 Likes

Dumb question, which should I tick for my uploads bucket?

Dumb question #2, what can I do about s3 site backup bucket, should I make that non-public too? I know the objects are already ACL’d so I guess it doesn’t matter

1 Like

UPDATE: I am adding this information to the OP as well.

As of the below PR, the secure uploads functionality has changed slightly. Now not just media files will be affected, now ALL uploads (images, video, audio, text, pdfs, zips, and others) will follow the secure upload rules. This includes attachments:

This is being done because there are communities out there that would like all attachments and media to be secure based on category rules but still allow anonymous users to download attachments in public places, which was not possible before.

This is not a dumb question, I am not even sure. I think its the top one. @schleifer can you please weigh in here?

11 Likes

Thanks, that’s what I thought too but when I enabled secure media, the first upload I did in the public part of our site was not visible, I don’t know if that’s on purpose or not but it doesn’t fit our use-case so I’ve switched it off, cool feature, though!

2 Likes

Block all public access option is the ideal choice, but it can’t be selected until all existing uploads – and all references to them in existing posts – are migrated.

7 Likes

To add to this, the correct rake task to run before changing anything about your S3 bucket is uploads:sync_s3_acls which will make sure every object is either private or public-read.

8 Likes