diff options
| -rw-r--r-- | app/models/pages_domain.rb | 50 | ||||
| -rw-r--r-- | app/services/projects/update_pages_configuration_service.rb | 22 | ||||
| -rw-r--r-- | app/services/projects/update_pages_service.rb | 3 | ||||
| -rw-r--r-- | app/views/projects/pages/show.html.haml | 4 | ||||
| -rw-r--r-- | db/schema.rb | 11 | ||||
| -rw-r--r-- | spec/factories/pages_domains.rb | 79 | ||||
| -rw-r--r-- | spec/models/pages_domain_spec.rb | 152 | ||||
| -rw-r--r-- | spec/models/project_spec.rb | 1 | ||||
| -rw-r--r-- | spec/services/projects/update_pages_service_spec.rb (renamed from spec/services/projects/update_pages_worker_spec.rb) | 0 |
9 files changed, 293 insertions, 29 deletions
diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index 985329bb856..b594957493a 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -6,7 +6,8 @@ class PagesDomain < ActiveRecord::Base validates :certificate, certificate: true, allow_nil: true, allow_blank: true validates :key, certificate_key: true, allow_nil: true, allow_blank: true - validate :validate_matching_key, if: ->(domain) { domain.certificate.present? && domain.key.present? } + validate :validate_pages_domain + validate :validate_matching_key, if: ->(domain) { domain.certificate.present? || domain.key.present? } validate :validate_intermediates, if: ->(domain) { domain.certificate.present? } attr_encrypted :key, mode: :per_attribute_iv_and_salt, key: Gitlab::Application.secrets.db_key_base @@ -30,8 +31,8 @@ class PagesDomain < ActiveRecord::Base end def has_matching_key? - return unless x509 - return unless pkey + return false unless x509 + return false unless pkey # We compare the public key stored in certificate with public key from certificate key x509.check_private_key(pkey) @@ -40,6 +41,9 @@ class PagesDomain < ActiveRecord::Base def has_intermediates? return false unless x509 + # self-signed certificates doesn't have the certificate chain + return true if x509.verify(x509.public_key) + store = OpenSSL::X509::Store.new store.set_default_paths @@ -66,23 +70,8 @@ class PagesDomain < ActiveRecord::Base return x509.subject.to_s end - def fingerprint - return unless x509 - @fingeprint ||= OpenSSL::Digest::SHA256.new(x509.to_der).to_s - end - - def x509 - return unless certificate - @x509 ||= OpenSSL::X509::Certificate.new(certificate) - rescue OpenSSL::X509::CertificateError - nil - end - - def pkey - return unless key - @pkey ||= OpenSSL::PKey::RSA.new(key) - rescue OpenSSL::PKey::PKeyError, OpenSSL::Cipher::CipherError - nil + def certificate_text + @certificate_text ||= x509.try(:to_text) end private @@ -102,4 +91,25 @@ class PagesDomain < ActiveRecord::Base self.errors.add(:certificate, 'misses intermediates') end end + + def validate_pages_domain + return unless domain + if domain.downcase.ends_with?(".#{Settings.pages.host}".downcase) + self.errors.add(:domain, "*.#{Settings.pages.host} is restricted") + end + end + + def x509 + return unless certificate + @x509 ||= OpenSSL::X509::Certificate.new(certificate) + rescue OpenSSL::X509::CertificateError + nil + end + + def pkey + return unless key + @pkey ||= OpenSSL::PKey::RSA.new(key) + rescue OpenSSL::PKey::PKeyError, OpenSSL::Cipher::CipherError + nil + end end diff --git a/app/services/projects/update_pages_configuration_service.rb b/app/services/projects/update_pages_configuration_service.rb index 53e9d9e2757..b5324587d0e 100644 --- a/app/services/projects/update_pages_configuration_service.rb +++ b/app/services/projects/update_pages_configuration_service.rb @@ -35,7 +35,7 @@ module Projects def reload_daemon # GitLab Pages daemon constantly watches for modification time of `pages.path` # It reloads configuration when `pages.path` is modified - File.touch(Settings.pages.path) + update_file(pages_update_file, SecureRandom.hex(64)) end def pages_path @@ -46,14 +46,24 @@ module Projects File.join(pages_path, 'config.json') end + def pages_update_file + File.join(Settings.pages.path, '.update') + end + def update_file(file, data) - if data - File.open(file, 'w') do |file| - file.write(data) - end - else + unless data File.rm(file, force: true) + return + end + + temp_file = "#{file}.#{SecureRandom.hex(16)}" + File.open(temp_file, 'w') do |file| + file.write(data) end + File.mv(temp_file, file, force: true) + ensure + # In case if the updating fails + File.rm(temp_file, force: true) end end end diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index ceabd29fd52..a9979bf1e96 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -1,5 +1,6 @@ module Projects - class UpdatePagesService < BaseService + class + UpdatePagesService < BaseService BLOCK_SIZE = 32.kilobytes MAX_SIZE = 1.terabyte SITE_PATH = 'public/' diff --git a/app/views/projects/pages/show.html.haml b/app/views/projects/pages/show.html.haml index 52493b1959b..8b7010b75b2 100644 --- a/app/views/projects/pages/show.html.haml +++ b/app/views/projects/pages/show.html.haml @@ -14,9 +14,9 @@ %td Certificate %td - - if @domain.x509 + - if @domain.certificate_text %pre - = @domain.x509.to_text + = @domain.certificate_text - else .light missing diff --git a/db/schema.rb b/db/schema.rb index 15f378b28ff..dc3d8c22e8d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -855,6 +855,17 @@ ActiveRecord::Schema.define(version: 20170130204620) do add_index "oauth_applications", ["owner_id", "owner_type"], name: "index_oauth_applications_on_owner_id_and_owner_type", using: :btree add_index "oauth_applications", ["uid"], name: "index_oauth_applications_on_uid", unique: true, using: :btree + create_table "pages_domains", force: :cascade do |t| + t.integer "project_id" + t.text "certificate" + t.text "encrypted_key" + t.string "encrypted_key_iv" + t.string "encrypted_key_salt" + t.string "domain" + end + + add_index "pages_domains", ["domain"], name: "index_pages_domains_on_domain", unique: true, using: :btree + create_table "personal_access_tokens", force: :cascade do |t| t.integer "user_id", null: false t.string "token", null: false diff --git a/spec/factories/pages_domains.rb b/spec/factories/pages_domains.rb new file mode 100644 index 00000000000..4608867087c --- /dev/null +++ b/spec/factories/pages_domains.rb @@ -0,0 +1,79 @@ +FactoryGirl.define do + factory :pages_domain, class: 'PagesDomain' do + domain 'my.domain.com' + + trait :with_certificate do + certificate '-----BEGIN CERTIFICATE----- +MIICGzCCAYSgAwIBAgIBATANBgkqhkiG9w0BAQUFADAbMRkwFwYDVQQDExB0ZXN0 +LWNlcnRpZmljYXRlMB4XDTE2MDIxMjE0MzIwMFoXDTIwMDQxMjE0MzIwMFowGzEZ +MBcGA1UEAxMQdGVzdC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEFAAOBjQAw +gYkCgYEApL4J9L0ZxFJ1hI1LPIflAlAGvm6ZEvoT4qKU5Xf2JgU7/2geNR1qlNFa +SvCc08Knupp5yTgmvyK/Xi09U0N82vvp4Zvr/diSc4A/RA6Mta6egLySNT438kdT +nY2tR5feoTLwQpX0t4IMlwGQGT5h6Of2fKmDxzuwuyffcIHqLdsCAwEAAaNvMG0w +DAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUxl9WSxBprB0z0ibJs3rXEk0+95AwCwYD +VR0PBAQDAgXgMBEGCWCGSAGG+EIBAQQEAwIGQDAeBglghkgBhvhCAQ0EERYPeGNh +IGNlcnRpZmljYXRlMA0GCSqGSIb3DQEBBQUAA4GBAGC4T8SlFHK0yPSa+idGLQFQ +joZp2JHYvNlTPkRJ/J4TcXxBTJmArcQgTIuNoBtC+0A/SwdK4MfTCUY4vNWNdese +5A4K65Nb7Oh1AdQieTBHNXXCdyFsva9/ScfQGEl7p55a52jOPs0StPd7g64uvjlg +YHi2yesCrOvVXt+lgPTd +-----END CERTIFICATE-----' + end + + trait :with_key do + key '-----BEGIN PRIVATE KEY----- +MIICdgIBADANBgkqhkiG9w0BAQEFAASCAmAwggJcAgEAAoGBAKS+CfS9GcRSdYSN +SzyH5QJQBr5umRL6E+KilOV39iYFO/9oHjUdapTRWkrwnNPCp7qaeck4Jr8iv14t +PVNDfNr76eGb6/3YknOAP0QOjLWunoC8kjU+N/JHU52NrUeX3qEy8EKV9LeCDJcB +kBk+Yejn9nypg8c7sLsn33CB6i3bAgMBAAECgYA2D26w80T7WZvazYr86BNMePpd +j2mIAqx32KZHzt/lhh40J/SRtX9+Kl0Y7nBoRR5Ja9u/HkAIxNxLiUjwg9r6cpg/ +uITEF5nMt7lAk391BuI+7VOZZGbJDsq2ulPd6lO+C8Kq/PI/e4kXcIjeH6KwQsuR +5vrXfBZ3sQfflaiN4QJBANBt8JY2LIGQF8o89qwUpRL5vbnKQ4IzZ5+TOl4RLR7O +AQpJ81tGuINghO7aunctb6rrcKJrxmEH1whzComybrMCQQDKV49nOBudRBAIgG4K +EnLzsRKISUHMZSJiYTYnablof8cKw1JaQduw7zgrUlLwnroSaAGX88+Jw1f5n2Lh +Vlg5AkBDdUGnrDLtYBCDEQYZHblrkc7ZAeCllDOWjxUV+uMqlCv8A4Ey6omvY57C +m6I8DkWVAQx8VPtozhvHjUw80rZHAkB55HWHAM3h13axKG0htCt7klhPsZHpx6MH +EPjGlXIT+aW2XiPmK3ZlCDcWIenE+lmtbOpI159Wpk8BGXs/s/xBAkEAlAY3ymgx +63BDJEwvOb2IaP8lDDxNsXx9XJNVvQbv5n15vNsLHbjslHfAhAbxnLQ1fLhUPqSi +nNp/xedE1YxutQ== +-----END PRIVATE KEY-----' + end + + trait :with_certificate_chain do + # This certificate is signed with different key + certificate '-----BEGIN CERTIFICATE----- +MIIDGTCCAgGgAwIBAgIBAjANBgkqhkiG9w0BAQUFADASMRAwDgYDVQQDEwdUZXN0 +IENBMB4XDTE2MDIxMjE0MjMwMFoXDTE3MDIxMTE0MjMwMFowHTEbMBkGA1UEAxMS +dGVzdC1jZXJ0aWZpY2F0ZS0yMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKC +AQEAw8RWetIUT0YymSuKvBpClzDv/jQdX0Ch+2iF7f4Lm3lcmoUuXgyhl/WRe5K9 +ONuMHPQlZbeavEbvWb0BsU7geInhsjd/zAu3EP17jfSIXToUdSD20wcSG/yclLdZ +qhb6NCtHTJKFUI8BktoS7kafkdvmeem/UJFzlvcA6VMyGDkS8ZN39a45R1jGmPEl +Yk0g1jW7lSKcBLjU1O/Csv59LyWXqBP6jR1vB8ijlUf1IyK8gOk7NHF13GHl7Z3A +/8zwuEt/pB3yK92o71P+FnSEcJ23zcAalz6H9ajVTzRr/AXttineBNVYnEuPXW+V +Rsboe+bBO/e4pVKXnQ1F3aMT7QIDAQABo28wbTAMBgNVHRMBAf8EAjAAMB0GA1Ud +DgQWBBSFwo3rhc26lD8ZVaBVcUY1NyCOLDALBgNVHQ8EBAMCBeAwEQYJYIZIAYb4 +QgEBBAQDAgZAMB4GCWCGSAGG+EIBDQQRFg94Y2EgY2VydGlmaWNhdGUwDQYJKoZI +hvcNAQEFBQADggEBABppUhunuT7qArM9gZ2gLgcOK8qyZWU8AJulvloaCZDvqGVs +Qom0iEMBrrt5+8bBevNiB49Tz7ok8NFgLzrlEnOw6y6QGjiI/g8sRKEiXl+ZNX8h +s8VN6arqT348OU8h2BixaXDmBF/IqZVApGhR8+B4fkCt0VQmdzVuHGbOQXMWJCpl +WlU8raZoPIqf6H/8JA97pM/nk/3CqCoHsouSQv+jGY4pSL22RqsO0ylIM0LDBbmF +m4AEaojTljX1tMJAF9Rbiw/omam5bDPq2JWtosrz/zB69y5FaQjc6FnCk0M4oN/+ +VM+d42lQAgoq318A84Xu5vRh1KCAJuztkhNbM+w= +-----END CERTIFICATE-----' + end + + trait :with_expired_certificate do + certificate '-----BEGIN CERTIFICATE----- +MIIBsDCCARmgAwIBAgIBATANBgkqhkiG9w0BAQUFADAeMRwwGgYDVQQDExNleHBp +cmVkLWNlcnRpZmljYXRlMB4XDTE1MDIxMjE0MzMwMFoXDTE2MDIwMTE0MzMwMFow +HjEcMBoGA1UEAxMTZXhwaXJlZC1jZXJ0aWZpY2F0ZTCBnzANBgkqhkiG9w0BAQEF +AAOBjQAwgYkCgYEApL4J9L0ZxFJ1hI1LPIflAlAGvm6ZEvoT4qKU5Xf2JgU7/2ge +NR1qlNFaSvCc08Knupp5yTgmvyK/Xi09U0N82vvp4Zvr/diSc4A/RA6Mta6egLyS +NT438kdTnY2tR5feoTLwQpX0t4IMlwGQGT5h6Of2fKmDxzuwuyffcIHqLdsCAwEA +ATANBgkqhkiG9w0BAQUFAAOBgQBNj+vWvneyW1KkbVK+b/cVmnYPSfbkHrYK6m8X +Hq9LkWn6WP4EHsesHyslgTQZF8C7kVLTbLn2noLnOE+Mp3vcWlZxl3Yk6aZMhKS+ +Iy6oRpHaCF/2obZdIdgf9rlyz0fkqyHJc9GkioSoOhJZxEV2SgAkap8yS0sX2tJ9 +ZDXgrA== +-----END CERTIFICATE-----' + end + end +end diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb new file mode 100644 index 00000000000..929b2a26549 --- /dev/null +++ b/spec/models/pages_domain_spec.rb @@ -0,0 +1,152 @@ +require 'spec_helper' + +describe PagesDomain, models: true do + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + describe :validate_domain do + subject { build(:pages_domain, domain: domain) } + + context 'is unique' do + let(:domain) { 'my.domain.com' } + + it { is_expected.to validate_uniqueness_of(:domain) } + end + + context 'valid domain' do + let(:domain) { 'my.domain.com' } + + it { is_expected.to be_valid } + end + + context 'no domain' do + let(:domain) { nil } + + it { is_expected.to_not be_valid } + end + + context 'invalid domain' do + let(:domain) { '0123123' } + + it { is_expected.to_not be_valid } + end + + context 'domain from .example.com' do + let(:domain) { 'my.domain.com' } + + before { allow(Settings.pages).to receive(:host).and_return('domain.com') } + + it { is_expected.to_not be_valid } + end + end + + describe 'validate certificate' do + subject { domain } + + context 'when only certificate is specified' do + let(:domain) { build(:pages_domain, :with_certificate) } + + it { is_expected.to_not be_valid } + end + + context 'when only key is specified' do + let(:domain) { build(:pages_domain, :with_key) } + + it { is_expected.to_not be_valid } + end + + context 'with matching key' do + let(:domain) { build(:pages_domain, :with_certificate, :with_key) } + + it { is_expected.to be_valid } + end + + context 'for not matching key' do + let(:domain) { build(:pages_domain, :with_certificate_chain, :with_key) } + + it { is_expected.to_not be_valid } + end + end + + describe :url do + subject { domain.url } + + context 'without the certificate' do + let(:domain) { build(:pages_domain) } + + it { is_expected.to eq('http://my.domain.com') } + end + + context 'with a certificate' do + let(:domain) { build(:pages_domain, :with_certificate) } + + it { is_expected.to eq('https://my.domain.com') } + end + end + + describe :has_matching_key? do + subject { domain.has_matching_key? } + + context 'for matching key' do + let(:domain) { build(:pages_domain, :with_certificate, :with_key) } + + it { is_expected.to be_truthy } + end + + context 'for invalid key' do + let(:domain) { build(:pages_domain, :with_certificate_chain, :with_key) } + + it { is_expected.to be_falsey } + end + end + + describe :has_intermediates? do + subject { domain.has_intermediates? } + + context 'for self signed' do + let(:domain) { build(:pages_domain, :with_certificate) } + + it { is_expected.to be_truthy } + end + + context 'for certificate chain without the root' do + let(:domain) { build(:pages_domain, :with_certificate_chain) } + + it { is_expected.to be_falsey } + end + end + + describe :expired? do + subject { domain.expired? } + + context 'for valid' do + let(:domain) { build(:pages_domain, :with_certificate) } + + it { is_expected.to be_falsey } + end + + context 'for expired' do + let(:domain) { build(:pages_domain, :with_expired_certificate) } + + it { is_expected.to be_truthy } + end + end + + describe :subject do + let(:domain) { build(:pages_domain, :with_certificate) } + + subject { domain.subject } + + it { is_expected.to eq('/CN=test-certificate') } + end + + describe :certificate_text do + let(:domain) { build(:pages_domain, :with_certificate) } + + subject { domain.certificate_text } + + # We test only existence of output, since the output is long + it { is_expected.to_not be_empty } + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 48b085781e7..5fde9194e93 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -60,6 +60,7 @@ describe Project, models: true do it { is_expected.to have_many(:runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } + it { is_expected.to have_many(:pages_domains) } it { is_expected.to have_many(:labels).class_name('ProjectLabel').dependent(:destroy) } it { is_expected.to have_many(:users_star_projects).dependent(:destroy) } it { is_expected.to have_many(:environments).dependent(:destroy) } diff --git a/spec/services/projects/update_pages_worker_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 68e66866340..68e66866340 100644 --- a/spec/services/projects/update_pages_worker_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb |