Secure Media Uploads

Great,

I just enabled that setting and will wait for the fix to land on tests-passed to try it out.

Thanks!

6 Likes

Hello!

I pulled the latest changes and ran the job again. PDFs are not rendering correctly in private messages, but they don’t have a secure link url like images do.

Images: https://community.debtcollective.org/secure-media-uploads/original/1X/XXXXXXXXXXXXX.png
PDFs: https://community.debtcollective.org/uploads/short-url/XXXXXXXXXXXXX.pdf

In both cases it generates a signed S3 link and seems to work fine. Should I worry about this?

Thanks!

2 Likes

You seem to be psychic Orlando :wink: That is another issue I have on my list to fix. The PDF will be secure with a private ACL, it just doesn’t have the same access checks that the secure-media-uploads URL has. I am hoping to have this fix done this week. Those other improvements I was talking about with the rake task are in this PR, and will be merged soon:

10 Likes

Awesome @mjrbrennan! Thank you for your work.

I’ll keep an eye on this and report the results.

Thanks again!

8 Likes

Yes please, any other secure media issues report them here. It’s helpful to stamp out any remaining problems!

8 Likes

Hey @eatcodetravel, the improvements to the Rake tasks are now merged, and I have also updated the OP with better instructions on how to enable secure media and running the associated rake tasks!

4 Likes

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