Unify auth/unauthenticated request authorization system

Before this change, unauthenticated REST requests where inconditionnaly
allowed, as long as they were to the certificate terminus.
This could be a security hole, so now the REST requests, authenticated
or unauthenticated are all submitted to the REST authorization
layer.
The default authorizations now contains directives to allow unauthenticated
requests to the various certificate terminus to allow new hosts.
The conf/auth.conf file has been modified to match such defaults.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

2 files changed, 28 insertions, 37 deletions

diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb
index 22be8b007..6e89cb85b 100644
--- a/lib/puppet/network/rest_authconfig.rb
+++ b/lib/puppet/network/rest_authconfig.rb
@@ -5,15 +5,17 @@ module Puppet
 
         attr_accessor :rights
 
-        DEFAULT_ACL = {
-            :facts =>   { :acl => "/facts", :method => [:save, :find] },
-            :catalog => { :acl => "/catalog", :method => :find },
+        DEFAULT_ACL = [
+            { :acl => "~ ^\/catalog\/([^\/]+)$", :method => :find, :allow => '$1', :authenticated => true },
             # this one will allow all file access, and thus delegate
             # to fileserver.conf
-            :file =>    { :acl => "/file" },
-            :cert =>    { :acl => "/certificate", :method => :find },
-            :reports => { :acl => "/report", :method => :save }
-        }
+            { :acl => "/file" },
+            { :acl => "/certificate_revocation_list/ca", :method => :find, :authenticated => true },
+            { :acl => "/report", :method => :save, :authenticated => true },
+            { :acl => "/certificate/ca", :method => :find, :authenticated => false },
+            { :acl => "/certificate/", :method => :find, :authenticated => false },
+            { :acl => "/certificate_request", :method => [:find, :save], :authenticated => false },
+        ]
 
         def self.main
             add_acl = @main.nil?
@@ -28,11 +30,15 @@ module Puppet
         def allowed?(request)
             read()
 
+            # we're splitting the request in part because
+            # fail_on_deny could as well be called in the XMLRPC context
+            # with a ClientRequest.
             @rights.fail_on_deny(build_uri(request),
                                     :node => request.node,
                                     :ip => request.ip,
                                     :method => request.method,
-                                    :environment => request.environment)
+                                    :environment => request.environment,
+                                    :authenticated => request.authenticated)
         end
 
         def initialize(file = nil, parsenow = true)
@@ -50,26 +56,30 @@ module Puppet
 
         # force regular ACLs to be present
         def insert_default_acl
-            DEFAULT_ACL.each do |name, acl|
+            DEFAULT_ACL.each do |acl|
                 unless rights[acl[:acl]]
-                    Puppet.warning "Inserting default '#{acl[:acl]}' acl because none were found in '%s'" % ( @file || "no file configured")
-                    mk_acl(acl[:acl], acl[:method])
+                    Puppet.warning "Inserting default '#{acl[:acl]}'(%s) acl because none were found in '%s'" % [acl[:authenticated] ? "auth" : "non-auth" , ( !exists? ? "no auth.conf file configured" : @file)]
+                    mk_acl(acl)
                 end
             end
             # queue an empty (ie deny all) right for every other path
             # actually this is not strictly necessary as the rights system
             # denies not explicitely allowed paths
-            rights.newright("/") unless rights["/"]
+            unless rights["/"]
+                rights.newright("/")
+                rights.restrict_authenticated("/", :any)
+            end
         end
 
-        def mk_acl(path, method = nil)
-            @rights.newright(path)
-            @rights.allow(path, "*")
+        def mk_acl(acl)
+            @rights.newright(acl[:acl])
+            @rights.allow(acl[:acl], acl[:allow] || "*")
 
-            if method
+            if method = acl[:method]
                 method = [method] unless method.is_a?(Array)
-                method.each { |m| @rights.restrict_method(path, m) }
+                method.each { |m| @rights.restrict_method(acl[:acl], m) }
             end
+            @rights.restrict_authenticated(acl[:acl], acl[:authenticated]) unless acl[:authenticated].nil?
         end
 
         def build_uri(request)
diff --git a/lib/puppet/network/rest_authorization.rb b/lib/puppet/network/rest_authorization.rb
index e6f62d914..1294a3dd1 100644
--- a/lib/puppet/network/rest_authorization.rb
+++ b/lib/puppet/network/rest_authorization.rb
@@ -16,29 +16,10 @@ module Puppet::Network
             @authconfig
         end
 
-        # Verify that our client has access.  We allow untrusted access to
-        # certificates terminus but no others.
+        # Verify that our client has access.  
         def check_authorization(request)
-            if request.authenticated?
-                authenticated_authorized?(request)
-            else
-                unless unauthenticated_authorized?(request)
-                    msg = "%s access to %s [%s]" % [ (request.node.nil? ? request.ip : "#{request.node}(#{request.ip})"), request.indirection_name, request.method ]
-                    Puppet.warning("Denying access: " + msg)
-                    raise AuthorizationError.new( "Forbidden request:" + msg )
-                end
-            end
-        end
-
-        # delegate to our authorization file
-        def authenticated_authorized?(request)
             authconfig.allowed?(request)
         end
-
-        # allow only certificate requests when not authenticated
-        def unauthenticated_authorized?(request)
-            request.indirection_name == :certificate or request.indirection_name == :certificate_request
-        end
     end
 end