diff --git a/lib/puppet/application/lookup.rb b/lib/puppet/application/lookup.rb index fad2aa79f7..bcdcd9eea5 100644 --- a/lib/puppet/application/lookup.rb +++ b/lib/puppet/application/lookup.rb @@ -390,7 +390,7 @@ def generate_scope _, x509 = cert.get_certificate(node) cert = OpenSSL::X509::Certificate.new(x509) Puppet::SSL::Oids.register_puppet_oids - trusted = Puppet::Context::TrustedInformation.remote(true, facts.values['certname'] || node, Puppet::SSL::Certificate.from_instance(cert)) + trusted = Puppet::Context::TrustedInformation.remote(true, facts.values['certname'] || node, cert) Puppet.override(trusted_information: trusted) do node = ni.find(node, facts: facts, environment: Puppet[:environment]) end diff --git a/lib/puppet/context/trusted_information.rb b/lib/puppet/context/trusted_information.rb index dcc2732855..b01e6164f1 100644 --- a/lib/puppet/context/trusted_information.rb +++ b/lib/puppet/context/trusted_information.rb @@ -54,7 +54,7 @@ def self.remote(authenticated, node_name, certificate) if certificate.nil? Puppet.info(_('TrustedInformation expected a certificate, but none was given.')) else - extensions = certificate.custom_extensions.to_h do |ext| + extensions = Puppet::SSL::Oids.custom_extensions_for(certificate).to_h do |ext| [ext['oid'].freeze, ext['value'].freeze] end end diff --git a/lib/puppet/ssl.rb b/lib/puppet/ssl.rb index 64f44b19a2..fba7d735b1 100644 --- a/lib/puppet/ssl.rb +++ b/lib/puppet/ssl.rb @@ -19,7 +19,6 @@ module Puppet::SSL require_relative 'ssl/verifier' require_relative 'ssl/ssl_provider' require_relative 'ssl/state_machine' - require_relative 'ssl/certificate' require_relative 'ssl/certificate_request' require_relative 'ssl/certificate_request_attributes' end diff --git a/lib/puppet/ssl/base.rb b/lib/puppet/ssl/base.rb index 7e9ccb866b..a0416376a1 100644 --- a/lib/puppet/ssl/base.rb +++ b/lib/puppet/ssl/base.rb @@ -85,13 +85,7 @@ def self.from_s(string, name = nil) # Read content from disk appropriately. def read(path) - # applies to Puppet::SSL::Certificate, Puppet::SSL::CertificateRequest - # nothing derives from Puppet::SSL::Certificate, but it is called by a number of other SSL Indirectors: - # Puppet::Indirector::CertificateStatus::File (.indirection.find) - # Puppet::Network::HTTP::WEBrick (.indirection.find) - # Puppet::Network::HTTP::RackREST (.from_instance) - # Puppet::Network::HTTP::WEBrickREST (.from_instance) - # Puppet::SSL::Inventory (.indirection.search, implements its own add / rebuild / serials with encoding UTF8) + # applies to Puppet::SSL::CertificateRequest @content = wrapped_class.new(Puppet::FileSystem.read(path, :encoding => Encoding::ASCII)) end diff --git a/lib/puppet/ssl/certificate.rb b/lib/puppet/ssl/certificate.rb deleted file mode 100644 index 5290f08ab4..0000000000 --- a/lib/puppet/ssl/certificate.rb +++ /dev/null @@ -1,98 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../puppet/ssl/base' - -# Manage certificates themselves. This class has no -# 'generate' method because the CA is responsible -# for turning CSRs into certificates; we can only -# retrieve them from the CA (or not, as is often -# the case). -# -# @deprecated Use {Puppet::SSL::SSLProvider} instead. -class Puppet::SSL::Certificate < Puppet::SSL::Base - # This is defined from the base class - wraps OpenSSL::X509::Certificate - - # Because of how the format handler class is included, this - # can't be in the base class. - def self.supported_formats - [:s] - end - - def self.subject_alt_names_for(cert) - alts = cert.extensions.find { |ext| ext.oid == "subjectAltName" } - return [] unless alts - - alts.value.split(/\s*,\s*/) - end - - def subject_alt_names - self.class.subject_alt_names_for(content) - end - - def expiration - return nil unless content - - content.not_after - end - - # This name is what gets extracted from the subject before being passed - # to the constructor, so it's not downcased - def unmunged_name - self.class.name_from_subject(content.subject.to_utf8) - end - - # Any extensions registered with custom OIDs as defined in module - # Puppet::SSL::Oids may be looked up here. - # - # A cert with a 'pp_uuid' extension having the value 'abcd' would return: - # - # [{ 'oid' => 'pp_uuid', 'value' => 'abcd'}] - # - # @return [Array String}>] An array of two element hashes, - # with key/value pairs for the extension's oid, and its value. - def custom_extensions - custom_exts = content.extensions.select do |ext| - Puppet::SSL::Oids.subtree_of?('ppRegCertExt', ext.oid) or - Puppet::SSL::Oids.subtree_of?('ppPrivCertExt', ext.oid) or - Puppet::SSL::Oids.subtree_of?('ppAuthCertExt', ext.oid) - end - - custom_exts.map do |ext| - { 'oid' => ext.oid, 'value' => get_ext_val(ext.oid) } - end - end - - private - - # Extract the extensions sequence from the wrapped certificate's raw ASN.1 form - def exts_seq - # See RFC-2459 section 4.1 (https://tools.ietf.org/html/rfc2459#section-4.1) - # to see where this is defined. Essentially this is saying "in the first - # sequence in the certificate, find the item that's tagged with 3. This - # is where the extensions are stored." - @extensions_tag ||= 3 - - @exts_seq ||= OpenSSL::ASN1.decode(content.to_der).value[0].value.find do |data| - (data.tag == @extensions_tag) && (data.tag_class == :CONTEXT_SPECIFIC) - end.value[0] - end - - # Get the DER parsed value of an X.509 extension by it's OID, or short name - # if one has been registered with OpenSSL. - def get_ext_val(oid) - ext_obj = exts_seq.value.find do |ext_seq| - ext_seq.value[0].value == oid - end - - raw_val = ext_obj.value.last.value - - begin - OpenSSL::ASN1.decode(raw_val).value - rescue OpenSSL::ASN1::ASN1Error - # This is required to maintain backward compatibility with the previous - # way trusted facts were signed. See PUP-3560 - raw_val - end - end -end diff --git a/lib/puppet/ssl/error.rb b/lib/puppet/ssl/error.rb index 64f7b045f0..4db11351c3 100644 --- a/lib/puppet/ssl/error.rb +++ b/lib/puppet/ssl/error.rb @@ -15,8 +15,9 @@ def initialize(message, code, cert) class CertMismatchError < Puppet::SSL::SSLError def initialize(peer_cert, host) - valid_certnames = [peer_cert.subject.to_utf8.sub(/.*=/, ''), - *Puppet::SSL::Certificate.subject_alt_names_for(peer_cert)].uniq + alts = peer_cert.extensions.find { |ext| ext.oid == "subjectAltName" } + alt_names = alts ? alts.value.split(/\s*,\s*/) : [] + valid_certnames = [peer_cert.subject.to_utf8.sub(/.*=/, ''), *alt_names].uniq if valid_certnames.size > 1 expected_certnames = _("expected one of %{certnames}") % { certnames: valid_certnames.join(', ') } else diff --git a/lib/puppet/ssl/oids.rb b/lib/puppet/ssl/oids.rb index 0807d22f0a..76e9fb1071 100644 --- a/lib/puppet/ssl/oids.rb +++ b/lib/puppet/ssl/oids.rb @@ -196,4 +196,43 @@ def self.subtree_of?(first, second, exclusive = false) rescue OpenSSL::ASN1::ASN1Error, TypeError false end + + # Extract custom Puppet certificate extensions from an OpenSSL::X509::Certificate. + # + # @param cert [OpenSSL::X509::Certificate] + # @return [Array] array of hashes with 'oid' and 'value' keys + def self.custom_extensions_for(cert) + exts_seq = extensions_sequence(cert) + + cert.extensions.select do |ext| + subtree_of?('ppRegCertExt', ext.oid) || + subtree_of?('ppPrivCertExt', ext.oid) || + subtree_of?('ppAuthCertExt', ext.oid) + end.map do |ext| + { 'oid' => ext.oid, 'value' => extension_value(exts_seq, ext.oid) } + end + end + + # @api private + def self.extensions_sequence(cert) + extensions_tag = 3 + OpenSSL::ASN1.decode(cert.to_der).value[0].value.find do |data| + (data.tag == extensions_tag) && (data.tag_class == :CONTEXT_SPECIFIC) + end.value[0] + end + private_class_method :extensions_sequence + + # @api private + def self.extension_value(exts_seq, oid) + ext_obj = exts_seq.value.find do |ext_seq| + ext_seq.value[0].value == oid + end + raw_val = ext_obj.value.last.value + begin + OpenSSL::ASN1.decode(raw_val).value + rescue OpenSSL::ASN1::ASN1Error + raw_val + end + end + private_class_method :extension_value end diff --git a/spec/integration/network/formats_spec.rb b/spec/integration/network/formats_spec.rb index ee2d87c7b8..843c42ada0 100644 --- a/spec/integration/network/formats_spec.rb +++ b/spec/integration/network/formats_spec.rb @@ -34,10 +34,6 @@ def self.canonical_order(s) @format = Puppet::Network::FormatHandler.format(:s) end - it "should support certificates" do - expect(@format).to be_supported(Puppet::SSL::Certificate) - end - it "should not support catalogs" do expect(@format).not_to be_supported(Puppet::Resource::Catalog) end diff --git a/spec/lib/puppet/test_ca.rb b/spec/lib/puppet/test_ca.rb index 2a51166dd2..01ae6c90e5 100644 --- a/spec/lib/puppet/test_ca.rb +++ b/spec/lib/puppet/test_ca.rb @@ -106,7 +106,7 @@ def sign(csr, opts = {}) cert.add_extension(ext) end cert.sign(@key, @digest) - Puppet::SSL::Certificate.from_instance(cert) + cert end def revoke(cert, crl = @crl, issuer_key = @key) @@ -122,7 +122,7 @@ def revoke(cert, crl = @crl, issuer_key = @key) def generate(name, opts) info = create_request(name) - cert = sign(info[:csr], opts).content + cert = sign(info[:csr], opts) info.merge(cert: cert) end diff --git a/spec/unit/certificate_factory_spec.rb b/spec/unit/certificate_factory_spec.rb index a763df1af5..55ea4c05c9 100644 --- a/spec/unit/certificate_factory_spec.rb +++ b/spec/unit/certificate_factory_spec.rb @@ -2,6 +2,7 @@ require 'puppet/test_ca' require 'puppet/certificate_factory' +require 'puppet/ssl/oids' describe Puppet::CertificateFactory, :unless => RUBY_PLATFORM == 'java' do let :serial do OpenSSL::BN.new('12') end @@ -136,13 +137,13 @@ cert = subject.build(:client, csr, issuer, serial) - # The cert must be signed before being later DER-decoding + # The cert must be signed before being later DER-decoded signer = Puppet::SSL::CertificateSigner.new signer.sign(cert, key) - wrapped_cert = Puppet::SSL::Certificate.from_instance cert + exts = Puppet::SSL::Oids.custom_extensions_for(cert) - priv_ext = wrapped_cert.custom_extensions.find {|ext| ext['oid'] == '1.3.6.1.4.1.34380.1.2.1'} - uuid_ext = wrapped_cert.custom_extensions.find {|ext| ext['oid'] == 'pp_uuid'} + priv_ext = exts.find {|ext| ext['oid'] == '1.3.6.1.4.1.34380.1.2.1'} + uuid_ext = exts.find {|ext| ext['oid'] == 'pp_uuid'} # The expected results should be DER encoded, the Puppet cert wrapper will turn # these into normal strings. diff --git a/spec/unit/context/trusted_information_spec.rb b/spec/unit/context/trusted_information_spec.rb index 6a14b43378..0146c9a4cc 100644 --- a/spec/unit/context/trusted_information_spec.rb +++ b/spec/unit/context/trusted_information_spec.rb @@ -18,11 +18,11 @@ end let(:cert) do - cert = Puppet::SSL::Certificate.from_instance(Puppet::CertificateFactory.build('ca', csr, csr.content, 1)) + cert = Puppet::CertificateFactory.build('ca', csr, csr.content, 1) # The cert must be signed so that it can be successfully be DER-decoded later signer = Puppet::SSL::CertificateSigner.new - signer.sign(cert.content, key) + signer.sign(cert, key) cert end diff --git a/spec/unit/ssl/base_spec.rb b/spec/unit/ssl/base_spec.rb index 06fe593b2a..e82e0f936d 100644 --- a/spec/unit/ssl/base_spec.rb +++ b/spec/unit/ssl/base_spec.rb @@ -1,12 +1,12 @@ require 'spec_helper' -require 'puppet/ssl/certificate' +require 'puppet/ssl/certificate_request' class TestCertificate < Puppet::SSL::Base - wraps(Puppet::SSL::Certificate) + wraps(OpenSSL::X509::Certificate) end -describe Puppet::SSL::Certificate do +describe Puppet::SSL::Base do before :each do @base = TestCertificate.new("name") @class = TestCertificate @@ -46,7 +46,9 @@ class TestCertificate < Puppet::SSL::Base describe "when initializing wrapped class from a file with #read" do it "should open the file with ASCII encoding" do path = '/foo/bar/cert' + cert = double('cert') expect(Puppet::FileSystem).to receive(:read).with(path, {:encoding => Encoding::ASCII}).and_return("bar") + allow(OpenSSL::X509::Certificate).to receive(:new).with("bar").and_return(cert) @base.read(path) end end diff --git a/spec/unit/ssl/certificate_spec.rb b/spec/unit/ssl/certificate_spec.rb deleted file mode 100644 index 9d014754a4..0000000000 --- a/spec/unit/ssl/certificate_spec.rb +++ /dev/null @@ -1,183 +0,0 @@ -require 'spec_helper' -require 'puppet/certificate_factory' - -require 'puppet/ssl/certificate' - -describe Puppet::SSL::Certificate do - let :key do OpenSSL::PKey::RSA.new(Puppet[:keylength]) end - - # Sign the provided cert so that it can be DER-decoded later - def sign_wrapped_cert(cert) - signer = Puppet::SSL::CertificateSigner.new - signer.sign(cert.content, key) - end - - before do - @class = Puppet::SSL::Certificate - end - - it "should only support the text format" do - expect(@class.supported_formats).to eq([:s]) - end - - describe "when converting from a string" do - it "should create a certificate instance with its name set to the certificate subject and its content set to the extracted certificate" do - cert = double( - 'certificate', - :subject => OpenSSL::X509::Name.parse("/CN=Foo.madstop.com"), - :is_a? => true - ) - expect(OpenSSL::X509::Certificate).to receive(:new).with("my certificate").and_return(cert) - - mycert = double('sslcert') - expect(mycert).to receive(:content=).with(cert) - - expect(@class).to receive(:new).with("Foo.madstop.com").and_return(mycert) - - @class.from_s("my certificate") - end - - it "should create multiple certificate instances when asked" do - cert1 = double('cert1') - expect(@class).to receive(:from_s).with("cert1").and_return(cert1) - cert2 = double('cert2') - expect(@class).to receive(:from_s).with("cert2").and_return(cert2) - - expect(@class.from_multiple_s("cert1\n---\ncert2")).to eq([cert1, cert2]) - end - end - - describe "when converting to a string" do - before do - @certificate = @class.new("myname") - end - - it "should return an empty string when it has no certificate" do - expect(@certificate.to_s).to eq("") - end - - it "should convert the certificate to pem format" do - certificate = double('certificate', :to_pem => "pem") - @certificate.content = certificate - expect(@certificate.to_s).to eq("pem") - end - - it "should be able to convert multiple instances to a string" do - cert2 = @class.new("foo") - expect(@certificate).to receive(:to_s).and_return("cert1") - expect(cert2).to receive(:to_s).and_return("cert2") - - expect(@class.to_multiple_s([@certificate, cert2])).to eq("cert1\n---\ncert2") - - end - end - - describe "when managing instances" do - def build_cert(opts) - key = OpenSSL::PKey::RSA.new(Puppet[:keylength]) - csr = Puppet::SSL::CertificateRequest.new('quux') - csr.generate(key, opts) - - raw_cert = Puppet::CertificateFactory.build('client', csr, csr.content, 14) - @class.from_instance(raw_cert) - end - - before do - @certificate = @class.new("myname") - end - - it "should have a name attribute" do - expect(@certificate.name).to eq("myname") - end - - it "should convert its name to a string and downcase it" do - expect(@class.new(:MyName).name).to eq("myname") - end - - it "should have a content attribute" do - expect(@certificate).to respond_to(:content) - end - - describe "#subject_alt_names", :unless => RUBY_PLATFORM == 'java' do - it "should list all alternate names when the extension is present" do - certificate = build_cert(:dns_alt_names => 'foo, bar,baz') - expect(certificate.subject_alt_names). - to match_array(['DNS:foo', 'DNS:bar', 'DNS:baz', 'DNS:quux']) - end - - it "should return an empty list of names if the extension is absent" do - certificate = build_cert({}) - expect(certificate.subject_alt_names).to be_empty - end - end - - describe "custom extensions", :unless => RUBY_PLATFORM == 'java' do - it "returns extensions under the ppRegCertExt" do - exts = {'pp_uuid' => 'abcdfd'} - cert = build_cert(:extension_requests => exts) - sign_wrapped_cert(cert) - expect(cert.custom_extensions).to include('oid' => 'pp_uuid', 'value' => 'abcdfd') - end - - it "returns extensions under the ppPrivCertExt" do - exts = {'1.3.6.1.4.1.34380.1.2.1' => 'x509 :('} - cert = build_cert(:extension_requests => exts) - sign_wrapped_cert(cert) - expect(cert.custom_extensions).to include('oid' => '1.3.6.1.4.1.34380.1.2.1', 'value' => 'x509 :(') - end - - it "returns extensions under the ppAuthCertExt" do - exts = {'pp_auth_role' => 'taketwo'} - cert = build_cert(:extension_requests => exts) - sign_wrapped_cert(cert) - expect(cert.custom_extensions).to include('oid' => 'pp_auth_role', 'value' => 'taketwo') - end - - it "doesn't return standard extensions" do - cert = build_cert(:dns_alt_names => 'foo') - expect(cert.custom_extensions).to be_empty - end - end - - it "should return a nil expiration if there is no actual certificate" do - allow(@certificate).to receive(:content).and_return(nil) - - expect(@certificate.expiration).to be_nil - end - - it "should use the expiration of the certificate as its expiration date" do - cert = double('cert') - allow(@certificate).to receive(:content).and_return(cert) - - expect(cert).to receive(:not_after).and_return("sometime") - - expect(@certificate.expiration).to eq("sometime") - end - - it "should be able to read certificates from disk" do - path = "/my/path" - expect(Puppet::FileSystem).to receive(:read).with(path, {:encoding => Encoding::ASCII}).and_return("my certificate") - certificate = double('certificate') - expect(OpenSSL::X509::Certificate).to receive(:new).with("my certificate").and_return(certificate) - expect(@certificate.read(path)).to equal(certificate) - expect(@certificate.content).to equal(certificate) - end - - it "should have a :to_text method that it delegates to the actual key" do - real_certificate = double('certificate') - expect(real_certificate).to receive(:to_text).and_return("certificatetext") - @certificate.content = real_certificate - expect(@certificate.to_text).to eq("certificatetext") - end - - it "should parse the old non-DER encoded extension values" do - cert = OpenSSL::X509::Certificate.new(File.read(my_fixture("old-style-cert-exts.pem"))) - wrapped_cert = Puppet::SSL::Certificate.from_instance cert - exts = wrapped_cert.custom_extensions - - expect(exts.find { |ext| ext['oid'] == 'pp_uuid'}['value']).to eq('I-AM-A-UUID') - expect(exts.find { |ext| ext['oid'] == 'pp_instance_id'}['value']).to eq('i_am_an_id') - expect(exts.find { |ext| ext['oid'] == 'pp_image_name'}['value']).to eq('i_am_an_image_name') - end - end -end