From fb48eaba4600c1e0e36afae6ec7638d52265a62c Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Mon, 10 Sep 2018 13:05:22 +0100
Subject: [PATCH] Encrypt webhook tokens and URLs in the database

---
 app/models/hooks/web_hook.rb                  | 44 +++++++++++++++++++
 .../unreleased/51021-more-attr-encrypted.yml  |  5 +++
 ..._add_attr_encrypted_columns_to_web_hook.rb | 15 +++++++
 db/schema.rb                                  |  4 ++
 lib/gitlab/import_export/import_export.yml    |  6 +++
 5 files changed, 74 insertions(+)
 create mode 100644 changelogs/unreleased/51021-more-attr-encrypted.yml
 create mode 100644 db/migrate/20180910115836_add_attr_encrypted_columns_to_web_hook.rb

diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 771a61b090f..68ba4b213b2 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -3,6 +3,16 @@
 class WebHook < ActiveRecord::Base
   include Sortable
 
+  attr_encrypted :token,
+                 mode:      :per_attribute_iv,
+                 algorithm: 'aes-256-gcm',
+                 key:       Settings.attr_encrypted_db_key_base_truncated
+
+  attr_encrypted :url,
+                 mode:      :per_attribute_iv,
+                 algorithm: 'aes-256-gcm',
+                 key:       Settings.attr_encrypted_db_key_base_truncated
+
   has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
 
   validates :url, presence: true, public_url: { allow_localhost: lambda(&:allow_local_requests?),
@@ -27,4 +37,38 @@ class WebHook < ActiveRecord::Base
   def allow_local_requests?
     false
   end
+
+  # In 11.4, the web_hooks table has both `token` and `encrypted_token` fields.
+  # Ensure that the encrypted version always takes precedence if present.
+  alias_method :attr_encrypted_token, :token
+  def token
+    attr_encrypted_token.presence || read_attribute(:token)
+  end
+
+  # In 11.4, the web_hooks table has both `token` and `encrypted_token` fields.
+  # Pending a background migration to encrypt all fields, we should just clear
+  # the unencrypted value whenever the new value is set.
+  alias_method :'attr_encrypted_token=', :'token='
+  def token=(value)
+    self.attr_encrypted_token = value
+
+    write_attribute(:token, nil)
+  end
+
+  # In 11.4, the web_hooks table has both `url` and `encrypted_url` fields.
+  # Ensure that the encrypted version always takes precedence if present.
+  alias_method :attr_encrypted_url, :url
+  def url
+    attr_encrypted_url.presence || read_attribute(:url)
+  end
+
+  # In 11.4, the web_hooks table has both `url` and `encrypted_url` fields.
+  # Pending a background migration to encrypt all fields, we should just clear
+  # the unencrypted value whenever the new value is set.
+  alias_method :'attr_encrypted_url=', :'url='
+  def url=(value)
+    self.attr_encrypted_url = value
+
+    write_attribute(:url, nil)
+  end
 end
diff --git a/changelogs/unreleased/51021-more-attr-encrypted.yml b/changelogs/unreleased/51021-more-attr-encrypted.yml
new file mode 100644
index 00000000000..0e18c59f1bb
--- /dev/null
+++ b/changelogs/unreleased/51021-more-attr-encrypted.yml
@@ -0,0 +1,5 @@
+---
+title: Encrypt webhook tokens and URLs in the database
+merge_request: 21645
+author:
+type: security
diff --git a/db/migrate/20180910115836_add_attr_encrypted_columns_to_web_hook.rb b/db/migrate/20180910115836_add_attr_encrypted_columns_to_web_hook.rb
new file mode 100644
index 00000000000..72f5c8d653b
--- /dev/null
+++ b/db/migrate/20180910115836_add_attr_encrypted_columns_to_web_hook.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class AddAttrEncryptedColumnsToWebHook < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :web_hooks, :encrypted_token, :string
+    add_column :web_hooks, :encrypted_token_iv, :string
+
+    add_column :web_hooks, :encrypted_url, :string
+    add_column :web_hooks, :encrypted_url_iv, :string
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ecb9d4391d7..13c6d65255e 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -2272,6 +2272,10 @@ ActiveRecord::Schema.define(version: 20180917172041) do
     t.boolean "job_events", default: false, null: false
     t.boolean "confidential_note_events"
     t.text "push_events_branch_filter"
+    t.string "encrypted_token"
+    t.string "encrypted_token_iv"
+    t.string "encrypted_url"
+    t.string "encrypted_url_iv"
   end
 
   add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index a19b3c88627..2bed470514b 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -147,6 +147,12 @@ excluded_attributes:
     - :reference
     - :reference_html
     - :epic_id
+  hooks:
+    - :token
+    - :encrypted_token
+    - :encrypted_token_iv
+    - :encrypted_url
+    - :encrypted_url_iv
 
 methods:
   labels:
-- 
GitLab