Implement auth token hashing

cupakromer · cupakromer · commit 1b721b07b0dc · 2018-05-11T10:56:07.000-04:00
I want to make this clear: > This is **NOT** intended to be a "secure" scheme. As everything is stored in memory that is our largest security measure here. What this is intended to do is add a additional layer of computation in case the data is leaked; and to add a minor obfuscation against internal employees attempting to view the data in-memory. This uses the same general token hashing algorithm that Devise uses for things like password recovery tokens. The Devise scheme is as follows: +-------------+ | column_name |---------+ +------+------+ | +--------------------+ | | | memory | | | |--------------------| +--v--+ | | | +------------------+ | | +-----+ | | +------------+ | | Rails secret_key +---> KDF +---> key |---+----->| cached key | | +------------------+ | | +-----+ | +------------+ | +-----+ | | | +---------|----------+ | +------------------------------+ | | +-------------------------+ | | database | | |-------------------------| +--v---+ | | +-------+ | | +--------+ | +-----------------+ | | token +---> HMAC +---> digest |------>| digested record | | +-------+ | | +--------+ | +-----------------+ | +------+ | | +-------------------------+ This implementation differs slightly from the above. Namely, this does not use the KDF based on the Rails secret token. Instead we use a time sensitive rotating key generated purely from `SecureRandom`: +----------------------+ | memory | |----------------------| | | | +--------------+ | +-----------------------+| rotating key | | | | +--------------+ | | | | +--v---+ | | +-------+ | | +--------+ | +--------------+ | | token +---> HMAC +---> digest |------>| token digest | | +-------+ | | +--------+ | +--------------+ | +------+ | | +----------------------+ This does not lessen the security. While the KDF is intentionally slow, the implementation Devise uses caches the result. This means the expensive computation is only performed once then stored in-memory. However, it is always computed the same way using PBKDF2-HMAC-SHA1 based on the Rails secret key and the salt "Devise COLUMN_NAME"; the security there is purely in keeping the Rails secret key secret (which is stored in memory). In our implementation, the secret is only ever stored in memory and always generated via `SecureRandom`. Thus there's no way to try to pre-compute the value without exploiting `SecureRandom`. Additionally, instead of being completely fixed, we change it at a relatively frequent interval. Thus if there is a compromise of the data (which is in-memory so either the system is compromised or there's a remote code execution vulnerability which means way worse problems) only a subset of the data from that time window is available. If more data is dumped again later, the key has changed; _nominally_ increasing computation costs of the attacker (similar, but not exactly like, a unique salt per token). Why not just salt+hash each token? ---------------------------------- The main problem with token auth is there's no way to look up the token to know which salt to use. This means you would essentially need to run through all of the salts to try to find a match. If we had thought about this more in the beginning we could have considered choosing longer tokens and using part of them as the identifying key. Thus allowing us to look up the specific token and treating the process just like a regular login. References ---------- - https://en.wikipedia.org/wiki/HMAC - https://crypto.stackexchange.com/questions/34864/key-size-for-hmac-sha256/34866#34866 - [Devise "generating" reset token](https://github.com/plataformatec/devise/blob/v4.4.3/lib/devise/models/recoverable.rb#L90) - [Devise algorithm](https://github.com/plataformatec/devise/blob/v4.4.3/lib/devise/token_generator.rb#L21) - [Devise config Rails KDF](https://github.com/plataformatec/devise/blob/v4.4.3/lib/devise/rails.rb#L41-L43) - [Rails KDF](https://github.com/rails/rails/blob/v5.2.0/activesupport/lib/active_support/key_generator.rb#L23)
diff --git a/lib/kracken/controllers/token_authenticatable.rb b/lib/kracken/controllers/token_authenticatable.rb
@@ -36,10 +36,30 @@ def request_http_token_authentication(realm = 'Application')
 
     module_function
 
+      # @private
+      def auth_cache_key(token)
+        # This key must **ALWAYS** be generated and stored either in-memory or
+        # "securely" on the server. Treat this like the Rails / Devise secret
+        # key.
+        #
+        # If in the future we move back to an external cache (say Redis) for
+        # the cached auth token storage this must still remain **ONLY** on the
+        # server.
+        key = TokenAuthenticatable.cache.fetch("KEY", AUTH_KEY_OPTS) {
+          # Clear all existing data generated by previous key
+          clear_auth_cache
+          # HMAC-SHA256 takes a maximum key size of 64-bytes
+          # See https://crypto.stackexchange.com/questions/34864/key-size-for-hmac-sha256/34866#34866
+          SecureRandom.random_bytes(64)
+        }
+        OpenSSL::HMAC.digest("SHA256", key, token)
+      end
+
       def cache_valid_auth(token, force: false, &generate_cache)
-        cache_key = token
-        val = TokenAuthenticatable.cache.read(cache_key) unless force
-        val ||= store_valid_auth(cache_key, &generate_cache)
+        cache_key = auth_cache_key(token)
+        val, nonce, hmac = TokenAuthenticatable.cache.read(cache_key) unless force
+        val = nil unless hmac == OpenSSL::HMAC.digest("SHA256", "#{nonce}#{token}", val.to_s)
+        val ||= store_valid_auth(token, cache_key, &generate_cache)
         shallow_freeze(val)
       end
 
@@ -52,14 +72,29 @@ def shallow_freeze(val)
         val.each { |_k, v| v.freeze }.freeze
       end
 
-      def store_valid_auth(cache_key)
-        val = yield
-        TokenAuthenticatable.cache.write(cache_key, val, CACHE_TTL_OPTS) if val
+      def store_valid_auth(token, cache_key = auth_cache_key(token))
+        return unless (val = yield(token))
+
+        # HMAC-SHA256 takes a maximum of 64-bytes for the key. When data is
+        # longer it is hashed, truncating to 32-bytes. We add the nonce here to
+        # force us over that 64-byte limit, thus hashing the result. This is to
+        # ensure at least 32-bytes of entropy from the token. This HMAC is
+        # mearly meant as a hash collision guard, not as added security.
+        nonce = SecureRandom.random_bytes(64)
+        hmac = OpenSSL::HMAC.digest("SHA256", "#{nonce}#{token}", val.to_s)
+        TokenAuthenticatable.cache.write cache_key,
+                                         [val, nonce, hmac].freeze,
+                                         CACHE_TTL_OPTS
         val
       end
 
     private
 
+      AUTH_KEY_OPTS = {
+        expires_in: 5.minutes, # Rotate key relatively frequently
+        race_condition_ttl: 1.second,
+      }.freeze
+
       CACHE_TTL_OPTS = {
         expires_in: ENV.fetch("KRACKEN_TOKEN_TTL", 1.minute).to_i,
         race_condition_ttl: 1.second,
diff --git a/spec/kracken/controllers/token_authenticatable_spec.rb b/spec/kracken/controllers/token_authenticatable_spec.rb
@@ -72,7 +72,9 @@ def authenticate_or_request_with_http_token(realm = nil)
             'HTTP_AUTHORIZATION' => "Token token=\"#{cached_token}\""
           }
 
-          Controllers::TokenAuthenticatable.cache.write cache_key, auth_info
+          Controllers::TokenAuthenticatable.store_valid_auth cached_token do
+            auth_info
+          end
           stub_const "Kracken::Authenticator", spy("Kracken::Authenticator")
         end
 
@@ -163,7 +165,9 @@ def authenticate_or_request_with_http_token(realm = nil)
           expect {
             a_controller.authenticate_user_with_token!
           }.not_to change {
-            Controllers::TokenAuthenticatable.cache.exist?("#{invalid_token}")
+            Controllers::TokenAuthenticatable.cache.exist?(
+              Controllers::TokenAuthenticatable.auth_cache_key(invalid_token)
+            )
           }.from false
         end
       end
@@ -218,20 +222,28 @@ def authenticate_or_request_with_http_token(realm = nil)
           expect(a_controller.current_team_ids).to be_frozen
         end
 
-        it "sets the auth info as the cache value" do
+        it "sets the auth info, along with a nonce, and HMAC as the cache value" do
           expect {
             a_controller.authenticate_user_with_token!
           }.to change {
-            Controllers::TokenAuthenticatable.cache.read("any token")
+            Controllers::TokenAuthenticatable.cache.read(
+              Controllers::TokenAuthenticatable.auth_cache_key("any token")
+            )
           }.from(nil).to(
-            id: :any_id,
-            team_ids: [:some, :team, :ids],
+            [
+              {
+                id: :any_id,
+                team_ids: [:some, :team, :ids],
+              },
+              be_a(String).and(satisfy { |s| s.size == 64 }),
+              be_a(String).and(satisfy { |s| s.size == 32 }),
+            ]
           )
         end
 
         it "sets the cache expiration to one minute by default" do
           expect(Controllers::TokenAuthenticatable.cache).to receive(:write).with(
-            "any token",
+            Controllers::TokenAuthenticatable.auth_cache_key("any token"),
             anything,
             include(expires_in: 1.minute),
           )
@@ -243,6 +255,51 @@ def authenticate_or_request_with_http_token(realm = nil)
           a_controller.authenticate_user_with_token!
           expect(a_controller.current_user).to be a_user
         end
+
+        it "treats an HMAC verification failure as a miss" do
+          initial_salt = "salt" * 16
+          initial_hmac = "hmac" * 8
+          cache_key = Controllers::TokenAuthenticatable.auth_cache_key(
+            "any token"
+          )
+          Controllers::TokenAuthenticatable.cache.write(
+            cache_key,
+            [
+              {
+                id: :any_id,
+                team_ids: [:some, :team, :ids],
+              },
+              initial_salt,
+              initial_hmac,
+            ],
+          )
+
+          expect {
+            a_controller.authenticate_user_with_token!
+          }.to change {
+            Controllers::TokenAuthenticatable.cache.read(cache_key)
+          }.from(
+            [
+              {
+                id: :any_id,
+                team_ids: [:some, :team, :ids],
+              },
+              initial_salt,
+              initial_hmac,
+            ],
+          ).to(
+            [
+              {
+                id: :any_id,
+                team_ids: [:some, :team, :ids],
+              },
+              be_a(String).and(satisfy { |s| s.size == 64 && s != initial_salt}),
+              be_a(String).and(satisfy { |s| s.size == 32 && s != initial_hmac}),
+            ]
+          )
+          expect(Authenticator).to have_received(:user_with_token)
+            .with(valid_token)
+        end
       end
     end
   end
