[puppet/el4] Apply upstream patches for security issues
Todd Zullinger
tmz at fedoraproject.org
Tue Oct 4 01:21:57 UTC 2011
commit 0d92e380ea69ab86d1aaa8d40ab5da7f36ce0295
Author: Todd Zullinger <tmz at pobox.com>
Date: Mon Oct 3 21:16:39 2011 -0400
Apply upstream patches for security issues
Includes patches for the following vulnerabilities:
CVE-2011-3848 - Directory traversal attack
CVE-2011-3869 - K5login content attack
CVE-2011-3870 - SSH authorized_keys symlink attack
CVE-2011-3871 - Predictable temp file using RAL
For further details refer to the upstream announcement:
http://groups.google.com/group/puppet-announce/browse_thread/thread/91e3b46d2328a1cb
...2-Predictable-temporary-filename-in-ralsh.patch | 69 ++++++++++
0.25.x-9791-TOCTOU-in-ssh-auth-keys-type.patch | 50 +++++++
...gin-can-overwrite-arbitrary-files-as-root.patch | 40 ++++++
...Resist-directory-traversal-attacks-0.25.x.patch | 140 ++++++++++++++++++++
puppet.spec | 14 ++
5 files changed, 313 insertions(+), 0 deletions(-)
---
diff --git a/0.25-9792-Predictable-temporary-filename-in-ralsh.patch b/0.25-9792-Predictable-temporary-filename-in-ralsh.patch
new file mode 100644
index 0000000..5eeee98
--- /dev/null
+++ b/0.25-9792-Predictable-temporary-filename-in-ralsh.patch
@@ -0,0 +1,69 @@
+From 21b7192320dbb79a8cfe1fd3e06d0d399c964c0f Mon Sep 17 00:00:00 2001
+From: Daniel Pittman <daniel at puppetlabs.com>
+Date: Wed, 28 Sep 2011 23:23:55 -0700
+Subject: [PATCH] (#9792) Predictable temporary filename in ralsh.
+
+When ralsh is used in edit mode the temporary filename is in a shared
+directory, and is absolutely predictable. Worse, it won't be touched until
+well after the startup of the command.
+
+It can be tricked into writing through a symlink to edit any file on the
+system, or to create through it, but worse - the file is reopened with the
+same name later, so it can have the target replaced between edit and
+operate...
+
+The only possible mitigation comes from the system editor and the behaviour it
+has around editing through symbolic links, which is very weak.
+
+This improves this to prefer the current working directory for the temporary
+file, and to be somewhat less predictable and more safe in conjuring it into
+being.
+
+Signed-off-by: Daniel Pittman <daniel at puppetlabs.com>
+---
+ lib/puppet/application/ralsh.rb | 27 +++++++++++++++++----------
+ 1 files changed, 17 insertions(+), 10 deletions(-)
+
+diff --git a/lib/puppet/application/ralsh.rb b/lib/puppet/application/ralsh.rb
+index b9f7a58..593d3c1 100644
+--- a/lib/puppet/application/ralsh.rb
++++ b/lib/puppet/application/ralsh.rb
+@@ -119,18 +119,25 @@ Puppet::Application.new(:ralsh) do
+ end.compact.join("\n")
+
+ if options[:edit]
+- file = "/tmp/x2puppet-#{Process.pid}.pp"
++ require 'tempfile'
++ # Prefer the current directory, which is more likely to be secure
++ # and, in the case of interactive use, accessible to the user.
++ tmpfile = Tempfile.new('x2puppet', Dir.pwd)
+ begin
+- File.open(file, "w") do |f|
+- f.puts text
+- end
+- ENV["EDITOR"] ||= "vi"
+- system(ENV["EDITOR"], file)
+- system("puppet -v " + file)
++ # sync write, so nothing buffers before we invoke the editor.
++ tmpfile.sync = true
++ tmpfile.puts text
++
++ # edit the content
++ system(ENV["EDITOR"] || 'vi', tmpfile.path)
++
++ # ...and, now, pass that file to puppet to apply. Because
++ # many editors rename or replace the original file we need to
++ # feed the pathname, not the file content itself, to puppet.
++ system('puppet -v ' + tmpfile.path)
+ ensure
+- #if FileTest.exists? file
+- # File.unlink(file)
+- #end
++ # The temporary file will be safely removed.
++ tmpfile.close(true)
+ end
+ else
+ puts text
+--
+1.7.6.4
+
diff --git a/0.25.x-9791-TOCTOU-in-ssh-auth-keys-type.patch b/0.25.x-9791-TOCTOU-in-ssh-auth-keys-type.patch
new file mode 100644
index 0000000..4740b95
--- /dev/null
+++ b/0.25.x-9791-TOCTOU-in-ssh-auth-keys-type.patch
@@ -0,0 +1,50 @@
+From e2c1cd5c957a236f89b9e8cb7b4e4f8769079e8c Mon Sep 17 00:00:00 2001
+From: Ricky Zhou <ricky at fedoraproject.org>
+Date: Mon, 29 Aug 2011 16:01:12 -0400
+Subject: [PATCH] Drop privileges before creating and chmodding SSH keys.
+
+Previously, potentially abusable chown and chmod calls were performed as
+root. This tries to moves as much as possible into code which is run
+after privileges have been dropped.
+
+Huge thanks to Ricky Zhou <ricky at fedoraproject.org> for discovering this and
+supplying the security fix. Awesome work.
+
+Signed-off-by: Daniel Pittman <daniel at puppetlabs.com>
+---
+ lib/puppet/provider/ssh_authorized_key/parsed.rb | 16 +++++++++-------
+ 1 files changed, 9 insertions(+), 7 deletions(-)
+
+diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb
+index fb4d095..7358872 100644
+--- a/lib/puppet/provider/ssh_authorized_key/parsed.rb
++++ b/lib/puppet/provider/ssh_authorized_key/parsed.rb
+@@ -62,16 +62,18 @@ Puppet::Type.type(:ssh_authorized_key).provide(:parsed,
+ end
+
+ def flush
+- raise Puppet::Error, "Cannot write SSH authorized keys without user" unless user
+- raise Puppet::Error, "User '#{user}' does not exist" unless uid = Puppet::Util.uid(user)
++ raise Puppet::Error, "Cannot write SSH authorized keys without user" unless user
++ raise Puppet::Error, "User '#{user}' does not exist" unless uid = Puppet::Util.uid(user)
++ Puppet::Util::SUIDManager.asuser(@resource.should(:user)) do
+ unless File.exist?(dir = File.dirname(target))
+- Puppet.debug "Creating #{dir}"
+- Dir.mkdir(dir, dir_perm)
+- File.chown(uid, nil, dir)
++ Puppet.debug "Creating #{dir}"
++ Dir.mkdir(dir, dir_perm)
+ end
+- Puppet::Util::SUIDManager.asuser(user) { super }
+- File.chown(uid, nil, target)
++
++ super
++
+ File.chmod(file_perm, target)
++ end
+ end
+
+ # parse sshv2 option strings, wich is a comma separated list of
+--
+1.7.6.4
+
diff --git a/0.25.x-9794-k5login-can-overwrite-arbitrary-files-as-root.patch b/0.25.x-9794-k5login-can-overwrite-arbitrary-files-as-root.patch
new file mode 100644
index 0000000..2125ae3
--- /dev/null
+++ b/0.25.x-9794-k5login-can-overwrite-arbitrary-files-as-root.patch
@@ -0,0 +1,40 @@
+From a4333c110ad084f205605708eaab52ad243d6c86 Mon Sep 17 00:00:00 2001
+From: Daniel Pittman <daniel at puppetlabs.com>
+Date: Thu, 29 Sep 2011 00:26:13 -0700
+Subject: [PATCH] (#9794) k5login can overwrite arbitrary files as root
+
+The k5login type is typically used to manage a file in the home directory of a
+user; the explicit purpose of the files is to allow access to other users.
+
+It writes to the target file directly, as root, without doing anything to
+secure the file. That would allow the owner of the home directory to symlink
+to anything on the system, and have it replaced with the correct content of
+the file. Which is a fairly obvious escalation to root the next time Puppet
+runs.
+
+Now, instead, fix that to securely write the target file in a predictable and
+secure fashion, using the `secure_open` helper.
+
+Signed-off-by: Daniel Pittman <daniel at puppetlabs.com>
+---
+ lib/puppet/type/k5login.rb | 4 +++-
+ 1 files changed, 3 insertions(+), 1 deletions(-)
+
+diff --git a/lib/puppet/type/k5login.rb b/lib/puppet/type/k5login.rb
+index 5526fda..b13b34d 100644
+--- a/lib/puppet/type/k5login.rb
++++ b/lib/puppet/type/k5login.rb
+@@ -81,7 +81,9 @@ Puppet::Type.newtype(:k5login) do
+
+ private
+ def write(value)
+- File.open(@resource[:name], "w") { |f| f.puts value.join("\n") }
++ Puppet::Util.secure_open(@resource[:name], "w") do |f|
++ f.puts value.join("\n")
++ end
+ end
+ end
+ end
+--
+1.7.6.4
+
diff --git a/0001-Resist-directory-traversal-attacks-0.25.x.patch b/0001-Resist-directory-traversal-attacks-0.25.x.patch
new file mode 100644
index 0000000..ecb0fbb
--- /dev/null
+++ b/0001-Resist-directory-traversal-attacks-0.25.x.patch
@@ -0,0 +1,140 @@
+From 6e5a821cbf94b220dfc021ff7ebad0831c60e207 Mon Sep 17 00:00:00 2001
+From: Daniel Pittman <daniel at puppetlabs.com>
+Date: Sat, 24 Sep 2011 12:44:20 -0700
+Subject: [PATCH] Resist directory traversal attacks through indirections.
+
+In various versions of Puppet it was possible to cause a directory traversal
+attack through the SSLFile indirection base class. This was variously
+triggered through the user-supplied key, or the Subject of the certificate, in
+the code.
+
+Now, we detect bad patterns down in the base class for our indirections, and
+fail hard on them. This reduces the attack surface with as little disruption
+to the overall codebase as possible, making it suitable to deploy as part of
+older, stable versions of Puppet.
+
+In the long term we will also address this higher up the stack, to prevent
+these problems from reoccurring, but for now this will suffice.
+
+Huge thanks to Kristian Erik Hermansen <kristian.hermansen at gmail.com> for the
+responsible disclosure, and useful analysis, around this defect.
+
+Signed-off-by: Daniel Pittman <daniel at puppetlabs.com>
+---
+ lib/puppet/indirector.rb | 8 +++++++-
+ lib/puppet/indirector/ssl_file.rb | 5 +++++
+ lib/puppet/indirector/yaml.rb | 5 +++++
+ spec/unit/indirector/ssl_file.rb | 19 +++++++++++++++++++
+ spec/unit/indirector/yaml.rb | 15 +++++++++++++++
+ 5 files changed, 51 insertions(+), 1 deletions(-)
+
+diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
+index 61ef2db..20a460d 100644
+--- a/lib/puppet/indirector.rb
++++ b/lib/puppet/indirector.rb
+@@ -31,7 +31,13 @@ module Puppet::Indirector
+ @indirection
+ end
+
+- module ClassMethods
++ # Helper definition for indirections that handle filenames.
++ BadNameRegexp = Regexp.union(/^\.\./,
++ %r{[\\/]},
++ "\0",
++ /(?i)^[a-z]:/)
++
++ module ClassMethods
+ attr_reader :indirection
+
+ def cache_class=(klass)
+diff --git a/lib/puppet/indirector/ssl_file.rb b/lib/puppet/indirector/ssl_file.rb
+index fc1e65d..9defcb5 100644
+--- a/lib/puppet/indirector/ssl_file.rb
++++ b/lib/puppet/indirector/ssl_file.rb
+@@ -54,6 +54,11 @@ class Puppet::Indirector::SslFile < Puppet::Indirector::Terminus
+
+ # Use a setting to determine our path.
+ def path(name)
++ if name =~ Puppet::Indirector::BadNameRegexp then
++ Puppet.crit("directory traversal detected in #{self.class}: #{name.inspect}")
++ raise ArgumentError, "invalid key"
++ end
++
+ if ca?(name) and ca_location
+ ca_location
+ elsif collection_directory
+diff --git a/lib/puppet/indirector/yaml.rb b/lib/puppet/indirector/yaml.rb
+index 782112e..a119b86 100644
+--- a/lib/puppet/indirector/yaml.rb
++++ b/lib/puppet/indirector/yaml.rb
+@@ -50,6 +50,11 @@ class Puppet::Indirector::Yaml < Puppet::Indirector::Terminus
+
+ # Return the path to a given node's file.
+ def path(name)
++ if name =~ Puppet::Indirector::BadNameRegexp then
++ Puppet.crit("directory traversal detected in #{self.class}: #{name.inspect}")
++ raise ArgumentError, "invalid key"
++ end
++
+ File.join(base, self.class.indirection_name.to_s, name.to_s + ".yaml")
+ end
+
+diff --git a/spec/unit/indirector/ssl_file.rb b/spec/unit/indirector/ssl_file.rb
+index 7a9d629..077ccc2 100755
+--- a/spec/unit/indirector/ssl_file.rb
++++ b/spec/unit/indirector/ssl_file.rb
+@@ -89,6 +89,25 @@ describe Puppet::Indirector::SslFile do
+ end
+ end
+
++ ['../foo', '..\\foo', './../foo', '.\\..\\foo',
++ '/foo', '//foo', '\\foo', '\\\\goo',
++ "test\0/../bar", "test\0\\..\\bar",
++ "..\\/bar", "/tmp/bar", "/tmp\\bar", "tmp\\bar",
++ " / bar", " /../ bar", " \\..\\ bar",
++ "c:\\foo", "c:/foo", "\\\\?\\UNC\\bar", "\\\\foo\\bar",
++ "\\\\?\\c:\\foo", "//?/UNC/bar", "//foo/bar",
++ "//?/c:/foo",
++ ].each do |input|
++ it "should resist directory traversal attacks (#{input.inspect})" do
++ expect { @searcher.path(input) }.to raise_error
++ end
++ end
++
++ # REVISIT: Should probably test MS-DOS reserved names here, too, since
++ # they would represent a vulnerability on a Win32 system, should we ever
++ # support that path. Don't forget that 'CON.foo' == 'CON'
++ # --daniel 2011-09-24
++
+ describe "when finding certificates on disk" do
+ describe "and no certificate is present" do
+ before do
+diff --git a/spec/unit/indirector/yaml.rb b/spec/unit/indirector/yaml.rb
+index 0e70708..c5d357f 100755
+--- a/spec/unit/indirector/yaml.rb
++++ b/spec/unit/indirector/yaml.rb
+@@ -50,6 +50,21 @@ describe Puppet::Indirector::Yaml, " when choosing file location" do
+ it "should use the object's name to determine the file name" do
+ @store.path(:me).should =~ %r{me.yaml$}
+ end
++
++ ['../foo', '..\\foo', './../foo', '.\\..\\foo',
++ '/foo', '//foo', '\\foo', '\\\\goo',
++ "test\0/../bar", "test\0\\..\\bar",
++ "..\\/bar", "/tmp/bar", "/tmp\\bar", "tmp\\bar",
++ " / bar", " /../ bar", " \\..\\ bar",
++ "c:\\foo", "c:/foo", "\\\\?\\UNC\\bar", "\\\\foo\\bar",
++ "\\\\?\\c:\\foo", "//?/UNC/bar", "//foo/bar",
++ "//?/c:/foo",
++ ].each do |input|
++ it "should resist directory traversal attacks (#{input.inspect})" do
++ expect { @store.path(input) }.to raise_error
++ end
++ end
++
+ end
+
+ describe Puppet::Indirector::Yaml, " when storing objects as YAML" do
+--
+1.7.4.4
+
diff --git a/puppet.spec b/puppet.spec
index c4bca66..aed6a44 100644
--- a/puppet.spec
+++ b/puppet.spec
@@ -19,6 +19,14 @@ Patch0: puppet-0.25.5-yumrepo-deprecation-warning.patch
Patch1: puppet-0.25.5-puppet.conf-line-endings.patch
# http://projects.puppetlabs.com/issues/2359
Patch2: puppet-0.25.5-capture-stderr-from-exec.patch
+# http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-3848
+Patch3: 0001-Resist-directory-traversal-attacks-0.25.x.patch
+# http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-3871
+Patch4: 0.25-9792-Predictable-temporary-filename-in-ralsh.patch
+# http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-3870
+Patch5: 0.25.x-9791-TOCTOU-in-ssh-auth-keys-type.patch
+# http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2011-3869
+Patch6: 0.25.x-9794-k5login-can-overwrite-arbitrary-files-as-root.patch
Group: System Environment/Base
@@ -76,6 +84,10 @@ The server can also function as a certificate authority and file server.
%patch0 -p1
%patch1 -p1
%patch2 -p1
+%patch3 -p1
+%patch4 -p1
+%patch5 -p1
+%patch6 -p1
patch -p1 < conf/redhat/rundir-perms.patch
%build
@@ -233,6 +245,8 @@ rm -rf %{buildroot}
%changelog
* Mon Oct 03 2011 Todd Zullinger <tmz at pobox.com> - 0.25.5-2
+- Apply upstream patches for CVE-2011-3848, CVE-2011-3869, CVE-2011-3870,
+ CVE-2011-3871
- Create and own /usr/share/puppet/modules (#615432)
- Silence deprecation warnings in yumrepo type (#615175, upstream #4252)
- Handle CR/LF in puppet.conf (upstream #3514)
More information about the scm-commits
mailing list