From 61df812ac688cb0848752f9f26f77d65eadf160a Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 2 Jun 2018 02:32:30 -0700
Subject: [PATCH 1/2] Fix attr_encryption key settings

attr_encrypted does different things with `key` depending on what mode you are using:

1. In `:per_attribute_iv_and_salt` mode, it generates a hash with the salt:
https://github.com/attr-encrypted/encryptor/blob/c3a62c4a9e74686dd95e0548f9dc2a361fdc95d1/lib/encryptor.rb#L77.
There is no need to truncate the key to 32 bytes here.

2. In `:per_attribute_iv` mode, it sets the key directly to the password, so
truncation to 32 bytes is necessary.

Closes #47166
---
 app/models/clusters/platforms/kubernetes.rb   |  4 ++--
 app/models/clusters/providers/gcp.rb          |  2 +-
 .../unreleased/sh-fix-secrets-not-working.yml |  5 ++++
 config/settings.rb                            | 23 ++++++++++++-------
 4 files changed, 23 insertions(+), 11 deletions(-)
 create mode 100644 changelogs/unreleased/sh-fix-secrets-not-working.yml

diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb
index 25eac5160f1..36631d57ad1 100644
--- a/app/models/clusters/platforms/kubernetes.rb
+++ b/app/models/clusters/platforms/kubernetes.rb
@@ -11,12 +11,12 @@ module Clusters
 
       attr_encrypted :password,
         mode: :per_attribute_iv,
-        key: Settings.attr_encrypted_db_key_base,
+        key: Settings.attr_encrypted_db_key_base_truncated,
         algorithm: 'aes-256-cbc'
 
       attr_encrypted :token,
         mode: :per_attribute_iv,
-        key: Settings.attr_encrypted_db_key_base,
+        key: Settings.attr_encrypted_db_key_base_truncated,
         algorithm: 'aes-256-cbc'
 
       before_validation :enforce_namespace_to_lower_case
diff --git a/app/models/clusters/providers/gcp.rb b/app/models/clusters/providers/gcp.rb
index eb2e42fd3fe..4db1bb35c12 100644
--- a/app/models/clusters/providers/gcp.rb
+++ b/app/models/clusters/providers/gcp.rb
@@ -11,7 +11,7 @@ module Clusters
 
       attr_encrypted :access_token,
         mode: :per_attribute_iv,
-        key: Settings.attr_encrypted_db_key_base,
+        key: Settings.attr_encrypted_db_key_base_truncated,
         algorithm: 'aes-256-cbc'
 
       validates :gcp_project_id,
diff --git a/changelogs/unreleased/sh-fix-secrets-not-working.yml b/changelogs/unreleased/sh-fix-secrets-not-working.yml
new file mode 100644
index 00000000000..044a873ecd9
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-secrets-not-working.yml
@@ -0,0 +1,5 @@
+---
+title: Fix attr_encryption key settings
+merge_request:
+author:
+type: fixed
diff --git a/config/settings.rb b/config/settings.rb
index 58f38d103ea..3f3481bb65d 100644
--- a/config/settings.rb
+++ b/config/settings.rb
@@ -85,17 +85,24 @@ class Settings < Settingslogic
       File.expand_path(path, Rails.root)
     end
 
-    # Returns a 256-bit key for attr_encrypted
-    def attr_encrypted_db_key_base
-      # Ruby 2.4+ requires passing in the exact required length for OpenSSL keys
-      # (https://github.com/ruby/ruby/commit/ce635262f53b760284d56bb1027baebaaec175d1).
-      # Previous versions quietly truncated the input.
-      #
-      # The default mode for the attr_encrypted gem is to use a 256-bit key.
-      # We truncate the 128-byte string to 32 bytes.
+    # Ruby 2.4+ requires passing in the exact required length for OpenSSL keys
+    # (https://github.com/ruby/ruby/commit/ce635262f53b760284d56bb1027baebaaec175d1).
+    # Previous versions quietly truncated the input.
+    #
+    # Use this when using :per_attribute_iv mode for attr_encrypted.
+    # We have to truncate the string to 32 bytes for a 256-bit cipher.
+    def attr_encrypted_db_key_base_truncated
       Gitlab::Application.secrets.db_key_base[0..31]
     end
 
+    # This should be used for :per_attribute_salt_and_iv mode. There is no
+    # need to truncate the key because the encryptor will use the salt to
+    # generate a hash of the password:
+    # https://github.com/attr-encrypted/encryptor/blob/c3a62c4a9e74686dd95e0548f9dc2a361fdc95d1/lib/encryptor.rb#L77
+    def attr_encrypted_db_key_base
+      Gitlab::Application.secrets.db_key_base
+    end
+
     private
 
     def base_url(config)

From ca466d5d152e1a4f35ef044f344d8b7b33617db2 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 2 Jun 2018 06:14:58 -0700
Subject: [PATCH 2/2] Fix missing key change in
 20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb

---
 ..._migrate_kubernetes_service_to_new_clusters_architectures.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb
index 1586a7eb92f..a957f107405 100644
--- a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb
+++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb
@@ -48,7 +48,7 @@ class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migrati
 
     attr_encrypted :token,
       mode: :per_attribute_iv,
-      key: Settings.attr_encrypted_db_key_base,
+      key: Settings.attr_encrypted_db_key_base_truncated,
       algorithm: 'aes-256-cbc'
   end