Merge branch '346290-proposal-for-adding-binary-file-support-to-ci-variables' into 'master'

Adding Secure Files data model and file uploader

See merge request gitlab-org/gitlab!77886

app/models/ci/secure_file.rb

+# frozen_string_literal: true
+
+module Ci
+  class SecureFile < Ci::ApplicationRecord
+    include FileStoreMounter
+
+    FILE_SIZE_LIMIT = 5.megabytes.freeze
+    CHECKSUM_ALGORITHM = 'sha256'
+
+    belongs_to :project, optional: false
+
+    validates :file, presence: true, file_size: { maximum: FILE_SIZE_LIMIT }
+    validates :checksum, :file_store, :name, :permissions, :project_id, presence: true
+
+    before_validation :assign_checksum
+
+    enum permissions: { read_only: 0, read_write: 1, execute: 2 }
+
+    default_value_for(:file_store) { Ci::SecureFileUploader.default_store }
+
+    mount_file_store_uploader Ci::SecureFileUploader
+
+    def checksum_algorithm
+      CHECKSUM_ALGORITHM
+    end
+
+    private
+
+    def assign_checksum
+      self.checksum = file.checksum if file.present? && file_changed?
+    end
+  end
+end

app/models/project.rb

@@ -340,6 +340,7 @@ class Project < ApplicationRecord
   has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
   has_many :variables, class_name: 'Ci::Variable'
   has_many :triggers, class_name: 'Ci::Trigger'
+  has_many :secure_files, class_name: 'Ci::SecureFile'
   has_many :environments
   has_many :environments_for_dashboard, -> { from(with_rank.unfoldered.available, :environments).where('rank <= 3') }, class_name: 'Environment'
   has_many :deployments

app/uploaders/ci/secure_file_uploader.rb

+# frozen_string_literal: true
+
+module Ci
+  class SecureFileUploader < GitlabUploader
+    include ObjectStorage::Concern
+
+    storage_options Gitlab.config.ci_secure_files
+
+    # Use Lockbox to encrypt/decrypt the stored file (registers CarrierWave callbacks)
+    encrypt(key: :key)
+
+    def key
+      OpenSSL::HMAC.digest('SHA256', Gitlab::Application.secrets.db_key_base, model.project_id.to_s)
+    end
+
+    def checksum
+      @checksum ||= Digest::SHA256.hexdigest(model.file.read)
+    end
+
+    def store_dir
+      dynamic_segment
+    end
+
+    private
+
+    def dynamic_segment
+      Gitlab::HashedPath.new('secure_files', model.id, root_hash: model.project_id)
+    end
+
+    class << self
+      # direct upload is disabled since the file
+      # must always be encrypted
+      def direct_upload_enabled?
+        false
+      end
+
+      def background_upload_enabled?
+        false
+      end
+
+      def default_store
+        object_store_enabled? ? ObjectStorage::Store::REMOTE : ObjectStorage::Store::LOCAL
+      end
+    end
+  end
+end

config/gitlab.yml.example

@@ -1425,6 +1425,18 @@ test:
         aws_secret_access_key: AWS_SECRET_ACCESS_KEY
         region: us-east-1
 
+  ci_secure_files:
+    enabled: true
+    storage_path: tmp/tests/ci_secure_files
+    object_store:
+      enabled: false
+      remote_directory: ci_secure_files
+      connection:
+        provider: AWS # Only AWS supported at the moment
+        aws_access_key_id: AWS_ACCESS_KEY_ID
+        aws_secret_access_key: AWS_SECRET_ACCESS_KEY
+        region: us-east-1 
+
   gitlab:
     host: localhost
     port: 80

config/initializers/1_settings.rb

@@ -246,6 +246,14 @@ Settings.gitlab_ci['add_pusher']            = false if Settings.gitlab_ci['add_p
 Settings.gitlab_ci['builds_path']           = Settings.absolute(Settings.gitlab_ci['builds_path'] || "builds/")
 Settings.gitlab_ci['url']                 ||= Settings.__send__(:build_gitlab_ci_url)
 
+#
+# CI Secure Files
+#
+Settings['ci_secure_files'] ||= Settingslogic.new({})
+Settings.ci_secure_files['enabled']      = true if Settings.ci_secure_files['enabled'].nil?
+Settings.ci_secure_files['storage_path'] = Settings.absolute(Settings.ci_secure_files['storage_path'] || File.join(Settings.shared['path'], "ci_secure_files"))
+Settings.ci_secure_files['object_store'] = ObjectStoreSettings.legacy_parse(Settings.ci_secure_files['object_store'])
+
 #
 # Reply by email
 #

config/object_store_settings.rb

@@ -2,7 +2,7 @@
 
 # Set default values for object_store settings
 class ObjectStoreSettings
-  SUPPORTED_TYPES = %w(artifacts external_diffs lfs uploads packages dependency_proxy terraform_state pages).freeze
+  SUPPORTED_TYPES = %w(artifacts external_diffs lfs uploads packages dependency_proxy terraform_state pages secure_files).freeze
   ALLOWED_OBJECT_STORE_OVERRIDES = %w(bucket enabled proxy_download).freeze
 
   # To ensure the one Workhorse credential matches the Rails config, we

db/migrate/20220110170953_create_ci_secure_files.rb

+# frozen_string_literal: true
+
+class CreateCiSecureFiles < Gitlab::Database::Migration[1.0]
+  def up
+    create_table :ci_secure_files do |t|
+      t.bigint :project_id, index: true, null: false
+      t.timestamps_with_timezone null: false
+      t.integer :file_store, limit: 2, null: false, default: 1
+      t.integer :permissions, null: false, default: 0, limit: 2
+      t.text :name, null: false, limit: 255
+      t.text :file, null: false, limit: 255
+      t.binary :checksum, null: false
+    end
+  end
+
+  def down
+    drop_table :ci_secure_files, if_exists: true
+  end
+end

db/schema_migrations/20220110170953

+da1c6f2db7cee1e4cb8b477d1892fa7206a95157a84864ad3d6022ab6cffbd1f
\ No newline at end of file

db/structure.sql

@@ -12209,6 +12209,29 @@ CREATE SEQUENCE ci_running_builds_id_seq
 
 ALTER SEQUENCE ci_running_builds_id_seq OWNED BY ci_running_builds.id;
 
+CREATE TABLE ci_secure_files (
+    id bigint NOT NULL,
+    project_id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    file_store smallint DEFAULT 1 NOT NULL,
+    permissions smallint DEFAULT 0 NOT NULL,
+    name text NOT NULL,
+    file text NOT NULL,
+    checksum bytea NOT NULL,
+    CONSTRAINT check_320790634d CHECK ((char_length(file) <= 255)),
+    CONSTRAINT check_402c7b4a56 CHECK ((char_length(name) <= 255))
+);
+
+CREATE SEQUENCE ci_secure_files_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE ci_secure_files_id_seq OWNED BY ci_secure_files.id;
+
 CREATE TABLE ci_sources_pipelines (
     id integer NOT NULL,
     project_id integer,
@@ -21440,6 +21463,8 @@ ALTER TABLE ONLY ci_runners ALTER COLUMN id SET DEFAULT nextval('ci_runners_id_s
 
 ALTER TABLE ONLY ci_running_builds ALTER COLUMN id SET DEFAULT nextval('ci_running_builds_id_seq'::regclass);
 
+ALTER TABLE ONLY ci_secure_files ALTER COLUMN id SET DEFAULT nextval('ci_secure_files_id_seq'::regclass);
+
 ALTER TABLE ONLY ci_sources_pipelines ALTER COLUMN id SET DEFAULT nextval('ci_sources_pipelines_id_seq'::regclass);
 
 ALTER TABLE ONLY ci_sources_projects ALTER COLUMN id SET DEFAULT nextval('ci_sources_projects_id_seq'::regclass);
@@ -22930,6 +22955,9 @@ ALTER TABLE ONLY ci_runners
 ALTER TABLE ONLY ci_running_builds
     ADD CONSTRAINT ci_running_builds_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY ci_secure_files
+    ADD CONSTRAINT ci_secure_files_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY ci_sources_pipelines
     ADD CONSTRAINT ci_sources_pipelines_pkey PRIMARY KEY (id);
 
@@ -25627,6 +25655,8 @@ CREATE INDEX index_ci_running_builds_on_project_id ON ci_running_builds USING bt
 
 CREATE INDEX index_ci_running_builds_on_runner_id ON ci_running_builds USING btree (runner_id);
 
+CREATE INDEX index_ci_secure_files_on_project_id ON ci_secure_files USING btree (project_id);
+
 CREATE INDEX index_ci_sources_pipelines_on_pipeline_id ON ci_sources_pipelines USING btree (pipeline_id);
 
 CREATE INDEX index_ci_sources_pipelines_on_project_id ON ci_sources_pipelines USING btree (project_id);

ee/spec/uploaders/every_gitlab_uploader_spec.rb

@@ -72,6 +72,7 @@ RSpec.describe 'Every GitLab uploader' do
     # Please see https://gitlab.com/gitlab-org/gitlab/-/issues/328491 for more details.
     def known_unimplemented_uploader?(uploader)
       [
+        Ci::SecureFileUploader, # TODO: Add Geo support for Secure Files https://gitlab.com/gitlab-org/gitlab/-/issues/349893
         DeletedObjectUploader,
         DependencyProxy::FileUploader,
         Packages::Composer::CacheUploader,

lib/gitlab/database/gitlab_loose_foreign_keys.yml

@@ -143,3 +143,7 @@ security_scans:
   - table: ci_builds
     column: build_id
     on_delete: async_delete
+ci_secure_files:
+  - table: projects
+    column: project_id
+    on_delete: async_delete

lib/gitlab/database/gitlab_schemas.yml

@@ -107,6 +107,7 @@ ci_runner_projects: :gitlab_ci
 ci_runners: :gitlab_ci
 ci_running_builds: :gitlab_ci
 ci_sources_pipelines: :gitlab_ci
+ci_secure_files: :gitlab_ci
 ci_sources_projects: :gitlab_ci
 ci_stages: :gitlab_ci
 ci_subscriptions_projects: :gitlab_ci

spec/factories/ci/secure_files.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ci_secure_file, class: 'Ci::SecureFile' do
+    name { 'filename' }
+    file { fixture_file_upload('spec/fixtures/ci_secure_files/upload-keystore.jks', 'application/octet-stream') }
+    checksum { 'foo1234' }
+    project
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -602,6 +602,7 @@ project:
 - bulk_import_exports
 - ci_project_mirror
 - sync_events
+- secure_files
 award_emoji:
 - awardable
 - user

spec/models/ci/secure_file_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::SecureFile do
+  let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') }
+
+  subject { create(:ci_secure_file) }
+
+  before do
+    stub_ci_secure_file_object_storage
+  end
+
+  it { is_expected.to be_a FileStoreMounter }
+
+  it { is_expected.to belong_to(:project).required }
+
+  it_behaves_like 'having unique enum values'
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:checksum) }
+    it { is_expected.to validate_presence_of(:file_store) }
+    it { is_expected.to validate_presence_of(:name) }
+    it { is_expected.to validate_presence_of(:permissions) }
+    it { is_expected.to validate_presence_of(:project_id) }
+  end
+
+  describe '#permissions' do
+    it 'defaults to read_only file permssions' do
+      expect(subject.permissions).to eq('read_only')
+    end
+  end
+
+  describe '#checksum' do
+    it 'computes SHA256 checksum on the file before encrypted' do
+      subject.file = CarrierWaveStringFile.new(sample_file)
+      subject.save!
+      expect(subject.checksum).to eq(Digest::SHA256.hexdigest(sample_file))
+    end
+  end
+
+  describe '#checksum_algorithm' do
+    it 'returns the configured checksum_algorithm' do
+      expect(subject.checksum_algorithm).to eq('sha256')
+    end
+  end
+
+  describe '#file' do
+    it 'returns the saved file' do
+      subject.file = CarrierWaveStringFile.new(sample_file)
+      subject.save!
+      expect(Base64.encode64(subject.file.read)).to eq(Base64.encode64(sample_file))
+    end
+  end
+end

spec/support/helpers/stub_object_storage.rb

@@ -91,6 +91,12 @@ module StubObjectStorage
                                  **params)
   end
 
+  def stub_ci_secure_file_object_storage(**params)
+    stub_object_storage_uploader(config: Gitlab.config.ci_secure_files.object_store,
+                                 uploader: Ci::SecureFileUploader,
+                                 **params)
+  end
+
   def stub_terraform_state_object_storage(**params)
     stub_object_storage_uploader(config: Gitlab.config.terraform_state.object_store,
                                  uploader: Terraform::StateUploader,

spec/uploaders/ci/secure_file_uploader_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::SecureFileUploader do
+  subject { ci_secure_file.file }
+
+  let(:project) { create(:project) }
+  let(:ci_secure_file) { create(:ci_secure_file) }
+  let(:sample_file) { fixture_file('ci_secure_files/upload-keystore.jks') }
+
+  before do
+    stub_ci_secure_file_object_storage
+  end
+
+  describe '#key' do
+    it 'creates a digest with a secret key and the project id' do
+      expect(OpenSSL::HMAC)
+        .to receive(:digest)
+        .with('SHA256', Gitlab::Application.secrets.db_key_base, ci_secure_file.project_id.to_s)
+        .and_return('digest')
+
+      expect(subject.key).to eq('digest')
+    end
+  end
+
+  describe '.checksum' do
+    it 'returns a SHA256 checksum for the unencrypted file' do
+      expect(subject.checksum).to eq(Digest::SHA256.hexdigest(sample_file))
+    end
+  end
+
+  describe 'encryption' do
+    it 'encrypts the stored file' do
+      expect(Base64.encode64(subject.file.read)).not_to eq(Base64.encode64(sample_file))
+    end
+
+    it 'decrypts the file when reading' do
+      expect(Base64.encode64(subject.read)).to eq(Base64.encode64(sample_file))
+    end
+  end
+
+  describe '.direct_upload_enabled?' do
+    it 'returns false' do
+      expect(described_class.direct_upload_enabled?).to eq(false)
+    end
+  end
+
+  describe '.background_upload_enabled?' do
+    it 'returns false' do
+      expect(described_class.background_upload_enabled?).to eq(false)
+    end
+  end
+
+  describe '.default_store' do
+    context 'when object storage is enabled' do
+      it 'returns REMOTE' do
+        expect(described_class.default_store).to eq(ObjectStorage::Store::REMOTE)
+      end
+    end
+
+    context 'when object storage is disabled' do
+      before do
+        stub_ci_secure_file_object_storage(enabled: false)
+      end
+
+      it 'returns LOCAL' do
+        expect(described_class.default_store).to eq(ObjectStorage::Store::LOCAL)
+      end
+    end
+  end
+end