Plist start permission issue


(Michael Cashwell) #1

Greetings,

I’ve observed that “sudo services start” installs its plist file with permissions 0600. This does not agree with the 0644 permissions that all other plists (including those installed in /System by Apple) use and it breaks tools like LaunchControl (a GUI app that allows one to manage all agents and daemons and is useful for inspection and status even in cases where Homebrew controls the plist).

Homebrew isn’t actually setting the wrong perms. It’s doing a cp under sudo and the 0600 seems to just fall out. But it does seem that Homebrew is neglecting to set the right permissions.

I’ve fixed this issue with the following patch:

diff --git a/cmd/brew-services.rb b/cmd/brew-services.rb
index ebba906…e57fcb6 100755
— a/cmd/brew-services.rb
+++ b/cmd/brew-services.rb
@@ -307,6 +307,7 @@ module ServicesCli
rm service.dest if service.dest.exist?
service.dest_dir.mkpath unless service.dest_dir.directory?
cp temp.path, service.dest

  •    chmod 0644, service.dest
    
       # Clear tempfile.
       temp.close
    

but I’m curious if this was deliberate or an oversight.

-Mike


(Mike McQuaid) #2

This is neither deliberate nor an oversight but it depends likely on your default, root umask. I’d suggest you change that yourself.


(Michael Cashwell) #3

I considered doing that but it’s a pretty weak and fragile solution.

Homebrew doing an install (using my umask) or doing “sudo service start …” (using root’s umask) will in general need to write different types of files for different purposes into different places. Expecting a single umask to be right for all of them doesn’t make sense.

And it shouldn’t. The umask is just a default. Writing a plist file is not a default case. It is a special file that should have very specific permissions explicitly applied. Homebrew and its Taps have many uses of chmod doing exactly that.

Surely writing a plist file should be one of them.

-MIke


(Mike McQuaid) #4

I don’t agree it’s fragile or weak; this is a problem that seems to be for you alone (or at least a small number of people) so I don’t think it warrants a change at this time, sorry.


(Michael Cashwell) #5

Setting the umask needed to get 0644 perms on a plist file runs the risk of inadvertently granting those same elevated permissions to other installed files that should not have them. Wouldn’t it be more reliable and secure to have the default perms be 0600 (as they are) and only grant more permissive permissions explicitly where needed on a case by case basis?

And that leads directly to LaunchDaemon plist files being such a case. Having 0600 permissions on them is simply wrong. I’ve done further digging and conversed with several Apple engineers about it. Here’s some of what I found:

https://developer.apple.com/documentation/servicemanagement/1431078-smjobbless

Apple’s SMJobBless function “obviates the need for a setuid helper invoked via AuthorizationExecuteWithPrivileges() in order to install a launchd plist”. Guess what permissions that function sets on the plist file it installs (regardless of the calling umask)? 0644, just like all of Apple’s other LaunchDaemon plists.

https://developer.apple.com/legacy/library/samplecode/BetterAuthorizationSample/Listings/Performing_Privileged_Operations_With_BetterAuthorizationSampleLib_txt.html#//apple_ref/doc/uid/DTS10004207-Performing_Privileged_Operations_With_BetterAuthorizationSampleLib_txt-DontLinkElementID_8

The readme from Apple’s stalwart BetterAuthorizationSample sample Xcode project shows:
/Library/LaunchDaemons/.plist rw-r–r-- root:wheel
and goes on to say “This is the standard location, name, and permissions for launchd configuration files”.

http://stattenfield.org/keith/
I asked a current Apple engineer (Keith Stattenfield) why launchd accepts plist files with 0600 permissions if they aren’t right. There were two parts to his response.

— 1 —
launchd is not the only process that reads the plist files:

“Various other bits of the system besides launchd also read the launchd job files, for both administrative and auditing purposes. Some of these daemons are running as role users, and having a launchd job file which is not world readable will make these daemons fail.

Speaking unofficially for Apple, but with some authority as the author of one of those system daemons.”

— 2 —
launchd only rejects excessive permissions like world write that would cause it to be a security risk. It is not launchd’s function to attempt to know or enforce the plist permission requirements of other processes. Thus expecting launchd to reject a plist file that lacks world-read needed by something else is not valid.

I see no legitimate reason why Homebrew should not explicitly install these plist files with the permissions Apple clearly intends for them to have.

-Mike


(Mike McQuaid) #6

Ok, we’ll review a PR.