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 <[email protected]> --- conf/auth.conf | 33 +++++++++++++++++++++ lib/puppet/network/rest_authconfig.rb | 21 ++++++++----- lib/puppet/network/rest_authorization.rb | 21 +------------- spec/unit/network/rest_authconfig.rb | 25 +++++++++++----- spec/unit/network/rest_authorization.rb | 46 ++++++----------------------- 5 files changed, 74 insertions(+), 72 deletions(-) diff --git a/conf/auth.conf b/conf/auth.conf index 784acc9..d42a347 100644 --- a/conf/auth.conf +++ b/conf/auth.conf @@ -11,6 +11,7 @@ # path /path/to/resource # [environment envlist] # [method methodlist] +# [auth[enthicated] {yes|no|on|off}] # allow [host|ip|*] # deny [host|ip] # @@ -24,6 +25,7 @@ # path ~ regex # [environment envlist] # [method methodlist] +# [auth[enthicated] {yes|no|on|off}] # allow [host|ip|*] # deny [host|ip] # @@ -36,24 +38,35 @@ # path ~ ^/path/to/resource # is essentially equivalent to path /path/to/resource # +# environment:: restrict an ACL to a specific set of environments +# method:: restrict an ACL to a specific set of methods +# auth:: restrict an ACL to an authenticated or unauthenticated request +# +### Authenticated ACL - those applies only when the client +### has a valid certificate and is thus authenticated + # allow nodes to drop and find their facts path /facts +auth yes method save,find allow * # allow all nodes to get their catalogs (ie their configuration) path /catalog +auth yes method find allow * # allow all nodes to access the certificates services path /certificate +auth yes method find allow * # allow all nodes to store their reports path /report +auth yes method save allow * @@ -61,6 +74,26 @@ allow * # which means in practice that fileserver.conf will # still be used path /file +auth yes +allow * + +### Unauthenticated ACL, for clients for which the current master doesn't +### have a valid certificate + +# allow access to the master CA +path /certificate/ca +auth no +method find +allow * + +path /certificate/ +auth no +method find +allow * + +path /certificate_request +auth no +method find, save allow * # this one is not stricly necessary, but it has the merit diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb index 22be8b0..2b15b96 100644 --- a/lib/puppet/network/rest_authconfig.rb +++ b/lib/puppet/network/rest_authconfig.rb @@ -6,13 +6,16 @@ module Puppet attr_accessor :rights DEFAULT_ACL = { - :facts => { :acl => "/facts", :method => [:save, :find] }, - :catalog => { :acl => "/catalog", :method => :find }, + :facts => { :acl => "/facts", :method => [:save, :find], :authenticated => true }, + :catalog => { :acl => "/catalog", :method => :find, :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 } + :cert => { :acl => "/certificate", :method => :find, :authenticated => true }, + :reports => { :acl => "/report", :method => :save, :authenticated => true }, + :cert_ca => { :acl => "/certificate/ca", :method => :find, :authenticated => false }, + :get_cert =>{ :acl => "/certificate/", :method => :find, :authenticated => false }, + :cert_request =>{ :acl => "/certificate_request", :method => [:find, :save], :authenticated => false }, } def self.main @@ -32,7 +35,8 @@ module Puppet :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) @@ -52,8 +56,8 @@ module Puppet def insert_default_acl DEFAULT_ACL.each do |name, 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" , ( @file || "no file configured")] + mk_acl(acl[:acl], acl[:method], acl[:authenticated]) end end # queue an empty (ie deny all) right for every other path @@ -62,7 +66,7 @@ module Puppet rights.newright("/") unless rights["/"] end - def mk_acl(path, method = nil) + def mk_acl(path, method = nil, authenticated = nil) @rights.newright(path) @rights.allow(path, "*") @@ -70,6 +74,7 @@ module Puppet method = [method] unless method.is_a?(Array) method.each { |m| @rights.restrict_method(path, m) } end + @rights.restrict_authenticated(path, authenticated) unless authenticated.nil? end def build_uri(request) diff --git a/lib/puppet/network/rest_authorization.rb b/lib/puppet/network/rest_authorization.rb index e6f62d9..1294a3d 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 diff --git a/spec/unit/network/rest_authconfig.rb b/spec/unit/network/rest_authconfig.rb index 7dc9738..7f6817a 100644 --- a/spec/unit/network/rest_authconfig.rb +++ b/spec/unit/network/rest_authconfig.rb @@ -16,8 +16,8 @@ describe Puppet::Network::RestAuthConfig do @acl = stub_everything 'rights' @authconfig.rights = @acl - @request = stub 'request', :indirection_name => "path", :key => "to/resource", :ip => "127.0.0.1", - :node => "me", :method => :save, :environment => :env + @request = stub 'request', :indirection_name => "path", :key => "to/resource", :ip => "127.0.0.1", + :node => "me", :method => :save, :environment => :env, :authenticated => true end it "should use the puppet default rest authorization file" do @@ -33,7 +33,7 @@ describe Puppet::Network::RestAuthConfig do end it "should ask for authorization to the ACL subsystem" do - @acl.expects(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env) + @acl.expects(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true) @authconfig.allowed?(@request) end @@ -53,6 +53,11 @@ describe Puppet::Network::RestAuthConfig do @acl.expects(:restrict_method).with(:path, :method) @authconfig.mk_acl(:path, :method) end + + it "should restrict the ACL to a specific authentication state" do + @acl.expects(:restrict_authenticated).with(:path, :authentication) + @authconfig.mk_acl(:path, nil, :authentication) + end end describe "when parsing the configuration file" do @@ -70,7 +75,7 @@ describe Puppet::Network::RestAuthConfig do @authconfig.rights.stubs(:[]).returns(true) @authconfig.rights.stubs(:[]).with(acl).returns(nil) - @authconfig.expects(:mk_acl).with { |a,m| a == acl } + @authconfig.expects(:mk_acl).with { |a,m,auth| a == acl } @authconfig.insert_default_acl end @@ -96,14 +101,18 @@ describe Puppet::Network::RestAuthConfig do describe "when adding default ACLs" do [ - { :acl => "/facts", :method => [:save, :find] }, - { :acl => "/catalog", :method => :find }, - { :acl => "/report", :method => :save }, + { :acl => "/facts", :method => [:save, :find], :authenticated => true }, + { :acl => "/catalog", :method => :find, :authenticated => true }, { :acl => "/file" }, + { :acl => "/certificate", :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 } ].each do |acl| it "should create a default right for #{acl[:acl]}" do @authconfig.stubs(:mk_acl) - @authconfig.expects(:mk_acl).with(acl[:acl], acl[:method]) + @authconfig.expects(:mk_acl).with(acl[:acl], acl[:method], acl[:authenticated]) @authconfig.insert_default_acl end end diff --git a/spec/unit/network/rest_authorization.rb b/spec/unit/network/rest_authorization.rb index 3acd23f..db42dd7 100644 --- a/spec/unit/network/rest_authorization.rb +++ b/spec/unit/network/rest_authorization.rb @@ -22,48 +22,22 @@ describe Puppet::Network::RestAuthorization do end describe "when testing request authorization" do - describe "when the client is not authenticated" do - before :each do - @request.stubs(:authenticated?).returns(false) - end + it "should delegate to the current rest authconfig" do + @authconfig.expects(:allowed?).with(@request).returns(true) - [ :certificate, :certificate_request].each do |indirection| - it "should allow #{indirection}" do - @request.stubs(:indirection_name).returns(indirection) - lambda { @auth.check_authorization(@request) }.should_not raise_error Puppet::Network::AuthorizationError - end - end - - [ :facts, :file_metadata, :file_content, :catalog, :report, :checksum, :runner ].each do |indirection| - it "should not allow #{indirection}" do - @request.stubs(:indirection_name).returns(indirection) - lambda { @auth.check_authorization(@request) }.should raise_error Puppet::Network::AuthorizationError - end - end + @auth.check_authorization(@request) end - describe "when the client is authenticated" do - before :each do - @request.stubs(:authenticated?).returns(true) - end - - it "should delegate to the current rest authconfig" do - @authconfig.expects(:allowed?).with(@request).returns(true) - - @auth.check_authorization(@request) - end + it "should raise an AuthorizationError if authconfig raises an AuthorizationError" do + @authconfig.expects(:allowed?).with(@request).raises(Puppet::Network::AuthorizationError.new("forbidden")) - it "should raise an AuthorizationError if authconfig raises an AuthorizationError" do - @authconfig.expects(:allowed?).with(@request).raises(Puppet::Network::AuthorizationError.new("forbidden")) - - lambda { @auth.check_authorization(@request) }.should raise_error Puppet::Network::AuthorizationError - end + lambda { @auth.check_authorization(@request) }.should raise_error Puppet::Network::AuthorizationError + end - it "should not raise an AuthorizationError if request is allowed" do - @authconfig.expects(:allowed?).with(@request).returns(true) + it "should not raise an AuthorizationError if request is allowed" do + @authconfig.expects(:allowed?).with(@request).returns(true) - lambda { @auth.check_authorization(@request) }.should_not raise_error Puppet::Network::AuthorizationError - end + lambda { @auth.check_authorization(@request) }.should_not raise_error Puppet::Network::AuthorizationError end end end \ No newline at end of file -- 1.6.0.2 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
