Skip to content
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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 30, 2014
Merged

Change the default owner of packaged files. See #129 #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16ac5f3
Add test to check control scripts create daemonUser if required
aparkinson Jan 25, 2014
90a23cb
Default package mappings now use 'appUser' for file ownership
aparkinson Jan 25, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions src/main/scala/com/typesafe/sbt/packager/GenericPackageSettings.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ trait GenericPackageSettings
* Files in `conf/` or `etc/` directories are automatically marked as configuration.
* `../man/...1` files are automatically compressed into .gz files.
*
* TODO - We need to figure out how to handle ownership here.
*/
def mapGenericMappingsToLinux(mappings: Seq[(File, String)])(rename: String => String): Seq[LinuxPackageMapping] = {
def mapGenericMappingsToLinux(mappings: Seq[(File, String)], user: String)(rename: String => String): Seq[LinuxPackageMapping] = {
val (directories, nondirectories) = mappings.partition(_._1.isDirectory)
val (binaries, nonbinaries) = nondirectories.partition(_._1.canExecute)
val (manPages, nonManPages) = nonbinaries partition {
Expand All @@ -54,10 +53,10 @@ trait GenericPackageSettings
}

Seq(
packageMappingWithRename((binaries ++ directories):_*) withUser Users.Root withGroup Users.Root withPerms "0755",
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",
Copy link
Member

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....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

packageMappingWithRename(compressedManPages:_*).gzipped withUser user withGroup user withPerms "0644",
packageMappingWithRename(configFiles:_*) withConfig() withUser user withGroup user withPerms "0644",
packageMappingWithRename(remaining:_*) withUser user withGroup user withPerms "0644"
)
}

Expand All @@ -68,16 +67,17 @@ trait GenericPackageSettings
defaultLinuxLogsLocation := "/var/log",

// First we look at the src/linux files
linuxPackageMappings <++= (sourceDirectory in Linux) map { dir =>
mapGenericMappingsToLinux((dir.*** --- dir) x relativeTo(dir))(identity)
linuxPackageMappings <++= (sourceDirectory in Linux, appUser in Linux) map { (dir, user) =>
mapGenericMappingsToLinux((dir.*** --- dir) x relativeTo(dir), user)(identity)
},
// Now we look at the src/universal files.
linuxPackageMappings <++= (normalizedName in Universal, mappings in Universal, defaultLinuxInstallLocation) map { (pkg, mappings, installLocation) =>
linuxPackageMappings <++= (normalizedName in Universal, mappings in Universal, defaultLinuxInstallLocation, appUser in Linux) map {
(pkg, mappings, installLocation, user) =>
// TODO - More windows filters...
def isWindowsFile(f: (File, String)): Boolean =
f._2 endsWith ".bat"

mapGenericMappingsToLinux(mappings filterNot isWindowsFile) { name =>
mapGenericMappingsToLinux(mappings filterNot isWindowsFile, user) { name =>
installLocation + "/" + pkg + "/" + name
}
},
Expand Down
14 changes: 7 additions & 7 deletions src/main/scala/com/typesafe/sbt/packager/archetypes/JavaServerApplication.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ object JavaServerAppPackaging {
def debianSettings: Seq[Setting[_]] =
Seq(
serverLoading := Upstart,
daemonUser := Users.Root,
daemonUser <<= appUser in Linux,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this principle.

// This one is begging for sbt 0.13 syntax...
debianScriptReplacements <<= (
maintainer in Debian, packageSummary in Debian, serverLoading in Debian, daemonUser in Debian, normalizedName,
Expand All @@ -55,15 +55,15 @@ object JavaServerAppPackaging {
map { (tmpDir, loader, replacements, template) =>
makeDebianMaintainerScript(JavaAppStartScript.startScript, Some(template))(tmpDir, loader, replacements)
},
linuxPackageMappings in Debian <++= (debianMakeStartScript, normalizedName, serverLoading in Debian)
map { (script, name, loader) =>
linuxPackageMappings in Debian <++= (debianMakeStartScript, normalizedName, serverLoading in Debian, appUser in Linux)
map { (script, name, loader, owner) =>
val (path, permissions) = loader match {
case Upstart => ("/etc/init/" + name + ".conf", "0644")
case SystemV => ("/etc/init.d/" + name, "0755")
}
for {
s <- script.toSeq
} yield LinuxPackageMapping(Seq(s -> path)).withPerms(permissions).withConfig()
} yield LinuxPackageMapping(Seq(s -> path), LinuxFileMetaData(owner, owner, permissions, "true"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe LinuxFileMetaData(owner, owner, permissions, config = "true")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these marked as conf files? Do we really want users altering this, or would we rather have them use /etc/default to configure stuff?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should use /etc/default but we can't stop them making changes to these files. Least this way they won't lose any changes they make when upgrading the package. Lintian will also complain about these files if they aren't marked as config files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Didnt' realize the bit about lintian. Looks good the way it is then.

},

// === etc config mapping ===
Expand All @@ -74,13 +74,13 @@ object JavaServerAppPackaging {
},
debianMakeEtcDefault <<= (normalizedName, target in Universal, serverLoading in Debian, linuxEtcDefaultTemplate in Debian)
map makeEtcDefaultScript,
linuxPackageMappings in Debian <++= (debianMakeEtcDefault, normalizedName) map { (conf, name) =>
conf.map(c => LinuxPackageMapping(Seq(c -> ("/etc/default/" + name))).withConfig()).toSeq
linuxPackageMappings in Debian <++= (debianMakeEtcDefault, normalizedName, appUser in Linux) map { (conf, name, owner) =>
conf.map(c => LinuxPackageMapping(Seq(c -> ("/etc/default/" + name)), LinuxFileMetaData(owner, owner)).withConfig()).toSeq
},
// TODO should we specify daemonGroup in configs?

// === logging directory mapping ===
linuxPackageMappings in Debian <+= (normalizedName, defaultLinuxLogsLocation, target in Debian, daemonUser in Debian) map {
linuxPackageMappings in Debian <+= (normalizedName, defaultLinuxLogsLocation, target in Debian, appUser in Linux) map {
(name, logsDir, target, user) =>
// create empty var/log directory
val d = target / logsDir
Expand Down
3 changes: 3 additions & 0 deletions src/main/scala/com/typesafe/sbt/packager/debian/Keys.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Contributor Author

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

Copy link
Contributor

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

Copy link

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

Copy link
Member

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?

Copy link
Contributor

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


//init script parameters
def daemonUser = linux.Keys.daemonUser
def serverLoading = linux.Keys.serverLoading
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/com/typesafe/sbt/packager/linux/Keys.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ trait Keys {
val packageSummary = SettingKey[String]("package-summary", "Summary of the contents of a linux package.")
val packageDescription = SettingKey[String]("package-description", "The description of the package. Used when searching.")
val maintainer = SettingKey[String]("maintainer", "The name/email address of a maintainer for the native package.")
val appUser = SettingKey[String]("app-user", "The owner of the files in the package")
val daemonUser = SettingKey[String]("daemon-user", "User to start application daemon")
val serverLoading = SettingKey[ServerLoader]("server-loader", "Loading system to be used for application start script")
val linuxPackageMappings = TaskKey[Seq[LinuxPackageMapping]]("linux-package-mappings", "File to install location mappings including owner and privileges.")
Expand Down
4 changes: 3 additions & 1 deletion src/main/scala/com/typesafe/sbt/packager/linux/LinuxPlugin.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package linux

import Keys._
import sbt._
import com.typesafe.sbt.packager.linux.LinuxPlugin.Users

/**
* Plugin trait containing all the generic values used for
Expand All @@ -25,7 +26,8 @@ trait LinuxPlugin extends Plugin {
}
},
packageSummary in Linux <<= packageSummary,
packageDescription in Linux <<= packageDescription)
packageDescription in Linux <<= packageDescription,
appUser := Users.Root)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is gonna be appUser in Linux <<= normalizedName

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had thought that might be a separate pull request due to the number of documentation changes required.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.


/** DSL for packaging files into .deb */
def packageMapping(files: (File, String)*) = LinuxPackageMapping(files)
Expand Down
36 changes: 36 additions & 0 deletions src/sbt-test/debian/daemon-user-deb/build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import NativePackagerKeys._
import com.typesafe.sbt.packager.archetypes.ServerLoader

packageArchetype.java_server

serverLoading in Debian := ServerLoader.Upstart

appUser in Linux := "daemonUser"

mainClass in Compile := Some("empty")

name := "debian-test"

version := "0.1.0"

maintainer := "Josh Suereth <[email protected]>"

packageSummary := "Test debian package"

packageDescription := """A fun package description of our software,
with multiple lines."""

TaskKey[Unit]("check-control-files") <<= (target, streams) map { (target, out) =>
val debian = target / "debian-test-0.1.0" / "DEBIAN"
val postinst = IO.read(debian / "postinst")
val postrm = IO.read(debian / "postrm")
assert(postinst contains "addgroup --system daemonUser", "postinst misses addgroup for daemonUser: " + postinst)
assert(postinst contains "useradd --system --no-create-home --gid daemonUser --shell /bin/false daemonUser", "postinst misses useradd for daemonUser: " + postinst)
assert(postinst contains "chown daemonUser:daemonUser /var/log/debian-test", "postinst misses chown daemonUser /var/log/debian-test: " + postinst)
assert(postinst contains "chown daemonUser:daemonUser /usr/share/debian-test/bin/debian-test", "postinst misses chown daemonUser /usr/share/debian-test/bin/debian-test: " + postinst)
assert(postrm contains "deluser --quiet --system daemonUser > /dev/null || true", "postrm misses purging daemonUser user: " + postrm)
assert(postrm contains "delgroup --quiet --system daemonUser > /dev/null || true", "postrm misses purging daemonUser group: " + postrm)
out.log.success("Successfully tested upstart control files")
()
}

1 change: 1 addition & 0 deletions src/sbt-test/debian/daemon-user-deb/project/plugins.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
addSbtPlugin("com.typesafe.sbt" % "sbt-native-packager" % sys.props("project.version"))
12 changes: 12 additions & 0 deletions src/sbt-test/debian/daemon-user-deb/test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Run the debian packaging.
> debian:package-bin
$ exists target/debian-test-0.1.0.deb

$ exists target/debian-test-0.1.0/etc
$ exists target/debian-test-0.1.0/etc/init/debian-test.conf
# Check defaults
$ exists target/debian-test-0.1.0/DEBIAN/prerm
$ exists target/debian-test-0.1.0/DEBIAN/postinst

# Check files for defaults
> check-control-files