(puppetlabs/puppet) New pull request: Fixed #663 - Changed default certification key lenght and hash
weareth...@puppetlabs.com
Greetings!
Please review the pull request #69: Fixed #663 - Changed default certification key lenght and hash opened by (jamtur01)
Some more information about the pull request:
- Opened: Sun Aug 28 19:54:37 UTC 2011
- Based on: puppetlabs:master (98db04eb290ad7767cbc6da43c0ab94971f0d8ef)
- Requested merge: jamtur01:tickets/master/6663 (3a223d62b633e24dfafa10015b6cc973320fdd9a)
Description:
Updated Puppet to use 2048-bit keys and SHA256 for all hashing related
to key signing. Tests updated also but no additional tests added.
Thanks!
The Pull Request Bot
Diff follows:
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 106e94b..3732e56 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -368,9 +368,9 @@ module Puppet
's' (seconds). The unit defaults to seconds. If this parameter
is set, ca_days is ignored. Examples are '3600' (one hour)
and '1825d', which is the same as '5y' (5 years) "],
- :ca_md => ["md5", "The type of hash used in certificates."],
+ :ca_md => ["sha256", "The type of hash used in certificates."],
:req_bits => [2048, "The bit length of the certificates."],
- :keylength => [1024, "The bit length of keys."],
+ :keylength => [2048, "The bit length of keys."],
:cert_inventory => {
:default => "$cadir/inventory.txt",
:mode => 0644,
diff --git a/lib/puppet/ssl/certificate_authority.rb b/lib/puppet/ssl/certificate_authority.rb
index a4cbaf7..da6ff7b 100644
--- a/lib/puppet/ssl/certificate_authority.rb
+++ b/lib/puppet/ssl/certificate_authority.rb
@@ -239,7 +239,7 @@ class Puppet::SSL::CertificateAuthority
cert = Puppet::SSL::Certificate.new(hostname)
cert.content = Puppet::SSL::CertificateFactory.new(cert_type, csr.content, issuer, next_serial).result
- 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}"
diff --git a/lib/puppet/ssl/certificate_request.rb b/lib/puppet/ssl/certificate_request.rb
index 8c83339..dfede3e 100644
--- a/lib/puppet/ssl/certificate_request.rb
+++ b/lib/puppet/ssl/certificate_request.rb
@@ -51,7 +51,7 @@ class Puppet::SSL::CertificateRequest < Puppet::SSL::Base
csr.version = 0
csr.subject = OpenSSL::X509::Name.new([["CN", common_name]])
csr.public_key = key.public_key
- 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)
diff --git a/lib/puppet/ssl/certificate_revocation_list.rb b/lib/puppet/ssl/certificate_revocation_list.rb
index 293f4b8..01c400a 100644
--- a/lib/puppet/ssl/certificate_revocation_list.rb
+++ b/lib/puppet/ssl/certificate_revocation_list.rb
@@ -38,7 +38,7 @@ class Puppet::SSL::CertificateRevocationList < Puppet::SSL::Base
# 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
@@ -77,7 +77,7 @@ class Puppet::SSL::CertificateRevocationList < Puppet::SSL::Base
# Keep CRL valid for 5 years
@content.next_update = time + 5 * 365*24*60*60
- @content.sign(cakey, OpenSSL::Digest::SHA1.new)
+ @content.sign(cakey, OpenSSL::Digest::SHA256.new)
Puppet::SSL::CertificateRevocationList.indirection.save(self)
end
diff --git a/lib/puppet/sslcertificates/ca.rb b/lib/puppet/sslcertificates/ca.rb
index f3321bd..f5a0d9b 100644
--- a/lib/puppet/sslcertificates/ca.rb
+++ b/lib/puppet/sslcertificates/ca.rb
@@ -57,9 +57,6 @@ class Puppet::SSLCertificates::CA
if Puppet[:capass]
if FileTest.exists?(Puppet[:capass])
- #puts "Reading #{Puppet[:capass]}"
- #system "ls -al #{Puppet[:capass]}"
- #File.read Puppet[:capass]
@config[:password] = self.getpass
else
# Don't create a password if the cert already exists
@@ -350,7 +347,7 @@ class Puppet::SSLCertificates::CA
end
end
- def sign_with_key(signable, digest = OpenSSL::Digest::SHA1.new)
+ def sign_with_key(signable, digest = OpenSSL::Digest::SHA256.new)
cakey = nil
if @config[:password]
begin
diff --git a/lib/puppet/sslcertificates/certificate.rb b/lib/puppet/sslcertificates/certificate.rb
index 2d30bb0..996b82e 100644
--- a/lib/puppet/sslcertificates/certificate.rb
+++ b/lib/puppet/sslcertificates/certificate.rb
@@ -128,11 +128,7 @@ class Puppet::SSLCertificates::Certificate
@csr.version = 0
@csr.subject = name
@csr.public_key = @key.public_key
- @csr.sign(@key, OpenSSL::Digest::SHA1.new)
-
- #File.open(@csrfile, "w") { |f|
- # f << @csr.to_pem
- #}
+ @csr.sign(@key, OpenSSL::Digest::SHA256.new)
raise Puppet::Error, "CSR sign verification failed" unless @csr.verify(@key.public_key)
@@ -142,28 +138,14 @@ class Puppet::SSLCertificates::Certificate
def mkkey
# @key is the file
- @key = OpenSSL::PKey::RSA.new(1024)
-# { |p,n|
-# case p
-# when 0; Puppet.info "key info: ." # BN_generate_prime
-# when 1; Puppet.info "key info: +" # BN_generate_prime
-# when 2; Puppet.info "key info: *" # searching good prime,
-# # n = #of try,
-# # but also data from BN_generate_prime
-# when 3; Puppet.info "key info: \n" # found good prime, n==0 - p, n==1 - q,
-# # but also data from BN_generate_prime
-# else; Puppet.info "key info: *" # BN_generate_prime
-# end
-# }
-
- if @password
- # passwdproc = proc { @password }
+ @key = OpenSSL::PKey::RSA.new(2048)
- keytext = @key.export(
+ if @password
+ keytext = @key.export(
- OpenSSL::Cipher::DES.new(:EDE3, :CBC),
+ OpenSSL::Cipher::DES.new(:EDE3, :CBC),
- @password
+ @password
)
File.open(@keyfile, "w", 0400) { |f|
f << keytext
@@ -173,8 +155,6 @@ class Puppet::SSLCertificates::Certificate
f << @key.to_pem
}
end
-
- #cmd = "#{ossl} genrsa -out #{@key} 1024"
end
def mkselfsigned
@@ -196,7 +176,7 @@ class Puppet::SSLCertificates::Certificate
end
@cert = SSLCertificates.mkcert(args)
- @cert.sign(@key, OpenSSL::Digest::SHA1.new) if @selfsign
+ @cert.sign(@key, OpenSSL::Digest::SHA256.new) if @selfsign
@cert
end
@@ -215,11 +195,6 @@ class Puppet::SSLCertificates::Certificate
end
end
- # verify that we can track down the cert chain or whatever
- def verify
- "openssl verify -verbose -CAfile /home/luke/.puppet/ssl/certs/ca.pem -purpose sslserver culain.madstop.com.pem"
- end
-
def write
files = {
@certfile => @cert,
diff --git a/lib/puppet/sslcertificates/support.rb b/lib/puppet/sslcertificates/support.rb
index 7d67081..0f9f105 100644
--- a/lib/puppet/sslcertificates/support.rb
+++ b/lib/puppet/sslcertificates/support.rb
@@ -72,7 +72,7 @@ module Puppet::SSLCertificates::Support
csr.version = 0
csr.subject = OpenSSL::X509::Name.new([["CN", Puppet[:certname]]])
csr.public_key = key.public_key
- csr.sign(key, OpenSSL::Digest::MD5.new)
+ csr.sign(key, OpenSSL::Digest::SHA256.new)
return csr
end
@@ -101,8 +101,6 @@ module Puppet::SSLCertificates::Support
end
Puppet.settings.write(:hostcert) do |f| f.print cert end
Puppet.settings.write(:localcacert) do |f| f.print cacert end
- #File.open(@certfile, "w", 0644) { |f| f.print cert }
- #File.open(@cacertfile, "w", 0644) { |f| f.print cacert }
begin
@cert = OpenSSL::X509::Certificate.new(cert)
@cacert = OpenSSL::X509::Certificate.new(cacert)
diff --git a/spec/unit/ssl/certificate_authority_spec.rb b/spec/unit/ssl/certificate_authority_spec.rb
index 3c5780a..555dc2e 100755
--- a/spec/unit/ssl/certificate_authority_spec.rb
+++ b/spec/unit/ssl/certificate_authority_spec.rb
@@ -380,7 +380,7 @@ describe Puppet::SSL::CertificateAuthority do
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 e45f013..872fe8e 100755
--- a/spec/unit/ssl/certificate_request_spec.rb
+++ b/spec/unit/ssl/certificate_request_spec.rb
@@ -146,7 +146,7 @@ describe Puppet::SSL::CertificateRequest do
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
diff --git a/spec/unit/ssl/certificate_revocation_list_spec.rb b/spec/unit/ssl/certificate_revocation_list_spec.rb
index 99058b3..c48504d 100755
--- a/spec/unit/ssl/certificate_revocation_list_spec.rb
+++ b/spec/unit/ssl/certificate_revocation_list_spec.rb
@@ -155,7 +155,7 @@ describe Puppet::SSL::CertificateRevocationList do
end
it "should sign the CRL with the CA's private key and a digest instance" do
- @crl.content.expects(:sign).with { |key, digest| key == @key and digest.is_a?(OpenSSL::Digest::SHA1) }
+ @crl.content.expects(:sign).with { |key, digest| key == @key and digest.is_a?(OpenSSL::Digest::SHA256) }
@crl.revoke(1, @key)
end
Daniel Pittman
> Please review the pull request #69: Fixed #663 - Changed default
> certification key lenght and hash opened by (jamtur01)
There is a typo in the commit message; #6663 vs #663.
Meanwhile, I am concerned that we still support RHEL4, which has a
version of OpenSSL old enough that SHA256 wasn't supported upstream.
Given that we know the Ruby OpenSSL bindings to fail, rather than
anything else, if you ask for an algorithm that doesn't exist in the
underlying libraries, I don't have complete confidence that this is
safe.
We should work out if we care about supporting OpenSSL that old (eg:
RHEL4) for the release that master will be, and if so, what to do
here.
Daniel
--
⎋ Puppet Labs Developer – http://puppetlabs.com
♲ Made with 100 percent post-consumer electrons
James Turnbull
> On Sun, Aug 28, 2011 at 13:00, <weareth...@puppetlabs.com> wrote:
>
>> Please review the pull request #69: Fixed #663 - Changed default
>> certification key lenght and hash opened by (jamtur01)
>
> There is a typo in the commit message; #6663 vs #663.
I fixed all of the typos but the GitHub bot seems to have pulled the old
message.
>
> Meanwhile, I am concerned that we still support RHEL4, which has a
> version of OpenSSL old enough that SHA256 wasn't supported upstream.
> Given that we know the Ruby OpenSSL bindings to fail, rather than
> anything else, if you ask for an algorithm that doesn't exist in the
> underlying libraries, I don't have complete confidence that this is
> safe.
So I checked this. SHA256 is supported since Lenny on Debian and it is
backported to the OpenSSL 0.9.8 version on RHEL4 but I couldn't work out
WHICH specific RHEL4 release it was backported too. It's supported in
Ruby back to at least 1.8.5 but as Daniel indicates it segfaults if the
OpenSSL library doesn't support SHA256 underneath.
>
> We should work out if we care about supporting OpenSSL that old (eg:
> RHEL4) for the release that master will be, and if so, what to do
> here.
I guess it depends on what release this goes into. I targetted Telly
and we'd need to consider which PE release will feature Telly and
whether we continue to support RHEL4 in that release.
Regards
James
--
James Turnbull
Puppet Labs
1-503-734-8571
Join us for PuppetConf <http://www.bit.ly/puppetconfsig>, September 22nd
and 23rd in Portland, Oregon, USA.
Michael Stahnke
Daniel Pittman
> Daniel Pittman wrote:
>> On Sun, Aug 28, 2011 at 13:00, <weareth...@puppetlabs.com> wrote:
>>
>> Meanwhile, I am concerned that we still support RHEL4, which has a
>> version of OpenSSL old enough that SHA256 wasn't supported upstream.
>> Given that we know the Ruby OpenSSL bindings to fail, rather than
>> anything else, if you ask for an algorithm that doesn't exist in the
>> underlying libraries, I don't have complete confidence that this is
>> safe.
>
> So I checked this. SHA256 is supported since Lenny on Debian and it is
> backported to the OpenSSL 0.9.8 version on RHEL4 but I couldn't work out
> WHICH specific RHEL4 release it was backported too. It's supported in
> Ruby back to at least 1.8.5 but as Daniel indicates it segfaults if the
> OpenSSL library doesn't support SHA256 underneath.
Well, to be clear, I know that it does that if you have a FIPS OpenSSL
without MD5, and we have field proof that the Ruby OpenSSL bindings
will segfault in that case. I also know that they decide if SHA256 is
supported based on the version number of the OpenSSL package, so...