Pages domain model specs

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

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 update_file(file, data)
-      if data
-        File.open(file, 'w') do |file|
-          file.write(data)
+    def pages_update_file
+      File.join(Settings.pages.path, '.update')
     end
-      else
+
+    def update_file(file, data)
+      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

app/services/projects/update_pages_service.rb

 module Projects
-  class UpdatePagesService < BaseService
+  class
+  UpdatePagesService < BaseService
     BLOCK_SIZE = 32.kilobytes
     MAX_SIZE = 1.terabyte
     SITE_PATH = 'public/'

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

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

spec/factories/pages_domains.rb

+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

spec/models/pages_domain_spec.rb

+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

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) }