feat: harden hmac binding
This commit is contained in:
@@ -30,8 +30,9 @@ class Public::Api::V1::Inboxes::ContactsController < Public::Api::V1::InboxesCon
|
||||
def process_hmac
|
||||
return if params[:identifier_hash].blank? && !@inbox_channel.hmac_mandatory
|
||||
raise StandardError, 'HMAC failed: Invalid Identifier Hash Provided' unless valid_hmac?
|
||||
raise StandardError, 'HMAC failed: Identifier does not match contact' if hmac_identifier_conflict?
|
||||
|
||||
@contact_inbox.update(hmac_verified: true) if @contact_inbox.present?
|
||||
@contact_inbox.update(hmac_verified: true) if @contact_inbox.present? && hmac_identifier_matches?
|
||||
end
|
||||
|
||||
def valid_hmac?
|
||||
@@ -42,6 +43,24 @@ class Public::Api::V1::Inboxes::ContactsController < Public::Api::V1::InboxesCon
|
||||
)
|
||||
end
|
||||
|
||||
# Rejects the request when the selected contact is already bound to a different identifier,
|
||||
# so a valid (identifier, identifier_hash) pair cannot be replayed against another contact.
|
||||
def hmac_identifier_conflict?
|
||||
return false if @contact_inbox.blank?
|
||||
|
||||
contact_identifier = @contact_inbox.contact.identifier.to_s
|
||||
contact_identifier.present? &&
|
||||
!ActiveSupport::SecurityUtils.secure_compare(contact_identifier, params[:identifier].to_s)
|
||||
end
|
||||
|
||||
# Only grants verified trust when the proven identifier matches the contact's own identifier.
|
||||
# A blank contact identifier never grants trust, preventing replay onto anonymous contacts.
|
||||
def hmac_identifier_matches?
|
||||
contact_identifier = @contact_inbox.contact.identifier.to_s
|
||||
contact_identifier.present? &&
|
||||
ActiveSupport::SecurityUtils.secure_compare(contact_identifier, params[:identifier].to_s)
|
||||
end
|
||||
|
||||
def permitted_params
|
||||
params.permit(:identifier, :identifier_hash, :email, :name, :avatar_url, :phone_number, custom_attributes: {})
|
||||
end
|
||||
|
||||
@@ -48,4 +48,52 @@ RSpec.describe 'Public Inbox Contacts API', type: :request do
|
||||
expect(data['name']).to eq 'John Smith'
|
||||
end
|
||||
end
|
||||
|
||||
describe 'HMAC identifier binding' do
|
||||
let!(:owner_contact) { create(:contact, account: api_channel.account, identifier: 'owner-identifier') }
|
||||
let!(:owner_inbox) { create(:contact_inbox, contact: owner_contact, inbox: api_channel.inbox) }
|
||||
let!(:victim_contact) { create(:contact, account: api_channel.account, identifier: 'victim-identifier', name: 'Victim') }
|
||||
let!(:victim_inbox) { create(:contact_inbox, contact: victim_contact, inbox: api_channel.inbox) }
|
||||
let(:valid_owner_hash) { OpenSSL::HMAC.hexdigest('sha256', api_channel.hmac_token, 'owner-identifier') }
|
||||
|
||||
it 'does not promote the victim contact inbox to hmac_verified when a valid pair is replayed against the victim source_id' do
|
||||
begin
|
||||
get "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{victim_inbox.source_id}",
|
||||
params: { identifier: 'owner-identifier', identifier_hash: valid_owner_hash }
|
||||
rescue StandardError
|
||||
# the fix fails closed by raising; the security invariant is asserted below
|
||||
end
|
||||
|
||||
expect(victim_inbox.reload.hmac_verified).to be(false)
|
||||
end
|
||||
|
||||
it 'does not mutate the victim contact when a valid pair is replayed against the victim source_id' do
|
||||
begin
|
||||
patch "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{victim_inbox.source_id}",
|
||||
params: { identifier: 'owner-identifier', identifier_hash: valid_owner_hash, name: 'Hacked' }
|
||||
rescue StandardError
|
||||
# the fix fails closed by raising; the security invariant is asserted below
|
||||
end
|
||||
|
||||
expect(victim_contact.reload.name).to eq('Victim')
|
||||
end
|
||||
|
||||
it 'still verifies the contact inbox when the owner presents a matching identifier pair' do
|
||||
get "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{owner_inbox.source_id}",
|
||||
params: { identifier: 'owner-identifier', identifier_hash: valid_owner_hash }
|
||||
|
||||
expect(response).to have_http_status(:success)
|
||||
expect(owner_inbox.reload.hmac_verified).to be(true)
|
||||
end
|
||||
|
||||
it 'does not verify an anonymous contact inbox when a valid pair is replayed against its source_id' do
|
||||
anonymous_contact = create(:contact, account: api_channel.account)
|
||||
anonymous_inbox = create(:contact_inbox, contact: anonymous_contact, inbox: api_channel.inbox)
|
||||
|
||||
get "/public/api/v1/inboxes/#{api_channel.identifier}/contacts/#{anonymous_inbox.source_id}",
|
||||
params: { identifier: 'owner-identifier', identifier_hash: valid_owner_hash }
|
||||
|
||||
expect(anonymous_inbox.reload.hmac_verified).to be(false)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user