Fix #1875 - Add a REST authorization system

This patch introduces a new configuration file (and configuration
setting to set it).
Each REST request is checked against this configuration file, and is
either allowed or denied.

The configuration file has the following format:

path /uripath
method <methods>
allow <ip> or <name>
deny <ip> or <name>

or
path ~ <regex>
method <methods>
allow <ip> or <name>
deny <ip> or <name>

where regex is a ruby regex.
This last syntax allows deny/allow interpolation from
the regex captures:

path ~ /files[^/]+/files/([^/]+)/([^/])/
method find
allow $2.$1

If you arrange your files/ directory to have files in
'domain.com/host/', then only the referenced host will
be able to access their files, other hosts will be denied.

For instance:
files/reductivelabs.com/dns/...
files/reductivelabs.com/www/...
then only files in dns can be accessible by dns.reductivelabs.com
and so on...

If the auth.conf file doesn't exist puppet uses sane defaults that allows
clients to check-in and ask for their configurations...

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

8 files changed, 406 insertions, 1 deletions

diff --git a/conf/auth.conf b/conf/auth.conf
new file mode 100644
index 000000000..1c037901d
--- /dev/null
+++ b/conf/auth.conf
@@ -0,0 +1,66 @@
+# This is an example auth.conf file, it mimics the puppetmasterd defaults
+# 
+# The ACL are checked in order of appearance in this file.
+# 
+# Supported syntax:
+# This file supports two different syntax depending on how 
+# you want to express the ACL.
+# 
+# Path syntax (the one used below):
+# ---------------------------------
+# path /path/to/resource
+# [method methodlist]
+# allow [host|ip|*]
+# deny [host|ip]
+# 
+# The path is matched as a prefix. That is /file match at
+# the same time /file_metadat and /file_content.
+# 
+# Regex syntax:
+# -------------
+# This one is differenciated from the path one by a '~'
+# 
+# path ~ regex
+# [method methodlist]
+# allow [host|ip|*]
+# deny [host|ip]
+# 
+# The regex syntax is the same as ruby ones.
+# 
+# Ex:
+# path ~ .pp$
+# will match every resource ending in .pp (manifests files for instance)
+# 
+# path ~ ^/path/to/resource
+# is essentially equivalent to path /path/to/resource
+# 
+
+# allow nodes to drop and find their facts
+path /facts
+method save,find
+allow *
+
+# allow all nodes to get their catalogs (ie their configuration)
+path /catalog
+method find
+allow *
+
+# allow all nodes to access the certificates services
+path /certificate
+method find
+allow *
+
+# allow all nodes to store their reports
+path /report
+method save
+allow *
+
+# inconditionnally allow access to all files services
+# which means in practice that fileserver.conf will
+# still be used
+path /file
+allow *
+
+# this one is not stricly necessary, but it has the merit
+# to show the default policy which is deny everything else
+path /
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index f21fbc26a..0eed2f884 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -395,6 +395,11 @@ module Puppet
             :group => "$group",
             :desc => "Where FileBucket files are stored."
         },
+        :rest_authconfig => [ "$confdir/auth.conf",
+            "The configuration file that defines the rights to the different
+            rest indirections.  This can be used as a fine-grained
+            authorization system for ``puppetmasterd``."
+        ],
         :ca => [true, "Wether the master should function as a certificate authority."],
         :modulepath => {:default => "$confdir/modules:/usr/share/puppet/modules",
            :desc => "The search path for modules as a colon-separated list of
diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index 04ba14401..20234b2da 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -2,9 +2,11 @@ module Puppet::Network::HTTP
 end
 
 require 'puppet/network/http/api/v1'
+require 'puppet/network/rest_authorization'
 
 module Puppet::Network::HTTP::Handler
     include Puppet::Network::HTTP::API::V1
+    include Puppet::Network::RestAuthorization
 
     attr_reader :server, :handler
 
@@ -38,7 +40,11 @@ module Puppet::Network::HTTP::Handler
     def process(request, response)
         indirection_request = uri2indirection(http_method(request), path(request), params(request))
 
-        send("do_%s" % indirection_request.method, indirection_request, request, response)
+        if authorized?(indirection_request)
+            send("do_%s" % indirection_request.method, indirection_request, request, response)
+        else
+            return do_exception(response, "Request forbidden by configuration %s %s" % [indirection_request.indirection_name, indirection_request.key], 403)
+        end
     rescue Exception => e
         return do_exception(response, e)
     end
diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb
new file mode 100644
index 000000000..58708e120
--- /dev/null
+++ b/lib/puppet/network/rest_authconfig.rb
@@ -0,0 +1,72 @@
+require 'puppet/network/authconfig'
+
+module Puppet
+    class Network::RestAuthConfig < Network::AuthConfig
+
+        attr_accessor :rights
+
+        DEFAULT_ACL = {
+            :facts =>   { :acl => "/facts", :method => [:save, :find] },
+            :catalog => { :acl => "/catalog", :method => :find },
+            # 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 }
+        }
+
+        def self.main
+            add_acl = @main.nil?
+            super
+            @main.insert_default_acl if add_acl and !@main.exists?
+            @main
+        end
+
+        # check wether this request is allowed in our ACL
+        def allowed?(request)
+            read()
+            return @rights.allowed?(build_uri(request), request.node, request.ip, request.method)
+        end
+
+        def initialize(file = nil, parsenow = true)
+            super(file || Puppet[:rest_authconfig], parsenow)
+
+            # if we didn't read a file (ie it doesn't exist)
+            # make sure we can create some default rights
+            @rights ||= Puppet::Network::Rights.new
+        end
+
+        def parse()
+            super()
+            insert_default_acl
+        end
+
+        # force regular ACLs to be present
+        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])
+                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["/"]
+        end
+
+        def mk_acl(path, method = nil)
+            @rights.newright(path)
+            @rights.allow(path, "*")
+
+            if method
+                method = [method] unless method.is_a?(Array)
+                method.each { |m| @rights.restrict_method(path, m) }
+            end
+        end
+
+        def build_uri(request)
+            "/#{request.indirection_name}/#{request.key}"
+        end
+    end
+end
diff --git a/lib/puppet/network/rest_authorization.rb b/lib/puppet/network/rest_authorization.rb
new file mode 100644
index 000000000..3278640fe
--- /dev/null
+++ b/lib/puppet/network/rest_authorization.rb
@@ -0,0 +1,49 @@
+require 'puppet/network/client_request'
+require 'puppet/network/rest_authconfig'
+
+module Puppet::Network
+    # Most of our subclassing is just so that we can get
+    # access to information from the request object, like
+    # the client name and IP address.
+    class InvalidClientRequest < Puppet::Error; end
+    module RestAuthorization
+
+        # Create our config object if necessary. If there's no configuration file
+        # we install our defaults
+        def authconfig
+            unless defined? @authconfig
+                @authconfig = Puppet::Network::RestAuthConfig.main
+            end
+
+            @authconfig
+        end
+
+        # Verify that our client has access.  We allow untrusted access to
+        # certificates terminus but no others.
+        def authorized?(request)
+            msg = "%s client %s access to %s [%s]" %
+                   [ request.authenticated? ? "authenticated" : "unauthenticated",
+                    (request.node.nil? ? request.ip : "#{request.node}(#{request.ip})"),
+                    request.indirection_name, request.method ]
+
+            if request.authenticated?
+                res = authenticated_authorized?(request, msg )
+            else
+                res = unauthenticated_authorized?(request, msg)
+            end
+            Puppet.notice((res ? "Allowing " : "Denying ") + msg)
+            return res
+        end
+
+        # delegate to our authorization file
+        def authenticated_authorized?(request, msg)
+            authconfig.allowed?(request)
+        end
+
+        # allow only certificate requests when not authenticated
+        def unauthenticated_authorized?(request, msg)
+            request.indirection_name == :certificate or request.indirection_name == :certificate_request
+        end
+    end
+end
+
diff --git a/spec/unit/network/http/handler.rb b/spec/unit/network/http/handler.rb
index 84b87025f..7b7ef4722 100755
--- a/spec/unit/network/http/handler.rb
+++ b/spec/unit/network/http/handler.rb
@@ -2,6 +2,7 @@
 
 require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/network/http/handler'
+require 'puppet/network/rest_authorization'
 
 class HttpHandled
     include Puppet::Network::HTTP::Handler
@@ -16,6 +17,10 @@ describe Puppet::Network::HTTP::Handler do
         Puppet::Network::HTTP::Handler.ancestors.should be_include(Puppet::Network::HTTP::API::V1)
     end
 
+    it "should include the Rest Authorization system" do
+        Puppet::Network::HTTP::Handler.ancestors.should be_include(Puppet::Network::RestAuthorization)
+    end
+
     it "should have a method for initializing" do
         @handler.should respond_to(:initialize_for_puppet)
     end
@@ -44,6 +49,8 @@ describe Puppet::Network::HTTP::Handler do
 
             @result = stub 'result', :render => "mytext"
 
+            @handler.stubs(:authorized?).returns(true)
+
             stub_server_interface
         end
 
@@ -82,6 +89,32 @@ describe Puppet::Network::HTTP::Handler do
             @handler.process(@request, @response)
         end
 
+        it "should delegate authorization to the RestAuthorization layer" do
+            request = stub 'request'
+            @handler.expects(:uri2indirection).returns request
+
+            request.expects(:method).returns "mymethod"
+
+            @handler.expects(:do_mymethod).with(request, @request, @response)
+
+            @handler.expects(:authorized?).with(request).returns(true)
+
+            @handler.process(@request, @response)
+        end
+
+        it "should return 403 if the request is not authorized" do
+            request = stub 'request'
+            @handler.expects(:uri2indirection).returns request
+
+            @handler.expects(:do_mymethod).never
+
+            @handler.expects(:authorized?).with(request).returns(false)
+
+            @handler.expects(:set_response)#.with { |response, body, status| status == 403 }
+
+            @handler.process(@request, @response)
+        end
+
         it "should serialize a controller exception when an exception is thrown while finding the model instance" do
             @handler.expects(:uri2indirection).returns stub("request", :method => :find)
 
diff --git a/spec/unit/network/rest_authconfig.rb b/spec/unit/network/rest_authconfig.rb
new file mode 100755
index 000000000..1f98f4082
--- /dev/null
+++ b/spec/unit/network/rest_authconfig.rb
@@ -0,0 +1,118 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/network/rest_authconfig'
+
+describe Puppet::Network::RestAuthConfig do
+    before :each do
+        FileTest.stubs(:exists?).returns(true)
+        File.stubs(:stat).returns(stub 'stat', :ctime => :now)
+        Time.stubs(:now).returns :now
+
+        @authconfig = Puppet::Network::RestAuthConfig.new("dummy", false)
+        @authconfig.stubs(:read)
+
+        @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
+    end
+
+    it "should use the puppet default rest authorization file" do
+        Puppet.expects(:[]).with(:rest_authconfig).returns("dummy")
+
+        Puppet::Network::RestAuthConfig.new(nil, false)
+    end
+
+    it "should read the config file when needed" do
+        @authconfig.expects(:read)
+
+        @authconfig.allowed?(@request)
+    end
+
+    it "should ask for authorization to the ACL subsystem" do
+        @acl.expects(:allowed?).with("/path/to/resource", "me", "127.0.0.1", :save)
+
+        @authconfig.allowed?(@request)
+    end
+
+    describe "when defining an acl with mk_acl" do
+        it "should create a new right for each default acl" do
+            @acl.expects(:newright).with(:path)
+            @authconfig.mk_acl(:path)
+        end
+
+        it "should allow everyone for each default right" do
+            @acl.expects(:allow).with(:path, "*")
+            @authconfig.mk_acl(:path)
+        end
+
+        it "should restrict the ACL to a method" do
+            @acl.expects(:restrict_method).with(:path, :method)
+            @authconfig.mk_acl(:path, :method)
+        end
+    end
+
+    describe "when parsing the configuration file" do
+        it "should check for missing ACL after reading the authconfig file" do
+            File.stubs(:open)
+
+            @authconfig.expects(:insert_default_acl)
+
+            @authconfig.parse()
+        end
+    end
+
+    [ "/facts", "/report", "/catalog", "/file"].each do |acl|
+        it "should insert #{acl} if not present" do
+            @authconfig.rights.stubs(:[]).returns(true)
+            @authconfig.rights.stubs(:[]).with(acl).returns(nil)
+
+            @authconfig.expects(:mk_acl).with { |a,m| a == acl }
+
+            @authconfig.insert_default_acl
+        end
+
+        it "should not insert #{acl} if present" do
+            @authconfig.rights.stubs(:[]).returns(true)
+            @authconfig.rights.stubs(:[]).with(acl).returns(true)
+
+            @authconfig.expects(:mk_acl).never
+
+            @authconfig.insert_default_acl
+        end
+    end
+
+    it "should create default ACL entries if no file have been read" do
+        Puppet::Network::RestAuthConfig.any_instance.stubs(:exists?).returns(false)
+
+        Puppet::Network::RestAuthConfig.any_instance.expects(:insert_default_acl)
+
+        Puppet::Network::RestAuthConfig.main
+    end
+
+    describe "when adding default ACLs" do
+
+        [
+            { :acl => "/facts", :method => [:save, :find] },
+            { :acl => "/catalog", :method => :find },
+            { :acl => "/report", :method => :save },
+            { :acl => "/file" },
+        ].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.insert_default_acl
+            end
+        end
+
+        it "should create a last catch-all deny all rule" do
+            @authconfig.stubs(:mk_acl)
+            @acl.expects(:newright).with("/")
+            @authconfig.insert_default_acl
+        end
+
+    end
+
+end
diff --git a/spec/unit/network/rest_authorization.rb b/spec/unit/network/rest_authorization.rb
new file mode 100755
index 000000000..15351b172
--- /dev/null
+++ b/spec/unit/network/rest_authorization.rb
@@ -0,0 +1,56 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/network/rest_authorization'
+
+class RestAuthorized
+    include Puppet::Network::RestAuthorization
+end
+
+
+describe Puppet::Network::RestAuthorization do
+    before :each do
+        @auth = RestAuthorized.new
+        @authconig = stub 'authconfig'
+        @auth.stubs(:authconfig).returns(@authconfig)
+
+        @request = stub_everything 'request'
+        @request.stubs(:method).returns(:find)
+        @request.stubs(:node).returns("node")
+    end
+
+    describe "when testing request authorization" do
+        describe "when the client is not authenticated" do
+            before :each do
+                @request.stubs(:authenticated?).returns(false)
+            end
+
+            [ :certificate, :certificate_request].each do |indirection|
+                it "should allow #{indirection}" do
+                    @request.stubs(:indirection_name).returns(indirection)
+                    @auth.authorized?(@request).should be_true
+                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)
+                    @auth.authorized?(@request).should be_false
+                end
+            end
+        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)
+
+                @auth.authorized?(@request)
+            end
+        end
+    end
+end
\ No newline at end of file