From 0e0047393f869330979b5e0ca205f4667476379e Mon Sep 17 00:00:00 2001 From: nfagerlund Date: Wed, 10 Aug 2011 16:39:59 -0700 Subject: (#3553) Explain that cron resources require time attributes The cron resource docs previously read, "All fields except the command and the user are optional, although specifying no periodic fields would result in the command being executed every minute." This was factually incorrect; instead, specifying no periodic fields results in a failure and an unhelpful error on Puppet 2.6 and 2.7. Although the issue will remain open as a behavior bug, this commit corrects the documentation of which attributes are required. It also changes the @doc string to a heredoc to simplify quote escaping. --- lib/puppet/type/cron.rb | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type/cron.rb b/lib/puppet/type/cron.rb index 4f6ea733c..5367517b2 100755 --- a/lib/puppet/type/cron.rb +++ b/lib/puppet/type/cron.rb @@ -3,11 +3,12 @@ require 'facter' require 'puppet/util/filetype' Puppet::Type.newtype(:cron) do - @doc = "Installs and manages cron jobs. All fields except the command - and the user are optional, although specifying no periodic - fields would result in the command being executed every - minute. While the name of the cron job is not part of the actual - job, it is used by Puppet to store and retrieve it. + @doc = <<-EOT + Installs and manages cron jobs. Every cron resource requires a command + and user attribute, as well as at least one periodic attribute (hour, + minute, month, monthday, weekday, or special). While the name of the cron + job is not part of the actual job, it is used by Puppet to store and + retrieve it. If you specify a cron job that matches an existing job in every way except name, then the jobs will be considered equivalent and the @@ -18,30 +19,30 @@ Puppet::Type.newtype(:cron) do Example: cron { logrotate: - command => \"/usr/sbin/logrotate\", + command => "/usr/sbin/logrotate", user => root, hour => 2, minute => 0 } - Note that all cron values can be specified as an array of values: + Note that all periodic attributes can be specified as an array of values: cron { logrotate: - command => \"/usr/sbin/logrotate\", + command => "/usr/sbin/logrotate", user => root, hour => [2, 4] } - Or using ranges, or the step syntax `*/2` (although there's no guarantee that - your `cron` daemon supports it): + ...or using ranges or the step syntax `*/2` (although there's no guarantee + that your `cron` daemon supports these): cron { logrotate: - command => \"/usr/sbin/logrotate\", + command => "/usr/sbin/logrotate", user => root, hour => ['2-4'], minute => '*/10' } - " + EOT ensurable # A base class for all of the Cron parameters, since they all have -- cgit From d7c9c765dbf28df3631e709832c44c343569cb53 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Wed, 10 Aug 2011 16:20:00 -0700 Subject: (#8740) Do not enumerate files in the root directory. Previously the command 'puppet resource file' would enumerate all files in the root directory, and generate an exception if the file type was not a directory, file, or link. Worse, it would also do this when a file or directory was specified, e.g. 'puppet resource file /etc/hosts'. Ideally, the find method of the ral terminus should not need to call the type's instances class method, instead just creating an instance of the type with the specified name and parameters. However, some types, like package, depend on this behavior. The type walks all providers and all instances that they provide, checking to see if the provider provides an instance with that name, and also warning if another provider provides an instance with the same name. Also, ideally, puppet should not blow up when encountering an unsupported file type, e.g. Unix domain socket, but that would be too big of a change for 2.6.x. This commit changes 'puppet resource file' to return a message saying that the operation is not supported: Listing all file instances is not supported. Please specify a file or directory, e.g. puppet resource file /etc The change is bit of a hack, as ideally, the file type's instances method could raise an exception when called in a 'search' context, but return an empty array in a 'find' context. But that also would be too big of a change for 2.6.x. This commit also adds spec tests for the resource application and file type, as well as an acceptance test, which creates a Unix domain socket in the root directory, while running 'puppet resource file'. Paired-with: Nick Lewis Reviewed-by: Jacob Helwig --- lib/puppet/application/resource.rb | 3 +++ lib/puppet/type/file.rb | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/application/resource.rb b/lib/puppet/application/resource.rb index f55caa58a..bc4faf5e4 100644 --- a/lib/puppet/application/resource.rb +++ b/lib/puppet/application/resource.rb @@ -81,6 +81,9 @@ class Puppet::Application::Resource < Puppet::Application [ Puppet::Resource.new( type, name, :parameters => params ).save( key ) ] end else + if type == "file" + raise "Listing all file instances is not supported. Please specify a file or directory, e.g. puppet resource file /etc" + end Puppet::Resource.search( key, {} ) end.map(&format).join("\n") diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb index 630ebe5de..e91929cf8 100644 --- a/lib/puppet/type/file.rb +++ b/lib/puppet/type/file.rb @@ -310,8 +310,8 @@ Puppet::Type.newtype(:file) do super(path.gsub(/\/+/, '/').sub(/\/$/, '')) end - def self.instances(base = '/') - return self.new(:name => base, :recurse => true, :recurselimit => 1, :audit => :all).recurse_local.values + def self.instances + return [] end @depthfirst = false -- cgit From 2a0de12af5305ed54dd99391ee17533e71e0d88e Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Tue, 9 Aug 2011 17:48:33 -0700 Subject: (#8770) Always fully drop privileges when changing user On Mac OS X, it is only possible to directly change the euid of a process, and not the uid. Thus, when a puppet master started as root on OS X would change to the service user (puppet), it would leave the uid of its process set to 0. This allowed any type of Ruby plugin executed on the master (a type, provider, function, etc.) to trivially regain root privileges (by setting the euid of its process back to 0) and potentially compromise the master. Now, when permanently changing user, we will first try Process::UID.change_privilege, before falling back to setting the euid/uid ourselves. change_privilege correctly sets the uid of the process to the desired new uid, preventing the process from later escalating itself back to root. Similar behavior is also used when changing group. This has no effect on the behavior when temporarily changing user/group (for instance, to execute a single command or create a file as a particular user). Reviewed-By: Jacob Helwig --- lib/puppet/util.rb | 56 ++++++++++++------------------------------ lib/puppet/util/suidmanager.rb | 54 +++++++++++++++++++++++++++++----------- 2 files changed, 56 insertions(+), 54 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index 34c6ec1ed..72c1f5f91 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -46,35 +46,24 @@ module Util # Change the process to a different user def self.chuser if group = Puppet[:group] - group = self.gid(group) - raise Puppet::Error, "No such group #{Puppet[:group]}" unless group - unless Puppet::Util::SUIDManager.gid == group - begin - Puppet::Util::SUIDManager.egid = group - Puppet::Util::SUIDManager.gid = group - rescue => detail - Puppet.warning "could not change to group #{group.inspect}: #{detail}" - $stderr.puts "could not change to group #{group.inspect}" - - # Don't exit on failed group changes, since it's - # not fatal - #exit(74) - end + begin + Puppet::Util::SUIDManager.change_group(group, true) + rescue => detail + Puppet.warning "could not change to group #{group.inspect}: #{detail}" + $stderr.puts "could not change to group #{group.inspect}" + + # Don't exit on failed group changes, since it's + # not fatal + #exit(74) end end if user = Puppet[:user] - user = self.uid(user) - raise Puppet::Error, "No such user #{Puppet[:user]}" unless user - unless Puppet::Util::SUIDManager.uid == user - begin - Puppet::Util::SUIDManager.initgroups(user) - Puppet::Util::SUIDManager.uid = user - Puppet::Util::SUIDManager.euid = user - rescue => detail - $stderr.puts "Could not change to user #{user}: #{detail}" - exit(74) - end + begin + Puppet::Util::SUIDManager.change_user(user, true) + rescue => detail + $stderr.puts "Could not change to user #{user}: #{detail}" + exit(74) end end end @@ -89,18 +78,14 @@ module Util if useself Puppet::Util::Log.create( - :level => level, :source => self, - :message => args ) else Puppet::Util::Log.create( - :level => level, - :message => args ) end @@ -261,9 +246,6 @@ module Util Puppet.debug "Executing '#{str}'" end - arguments[:uid] = Puppet::Util::SUIDManager.convert_xid(:uid, arguments[:uid]) if arguments[:uid] - arguments[:gid] = Puppet::Util::SUIDManager.convert_xid(:gid, arguments[:gid]) if arguments[:gid] - if execution_stub = Puppet::Util::ExecutionStub.current_value return execution_stub.call(command, arguments) end @@ -305,14 +287,8 @@ module Util $stderr.reopen(error_file) 3.upto(256){|fd| IO::new(fd).close rescue nil} - if arguments[:gid] - Process.egid = arguments[:gid] - Process.gid = arguments[:gid] unless @@os == "Darwin" - end - if arguments[:uid] - Process.euid = arguments[:uid] - Process.uid = arguments[:uid] unless @@os == "Darwin" - end + Puppet::Util::SUIDManager.change_group(arguments[:gid], true) if arguments[:gid] + Puppet::Util::SUIDManager.change_user(arguments[:uid], true) if arguments[:uid] ENV['LANG'] = ENV['LC_ALL'] = ENV['LC_MESSAGES'] = ENV['LANGUAGE'] = 'C' if command.is_a?(Array) Kernel.exec(*command) diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb index 6633de002..2e12b220f 100644 --- a/lib/puppet/util/suidmanager.rb +++ b/lib/puppet/util/suidmanager.rb @@ -36,12 +36,6 @@ module Puppet::Util::SUIDManager end module_function :groups= - if Facter['kernel'].value == 'Darwin' - # Cannot change real UID on Darwin so we set euid - alias :uid :euid - alias :gid :egid - end - def self.root? Process.uid == 0 end @@ -50,23 +44,55 @@ module Puppet::Util::SUIDManager def asuser(new_uid=nil, new_gid=nil) return yield if Puppet.features.microsoft_windows? or !root? - # We set both because some programs like to drop privs, i.e. bash. - old_uid, old_gid = self.uid, self.gid old_euid, old_egid = self.euid, self.egid - old_groups = self.groups begin - self.egid = convert_xid :gid, new_gid if new_gid - self.initgroups(convert_xid(:uid, new_uid)) if new_uid - self.euid = convert_xid :uid, new_uid if new_uid + change_group(new_gid) if new_gid + change_user(new_uid) if new_uid yield ensure - self.euid, self.egid = old_euid, old_egid - self.groups = old_groups + change_group(old_egid) + change_user(old_euid) end end module_function :asuser + def change_group(group, permanently=false) + gid = convert_xid(:gid, group) + raise Puppet::Error, "No such group #{group}" unless gid + + if permanently + begin + Process::GID.change_privilege(gid) + rescue NotImplementedError + Process.egid = gid + Process.gid = gid + end + else + Process.egid = gid + end + end + module_function :change_group + + def change_user(user, permanently=false) + uid = convert_xid(:uid, user) + raise Puppet::Error, "No such user #{user}" unless uid + + if permanently + begin + Process::UID.change_privilege(uid) + rescue NotImplementedError + initgroups(uid) + Process.euid = uid + Process.uid = uid + end + else + initgroups(uid) + Process.euid = uid + end + end + module_function :change_user + # Make sure the passed argument is a number. def convert_xid(type, id) map = {:gid => :group, :uid => :user} -- cgit From bb224dd1549817190b6471e677e43fa02bb766a3 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Fri, 12 Aug 2011 12:18:51 -0700 Subject: (#8770) Don't fail to set supplementary groups when changing user to root Previously, Puppet::Util::SUIDManager.change_user would always try to set supplementary groups (Process.initgroups) before changing its EUID. Process.initgroups requires the calling process to have EUID 0 in order to succeed. This worked fine in the case where the process was changing from root to a normal user, as it would set groups as root and then change EUID to 0. However, in the case where the process was changing back to root from a normal user, it would attempt to set groups as the normal user, and fail. Now, we check Process.euid before changing, and will set groups first if root, and will set euid first if not root. This ensures we can freely switch back and forth between root. This behavior is maintained inside of the change_user, rather than being broken into eg. raise_privilege and lower_privilege, because it is a relatively minor behavior difference, and the helper methods on their own would not have been generically useful. --- lib/puppet/util/suidmanager.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/util/suidmanager.rb b/lib/puppet/util/suidmanager.rb index 2e12b220f..697bce111 100644 --- a/lib/puppet/util/suidmanager.rb +++ b/lib/puppet/util/suidmanager.rb @@ -82,13 +82,21 @@ module Puppet::Util::SUIDManager begin Process::UID.change_privilege(uid) rescue NotImplementedError + # If changing uid, we must be root. So initgroups first here. initgroups(uid) Process.euid = uid Process.uid = uid end else - initgroups(uid) - Process.euid = uid + # If we're already root, initgroups before changing euid. If we're not, + # change euid (to root) first. + if Process.euid == 0 + initgroups(uid) + Process.euid = uid + else + Process.euid = uid + initgroups(uid) + end end end module_function :change_user -- cgit From 35c10060ed647ba221289d1edf90ce0c587b6c74 Mon Sep 17 00:00:00 2001 From: Dominic Cleal Date: Wed, 17 Aug 2011 08:02:10 +0100 Subject: (#9039) Update Augeas commands documentation Added documentation on commands added as part of #6494 and clarified existing commands documentation. --- lib/puppet/type/augeas.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type/augeas.rb b/lib/puppet/type/augeas.rb index f4d3c43e1..05dd42625 100644 --- a/lib/puppet/type/augeas.rb +++ b/lib/puppet/type/augeas.rb @@ -95,13 +95,18 @@ Puppet::Type.newtype(:augeas) do Commands supported are: set [PATH] [VALUE] Sets the value VALUE at loction PATH + setm [PATH] [SUB] [VALUE] Sets multiple nodes matching SUB relative to PATH, to VALUE rm [PATH] Removes the node at location PATH remove [PATH] Synonym for rm - clear [PATH] Keeps the node at PATH, but removes the value. + clear [PATH] Sets the node at PATH to NULL, creating it if needed ins [LABEL] [WHERE] [PATH] Inserts an empty node LABEL either [WHERE={before|after}] PATH. insert [LABEL] [WHERE] [PATH] Synonym for ins + mv [PATH] [PATH] Moves a node at PATH to the new location PATH + move [PATH] [PATH] Synonym for mv + defvar [NAME] [PATH] Sets Augeas variable $NAME to PATH + defnode [NAME] [PATH] [VALUE] Sets Augeas variable $NAME to PATH, creating it with VALUE if needed - If the parameter 'context' is set that value is prepended to PATH" + If the parameter 'context' is set that value is prepended to a relative PATH" end -- cgit From 7ac10930b30f13b5fdb7aea094283c8c122a0918 Mon Sep 17 00:00:00 2001 From: nfagerlund Date: Tue, 9 Aug 2011 17:33:14 -0700 Subject: (#8037) Fix incorrect example in Augeas type reference The changes attribute for the Augeas type's second example was incorrect, as it had leading slashes that took the paths out of the context of /files. This commit fixes the bad example, and changes the doc string to a heredoc to eliminate some messy escaping. --- lib/puppet/type/augeas.rb | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type/augeas.rb b/lib/puppet/type/augeas.rb index f4d3c43e1..c96986e7e 100644 --- a/lib/puppet/type/augeas.rb +++ b/lib/puppet/type/augeas.rb @@ -20,7 +20,8 @@ Puppet::Type.newtype(:augeas) do feature :need_to_run?, "If the command should run" feature :execute_changes, "Actually make the changes" - @doc = "Apply the changes (single or array of changes) to the filesystem + @doc = <<-EOT + Apply the changes (single or array of changes) to the filesystem via the augeas tool. Requires: @@ -30,24 +31,24 @@ Puppet::Type.newtype(:augeas) do Sample usage with a string: - augeas{\"test1\" : - context => \"/files/etc/sysconfig/firstboot\", - changes => \"set RUN_FIRSTBOOT YES\", - onlyif => \"match other_value size > 0\", + augeas{"test1" : + context => "/files/etc/sysconfig/firstboot", + changes => "set RUN_FIRSTBOOT YES", + onlyif => "match other_value size > 0", } Sample usage with an array and custom lenses: - augeas{\"jboss_conf\": - context => \"/files\", + augeas{"jboss_conf": + context => "/files", changes => [ - \"set /etc/jbossas/jbossas.conf/JBOSS_IP $ipaddress\", - \"set /etc/jbossas/jbossas.conf/JAVA_HOME /usr\" + "set etc/jbossas/jbossas.conf/JBOSS_IP $ipaddress", + "set etc/jbossas/jbossas.conf/JAVA_HOME /usr", ], - load_path => \"$/usr/share/jbossas/lenses\", + load_path => "$/usr/share/jbossas/lenses", } - " + EOT newparam (:name) do desc "The name of this task. Used for uniqueness" -- cgit