summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Kozono <mkozono@gmail.com>2019-09-07 00:29:06 +0000
committerMichael Kozono <mkozono@gmail.com>2019-09-07 00:29:06 +0000
commit093858adc76f893c35b0bcf25127c3f229a7bcb6 (patch)
treed40d8446bf384a0cffb95e818bbeac4f55221a8a
parent7920ff1147051324e63c6b28cd93ca616d5b3165 (diff)
parent8c3d0703ed71e9ac166b221146176a3ea7e23989 (diff)
downloadgitlab-ce-093858adc76f893c35b0bcf25127c3f229a7bcb6.tar.gz
Merge branch 'ecdsa_pages_certificates' into 'master'
Allow to load ECDSA certificates for pages domains See merge request gitlab-org/gitlab-ce!32393
-rw-r--r--app/models/pages_domain.rb4
-rw-r--r--app/validators/certificate_key_validator.rb4
-rw-r--r--app/validators/named_ecdsa_key_validator.rb34
-rw-r--r--changelogs/unreleased/ecdsa_pages_certificates.yml5
-rw-r--r--spec/factories/pages_domains.rb83
-rw-r--r--spec/models/pages_domain_spec.rb18
-rw-r--r--spec/validators/named_ecdsa_key_validator_spec.rb54
7 files changed, 198 insertions, 4 deletions
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