diff --git a/app/models/concerns/tokenable.rb b/app/models/concerns/tokenable.rb index ee795186ab..eb5b26d215 100644 --- a/app/models/concerns/tokenable.rb +++ b/app/models/concerns/tokenable.rb @@ -28,7 +28,7 @@ def generate_hashed_token(attribute, length: 64, version: ALGO_VERSION) # length, since the first 66 chars of our string consist of the account # and the bearer's UUID. This lets us use larger tokens (as seen here) # and avoid the nasty truncation. - res = BCrypt::Password.create Digest::SHA256.digest(raw) + res = BCrypt::Password.create Digest::SHA256.hexdigest(raw) when "v2" raw = SecureRandom.hex(length).gsub /.{#{version.length}}\z/, version raw = yield raw if block_given? @@ -53,6 +53,7 @@ def compare_hashed_token(attribute, token, version: ALGO_VERSION) a = self.send attribute b = nil + c = nil case version when "v1" @@ -64,6 +65,9 @@ def compare_hashed_token(attribute, token, version: ALGO_VERSION) end b = BCrypt::Engine.hash_secret digest, bcrypt.salt + + # FIXME(ezekg) support rehashing: https://github.com/bcrypt-ruby/bcrypt-ruby/pull/168 + c = BCrypt::Engine.hash_secret Digest::SHA256.hexdigest(token), bcrypt.salt when "v2" b = OpenSSL::HMAC.hexdigest "SHA512", account.private_key, token when "v3" @@ -72,7 +76,11 @@ def compare_hashed_token(attribute, token, version: ALGO_VERSION) raise NotImplementedError.new "token #{version} not implemented" end - secure_compare a, b + unless c.nil? + secure_compare(a, b) || secure_compare(a, c) + else + secure_compare(a, b) + end rescue false end diff --git a/app/services/license_key_lookup_service.rb b/app/services/license_key_lookup_service.rb index f4e05a53be..c7fffd0791 100644 --- a/app/services/license_key_lookup_service.rb +++ b/app/services/license_key_lookup_service.rb @@ -22,6 +22,17 @@ def call if license&.compare_hashed_token(:key, key, version: 'v1') Keygen.logger.warn { "[license-key-lookup-service] v1 keys are deprecated and must be regenerated: license_id=#{license.id.inspect}" } + # FIXME(ezekg) jit rehash key: https://github.com/bcrypt-ruby/bcrypt-ruby/pull/168 + digest = BCrypt::Engine.hash_secret( + Digest::SHA256.hexdigest(key), + BCrypt::Password.new(license.key).salt, # reuse salt + ) + unless license.send(:secure_compare, digest, license.key) + Keygen.logger.warn { "[license-key-lookup-service] rehashing key: license_id=#{license.id.inspect}" } + + license.update!(key: digest) + end + license else nil diff --git a/app/services/token_lookup_service.rb b/app/services/token_lookup_service.rb index 0874b71a42..8b60ae91b4 100644 --- a/app/services/token_lookup_service.rb +++ b/app/services/token_lookup_service.rb @@ -34,6 +34,17 @@ def call if instance&.compare_hashed_token(:digest, token, version: 'v1') Keygen.logger.warn { "[token-lookup-service] v1 tokens are deprecated and must be regenerated: bearer_type=#{instance.bearer.class.name.inspect} bearer_id=#{instance.bearer.id.inspect} token_id=#{instance.id.inspect}" } + # FIXME(ezekg) jit rehash token: https://github.com/bcrypt-ruby/bcrypt-ruby/pull/168 + digest = BCrypt::Engine.hash_secret( + Digest::SHA256.hexdigest(token), + BCrypt::Password.new(instance.digest).salt, # reuse salt + ) + unless instance.send(:secure_compare, digest, instance.digest) + Keygen.logger.warn { "[license-key-lookup-service] rehashing token: token_id=#{instance.id.inspect}" } + + instance.update!(digest:) + end + instance else nil diff --git a/spec/services/license_key_lookup_service_spec.rb b/spec/services/license_key_lookup_service_spec.rb new file mode 100644 index 0000000000..254b3c4995 --- /dev/null +++ b/spec/services/license_key_lookup_service_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'spec_helper' + +describe LicenseKeyLookupService do + let(:account) { create(:account) } + + context 'when key is a legacy encrypted key' do + let(:key) { 'bdcecdc23c0f48229f77357151d1e67f-8e1ae3236636bfe57893b835871553eb-524ffec53ad05c14bbf3339eec741300-295512a0539cd0863abe42ddc29cf9v1' } + let(:license_id) { key.split('-').first } + + context 'when key is a good key' do + let(:digest) { '$2a$12$kh33FHu.TWYMZlWldcOMKet4YiVNqMHq8/A343/GTfPz.NRCPq2Q.' } + + subject! { + create(:license, id: license_id, key: digest, account:) + } + + it 'should fail unencrypted lookup' do + record = described_class.call(key:, account:) + + expect(record).to be nil + end + + it 'should pass encrypted lookup' do + record = described_class.call(key:, account:, legacy_encrypted: true) + + expect(record).to eq subject + end + + it 'should not rehash' do + expect { described_class.call(key:, account:, legacy_encrypted: true) }.to_not( + change { subject.reload.key }, + ) + end + end + + context 'when key is a bad key' do + let(:digest) { '$2a$12$kh33FHu.TWYMZlWldcOMKevM0Jj4VrN/8.EbfAj4cBEcS51A7hYUK' } + + subject!{ + create(:license, id: license_id, key: digest, account:) + } + + it 'should fail unencrypted lookup' do + record = described_class.call(key:, account:) + + expect(record).to be nil + end + + it 'should pass encrypted lookup' do + record = described_class.call(key:, account:, legacy_encrypted: true) + + expect(record).to eq subject + end + + it 'should rehash' do + expect { described_class.call(key:, account:, legacy_encrypted: true) }.to( + change { subject.reload.key }, + ) + + # sanity check + ok = subject.compare_hashed_token(:key, key, version: 'v1') + expect(ok).to be true + end + end + end + + context 'when key is a normal key' do + let(:key) { 'FC1ECF-659627-58D58E-42130E-ADD88F-V3' } + + subject! { + create(:license, key:, account:) + } + + it 'should pass unencrypted lookup' do + record = described_class.call(key:, account:) + + expect(record).to eq subject + end + + it 'should fail encrypted lookup' do + record = described_class.call(key:, account:, legacy_encrypted: true) + + expect(record).to be nil + end + + it 'should not rehash' do + expect { described_class.call(key:, account:) }.to_not( + change { subject.reload.key }, + ) + end + end + + context 'when key is invalid' do + let(:key) { '6798CE-A9478B-42027F-4046E8-3FFD66-V3' } + + it 'should fail unencrypted lookup' do + record = described_class.call(key:, account:) + + expect(record).to be nil + end + + it 'should fail encrypted lookup' do + record = described_class.call(key:, account:, legacy_encrypted: true) + + expect(record).to be nil + end + end +end diff --git a/spec/services/token_lookup_service_spec.rb b/spec/services/token_lookup_service_spec.rb new file mode 100644 index 0000000000..3e3ecd637d --- /dev/null +++ b/spec/services/token_lookup_service_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'spec_helper' + +describe TokenLookupService do + context 'when token is a v1 token' do + let(:token) { '1780aeccbbb14b4dad0cf8b8165faba2.52f61713fd0c49a2a6537e3142d043cb.3e2373c60b7df8514ff763dad707faad44956c2421fdb2588fff2a73099b07v1' } + let(:account_id) { token.split('.').first } + let(:token_id) { token.split('.').second } + let(:account) { create(:account, id: account_id) } + let(:product) { create(:product, account:) } + + context 'when token is a good token' do + let(:digest) { '$2a$12$7sgr9E9ZURyXzttcuIl/muYRww.wHxNgCpgUHsKODV2ZXpPWiqLkq' } + + subject! { + record = create(:token, id: token_id, bearer: product, account:) + record.update!(digest:) + record + } + + it 'should pass lookup' do + record = described_class.call(token:, account:) + + expect(record).to eq subject + end + + it 'should not rehash' do + expect { described_class.call(token:, account:) }.to_not( + change { subject.reload.digest }, + ) + end + end + + context 'when token is a bad token' do + let(:digest) { '$2a$12$7sgr9E9ZURyXzttcuIl/muSl7JViEcBcytnbouQooX22PP43AzZ4C' } + + subject! { + record = create(:token, id: token_id, bearer: product, account:) + record.update!(digest:) + record + } + + it 'should pass lookup' do + record = described_class.call(token:, account:) + + expect(record).to eq subject + end + + it 'should rehash' do + expect { described_class.call(token:, account:) }.to( + change { subject.reload.digest }, + ) + + # sanity check + ok = subject.compare_hashed_token(:digest, token, version: 'v1') + expect(ok).to be true + end + end + end + + context 'when token is a v2 token' do + let(:token) { 'prod-c5cf2bc0986bb90cae46dade120172c1451abfc3429a4f9057a9786738d192v2' } + let(:digest) { '0c8f765a79a45992c1031f8cc69b858a960257ee16bfd67e26487913565e04337c25897b62011d45b75a7bc20a7e16057fef2573d32f91c1f264f595cfdd2a04' } + let(:account) { create(:account) } + let(:product) { create(:product, account:) } + + subject! { + record = create(:token, bearer: product, account:) + record.update!(digest:) + record + } + + it 'should pass lookup' do + record = described_class.call(token:, account:) + + expect(record).to eq subject + end + + it 'should not rehash' do + expect { described_class.call(token:, account:) }.to_not( + change { subject.reload.digest }, + ) + end + end + + context 'when token is a v3 token' do + let(:secret_key) { '9ef57edb7f2a91bed90805744cdbf4ece13905ad2670bc1f54212074043cede710ca6a60c95114cf16fbbef7d696b0c61649250a9baf14aab878e5f85f769836' } + let(:token) { 'prod-580839d4388a61216398c83f2c987a80dee472f556db7bc9aa66775574de54f4v3' } + let(:digest) { '0e555b39a256a319674356f834d39de70c06b0d0ef94f4b8f9c7d34bdc287f93' } + let(:product) { create(:product, account:) } + let(:account) { + record = create(:account) + record.update(secret_key:) + record + } + + subject! { + record = create(:token, bearer: product, account:) + record.update!(digest:) + record + } + + it 'should pass lookup' do + record = described_class.call(token:, account:) + + expect(record).to eq subject + end + + it 'should not rehash' do + expect { described_class.call(token:, account:) }.to_not( + change { subject.reload.digest }, + ) + end + end + + context 'when token is invalid' do + let(:token) { 'user-f25149752c704e0c8281150c6751ba77052111ad14214d5adacd0a80d45cc957v3' } + let(:account) { create(:account) } + + it 'should fail lookup' do + record = described_class.call(token:, account:) + + expect(record).to be nil + end + end +end