directory/file provider fail on OS X 10.11 due to SIP (System Integrity Protection)

[natewalck]

With OS X 10.11, Apple has implemented a new system called SIP (System Integrity Protection, more info here with some great source links: https://en.wikipedia.org/wiki/System_Integrity_Protection)

I've dug into this a bit and here is what I've found:

If you want to write to '/usr/local', chef checks to see if '/usr' is writable:

https://github.com/chef/chef/blob/master/lib/chef/provider/directory.rb#L76-L79

This is normally ok, but because '/usr' is a protected directory, the call will return false. This is using ::File::writable? as found here: https://github.com/chef/chef/blob/master/lib/chef/file_access_control/unix.rb#L31-L32

On OS X 10.11 + SIP, '/usr/local' is writable, despite the fact that '/usr' itself is not. This kind of breaks how unix file permissions work (a little).

I've worked on two different solutions to this problem, with one of them working, but it is a real kludge.

Option 1 (AKA the kludge) - Change the directory/file/etc providers so they behave differently on OS X 10.11+.

• Check to see if the file is writable as you normally would (https://github.com/chef/chef/blob/master/lib/chef/provider/directory.rb#L76-L79), but if false and you are on >= 10.11, do an additional check to see if it is a location that has a SIP exception (Config for SIP can be found at the following path on 10.11: '/System/Library/Sandbox/rootless.conf').
• If the path starts with anything on the exception list, lie and say that the path is writable. This is mostly true. Edge cases are chef run as non-root and file attributes/ACLs that prevent root from writing this directory (and probably others).

Code: https://gist.github.com/natewalck/7340096da52a01722333
This also has some edge cases and isn't 100% complete.

Option 2
I've also written code that adds FileAccessControl::Macosx and includes all the functions from FileAccessControl::Unix, then overriding the writable? function to do all the extra SIP logic. To make this work, we would need to pass two more pieces of information everywhere writable? is called:

• The path actually being written (not just the parent path)
• The platform_version to make sure we can scope the SIP functionality correctly.

Here is the code for this one: https://gist.github.com/natewalck/33cd7705e6d5ccdb2929
This example doesn't implement everything as I described it, but you can get the general idea.

Adding these things also feels messy, so I would love other suggestions on how to fix this. I am more than willing to do the leg work and test it, but I want to make sure that I am doing it in the right way. I have code examples for the above things, as well as parsing out rootless.conf to get the list of SIP exception directories, which I will add to this issue as soon as I confirm a few things.

[mikedodge04]

@jtimberman @tyler-ball

[lamont-granquist]

Option 2 definitely sounds better, or at least I'm not a fan of Option 1.

FAC is really ugly code, though, and I'm hesitant to make it more complicated. It also sort of feels like #writable? should probably be extracted into Chef::Util somewhere since I'll bet that the code in FAC is only the first case where we'll run into this problem and that other checks will also need to be modified.

[lamont-granquist]

Actually could you describe what you're trying to do in code? "If you want to write to '/usr/local'" is ambiguous since you might be referring to creating entries under /usr/local, or modifying perms on /usr/local which are two different things entirely.

Creating entries under /usr/local should succeed fine if /usr is read-only, and we shouldn't be checking that. If that's the case then its a bug. I don't think that's what you're talking about?

If you're trying to manage perms on /usr/local then that makes more sense about why we'd be checking #writable?("/usr") but it also doesn't make a lot of sense that /usr fails the #writable? check by still being actually writable. Although now I think I see the problem. So, yes, we need to not be checking #writable?("/usr") but need to be checking #can_update_acls?("/usr/local") which on normal Unix would just be #writable?("/usr") but needs to be modified on OSX now.

[coderanger]

On 10.11, you can mkdir /usr/local even though /usr is not writable.

[natewalck]

@lamont-granquist I think you have it...I am referring to the behavior about checking the parent directory.

Before any directory is created or has the permissions changed, it checks the parent directory to see if it is writable. If it isn't, it fails. This is done for recursive directory management as well. See:

• https://github.com/chef/chef/blob/master/lib/chef/provider/directory.rb#L67
• https://github.com/chef/chef/blob/master/lib/chef/provider/directory.rb#L77

with failure to be "writable" throwing:

• https://github.com/chef/chef/blob/master/lib/chef/provider/directory.rb#L84

This is the exact error thrown on 10.11 Beta 2:

* directory[/usr/local] action create[2015-07-01T13:53:38-07:00] INFO: Processing directory[/usr/local] action create (some_files::some_bins line 19)

    * Cannot create directory[/usr/local] at /usr/local due to insufficient permissions
    ================================================================================
    Error executing action `create` on resource 'directory[/usr/local]'
    ================================================================================

    Chef::Exceptions::InsufficientPermissions
    -----------------------------------------
    Cannot create directory[/usr/local] at /usr/local due to insufficient permissions

    Resource Declaration:
    ---------------------
    # In /etc/chef/local-mode-cache/cache/cookbooks/some_files/recipes/some_bins.rb

     19: directory '/usr/local' do
     20:   only_if { File.exist?('/usr/local') }
     21:   owner username
     22:   group 'admin'
     23:   mode 0755
     24: end

    Compiled Resource:
    ------------------
    # Declared in /etc/chef/local-mode-cache/cache/cookbooks/some_files/recipes/some_bins.rb:19:in `from_file'

    directory("/usr/local") do
      action :create
      retries 0
      retry_delay 2
      default_guard_interpreter :default
      path "/usr/local"
      declared_type :directory
      cookbook_name "some_files"
      recipe_name "some_bins"
      owner "someuser"
      group "admin"
      mode 493
      not_if { #code block }
      only_if { #code block }
    end

[natewalck]

@lamont-granquist If I know where you want the functionality, I don't mind writing the code and testing. Let me know how you want to handle it ;)

[lamont-granquist]

My first guess of where it should go is here:

https://github.com/chef/chef/blob/master/chef-config/lib/chef-config/path_helper.rb

And be used in chef via Chef::Utils::PathHelper. Although I'm not sure this is a "Path" helper, and a lot of that is Windows-specific. But implementing Chef::Utils::PathHelper.can_update_acls?("/usr/local") is where I'd start (although I'd definitely try to tighten up the wording on the method, and this might call for creating a new Chef::Utils::FileUtils class).

[natewalck]

I believe I have the PathHelper done for SIP related checks, but I was curious where to put the actual logic.

My proposal is to put it in the file and directory provider similar to this:

if !whyrun_mode? || ::File.exists?(parent_directory)
                if Chef::FileAccessControl.writable?(parent_directory)
                  true
                elsif Gem::Version.new(node['platform_version']) >= Gem::Version.new('10.11') && Chef::Util::PathHelper.is_sip_path?(parent_directory)
                  Chef::Util::PathHelper.writable_sip_path?(@new_resource.path)
                else
                  false
                end
              else
                true
              end
...

This would only check if the path is a SIP path on OS X 10.11 or newer and if it is a SIP path, it would see if it is on the exclusion list and return true if so.

Let me know if there is a better place to put this or if you think it should be done differently.

[natewalck]

Here are more details on my implementation. It seems to be working now.

PathHelper: https://gist.github.com/natewalck/12ab83d8a69ec3ecb80d#file-path_helper-rb
Directory: https://gist.github.com/natewalck/12ab83d8a69ec3ecb80d#file-directory-rb

I confirmed this allows me to change permissions on /usr/local, yet it will still prevent files from being written to /usr/* with the correct error message regarding insufficient permissions:

Chef::Exceptions::InsufficientPermissions: Cannot create directory[/usr/test] at /usr/test due to insufficient permissions

Let me know if this should be refactored and if I need to add any tests for it.

[lamont-granquist]

Yeah it'll be most useful if you convert that into a PR so that we can review it with the diffs easier. It will definitely need some tests around it.

[natewalck]

Sure thing. I'll put the changes into file.rb as well and then we can iterate from there. Thanks for all the help!

[natewalck]

PR is in for this: #3704

[tas50]

Closing this since #3704 was merged

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.