thoughtbot / paperclip

Easy file attachment management for ActiveRecord
https://thoughtbot.com
Other
9.01k stars 2.43k forks source link

Files upload with wrong security context on SELinux #2244

Closed rossettistone closed 8 years ago

rossettistone commented 8 years ago

When uploading files with Paperclip on a RHEL 6.7, the files are created with security context unconfined_u:object_r:user_tmp_t:s0, which makes them inaccessible to Apache. Other files that are available to be served by Apache have context unconfined_u:object_r:httpd_sys_content_t:s0

It's possible to fix the security context of any given file with the chcon (an analogue to chmod/chown) terminal command, but this only applies to existing files, and doesn't make future uploads accessible.

Would be great to have an option in Paperclip's config to chcon files upon upload. Or is there another fix for this that I haven't though of? I'm pretty stumped.

bdewater commented 8 years ago

chcon sets a temporary SELinux context which gets reset when using restorecon. The proper way to do this is to set the right default context for your upload directory using semanage fcontext because new files and directories inherit the context of their parent directory. It is described here in the Red Hat documentation - for similar OS questions you'll probably have more luck looking on sites like ServerFault.

rossettistone commented 8 years ago

Thank you very much, @bdewater! I'll give that a shot. I had scoured ServerFault and such, but missed the distinction between chcon and semanage fcontext - sounds like this will do the trick.

tute commented 8 years ago

Thank you both!

ball-hayden commented 7 years ago

I hope you'll excuse me commenting on this old issue, but I've found that using fcontext to set the context for the upload is insufficient.

When moving files between directories selinux will maintain the context of the file. As files are initially stored in a temp file, they are given a tmp_t context. When paperclip moves the file it retains this type.

I'd be interested to know if @rossettistone solved this issue, or whether the solution for now is to allow my webserver to read tmp_t files.

I'm guessing (given the lack of discussion about this issue) that very few people are having this issue, but I wonder if it's worth putting together a PR that runs restorecon after moving the uploaded file?

rossettistone commented 7 years ago

I ultimately ended up redeploying on Heroku for other reasons, and thereby bypassing this problem entirely. Sorry I can't be of more help!

nlevchuk commented 7 years ago

@ball-hayden It can be fixed here https://github.com/thoughtbot/paperclip/blob/8c7cb379d3e24eeaff3e65b735da7cf2f612f5ac/lib/paperclip/storage/filesystem.rb#L95 by changing mv to cp + rm.

@bdewater what do you think?

nlevchuk commented 7 years ago

For people who're using Selinux and paperclip it's a big problem.

ball-hayden commented 7 years ago

@nlevchuk I'm using the following monkey patch:

require 'paperclip'

module Paperclip
  # Monkeypatch Paperclip to run restorecon after moving files
  class Attachment
    def after_flush_writes
      unlink_files(@queued_for_write.values)

      # Correct selinux context
      @queued_for_write.keys.each do |style_name|
        Paperclip.run('restorecon', ':path', path: path(style_name))
      end
    rescue Cocaine::CommandNotFoundError, Cocaine::ExitStatusError => e
      Rails.logger.warn "Unable to restorecon: #{e.message}"
    end
  end
end

Although switching to cp + rm does seem like a simpler solution :smile:

bdewater commented 7 years ago

@nlevchuk I'm not working on the project where I encountered the problem and fixed it, so I can't check some things for you anymore 😞

nlevchuk commented 7 years ago

@bdewater ок, thanks for reply.

nlevchuk commented 7 years ago

@ball-hayden I found workaround for this issue.

For creating temp files Paperclip uses the Ruby's Tempfile class (link)

file = Tempfile.new([basename, extension])

The class takes tmpdir argument which let us to change it through the environment variables (link)

  # The temporary file will be placed in the directory as specified
  # by the +tmpdir+ parameter. By default, this is +Dir.tmpdir+.
  # When $SAFE > 0 and the given +tmpdir+ is tainted, it uses
  # '/tmp' as the temporary directory. Please note that ENV values
  # are tainted by default, and +Dir.tmpdir+'s return value might
  # come from environment variables (e.g. <tt>$TMPDIR</tt>).

We can use TMPDIR, TMP or TEMP variables as path to directory for temporary files (link)

 [ENV['TMPDIR'], ENV['TMP'], ENV['TEMP'], @@systmpdir, '/tmp', '.'].each do |dir|

Thus we can use any directory as TMPDIR and paperclip will be use it. Besides we can assign right security context on TMPDIR (recursively), ex. httpd_sys_content_t, and the files will be used for apache's response correctly. And no need to change Paperclip's code.

* Caveat:

For some reasons TMPDIR(TMP, TEMP) should not set up globally through ~/.bashrc or something like this. Only for running processes, ex. ruby servers (unicorn, thin) and background jobs (DJ, Sidekiq)