diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index c2f2c1d85..5e9f1319c 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -89,5 +89,8 @@ class AdminController < ApplicationController [:text_mermaid_available, Redmine::Mermaid.available?] ] @checklist << [:text_default_active_job_queue_changed, Rails.application.config.active_job.queue_adapter != :async] if Rails.env.production? + if CustomField.exists?(:field_format => 'encrypted_string') + @checklist << [:text_database_cipher_key_configured, Redmine::Ciphering.cipher_key.present?] + end end end diff --git a/app/models/custom_value.rb b/app/models/custom_value.rb index 972331dd7..37bc1ea0a 100644 --- a/app/models/custom_value.rb +++ b/app/models/custom_value.rb @@ -18,9 +18,12 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class CustomValue < ApplicationRecord + include Redmine::Ciphering + belongs_to :custom_field belongs_to :customized, :polymorphic => true + before_save :encrypt_clear_value after_save :custom_field_after_save_custom_value def initialize(attributes=nil, *args) @@ -61,8 +64,60 @@ class CustomValue < ApplicationRecord value.to_s end + # Returns the decrypted value for encrypted custom fields, or nil if + # it can not be decrypted (eg. the cipher key is missing or wrong) + def value + text = super + # Checks the format only for ciphered texts to avoid loading the custom field + return text unless ciphered?(text) && encrypted? + return nil if Redmine::Ciphering.cipher_key.blank? + + clear = read_ciphered_attribute(:value).force_encoding('UTF-8') + clear.valid_encoding? ? clear : nil + rescue OpenSSL::Cipher::CipherError, ArgumentError + nil + end + + def value=(arg) + if !encrypted? + # The format is unknown if the value is assigned before the custom field + @value_assigned_without_custom_field = custom_field.nil? + super + # Keeps the stored value if unchanged, as it would be encrypted + # with a new IV each time + elsif !ciphered?(self[:value]) || arg.to_s != value.to_s + write_ciphered_attribute(:value, arg) + end + end + + def value_undecryptable? + encrypted? && ciphered?(self[:value]) && value.nil? + end + private + def encrypted? + custom_field&.field_format == 'encrypted_string' + end + + def ciphered?(text) + text.to_s.start_with?('aes-256-cbc:') + end + + # Encrypts a value that was assigned before the custom field (eg. by + # CustomValue.new(:value => 'x', :custom_field => field)), and refuses + # to store it in clear text if the cipher key is missing + def encrypt_clear_value + return unless @value_assigned_without_custom_field && encrypted? + + @value_assigned_without_custom_field = false + return if self[:value].blank? || ciphered?(self[:value]) + + throw :abort if Redmine::Ciphering.cipher_key.blank? + + write_ciphered_attribute(:value, self[:value]) + end + def custom_field_after_save_custom_value custom_field.after_save_custom_value(self) end diff --git a/app/models/issue_custom_field.rb b/app/models/issue_custom_field.rb index 146f0925f..c462f2005 100644 --- a/app/models/issue_custom_field.rb +++ b/app/models/issue_custom_field.rb @@ -36,6 +36,9 @@ class IssueCustomField < CustomField end def visible_by?(project, user=User.current) + # Encrypted values can not be read or written without the cipher key + return false if field_format == 'encrypted_string' && !format.cipher_key_configured? + super || roles.intersect?(user.roles_for_project(project)) end diff --git a/app/models/journal.rb b/app/models/journal.rb index 8435ec069..5592862b8 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -282,11 +282,13 @@ class Journal < ApplicationRecord # custom fields changes if @custom_values_before_change values_by_custom_field_id = {} + encrypted_custom_field_ids = [] @custom_values_before_change.each_key do |custom_field_id| values_by_custom_field_id[custom_field_id] = nil end journalized.custom_field_values.each do |c| values_by_custom_field_id[c.custom_field_id] = c.value + encrypted_custom_field_ids << c.custom_field_id if c.custom_field.field_format == 'encrypted_string' end values_by_custom_field_id.each do |custom_field_id, after| @@ -305,6 +307,9 @@ class Journal < ApplicationRecord (after - before).reject(&:blank?).each do |value| add_custom_field_detail(custom_field_id, nil, value) end + elsif encrypted_custom_field_ids.include?(custom_field_id) + # Encrypted values must not be stored in clear text + add_custom_field_detail(custom_field_id, nil, nil) else add_custom_field_detail(custom_field_id, before, after) end diff --git a/app/views/custom_fields/formats/_encrypted_string.html.erb b/app/views/custom_fields/formats/_encrypted_string.html.erb new file mode 100644 index 000000000..e65e630c1 --- /dev/null +++ b/app/views/custom_fields/formats/_encrypted_string.html.erb @@ -0,0 +1 @@ +<%= render :partial => 'custom_fields/formats/regexp', :locals => {:f => f, :custom_field => custom_field} %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 8f9917b3b..6c7e01d22 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1509,3 +1509,6 @@ en: reaction_text_x_other_users: one: "1 other" other: "%{count} others" + label_encrypted_string: Encrypted text + error_database_cipher_key_not_configured: "The database cipher key is not configured in config/configuration.yml" + text_database_cipher_key_configured: Database cipher key configured for encrypted custom fields diff --git a/lib/redmine/field_format.rb b/lib/redmine/field_format.rb index 8634f7ea6..afa2e192b 100644 --- a/lib/redmine/field_format.rb +++ b/lib/redmine/field_format.rb @@ -475,6 +475,63 @@ module Redmine end end + # Single line text whose values are stored encrypted in the database + # with the key set by database_cipher_key in config/configuration.yml. + # Values can not be used in SQL statements, so filtering, searching, + # sorting and grouping are not supported, and values are not stored + # in the journal details. + class EncryptedStringFormat < Unbounded + add 'encrypted_string' + self.customized_class_names = %w(Issue) + self.is_filter_supported = false + self.bulk_edit_supported = false + self.change_no_details = true + self.form_partial = 'custom_fields/formats/encrypted_string' + + def cipher_key_configured? + Redmine::Ciphering.cipher_key.present? + end + + def validate_custom_field(custom_field) + errors = super + if custom_field.new_record? && !cipher_key_configured? + errors << [:field_format, l(:error_database_cipher_key_not_configured)] + end + errors + end + + def validate_custom_value(custom_value) + errors = super + if custom_value.value.present? && !cipher_key_configured? + errors << l(:error_database_cipher_key_not_configured) + end + errors + end + + def order_statement(custom_field) + nil + end + + # Renders a password field that is submitted only if the user edits it, + # so that the stored value is not sent to the browser (same as the + # repository and LDAP passwords) + def edit_tag(view, tag_id, tag_name, custom_value, options={}) + if custom_value.value.present? && custom_value.value == custom_value.value_was + view.password_field_tag( + 'ignore', 'x' * 15, + options.merge(:id => tag_id, :autocomplete => 'new-password', + :onfocus => "this.value=''; this.name='#{tag_name}';", + :onchange => "this.name='#{tag_name}';") + ) + else + view.password_field_tag( + tag_name, custom_value.value, + options.merge(:id => tag_id, :autocomplete => 'new-password') + ) + end + end + end + class Numeric < Unbounded self.form_partial = 'custom_fields/formats/numeric' self.totalable_supported = true diff --git a/lib/tasks/ciphering.rake b/lib/tasks/ciphering.rake index 8c558321b..3748815ad 100644 --- a/lib/tasks/ciphering.rake +++ b/lib/tasks/ciphering.rake @@ -17,20 +17,29 @@ namespace :db do - desc 'Encrypts SCM and LDAP passwords in the database.' + encrypted_custom_values = lambda do + CustomValue.where(:custom_field_id => CustomField.where(:field_format => 'encrypted_string').select(:id)) + end + + desc 'Encrypts SCM and LDAP passwords and encrypted custom field values in the database.' task :encrypt => :environment do unless (Repository.encrypt_all(:password) && AuthSource.encrypt_all(:account_password) && - User.encrypt_all(:twofa_totp_key)) + User.encrypt_all(:twofa_totp_key) && + encrypted_custom_values.call.encrypt_all(:value)) raise "Some objects could not be saved after encryption, update was rolled back." end end - desc 'Decrypts SCM and LDAP passwords in the database.' + desc 'Decrypts SCM and LDAP passwords and encrypted custom field values in the database.' task :decrypt => :environment do + if encrypted_custom_values.call.any?(&:value_undecryptable?) + raise "Some encrypted custom field values could not be decrypted, check the database_cipher_key." + end unless (Repository.decrypt_all(:password) && AuthSource.decrypt_all(:account_password) && - User.decrypt_all(:twofa_totp_key)) + User.decrypt_all(:twofa_totp_key) && + encrypted_custom_values.call.decrypt_all(:value)) raise "Some objects could not be saved after decryption, update was rolled back." end end diff --git a/test/functional/admin_controller_test.rb b/test/functional/admin_controller_test.rb index 7f9a52db7..3644c4412 100644 --- a/test/functional/admin_controller_test.rb +++ b/test/functional/admin_controller_test.rb @@ -213,6 +213,23 @@ class AdminControllerTest < Redmine::ControllerTest assert_response :success end + def test_info_should_check_cipher_key_if_encrypted_custom_fields_exist + get :info + assert_select 'td.name', :text => I18n.t(:text_database_cipher_key_configured), :count => 0 + + Redmine::Configuration.with 'database_cipher_key' => 'secret' do + IssueCustomField.create!(:name => 'Secret', :field_format => 'encrypted_string') + end + Redmine::Configuration.with 'database_cipher_key' => nil do + get :info + end + assert_select 'tr' do |rows| + row = rows.detect {|tr| tr.at('td.name')&.text == I18n.t(:text_database_cipher_key_configured)} + assert_not_nil row + assert_not_nil row.at('td.tick .icon-error') + end + end + def test_admin_menu_plugin_extension Redmine::MenuManager.map :admin_menu do |menu| menu.push :test_admin_menu_plugin_extension, '/foo/bar', :caption => 'Test' diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 4da71e8d9..8398cb537 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -6749,6 +6749,58 @@ class IssuesControllerTest < Redmine::ControllerTest assert_mail_body_match "Searchable field changed from 125 to New custom value", mail end + def test_get_edit_should_not_render_encrypted_custom_field_value + Redmine::Configuration.with 'database_cipher_key' => 'secret' do + field = IssueCustomField.create!(:name => 'Secret', :field_format => 'encrypted_string', + :is_for_all => true, :tracker_ids => [1]) + issue = Issue.find(1) + issue.custom_field_values = {field.id.to_s => 'my secret'} + issue.save! + + @request.session[:user_id] = 2 + get(:edit, :params => {:id => 1}) + assert_response :success + assert_select "input[type=password][id=issue_custom_field_values_#{field.id}][name=ignore]" + assert_not_include 'my secret', response.body + end + end + + def test_put_update_with_encrypted_custom_field_change + Redmine::Configuration.with 'database_cipher_key' => 'secret' do + field = IssueCustomField.create!(:name => 'Secret', :field_format => 'encrypted_string', + :is_for_all => true, :tracker_ids => [1]) + @request.session[:user_id] = 2 + with_settings :notified_events => %w(issue_updated) do + put( + :update, + :params => {:id => 1, :issue => {:custom_field_values => {field.id.to_s => 'my secret'}}} + ) + end + assert_redirected_to :action => 'show', :id => '1' + assert_equal 'my secret', Issue.find(1).custom_field_value(field) + + detail = Journal.last.details.detect {|d| d.prop_key == field.id.to_s} + assert_nil detail.value + mail = ActionMailer::Base.deliveries.last + assert_mail_body_match 'Secret updated', mail + end + end + + def test_put_update_should_keep_encrypted_custom_field_value_if_not_edited + Redmine::Configuration.with 'database_cipher_key' => 'secret' do + field = IssueCustomField.create!(:name => 'Secret', :field_format => 'encrypted_string', + :is_for_all => true, :tracker_ids => [1]) + issue = Issue.find(1) + issue.custom_field_values = {field.id.to_s => 'my secret'} + issue.save! + + @request.session[:user_id] = 2 + put(:update, :params => {:id => 1, :ignore => 'xxxxxxxxxxxxxxx', :issue => {:subject => 'Updated'}}) + assert_redirected_to :action => 'show', :id => '1' + assert_equal 'my secret', Issue.find(1).custom_field_value(field) + end + end + def test_put_update_with_multi_custom_field_change field = CustomField.find(1) field.update_attribute :multiple, true diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb index f9e2f46d2..0bb3361e9 100644 --- a/test/integration/api_test/issues_test.rb +++ b/test/integration/api_test/issues_test.rb @@ -970,6 +970,23 @@ class Redmine::ApiTest::IssuesTest < Redmine::ApiTest::Base assert_equal ['MySQL', 'PostgreSQL'], issue.custom_field_value(1).sort end + test "PUT /issues/:id.json with encrypted custom field" do + Redmine::Configuration.with 'database_cipher_key' => 'secret' do + field = IssueCustomField.create!(:name => 'Secret', :field_format => 'encrypted_string', + :is_for_all => true, :tracker_ids => [1]) + put( + '/issues/3.json', + :params => {:issue => {:custom_fields => [{'id' => field.id.to_s, 'value' => 'my secret'}]}}, + :headers => credentials('jsmith')) + assert_response :no_content + assert_match /\Aaes-256-cbc:/, Issue.find(3).custom_value_for(field).read_attribute(:value) + + get '/issues/3.json', :headers => credentials('jsmith') + json = ActiveSupport::JSON.decode(response.body) + assert_equal 'my secret', json['issue']['custom_fields'].detect {|f| f['id'] == field.id}['value'] + end + end + test "PUT /issues/:id.xml with project change" do put( '/issues/3.xml', diff --git a/test/unit/lib/redmine/field_format/encrypted_string_format_test.rb b/test/unit/lib/redmine/field_format/encrypted_string_format_test.rb new file mode 100644 index 000000000..3190d6a47 --- /dev/null +++ b/test/unit/lib/redmine/field_format/encrypted_string_format_test.rb @@ -0,0 +1,205 @@ +# frozen_string_literal: true + +# Redmine - project management software +# Copyright (C) 2006- Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require_relative '../../../../test_helper' +require 'redmine/field_format' + +class Redmine::EncryptedStringFieldFormatTest < ActionView::TestCase + def setup + User.current = nil + @field = with_cipher_key do + IssueCustomField.create!(:name => 'Secret', :field_format => 'encrypted_string', + :is_for_all => true, :tracker_ids => [1]) + end + end + + def test_value_should_be_stored_encrypted + with_cipher_key do + issue = save_issue_with_secret('my secret') + custom_value = CustomValue.find_by(:customized => issue, :custom_field => @field) + + assert_match /\Aaes-256-cbc:.+\z/, custom_value.read_attribute(:value) + assert_not_includes custom_value.read_attribute(:value), 'my secret' + assert_equal 'my secret', custom_value.value + assert_equal 'my secret', Issue.find(issue.id).custom_field_value(@field) + end + end + + def test_unchanged_value_should_not_be_encrypted_again + with_cipher_key do + issue = save_issue_with_secret('my secret') + stored = CustomValue.find_by(:customized => issue, :custom_field => @field).read_attribute(:value) + + issue = Issue.find(issue.id) + issue.custom_field_values = {@field.id.to_s => 'my secret'} + issue.subject = 'Updated' + issue.save! + + assert_equal stored, CustomValue.find_by(:customized => issue, :custom_field => @field).read_attribute(:value) + end + end + + def test_journal_details_should_not_store_values + with_cipher_key do + issue = save_issue_with_secret('old secret') + issue = Issue.find(issue.id) + issue.init_journal(User.find(1)) + issue.custom_field_values = {@field.id.to_s => 'new secret'} + issue.save! + + detail = issue.journals.last.details.detect {|d| d.prop_key == @field.id.to_s} + assert_not_nil detail + assert_nil detail.old_value + assert_nil detail.value + end + end + + def test_custom_field_should_not_be_created_without_cipher_key + with_cipher_key(nil) do + field = IssueCustomField.new(:name => 'Other secret', :field_format => 'encrypted_string') + assert_not field.save + assert_include ::I18n.t(:error_database_cipher_key_not_configured), field.errors[:field_format] + end + end + + def test_custom_field_should_not_be_visible_without_cipher_key + issue = with_cipher_key {save_issue_with_secret('my secret')} + + with_cipher_key(nil) do + issue = Issue.find(issue.id) + admin = User.find(1) + assert_not @field.visible_by?(issue.project, admin) + assert_not_include @field, issue.visible_custom_field_values(admin).map(&:custom_field) + assert_not_include @field, issue.editable_custom_fields(admin) + assert_nil issue.custom_field_value(@field) + end + end + + def test_stored_value_should_be_kept_without_cipher_key + issue = with_cipher_key {save_issue_with_secret('my secret')} + stored = CustomValue.find_by(:customized => issue, :custom_field => @field).read_attribute(:value) + + with_cipher_key(nil) do + issue = Issue.find(issue.id) + issue.init_journal(User.find(1)) + issue.safe_attributes = {'subject' => 'Updated', 'custom_field_values' => {@field.id.to_s => 'other'}} + issue.save! + end + + assert_equal stored, CustomValue.find_by(:customized => issue, :custom_field => @field).read_attribute(:value) + with_cipher_key {assert_equal 'my secret', Issue.find(issue.id).custom_field_value(@field)} + end + + def test_value_should_be_nil_with_wrong_cipher_key + issue = with_cipher_key {save_issue_with_secret('my secret')} + stored = CustomValue.find_by(:customized => issue, :custom_field => @field).read_attribute(:value) + + with_cipher_key('wrong') do + issue = Issue.find(issue.id) + assert_nil issue.custom_field_value(@field) + + issue.subject = 'Updated' + issue.save! + end + + assert_equal stored, CustomValue.find_by(:customized => issue, :custom_field => @field).read_attribute(:value) + end + + def test_value_should_be_encrypted_by_encrypt_all + issue = with_cipher_key(nil) {save_issue_with_secret('my secret')} + custom_value = CustomValue.find_by(:customized => issue, :custom_field => @field) + custom_value.update_column(:value, 'my secret') + + with_cipher_key do + assert CustomValue.where(:id => custom_value.id).encrypt_all(:value) + assert_match /\Aaes-256-cbc:/, custom_value.reload.read_attribute(:value) + assert_equal 'my secret', custom_value.value + + assert CustomValue.where(:id => custom_value.id).decrypt_all(:value) + assert_equal 'my secret', custom_value.reload.read_attribute(:value) + end + end + + def test_value_assigned_before_custom_field_should_be_encrypted + with_cipher_key do + custom_value = CustomValue.create!(:value => 'my secret', :customized => Issue.find(1), :custom_field => @field) + + assert_match /\Aaes-256-cbc:/, custom_value.reload.read_attribute(:value) + assert_equal 'my secret', custom_value.value + end + end + + def test_value_assigned_before_custom_field_should_not_be_saved_without_cipher_key + with_cipher_key(nil) do + custom_value = CustomValue.new(:value => 'my secret', :customized => Issue.find(1), :custom_field => @field) + + assert_not custom_value.save + assert_not CustomValue.where(:custom_field => @field).exists? + end + end + + def test_value_of_other_formats_should_not_be_decrypted + field = IssueCustomField.create!(:name => 'Text', :field_format => 'string') + custom_value = CustomValue.new(:custom_field => field, :value => 'aes-256-cbc:foo') + + with_cipher_key do + assert_equal 'aes-256-cbc:foo', custom_value.value + end + end + + def test_format_should_not_support_filter_sort_and_bulk_edit + assert_not @field.format.is_filter_supported + assert_not @field.format.searchable_supported + assert_not @field.format.bulk_edit_supported + assert_nil @field.order_statement + assert_nil @field.group_statement + end + + def test_edit_tag_should_not_render_stored_value + custom_value = CustomFieldValue.new(:custom_field => @field, :customized => Issue.new) + custom_value.instance_variable_set(:@value, 'my secret') + custom_value.value_was = 'my secret' + + html = @field.format.edit_tag(self, 'issue_custom_field_values_1', 'issue[custom_field_values][1]', custom_value) + assert_select_in html, 'input[type=password][name=ignore][value=?]', 'x' * 15 + assert_not_includes html, 'my secret' + end + + def test_edit_tag_without_stored_value + custom_value = CustomFieldValue.new(:custom_field => @field, :customized => Issue.new, :value => '') + + html = @field.format.edit_tag(self, 'issue_custom_field_values_1', 'issue[custom_field_values][1]', custom_value) + assert_select_in html, 'input[type=password][name=?]', 'issue[custom_field_values][1]' + end + + private + + def with_cipher_key(key='secret') + result = nil + Redmine::Configuration.with('database_cipher_key' => key) {result = yield} + result + end + + def save_issue_with_secret(secret) + issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :subject => 'Secret') + issue.custom_field_values = {@field.id.to_s => secret} + issue.save! + issue + end +end