This should be the commit that brings us to Beta 1. All tests pass, although I get some (gracefully handled) failures in tc_metrics.rb, and there is now a config file for the fileserver module, including authorization specification for it. I have also reworked error handling in the xmlrpc client and server so errors should propagate more correctly.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@594 980ebf18-57e1-0310-9a29-db15c13687c0

5 files changed, 99 insertions, 33 deletions

diff --git a/lib/puppet/server/ca.rb b/lib/puppet/server/ca.rb
index 20caee9bb..669fe1290 100644
--- a/lib/puppet/server/ca.rb
+++ b/lib/puppet/server/ca.rb
@@ -67,7 +67,7 @@ class Server
 
         # our client sends us a csr, and we either store it for later signing,
         # or we sign it right away
-        def getcert(csrtext, request = nil)
+        def getcert(csrtext, client = nil, clientip = nil)
             # okay, i need to retrieve the hostname from the csr, and then
             # verify that i get the same hostname through reverse lookup or
             # something
diff --git a/lib/puppet/server/filebucket.rb b/lib/puppet/server/filebucket.rb
index fa02d967a..9c02e0794 100755
--- a/lib/puppet/server/filebucket.rb
+++ b/lib/puppet/server/filebucket.rb
@@ -60,7 +60,7 @@ class Server
         end
 
         # accept a file from a client
-        def addfile(string,path, request = nil)
+        def addfile(string,path, client = nil, clientip = nil)
             #puts "entering addfile"
             contents = Base64.decode64(string)
             #puts "string is decoded"
@@ -129,7 +129,7 @@ class Server
             return md5
         end
 
-        def getfile(md5, request = nil)
+        def getfile(md5, client = nil, clientip = nil)
             bpath, bfile, bpaths = FileBucket.paths(@bucket,md5)
 
             unless FileTest.exists?(bfile)
diff --git a/lib/puppet/server/fileserver.rb b/lib/puppet/server/fileserver.rb
index 179459f42..6eec3503d 100755
--- a/lib/puppet/server/fileserver.rb
+++ b/lib/puppet/server/fileserver.rb
@@ -1,4 +1,5 @@
 require 'puppet'
+require 'webrick/httpstatus'
 require 'cgi'
 
 module Puppet
@@ -40,18 +41,22 @@ class Server
             return obj
         end
 
-        def describe(file, request = nil)
+        def describe(file, client = nil, clientip = nil)
             mount, path = splitpath(file)
 
-            subdir = nil
-            unless subdir = subdir(mount, path)
+            unless @mounts[mount].allowed?(client, clientip)
+                raise Puppet::Server::AuthorizationError, "Cannot access %s" % mount
+            end
+
+            sdir = nil
+            unless sdir = subdir(mount, path)
                 Puppet.notice "Could not find subdirectory %s" %
                     "//%s/%s" % [mount, path]
                 return ""
             end
 
             obj = nil
-            unless obj = self.check(subdir)
+            unless obj = self.check(sdir)
                 return ""
             end
 
@@ -112,9 +117,13 @@ class Server
             end
         end
 
-        def list(dir, recurse = false, sum = "md5", request = nil)
+        def list(dir, recurse = false, client = nil, clientip = nil)
             mount, path = splitpath(dir)
 
+            unless @mounts[mount].allowed?(client, clientip)
+                raise Puppet::Server::AuthorizationError, "Cannot access %s" % mount
+            end
+
             subdir = nil
             unless subdir = subdir(mount, path)
                 Puppet.notice "Could not find subdirectory %s" %
@@ -194,11 +203,25 @@ class Server
                                 mount.path = value
                             when "allow":
                                 value.split(/\s*,\s*/).each { |val|
-                                    mount.allow(val)
+                                    begin
+                                        Puppet.info "Allowing %s access to %s" %
+                                            [val, mount.name]
+                                        mount.allow(val)
+                                    rescue AuthStoreError => detail
+                                        raise Puppet::Error, "%s at line %s of %s" %
+                                            [detail.to_s, count, @config]
+                                    end
                                 }
                             when "deny":
                                 value.split(/\s*,\s*/).each { |val|
-                                    mount.deny(val)
+                                    begin
+                                        Puppet.info "Denying %s access to %s" %
+                                            [val, mount.name]
+                                        mount.deny(val)
+                                    rescue AuthStoreError => detail
+                                        raise Puppet::Error, "%s at line %s of %s" %
+                                            [detail.to_s, count, @config]
+                                    end
                                 }
                             else
                                 raise Puppet::Error,
@@ -218,12 +241,15 @@ class Server
             end
         end
 
-        def retrieve(file, request = nil)
+        def retrieve(file, client = nil, clientip = nil)
             mount, path = splitpath(file)
 
             unless (@mounts.include?(mount))
-                # FIXME I really need some better way to pass and handle xmlrpc errors
-                raise FileServerError, "%s not mounted" % mount
+                raise Puppet::Server::FileServerError, "%s not mounted" % mount
+            end
+
+            unless @mounts[mount].allowed?(client, clientip)
+                raise Puppet::Server::AuthorizationError, "Cannot access %s" % mount
             end
 
             fpath = nil
@@ -327,30 +353,20 @@ class Server
             return dirname
         end
 
-        class Mount
+        class Mount < AuthStore
             attr_reader :path, :name
 
-            def allow(pattern)
-            end
-
-            def allowed?(host)
-            end
-
-            def deny(pattern)
-            end
-
             def initialize(name, path = nil)
                 unless name =~ %r{^\w+$}
                     raise FileServerError, "Invalid name format '%s'" % name
                 end
                 @name = name
 
-                @allow = []
-                @deny = []
-
                 if path
                     self.path = path
                 end
+
+                super()
             end
 
             def path=(path)
diff --git a/lib/puppet/server/master.rb b/lib/puppet/server/master.rb
index 00d62eb5b..b45f91210 100644
--- a/lib/puppet/server/master.rb
+++ b/lib/puppet/server/master.rb
@@ -17,6 +17,7 @@ class Server
 
         def initialize(hash = {})
 
+            # FIXME this should all be s/:File/:Manifest/g or something
             # build our AST
             @file = hash[:File] || Puppet[:manifest]
             @parser = Puppet::Parser::Parser.new()
@@ -37,8 +38,8 @@ class Server
             end
         end
 
-        def getconfig(facts, request = nil)
-            if request
+        def getconfig(facts, client = nil, clientip = nil)
+            if client
                 #Puppet.warning request.inspect
             end
             if @local
diff --git a/lib/puppet/server/servlet.rb b/lib/puppet/server/servlet.rb
index b14efe645..4c45ebc62 100644
--- a/lib/puppet/server/servlet.rb
+++ b/lib/puppet/server/servlet.rb
@@ -4,6 +4,8 @@ module Puppet
 class Server
     class ServletError < RuntimeError; end
     class Servlet < XMLRPC::WEBrickServlet
+        ERR_UNAUTHORIZED = 30
+
         attr_accessor :request
 
         # this is just a duplicate of the normal method; it's here for
@@ -12,6 +14,10 @@ class Server
             self.new(server, *options)
         end
 
+        def authorize(request, method)
+            true
+        end
+
         def initialize(server, handlers)
             #Puppet.info server.inspect
             
@@ -28,28 +34,55 @@ class Server
             }
 
             @request = nil
+            @client = nil
+            @clientip = nil
             self.set_service_hook { |obj, *args|
                 #raise "crap!"
-                if @request
-                    args.push @request
+                if @client and @clientip
+                    args.push(@client, @clientip)
                     #obj.call(args, @request)
                 end
                 begin
                     obj.call(*args)
+                rescue Puppet::Server::AuthorizationError => detail
+                    Puppet.warning obj.inspect
+                    Puppet.warning args.inspect
+                    Puppet.err "Permission denied: %s" % detail.to_s
+                    raise XMLRPC::FaultException.new(
+                        1, detail.to_s
+                    )
+                rescue Puppet::Error => detail
+                    Puppet.warning obj.inspect
+                    Puppet.warning args.inspect
+                    Puppet.err "Puppet error: %s" % detail.to_s
+                    raise XMLRPC::FaultException.new(
+                        1, detail.to_s
+                    )
                 rescue => detail
                     Puppet.warning obj.inspect
                     Puppet.warning args.inspect
                     Puppet.err "Could not call: %s" % detail.to_s
+                    raise error
                 end
             }
         end
 
         def service(request, response)
             @request = request
-            if @request.client_cert
-                Puppet.info "client cert is %s" % @request.client_cert
+            if peer = request.peeraddr
+                @client = peer[2]
+                @clientip = peer[3]
+            else
+                raise XMLRPC::FaultException.new(
+                    ERR_UNCAUGHT_EXCEPTION,
+                    "Could not retrieve client information"
+                )
+            end
+
+            if request.client_cert
+                Puppet.info "client cert is %s" % request.client_cert
             end
-            if @request.server_cert
+            if request.server_cert
                 #Puppet.info "server cert is %s" % @request.server_cert
             end
             #p @request
@@ -59,6 +92,8 @@ class Server
                 Puppet.err "Could not service request: %s: %s" %
                     [detail.class, detail]
             end
+            @client = nil
+            @clientip = nil
             @request = nil
         end
 
@@ -66,7 +101,21 @@ class Server
 
         # this is pretty much just a copy of the original method but with more
         # feedback
+        # here's where we have our authorization hooks
         def dispatch(methodname, *args)
+
+            if defined? @request and @request
+                unless self.authorize(@request, methodname)
+                    raise XMLRPC::FaultException.new(
+                        ERR_UNAUTHORIZED,
+                        "Host %s not authorized to call %s" %
+                            [@request.host, methodname]
+                    )
+                end
+            else
+                raise Puppet::DevError, "Did not get request in dispatch"
+            end
+
             #Puppet.warning "dispatch on %s called with %s" %
             #    [methodname, args.inspect]
             for name, obj in @handler