-
Notifications
You must be signed in to change notification settings - Fork 446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change the default owner of packaged files. See #129 #139
Change the default owner of packaged files. See #129 #139
Conversation
@@ -83,6 +83,9 @@ object Keys extends DebianKeys { | |||
def target = sbt.Keys.target | |||
def streams = sbt.Keys.streams | |||
|
|||
// file ownership | |||
def appUser = linux.Keys.appUser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should take the chance and add appGroup
, too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sorting that out now while I'm rearranging which user accounts need creating in postinst
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently trying to sort things out with the debian policy.
The postinst
and postrm
looks very promising. However I have no experience with the magic setuid
(this is the s in chmod) and dpkg-statoverride
.
# 5. adjust file and directory permissions
if ! dpkg-statoverride --list $SERVER_HOME >/dev/null
then
chown -R $SERVER_USER:adm $SERVER_HOME
chmod u=rwx,g=rxs,o= $SERVER_HOME
fi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for appGroup (should be used for %files at rpm build script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File group name should automatically be used for RPM %files.
Did I miss something about the setuid bit? I've used this in the past when you have something like ping
which needs to run as root, but you want to allow users to run the exe. However, I'm not sure I see why we'd want that here. Is it in the debian best practices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's described here. However I think, even it is considered best-practices, we should not use it.
http://www.debian.org/doc/manuals/securing-debian-howto/ch9.en.html#s-bpp-lower-privs
I think this is a good start. We may add the |
packageMappingWithRename(compressedManPages:_*).gzipped withUser Users.Root withGroup Users.Root withPerms "0644", | ||
packageMappingWithRename(configFiles:_*) withConfig() withUser Users.Root withGroup Users.Root withPerms "0644", | ||
packageMappingWithRename(remaining:_*) withUser Users.Root withGroup Users.Root withPerms "0644" | ||
packageMappingWithRename((binaries ++ directories):_*) withUser user withGroup user withPerms "0755", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be nice to specify group too, and have it default to the user....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
This is looking great. +1 for the tests as well. |
SO, I think assuming we plan to have a follow-up pull request for daemonGroup setting, this PR is good to merge from me. @muuki88 any addiitonal comments? |
@jsuereth I haven't quite finished this request
|
…-files Change the default owner of packaged files. See #129
Looking forward for the next one :) |
I've introduced
appUser
to complementdaemonUser
with the default value ofroot
(This should be changed at a later date).daemonUser
defaults to the value ofappUser
and there is further scope to introduceappGroup
etc...Any Linux Generic Mappings use
appUser
as the owner of any files.This is still WIP but I would like a sanity check on the present work...