Please review pull request #616: (#13435) Change default signing digest algorithm opened by (jeffweiss)
Description:
Change the default digest algorithm away from MD5
This commit is one step along the way to FIPS 140-2 compliance (#8120).
In a FIPS 140-2 environment, MD5 is not available. Older versions of
Ruby (1.8.7, 1.9.2) will SIGABRT when trying to use MD5 because they
don't properly check the return code from openssl.
Because the fingerprints between agent and master aren't
machine-verified and puppet cert list --digest <digest> supports any
of the digests, this commit is backwards and forwards compatibile.
Later portions of #8120 will make the default digest algorithm
configurable.
- Opened: Fri Mar 30 23:32:57 UTC 2012
- Based on: puppetlabs:master (ba112e94619e6d35ca1d2d89975f787e537fe1e3)
- Requested merge: jeffweiss:ticket/master/13435_csrs_signed_with_sha1 (6362e5333fa944772f71b04111c744c3a1c257c0)
Diff follows:
diff --git a/lib/puppet/ssl/base.rb b/lib/puppet/ssl/base.rb
index f9bbcac..668134f 100644
--- a/lib/puppet/ssl/base.rb
+++ b/lib/puppet/ssl/base.rb
@@ -54,8 +54,8 @@ def to_text
content.to_text
end
- def fingerprint(md = :MD5)
- require 'openssl/digest'
+ def fingerprint(md = :SHA256)
+ require 'openssl'
# ruby 1.8.x openssl digest constants are string
# but in 1.9.x they are symbols
diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb
index 179acd9..f7b8a9d 100644
--- a/lib/puppet/ssl/certificate_authority.rb
+++ b/lib/puppet/ssl/certificate_authority.rb
@@ -274,7 +274,7 @@ def sign(hostname, allow_dns_alt_names = false, self_signing_csr = nil)
cert = Puppet::SSL::Certificate.new(hostname)
cert.content = Puppet::SSL::CertificateFactory.
build(cert_type, csr, issuer, next_serial)
- cert.content.sign(host.key.content, OpenSSL::Digest::SHA1.new)
+ cert.content.sign(host.key.content, OpenSSL::Digest::SHA256.new)
Puppet.notice "Signed certificate request for #{hostname}"
@@ -348,7 +348,7 @@ def verify(name)
raise CertificateVerificationError.new(store.error), store.error_string unless store.verify(cert.content)
end
- def fingerprint(name, md = :MD5)
+ def fingerprint(name, md = :SHA256)
unless cert = Puppet::SSL::Certificate.indirection.find(name) || Puppet::SSL::CertificateRequest.indirection.find(name)
raise ArgumentError, "Could not find a certificate or csr for #{name}"
end
diff --git a/lib/puppet/ssl/certificate_authority/interface.rb b/lib/puppet/ssl/certificate_authority/interface.rb
index 8a978b0..8f8371d 100644
--- a/lib/puppet/ssl/certificate_authority/interface.rb
+++ b/lib/puppet/ssl/certificate_authority/interface.rb
@@ -41,7 +41,7 @@ def generate(ca)
def initialize(method, options)
self.method = method
self.subjects = options.delete(:to)
- @digest = options.delete(:digest) || :MD5
+ @digest = options.delete(:digest) || :SHA256
@options = options
end
diff --git a/lib/puppet/ssl/certificate_request.rb b/lib/puppet/ssl/certificate_request.rb
index 461dc57..d028ec2 100644
--- a/lib/puppet/ssl/certificate_request.rb
+++ b/lib/puppet/ssl/certificate_request.rb
@@ -68,12 +68,12 @@ def generate(key, options = {})
csr.add_attribute(OpenSSL::X509::Attribute.new("extReq", extReq))
end
- csr.sign(key, OpenSSL::Digest::MD5.new)
+ csr.sign(key, OpenSSL::Digest::SHA256.new)
raise Puppet::Error, "CSR sign verification failed; you need to clean the certificate request for #{name} on the server" unless csr.verify(key.public_key)
@content = csr
- Puppet.info "Certificate Request fingerprint (md5): #{fingerprint}"
+ Puppet.info "Certificate Request fingerprint (sha256): #{fingerprint}"
@content
end
diff --git a/lib/puppet/ssl/certificate_revocation_list.rb b/lib/puppet/ssl/certificate_revocation_list.rb
index 293f4b8..a682093 100644
--- a/lib/puppet/ssl/certificate_revocation_list.rb
+++ b/lib/puppet/ssl/certificate_revocation_list.rb
@@ -38,7 +38,7 @@ def generate(cert, cakey)
# Keep CRL valid for 5 years
@content.next_update = Time.now + 5 * 365*24*60*60
- @content.sign(cakey, OpenSSL::Digest::SHA1.new)
+ @content.sign(cakey, OpenSSL::Digest::SHA256.new)
@content
end
diff --git a/spec/unit/ssl/base_spec.rb b/spec/unit/ssl/base_spec.rb
index 125623b..837fd61 100755
--- a/spec/unit/ssl/base_spec.rb
+++ b/spec/unit/ssl/base_spec.rb
@@ -14,7 +14,7 @@ class TestCertificate < Puppet::SSL::Base; end
before :each do
@cert = stub 'cert', :to_der => "DER"
@base.stubs(:content).returns(@cert)
- OpenSSL::Digest.stubs(:constants).returns ["MD5", "DIGEST"]
+ OpenSSL::Digest.stubs(:constants).returns ["MD5", "SHA1", "SHA256", "SHA512", "DIGEST"]
@digest = stub_everything
OpenSSL::Digest.stubs(:const_get).returns @digest
end
diff --git a/spec/unit/ssl/certificate_authority/interface_spec.rb b/spec/unit/ssl/certificate_authority/interface_spec.rb
index 41a57b8..dbb8727 100755
--- a/spec/unit/ssl/certificate_authority/interface_spec.rb
+++ b/spec/unit/ssl/certificate_authority/interface_spec.rb
@@ -45,9 +45,9 @@
interface.digest.should == :digest
end
- it "should set the digest to md5 if none given" do
+ it "should set the digest to SHA256 if none given" do
interface = @class.new(:generate, :to => :all)
- interface.digest.should == :MD5
+ interface.digest.should == :SHA256
end
end
@@ -347,10 +347,10 @@
@applier = @class.new(:fingerprint, :to => :all)
- @ca.expects(:fingerprint).with("host1", :MD5).returns "fingerprint1"
+ @ca.expects(:fingerprint).with("host1", :SHA256).returns "fingerprint1"
@applier.expects(:puts).with "host1 fingerprint1"
- @ca.expects(:fingerprint).with("host2", :MD5).returns "fingerprint2"
+ @ca.expects(:fingerprint).with("host2", :SHA256).returns "fingerprint2"
@applier.expects(:puts).with "host2 fingerprint2"
@applier.apply(@ca)
@@ -361,10 +361,10 @@
it "should print each named certificate if found" do
@applier = @class.new(:fingerprint, :to => %w{host1 host2})
- @ca.expects(:fingerprint).with("host1", :MD5).returns "fingerprint1"
+ @ca.expects(:fingerprint).with("host1", :SHA256).returns "fingerprint1"
@applier.expects(:puts).with "host1 fingerprint1"
- @ca.expects(:fingerprint).with("host2", :MD5).returns "fingerprint2"
+ @ca.expects(:fingerprint).with("host2", :SHA256).returns "fingerprint2"
@applier.expects(:puts).with "host2 fingerprint2"
@applier.apply(@ca)
diff --git a/spec/unit/ssl/certificate_authority_spec.rb b/spec/unit/ssl/certificate_authority_spec.rb
index 222b72e..33b6929 100755
--- a/spec/unit/ssl/certificate_authority_spec.rb
+++ b/spec/unit/ssl/certificate_authority_spec.rb
@@ -417,7 +417,7 @@ def stub_ca_host
it "should sign the resulting certificate using its real key and a digest" do
digest = mock 'digest'
- OpenSSL::Digest::SHA1.expects(:new).returns digest
+ OpenSSL::Digest::SHA256.expects(:new).returns digest
key = stub 'key', :content => "real_key"
@ca.host.stubs(:key).returns key
diff --git a/spec/unit/ssl/certificate_request_spec.rb b/spec/unit/ssl/certificate_request_spec.rb
index 9a27397..9731ed3 100755
--- a/spec/unit/ssl/certificate_request_spec.rb
+++ b/spec/unit/ssl/certificate_request_spec.rb
@@ -207,7 +207,7 @@
it "should sign the csr with the provided key and a digest" do
digest = mock 'digest'
- OpenSSL::Digest::MD5.expects(:new).returns(digest)
+ OpenSSL::Digest::SHA256.expects(:new).returns(digest)
@request.expects(:sign).with(@key, digest)
@instance.generate(@key)
end
-- 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.
