adding extra checks to make sure networking is secure, and refactoring a heckuva lot of test

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

5 files changed, 134 insertions, 36 deletions

diff --git a/lib/puppet/client.rb b/lib/puppet/client.rb
index 38b9fa7fa..679db2c51 100644
--- a/lib/puppet/client.rb
+++ b/lib/puppet/client.rb
@@ -47,6 +47,11 @@ module Puppet
                         #Puppet.info "cert is %s" % @http.cert
                         begin
                             call("%s.%s" % [namespace, method.to_s],*args)
+                        rescue OpenSSL::SSL::SSLError => detail
+                            Puppet.err "Could not call %s.%s: Untrusted certificates" %
+                                [namespace, method]
+                            raise NetworkClientError,
+                                "Certificates were not trusted"
                         rescue XMLRPC::FaultException => detail
                             Puppet.err "Could not call %s.%s: %s" %
                                 [namespace, method, detail.faultString]
@@ -61,16 +66,26 @@ module Puppet
                 }
             }
 
-            [:key, :cert, :ca_file].each { |method|
-                setmethod = method.to_s + "="
-                #self.send(:define_method, method) {
-                #    @http.send(method)
-                #}
-                self.send(:define_method, setmethod) { |*args|
-                    Puppet.debug "Setting %s" % method 
-                    @http.send(setmethod, *args)
-                }
-            }
+            def ca_file=(cafile)
+                @http.ca_file = cafile
+                store = OpenSSL::X509::Store.new
+                cacert = OpenSSL::X509::Certificate.new(
+                    File.read(cafile)
+                )
+                store.add_cert(cacert) 
+                store.purpose = OpenSSL::X509::PURPOSE_SSL_CLIENT
+                @http.cert_store = store
+            end
+
+            def cert=(cert)
+                Puppet.info "Adding certificate"
+                @http.cert = cert
+                @http.verify_mode = OpenSSL::SSL::VERIFY_PEER
+            end
+
+            def key=(key)
+                @http.key = key
+            end
 
             def initialize(hash)
                 hash[:Path] ||= "/RPC2"
@@ -89,23 +104,17 @@ module Puppet
                 )
 
                 if hash[:Certificate]
-                    @http.cert = hash[:Certificate]
+                    self.cert = hash[:Certificate]
+                else
+                    Puppet.err "No certificate; running with reduced functionality."
                 end
 
                 if hash[:Key]
-                    @http.key = hash[:Key]
+                    self.key = hash[:Key]
                 end
 
                 if hash[:CAFile]
-                    @http.ca_file = hash[:CAFile]
-                    store = OpenSSL::X509::Store.new
-                    cacert = OpenSSL::X509::Certificate.new(
-                        File.read(hash[:CAFile])
-                    )
-                    store.add_cert(cacert) 
-                    store.purpose = OpenSSL::X509::PURPOSE_SSL_CLIENT
-                    @http.cert_store = store
-                    @http.verify_mode = OpenSSL::SSL::VERIFY_NONE
+                    self.ca_file = hash[:CAFile]
                 end
 
                 # from here, i need to add the key, cert, and ca cert
@@ -141,14 +150,13 @@ module Puppet
                 end
             end
 
+            # unless we have a driver, we're a local client and we can't add
+            # certs anyway, so it doesn't matter
             unless @driver
                 return true
             end
 
-            Puppet.info "setting cert and key and such"
-            @driver.cert = @cert
-            @driver.key = @key
-            @driver.ca_file = @cacertfile
+            self.setcerts
         end
 
         def initialize(hash)
@@ -196,6 +204,12 @@ module Puppet
             end
         end
 
+        def setcerts
+            @driver.cert = @cert
+            @driver.key = @key
+            @driver.ca_file = @cacertfile
+        end
+
         class MasterClient < Puppet::Client
             @drivername = :Master
 
@@ -217,8 +231,6 @@ module Puppet
                     raise Puppet::Error, "Cannot apply; objects not defined"
                 end
 
-                # XXX this is kind of a problem; if the user changes the state file
-                # after this, then we have to reload the file and everything...
                 begin
                     Puppet::Storage.init
                     Puppet::Storage.load
@@ -398,6 +410,72 @@ module Puppet
             end
         end
 
+        # unlike the other client classes (again, this design sucks) this class
+        # is basically just a proxy class -- it calls its methods on the driver
+        # and that's about it
+        class ProxyClient < Puppet::Client
+            def self.mkmethods
+                interface = @handler.interface
+                namespace = interface.prefix
+
+                interface.methods.each { |ary|
+                    method = ary[0]
+                    Puppet.debug "%s: defining %s.%s" % [self, namespace, method]
+                    self.send(:define_method,method) { |*args|
+                        begin
+                            @driver.send(method, *args)
+                        rescue XMLRPC::FaultException => detail
+                            Puppet.err "Could not call %s.%s: %s" %
+                                [namespace, method, detail.faultString]
+                            raise NetworkClientError,
+                                "XMLRPC Error: %s" % detail.faultString
+                        end
+                    }
+                }
+            end
+        end
+
+        class FileClient < Puppet::Client::ProxyClient
+            @drivername = :FileServer
+
+            # set up the appropriate interface methods
+            @handler = Puppet::Server::FileServer
+
+            self.mkmethods
+
+            def initialize(hash = {})
+                if hash.include?(:FileServer)
+                    unless hash[:FileServer].is_a?(Puppet::Server::FileServer)
+                        raise Puppet::DevError, "Must pass an actual FS object"
+                    end
+                end
+
+                super(hash)
+            end
+        end
+
+        class CAClient < Puppet::Client::ProxyClient
+            @drivername = :CA
+
+            # set up the appropriate interface methods
+            @handler = Puppet::Server::CA
+            self.mkmethods
+
+            def initialize(hash = {})
+                if hash.include?(:CA)
+                    hash[:CA] = Puppet::Server::CA.new()
+                end
+
+                super(hash)
+            end
+        end
+
+        class StatusClient < Puppet::Client::ProxyClient
+            # set up the appropriate interface methods
+            @handler = Puppet::Server::ServerStatus
+            self.mkmethods
+        end
+
     end
 #---------------------------------------------------------------
 end
diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index 4c2f699c0..1918f60e9 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -11,6 +11,8 @@ module Puppet
         class ASTError < RuntimeError; end
         #---------------------------------------------------------------
         class AST
+            Puppet.setdefault(:typecheck, true)
+            Puppet.setdefault(:paramcheck, true)
             attr_accessor :line, :file, :parent
 
             @@pink = ""
@@ -482,6 +484,8 @@ module Puppet
                     # for types
                     objtype = @type.value
 
+                    # This will basically always be on, but I wanted to make it at
+                    # least simple to turn off if it came to that
                     if Puppet[:typecheck]
                         builtin = false
                         begin
@@ -492,6 +496,8 @@ module Puppet
                         if builtin
                             # we're a builtin type
                             #Puppet.debug "%s is a builtin type" % objtype
+                            # like :typecheck, this always defaults to on, but
+                            # at least it's easy to turn off if necessary
                             if Puppet[:paramcheck]
                                 @params.each { |param|
                                     #p self.name
diff --git a/lib/puppet/server.rb b/lib/puppet/server.rb
index 6c9dbf37b..15b27f849 100644
--- a/lib/puppet/server.rb
+++ b/lib/puppet/server.rb
@@ -76,7 +76,7 @@ module Puppet
                 hash[:SSLStartImmediately] = true
                 hash[:SSLEnable] = true
                 hash[:SSLCACertificateFile] = @cacertfile
-                hash[:SSLVerifyClient] = OpenSSL::SSL::VERIFY_NONE
+                hash[:SSLVerifyClient] = OpenSSL::SSL::VERIFY_PEER
                 hash[:SSLCertName] = nil
 
                 super(hash)
diff --git a/lib/puppet/server/servlet.rb b/lib/puppet/server/servlet.rb
index 4c45ebc62..2bc2dffc1 100644
--- a/lib/puppet/server/servlet.rb
+++ b/lib/puppet/server/servlet.rb
@@ -15,7 +15,20 @@ class Server
         end
 
         def authorize(request, method)
-            true
+            if request.client_cert
+                Puppet.info "Allowing %s(%s) trusted access to %s" %
+                    [request.peeraddr[2], request.peeraddr[3], method]
+                return true
+            else
+                if method =~ /^puppetca\./
+                    Puppet.notice "Allowing %s(%s) untrusted access to CA methods" %
+                        [request.peeraddr[2], request.peeraddr[3]]
+                else
+                    Puppet.err "Unauthenticated client %s(%s) cannot call %s" %
+                        [request.peeraddr[2], request.peeraddr[3], method]
+                    return false
+                end
+            end
         end
 
         def initialize(server, handlers)
@@ -79,12 +92,12 @@ class Server
                 )
             end
 
-            if request.client_cert
-                Puppet.info "client cert is %s" % request.client_cert
-            end
-            if request.server_cert
-                #Puppet.info "server cert is %s" % @request.server_cert
-            end
+            #if request.client_cert
+            #    Puppet.info "client cert is %s" % request.client_cert
+            #end
+            #if request.server_cert
+            #    Puppet.info "server cert is %s" % @request.server_cert
+            #end
             #p @request
             begin
                 super
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 4ae9cb97a..df22c59b2 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -1305,7 +1305,8 @@ module Puppet
                     if uri.port
                         args[:Port] = uri.port
                     end
-                    sourceobj.server = Puppet::NetworkClient.new(args)
+                    #sourceobj.server = Puppet::NetworkClient.new(args)
+                    sourceobj.server = Puppet::Client::FileClient.new(args)
 
                     tmp = uri.path
                     if tmp =~ %r{^/(\w+)}