Commit fb48eaba authored by Nick Thomas's avatar Nick Thomas

Encrypt webhook tokens and URLs in the database

parent 1b7fd53a
...@@ -3,6 +3,16 @@ ...@@ -3,6 +3,16 @@
class WebHook < ActiveRecord::Base class WebHook < ActiveRecord::Base
include Sortable 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 has_many :web_hook_logs, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
validates :url, presence: true, public_url: { allow_localhost: lambda(&:allow_local_requests?), validates :url, presence: true, public_url: { allow_localhost: lambda(&:allow_local_requests?),
...@@ -27,4 +37,38 @@ class WebHook < ActiveRecord::Base ...@@ -27,4 +37,38 @@ class WebHook < ActiveRecord::Base
def allow_local_requests? def allow_local_requests?
false false
end 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 end
---
title: Encrypt webhook tokens and URLs in the database
merge_request: 21645
author:
type: security
# 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
...@@ -2272,6 +2272,10 @@ ActiveRecord::Schema.define(version: 20180917172041) do ...@@ -2272,6 +2272,10 @@ ActiveRecord::Schema.define(version: 20180917172041) do
t.boolean "job_events", default: false, null: false t.boolean "job_events", default: false, null: false
t.boolean "confidential_note_events" t.boolean "confidential_note_events"
t.text "push_events_branch_filter" 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 end
add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree
......
...@@ -147,6 +147,12 @@ excluded_attributes: ...@@ -147,6 +147,12 @@ excluded_attributes:
- :reference - :reference
- :reference_html - :reference_html
- :epic_id - :epic_id
hooks:
- :token
- :encrypted_token
- :encrypted_token_iv
- :encrypted_url
- :encrypted_url_iv
methods: methods:
labels: labels:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment