From 8c3d0703ed71e9ac166b221146176a3ea7e23989 Mon Sep 17 00:00:00 2001 From: Vladimir Shushlin Date: Sat, 7 Sep 2019 00:29:03 +0000 Subject: Allow to load ECDSA certificates for pages domains Just replace RSA.new with PKey.read --- app/models/pages_domain.rb | 4 +- app/validators/certificate_key_validator.rb | 4 +- app/validators/named_ecdsa_key_validator.rb | 34 +++++++++ changelogs/unreleased/ecdsa_pages_certificates.yml | 5 ++ spec/factories/pages_domains.rb | 83 ++++++++++++++++++++++ spec/models/pages_domain_spec.rb | 18 +++++ spec/validators/named_ecdsa_key_validator_spec.rb | 54 ++++++++++++++ 7 files changed, 198 insertions(+), 4 deletions(-) create mode 100644 app/validators/named_ecdsa_key_validator.rb create mode 100644 changelogs/unreleased/ecdsa_pages_certificates.yml create mode 100644 spec/validators/named_ecdsa_key_validator_spec.rb diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 12ce717efd7..a2a471074a9 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -17,7 +17,7 @@ class PagesDomain < ApplicationRecord validates :certificate, certificate: true, if: ->(domain) { domain.certificate.present? } validates :key, presence: { message: 'must be present if HTTPS-only is enabled' }, if: :certificate_should_be_present? - validates :key, certificate_key: true, if: ->(domain) { domain.key.present? } + validates :key, certificate_key: true, named_ecdsa_key: true, if: ->(domain) { domain.key.present? } validates :verification_code, presence: true, allow_blank: false validate :validate_pages_domain @@ -247,7 +247,7 @@ class PagesDomain < ApplicationRecord def pkey return unless key - @pkey ||= OpenSSL::PKey::RSA.new(key) + @pkey ||= OpenSSL::PKey.read(key) rescue OpenSSL::PKey::PKeyError, OpenSSL::Cipher::CipherError nil end diff --git a/app/validators/certificate_key_validator.rb b/app/validators/certificate_key_validator.rb index 5b2bbffc066..b9d54d9636e 100644 --- a/app/validators/certificate_key_validator.rb +++ b/app/validators/certificate_key_validator.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# UrlValidator +# CertificateKeyValidator # # Custom validator for private keys. # @@ -20,7 +20,7 @@ class CertificateKeyValidator < ActiveModel::EachValidator def valid_private_key_pem?(value) return false unless value - pkey = OpenSSL::PKey::RSA.new(value) + pkey = OpenSSL::PKey.read(value) pkey.private? rescue OpenSSL::PKey::PKeyError false diff --git a/app/validators/named_ecdsa_key_validator.rb b/app/validators/named_ecdsa_key_validator.rb new file mode 100644 index 00000000000..42ee02b6ad4 --- /dev/null +++ b/app/validators/named_ecdsa_key_validator.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# NamedEcdsaKeyValidator +# +# Custom validator for ecdsa private keys. +# Golang currently doesn't support explicit curves for ECDSA certificates +# This validator checks if curve is set by name, not by parameters +# +# class Project < ActiveRecord::Base +# validates :certificate_key, named_ecdsa_key: true +# end +# +class NamedEcdsaKeyValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + if explicit_ec?(value) + record.errors.add(attribute, "ECDSA keys with explicit curves are not supported") + end + end + + private + + UNNAMED_CURVE = "UNDEF" + + def explicit_ec?(value) + return false unless value + + pkey = OpenSSL::PKey.read(value) + return false unless pkey.is_a?(OpenSSL::PKey::EC) + + pkey.group.curve_name == UNNAMED_CURVE + rescue OpenSSL::PKey::PKeyError + false + end +end diff --git a/changelogs/unreleased/ecdsa_pages_certificates.yml b/changelogs/unreleased/ecdsa_pages_certificates.yml new file mode 100644 index 00000000000..059cb434b62 --- /dev/null +++ b/changelogs/unreleased/ecdsa_pages_certificates.yml @@ -0,0 +1,5 @@ +--- +title: Allow ECDSA certificates for pages domains +merge_request: 32393 +author: +type: added diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb index ee5be82cd19..ae3988bdd69 100644 --- a/spec/factories/pages_domains.rb +++ b/spec/factories/pages_domains.rb @@ -271,5 +271,88 @@ ZDXgrA== auto_ssl_enabled { true } certificate_source { :gitlab_provided } end + + trait :explicit_ecdsa do + certificate '-----BEGIN CERTIFICATE----- +MIID1zCCAzkCCQDatOIwBlktwjAKBggqhkjOPQQDAjBPMQswCQYDVQQGEwJVUzEL +MAkGA1UECAwCTlkxCzAJBgNVBAcMAk5ZMQswCQYDVQQLDAJJVDEZMBcGA1UEAwwQ +dGVzdC1jZXJ0aWZpY2F0ZTAeFw0xOTA4MjkxMTE1NDBaFw0yMTA4MjgxMTE1NDBa +ME8xCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJOWTELMAkGA1UEBwwCTlkxCzAJBgNV +BAsMAklUMRkwFwYDVQQDDBB0ZXN0LWNlcnRpZmljYXRlMIICXDCCAc8GByqGSM49 +AgEwggHCAgEBME0GByqGSM49AQECQgH///////////////////////////////// +/////////////////////////////////////////////////////zCBngRCAf// +//////////////////////////////////////////////////////////////// +///////////////////8BEFRlT65YY4cmh+SmiGgtoVA7qLacluZsxXzuLSJkY7x +CeFWGTlR7H6TexZSwL07sb8HNXPfiD0sNPHvRR/Ua1A/AAMVANCeiAApHLhTlsxn +FzkyhKqg2mS6BIGFBADGhY4GtwQE6c2ePstmI5W0QpxkgTkFP7Uh+CivYGtNPbqh +S1537+dZKP4dwSei/6jeM0izwYVqQpv5fn4xwuW9ZgEYOSlqeJo7wARcil+0LH0b +2Zj1RElXm0RoF6+9Fyc+ZiyX7nKZXvQmQMVQuQE/rQdhNTxwhqJywkCIvpR2n9Fm +UAJCAf//////////////////////////////////////////+lGGh4O/L5Zrf8wB +SPcJpdA7tcm4iZxHrrtvtx6ROGQJAgEBA4GGAAQBVG/4c/hgl36toHj+eGL4pqv7 +l7M+ZKQJ4vz0Y9E6xIx+gvfVaZ58krmbBAP53ikwneQbFdcvw3L/ACPEib/qWjkB +ogykguy3OwHtKLYNnDWIsfiLumEjElhcBMZVXiXhb5txf11uXAWn5n6Qhey5YKPM +NjLLqDqaG19efCLCd21A0TcwCgYIKoZIzj0EAwIDgYsAMIGHAkEm68kYFVnN1c2N +OjSJpIDdFWGVYJHyMDI5WgQyhm4hAioXJ0T22Zab8Wmq+hBYRJNcHoaV894blfqR +V3ZJgam8EQJCAcnPpJQ0IqoT1pAQkaL3+Ka8ZaaCd6/8RnoDtGvWljisuyH65SRu +kmYv87bZe1KqOZDoaDBdfVsoxcGbik19lBPV +-----END CERTIFICATE-----' + + key '-----BEGIN EC PARAMETERS----- +MIIBwgIBATBNBgcqhkjOPQEBAkIB//////////////////////////////////// +//////////////////////////////////////////////////8wgZ4EQgH///// +//////////////////////////////////////////////////////////////// +/////////////////ARBUZU+uWGOHJofkpohoLaFQO6i2nJbmbMV87i0iZGO8Qnh +Vhk5Uex+k3sWUsC9O7G/BzVz34g9LDTx70Uf1GtQPwADFQDQnogAKRy4U5bMZxc5 +MoSqoNpkugSBhQQAxoWOBrcEBOnNnj7LZiOVtEKcZIE5BT+1Ifgor2BrTT26oUte +d+/nWSj+HcEnov+o3jNIs8GFakKb+X5+McLlvWYBGDkpaniaO8AEXIpftCx9G9mY +9URJV5tEaBevvRcnPmYsl+5ymV70JkDFULkBP60HYTU8cIaicsJAiL6Udp/RZlAC +QgH///////////////////////////////////////////pRhoeDvy+Wa3/MAUj3 +CaXQO7XJuImcR667b7cekThkCQIBAQ== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MIICnQIBAQRCAZZRG4FJO+OK29ygycrNzjxQDB+dp+QPo1Pk6RAl5PcraohyhFnI +MGUL4ba1efZUxCbAWxjVRSi7QEUNYCCdUPAtoIIBxjCCAcICAQEwTQYHKoZIzj0B +AQJCAf////////////////////////////////////////////////////////// +////////////////////////////MIGeBEIB//////////////////////////// +//////////////////////////////////////////////////////////wEQVGV +PrlhjhyaH5KaIaC2hUDuotpyW5mzFfO4tImRjvEJ4VYZOVHsfpN7FlLAvTuxvwc1 +c9+IPSw08e9FH9RrUD8AAxUA0J6IACkcuFOWzGcXOTKEqqDaZLoEgYUEAMaFjga3 +BATpzZ4+y2YjlbRCnGSBOQU/tSH4KK9ga009uqFLXnfv51ko/h3BJ6L/qN4zSLPB +hWpCm/l+fjHC5b1mARg5KWp4mjvABFyKX7QsfRvZmPVESVebRGgXr70XJz5mLJfu +cple9CZAxVC5AT+tB2E1PHCGonLCQIi+lHaf0WZQAkIB//////////////////// +///////////////////////6UYaHg78vlmt/zAFI9wml0Du1ybiJnEeuu2+3HpE4 +ZAkCAQGhgYkDgYYABAFUb/hz+GCXfq2geP54Yvimq/uXsz5kpAni/PRj0TrEjH6C +99VpnnySuZsEA/neKTCd5BsV1y/Dcv8AI8SJv+paOQGiDKSC7Lc7Ae0otg2cNYix ++Iu6YSMSWFwExlVeJeFvm3F/XW5cBafmfpCF7Llgo8w2MsuoOpobX158IsJ3bUDR +Nw== +-----END EC PRIVATE KEY-----' + end + + trait :ecdsa do + certificate '-----BEGIN CERTIFICATE----- +MIIB8zCCAVUCCQCGKuPQ6SBxUTAKBggqhkjOPQQDAjA+MQswCQYDVQQGEwJVUzEL +MAkGA1UECAwCVVMxCzAJBgNVBAcMAlVTMRUwEwYDVQQDDAxzaHVzaGxpbi5kZXYw +HhcNMTkwOTAyMDkyMDUxWhcNMjEwOTAxMDkyMDUxWjA+MQswCQYDVQQGEwJVUzEL +MAkGA1UECAwCVVMxCzAJBgNVBAcMAlVTMRUwEwYDVQQDDAxzaHVzaGxpbi5kZXYw +gZswEAYHKoZIzj0CAQYFK4EEACMDgYYABAH9Jd7ZWnTasgltZRbIMreihycOh/G4 +TXpkp8tTtEsuD+sh8au3Jywsi89RSZ6vgVoCY7//DQ2vamYnyBZqbL+cTQBsQ7wD +UEaSyP0R3P4b6Ox347pYzXwSdSOra9Cm4TMQe+prVMesxulqIm7G7CTI+9J8LHlJ +z0wUDQz/o+tUSYwv6zAKBggqhkjOPQQDAgOBiwAwgYcCQUOlTnn2QP/uYSh1dUSl +R9WYUg5+PQMg7kS+4K/5+5gonWCvaMcP+2P7hltUcvq41l3uMKKCZRU/x60/FMHc +1ZXdAkIBuVtm9RJXziNOKS4TcpH9os/FuREW8YQlpec58LDZdlivcHnikHZ4LCri +T7zu3VY6Rq+V/IKpsQwQjmoTJ0IpCM8= +-----END CERTIFICATE-----' + + key '-----BEGIN EC PARAMETERS----- +BgUrgQQAIw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MIHbAgEBBEFa72+eREW25IHbke0TiWFdW1R1ad9Nyqaz7CDtv5Kqdgd6Kcl8V2az +Lr6z1PS+JSERWzRP+fps7kdFRrtqy/ECpKAHBgUrgQQAI6GBiQOBhgAEAf0l3tla +dNqyCW1lFsgyt6KHJw6H8bhNemSny1O0Sy4P6yHxq7cnLCyLz1FJnq+BWgJjv/8N +Da9qZifIFmpsv5xNAGxDvANQRpLI/RHc/hvo7HfjuljNfBJ1I6tr0KbhMxB76mtU +x6zG6WoibsbsJMj70nwseUnPTBQNDP+j61RJjC/r +-----END EC PRIVATE KEY-----' + end end end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb index 519c519fbcf..5168064bb84 100644 --- a/spec/models/pages_domain_spec.rb +++ b/spec/models/pages_domain_spec.rb @@ -151,6 +151,24 @@ describe PagesDomain do end end end + + context 'with ecdsa certificate' do + it "is valid" do + domain = build(:pages_domain, :ecdsa) + + expect(domain).to be_valid + end + + context 'when curve is set explicitly by parameters' do + it 'adds errors to private key' do + domain = build(:pages_domain, :explicit_ecdsa) + + expect(domain).to be_invalid + + expect(domain.errors[:key]).not_to be_empty + end + end + end end describe 'validations' do diff --git a/spec/validators/named_ecdsa_key_validator_spec.rb b/spec/validators/named_ecdsa_key_validator_spec.rb new file mode 100644 index 00000000000..044c5b84a56 --- /dev/null +++ b/spec/validators/named_ecdsa_key_validator_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe NamedEcdsaKeyValidator do + let(:validator) { described_class.new(attributes: [:key]) } + let!(:domain) { build(:pages_domain) } + + subject { validator.validate_each(domain, :key, value) } + + context 'with empty value' do + let(:value) { nil } + + it 'does not add any error if value is empty' do + subject + + expect(domain.errors).to be_empty + end + end + + shared_examples 'does not add any error' do + it 'does not add any error' do + expect(value).to be_present + + subject + + expect(domain.errors).to be_empty + end + end + + context 'when key is not EC' do + let(:value) { attributes_for(:pages_domain)[:key] } + + include_examples 'does not add any error' + end + + context 'with ECDSA certificate with named curve' do + let(:value) { attributes_for(:pages_domain, :ecdsa)[:key] } + + include_examples 'does not add any error' + end + + context 'with ECDSA certificate with explicit curve params' do + let(:value) { attributes_for(:pages_domain, :explicit_ecdsa)[:key] } + + it 'adds errors' do + expect(value).to be_present + + subject + + expect(domain.errors[:key]).not_to be_empty + end + end +end -- cgit v1.2.1