[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 0.25.4-89-gcbbd363
James Turnbull
james at lovedthanlost.net
Tue May 18 09:04:20 UTC 2010
The following commit has been merged in the upstream branch:
commit 306d08259b7f67670e2143691f6dc50d655d832d
Author: Markus Roberts <Markus at reality.com>
Date: Mon Mar 29 17:16:05 2010 -0700
Revert the guts of #2890
This patch reverts the semantically significant parts of #2890 due to the
issues discussed on #3360 (security concerns when used with autosign,
inconsistency between REST & XMLRPC semantics) but leaves the semantically
neutral changes (code cleanup, added tests) in place.
This patch is intended for 0.25.x, but may also be applied as a step in the
resolution of #3450 (refactored #2890, add "remove_certs" flag) in Rolwf.
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index d762701..dc7e58f 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -161,19 +161,22 @@ class Puppet::Indirector::Indirection
end
end
- # Expire a cached object, if one is cached. Note that we now actually
- # remove it if possible, and only mark it as expired if destroy isn't
- # supported.
+ # Expire a cached object, if one is cached. Note that we don't actually
+ # remove it, we expire it and write it back out to disk. This way people
+ # can still use the expired object if they want.
def expire(key, *args)
- if cache? and instance = cache.find(request(:find, key, *args))
- Puppet.info "Expiring the #{name} cache of #{instance.name}"
- if cache.respond_to? :destroy
- cache.destroy(request(:destroy, instance, *args))
- else
- instance.expiration = Time.now - 1
- cache.save(request(:save,instance,*args))
- end
- end
+ request = request(:expire, key, *args)
+
+ return nil unless cache?
+
+ return nil unless instance = cache.find(request(:find, key, *args))
+
+ Puppet.info "Expiring the %s cache of %s" % [self.name, instance.name]
+
+ # Set an expiration date in the past
+ instance.expiration = Time.now - 60
+
+ cache.save(request(:save, instance, *args))
end
# Search for an instance in the appropriate terminus, caching the
@@ -213,7 +216,7 @@ class Puppet::Indirector::Indirection
return nil
end
- Puppet.debug "Using cached #{name} for #{request.key}, good until #{cached.expiration}"
+ Puppet.debug "Using cached %s for %s" % [self.name, request.key]
return cached
end
diff --git a/lib/puppet/ssl/certificate.rb b/lib/puppet/ssl/certificate.rb
index b6cba99..f9297f3 100644
--- a/lib/puppet/ssl/certificate.rb
+++ b/lib/puppet/ssl/certificate.rb
@@ -28,8 +28,7 @@ class Puppet::SSL::Certificate < Puppet::SSL::Base
end
def expiration
- # Our expiration is either that of the cache or the content, whichever comes first
- cache_expiration = @expiration
- [(content and content.not_after), cache_expiration].compact.sort.first
+ return nil unless content
+ return content.not_after
end
end
diff --git a/lib/puppet/ssl/host.rb b/lib/puppet/ssl/host.rb
index 7d34a4f..4cc519c 100644
--- a/lib/puppet/ssl/host.rb
+++ b/lib/puppet/ssl/host.rb
@@ -154,19 +154,26 @@ class Puppet::SSL::Host
end
def certificate
- @certificate ||= (
+ unless @certificate
+ generate_key unless key
+
# get the CA cert first, since it's required for the normal cert
# to be of any use.
- if not (key or generate_key) or not (ca? or Certificate.find("ca")) or not (cert = Certificate.find(name)) or cert.expired?
- nil
- elsif not cert.content.check_private_key(key.content)
- Certificate.expire(name)
- Puppet.warning "Retrieved certificate does not match private key"
- nil
- else
- cert
+ return nil unless Certificate.find("ca") unless ca?
+ return nil unless @certificate = Certificate.find(name)
+
+ unless certificate_matches_key?
+ raise Puppet::Error, "Retrieved certificate does not match private key; please remove certificate from server and regenerate it with the current key"
end
- )
+ end
+ @certificate
+ end
+
+ def certificate_matches_key?
+ return false unless key
+ return false unless certificate
+
+ return certificate.content.check_private_key(key.content)
end
# Generate all necessary parts of our ssl host.
diff --git a/lib/puppet/sslcertificates/ca.rb b/lib/puppet/sslcertificates/ca.rb
index f9efc02..f6bcbc1 100644
--- a/lib/puppet/sslcertificates/ca.rb
+++ b/lib/puppet/sslcertificates/ca.rb
@@ -278,13 +278,12 @@ class Puppet::SSLCertificates::CA
host = thing2name(csr)
csrfile = host2csrfile(host)
- raise Puppet::Error, "Certificate request for #{host} already exists" if File.exists?(csrfile)
- Puppet.settings.writesub(:csrdir, csrfile) { |f| f.print csr.to_pem }
+ if File.exists?(csrfile)
+ raise Puppet::Error, "Certificate request for %s already exists" % host
+ end
- certfile = host2certfile(host)
- if File.exists?(certfile)
- Puppet.notice "Removing previously signed certificate #{certfile} for #{host}"
- Puppet::SSLCertificates::Inventory::rebuild
+ Puppet.settings.writesub(:csrdir, csrfile) do |f|
+ f.print csr.to_pem
end
end
diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb
index ca2a412..311cc53 100755
--- a/spec/unit/indirector/indirection.rb
+++ b/spec/unit/indirector/indirection.rb
@@ -545,44 +545,33 @@ describe Puppet::Indirector::Indirection do
@indirection.expire("/my/key")
end
- describe "and the terminus supports removal of cache items with destroy" do
- it "should destroy the cached instance" do
- @cache.expects(:find).returns @cached
- @cache.expects(:destroy).with { |r| r.method == :destroy and r.key == "/my/key" }
- @cache.expects(:save).never
- @indirection.expire("/my/key")
- end
- end
-
- describe "and the terminus does not support removal of cache items with destroy" do
- it "should set the cached instance's expiration to a time in the past" do
- @cache.expects(:find).returns @cached
- @cache.stubs(:save)
+ it "should set the cached instance's expiration to a time in the past" do
+ @cache.expects(:find).returns @cached
+ @cache.stubs(:save)
- @cached.expects(:expiration=).with { |t| t < Time.now }
+ @cached.expects(:expiration=).with { |t| t < Time.now }
- @indirection.expire("/my/key")
- end
+ @indirection.expire("/my/key")
+ end
- it "should save the now expired instance back into the cache" do
- @cache.expects(:find).returns @cached
+ it "should save the now expired instance back into the cache" do
+ @cache.expects(:find).returns @cached
- @cached.expects(:expiration=).with { |t| t < Time.now }
+ @cached.expects(:expiration=).with { |t| t < Time.now }
- @cache.expects(:save)
+ @cache.expects(:save)
- @indirection.expire("/my/key")
- end
+ @indirection.expire("/my/key")
+ end
- it "should use a request to save the expired resource to the cache" do
- @cache.expects(:find).returns @cached
+ it "should use a request to save the expired resource to the cache" do
+ @cache.expects(:find).returns @cached
- @cached.expects(:expiration=).with { |t| t < Time.now }
+ @cached.expects(:expiration=).with { |t| t < Time.now }
- @cache.expects(:save).with { |r| r.is_a?(Puppet::Indirector::Request) and r.instance == @cached and r.method == :save }.returns(@cached)
+ @cache.expects(:save).with { |r| r.is_a?(Puppet::Indirector::Request) and r.instance == @cached and r.method == :save }.returns(@cached)
- @indirection.expire("/my/key")
- end
+ @indirection.expire("/my/key")
end
end
end
diff --git a/spec/unit/ssl/host.rb b/spec/unit/ssl/host.rb
index 9174f2e..a09d031 100755
--- a/spec/unit/ssl/host.rb
+++ b/spec/unit/ssl/host.rb
@@ -90,6 +90,55 @@ describe Puppet::SSL::Host do
Puppet::SSL::Host.localhost.should equal(two)
end
+ it "should be able to verify its certificate matches its key" do
+ Puppet::SSL::Host.new("foo").should respond_to(:certificate_matches_key?)
+ end
+
+ it "should consider the certificate invalid if it cannot find a key" do
+ host = Puppet::SSL::Host.new("foo")
+ host.expects(:key).returns nil
+
+ host.should_not be_certificate_matches_key
+ end
+
+ it "should consider the certificate invalid if it cannot find a certificate" do
+ host = Puppet::SSL::Host.new("foo")
+ host.expects(:key).returns mock("key")
+ host.expects(:certificate).returns nil
+
+ host.should_not be_certificate_matches_key
+ end
+
+ it "should consider the certificate invalid if the SSL certificate's key verification fails" do
+ host = Puppet::SSL::Host.new("foo")
+
+ key = mock 'key', :content => "private_key"
+ sslcert = mock 'sslcert'
+ certificate = mock 'cert', :content => sslcert
+
+ host.stubs(:key).returns key
+ host.stubs(:certificate).returns certificate
+
+ sslcert.expects(:check_private_key).with("private_key").returns false
+
+ host.should_not be_certificate_matches_key
+ end
+
+ it "should consider the certificate valid if the SSL certificate's key verification succeeds" do
+ host = Puppet::SSL::Host.new("foo")
+
+ key = mock 'key', :content => "private_key"
+ sslcert = mock 'sslcert'
+ certificate = mock 'cert', :content => sslcert
+
+ host.stubs(:key).returns key
+ host.stubs(:certificate).returns certificate
+
+ sslcert.expects(:check_private_key).with("private_key").returns true
+
+ host.should be_certificate_matches_key
+ end
+
describe "when specifying the CA location" do
before do
[Puppet::SSL::Key, Puppet::SSL::Certificate, Puppet::SSL::CertificateRequest, Puppet::SSL::CertificateRevocationList].each do |klass|
@@ -359,11 +408,10 @@ describe Puppet::SSL::Host do
describe "when managing its certificate" do
before do
@realcert = mock 'certificate'
- @realcert.stubs(:check_private_key).with('private key').returns true
-
- @cert = stub 'cert', :content => @realcert, :expired? => false
+ @cert = stub 'cert', :content => @realcert
- @host.stubs(:key).returns stub("key",:content => 'private key' )
+ @host.stubs(:key).returns mock("key")
+ @host.stubs(:certificate_matches_key?).returns true
end
it "should find the CA certificate if it does not have a certificate" do
@@ -411,22 +459,12 @@ describe Puppet::SSL::Host do
@host.certificate.should equal(@cert)
end
- it "should immediately expire the cached copy if the found certificate does not match the private key" do
- @realcert.expects(:check_private_key).with('private key').returns false
-
- Puppet::SSL::Certificate.stubs(:find).returns @cert
- Puppet::SSL::Certificate.expects(:expire).with("myname")
-
- @host.certificate
- end
-
- it "should not return a certificate if it does not match the private key" do
- @realcert.expects(:check_private_key).with('private key').returns false
+ it "should fail if the found certificate does not match the private key" do
+ @host.expects(:certificate_matches_key?).returns false
Puppet::SSL::Certificate.stubs(:find).returns @cert
- Puppet::SSL::Certificate.stubs(:expire).with("myname")
- @host.certificate.should == nil
+ lambda { @host.certificate }.should raise_error(Puppet::Error)
end
it "should return any previously found certificate" do
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list