CVE-2013-4455: Permissions, Privileges, and Access Controls (CWE-264)
value: Katello Installer before 0.0.18 uses world-readable permissions for /etc/pki/tls/private/katello-node.key when deploying a child Pulp node, which allows local users to obtain the private key by reading the file.
CWE description: Weaknesses in this category are related to the management of permissions, privileges, and other security features that are used to perform access control.
Published: 2014-05-14T19:55Z
Repository: https://github.com/Katello/katello-installer
Language: Other
Files changed: modules/apache/manifests/certs.pp, modules/certs/lib/puppet/provider/katello_ssl_tool.rb, modules/certs/lib/puppet/provider/privkey/katello_ssl_tool.rb, modules/kafo/manifests/foreman_certs.pp, modules/kafo/manifests/foreman_proxy_certs.pp, modules/kafo/manifests/puppet_certs.pp
Commit date: 2013-10-22 09:04:02 +0200
Commit message: 1021119 - make sure private keys are never world readable

This should prevent similar mistakes in future.

Diff: diff --git a/modules/apache/manifests/certs.pp b/modules/apache/manifests/certs.pp
index f94169a..d4d609e 100644
--- a/modules/apache/manifests/certs.pp
+++ b/modules/apache/manifests/certs.pp
@@ -32,10 +32,15 @@ class apache::certs (
       cert => Cert["${::certs::node_fqdn}-ssl"]
     } ~>
     privkey { $apache_ssl_key:
       ensure => present,
       cert => Cert["${::certs::node_fqdn}-ssl"]
+    } ->
+    file { $apache_ssl_key:
+      owner => $apache::params::user,
+      group => $apache::params::group,
+      mode  => '0400';
     }
 
     file { "${apache::params::configdir}/ssl.conf":
       content => template("apache/ssl.conf.erb"),
       mode   => '0644',
diff --git a/modules/certs/lib/puppet/provider/katello_ssl_tool.rb b/modules/certs/lib/puppet/provider/katello_ssl_tool.rb
index 4ee1b9c..cb77212 100644
--- a/modules/certs/lib/puppet/provider/katello_ssl_tool.rb
+++ b/modules/certs/lib/puppet/provider/katello_ssl_tool.rb
@@ -173,11 +173,11 @@ module Puppet::Provider::KatelloSslTool
       return false unless File.exists?(resource[:path])
       checksum(expected_content) == checksum(current_content)
     end
 
     def create
-      File.open(resource[:path], "w") { |f| f << expected_content }
+      File.open(resource[:path], "w", mode) { |f| f << expected_content }
     end
 
     protected
 
     def expected_content
@@ -196,10 +196,14 @@ module Puppet::Provider::KatelloSslTool
     # what path to copy from
     def source_path
       raise NotImplementedError
     end
 
+    def mode
+      0644
+    end
+
     def cert_details
       return @cert_details if defined? @cert_details
       if cert_resource = @resource[:cert]
         name = cert_resource.to_hash[:name]
         @cert_details = Puppet::Provider::KatelloSslTool::Cert.details(name)
diff --git a/modules/certs/lib/puppet/provider/privkey/katello_ssl_tool.rb b/modules/certs/lib/puppet/provider/privkey/katello_ssl_tool.rb
index e944256..a73e4bf 100644
--- a/modules/certs/lib/puppet/provider/privkey/katello_ssl_tool.rb
+++ b/modules/certs/lib/puppet/provider/privkey/katello_ssl_tool.rb
@@ -6,6 +6,10 @@ Puppet::Type.type(:privkey).provide(:katello_ssl_tool, :parent => Puppet::Provid
 
   def source_path
     cert_details[:privkey]
   end
 
+  def mode
+    0400
+  end
+
 end
diff --git a/modules/kafo/manifests/foreman_certs.pp b/modules/kafo/manifests/foreman_certs.pp
index 130adfc..121cc6b 100644
--- a/modules/kafo/manifests/foreman_certs.pp
+++ b/modules/kafo/manifests/foreman_certs.pp
@@ -34,11 +34,11 @@ class kafo::foreman_certs (
       cert => Cert["${::kafo::foreman_certs::hostname}-foreman-client"],
     } ->
 
     file { $client_key:
       owner => "foreman",
-      mode => "400"
+      mode => "0400"
     }
 
     pubkey { $client_ca:
       cert => $ca,
     }
diff --git a/modules/kafo/manifests/foreman_proxy_certs.pp b/modules/kafo/manifests/foreman_proxy_certs.pp
index 49f01d2..f12ed0f 100644
--- a/modules/kafo/manifests/foreman_proxy_certs.pp
+++ b/modules/kafo/manifests/foreman_proxy_certs.pp
@@ -34,11 +34,11 @@ class kafo::foreman_proxy_certs (
       cert => Cert["${::kafo::foreman_proxy_certs::hostname}-foreman-proxy"],
     } ->
 
     file { $proxy_key:
       owner => "foreman-proxy",
-      mode  => "400"
+      mode  => "0400"
     }
 
     pubkey { $proxy_ca:
       cert => $ca,
     }
diff --git a/modules/kafo/manifests/puppet_certs.pp b/modules/kafo/manifests/puppet_certs.pp
index 6dadf46..a39a92c 100644
--- a/modules/kafo/manifests/puppet_certs.pp
+++ b/modules/kafo/manifests/puppet_certs.pp
@@ -34,11 +34,11 @@ class kafo::puppet_certs (
       cert => Cert["${::kafo::puppet_certs::hostname}-puppet-client"],
     } ->
 
     file { $client_key:
       owner => "puppet",
-      mode => "400"
+      mode => "0400"
     }
 
     pubkey { $client_ca:
       cert => $ca,
     }

Vulnerable code:       File.open(resource[:path], "w") { |f| f << expected_content }
      mode => "400"
      mode  => "400"
      mode => "400"
Fixed code:     } ->
    file { $apache_ssl_key:
      owner => $apache::params::user,
      group => $apache::params::group,
      mode  => '0400';
      File.open(resource[:path], "w", mode) { |f| f << expected_content }
    def mode
      0644
    end

  def mode
    0400
  end

      mode => "0400"
      mode  => "0400"
      mode => "0400"