diff --git a/app/lib/activitypub/linked_data_signature.rb b/app/lib/activitypub/linked_data_signature.rb index e0c0606e446..35427e0fef7 100644 --- a/app/lib/activitypub/linked_data_signature.rb +++ b/app/lib/activitypub/linked_data_signature.rb @@ -34,18 +34,19 @@ class ActivityPub::LinkedDataSignature end def sign!(creator, sign_with: nil) + keypair = sign_with.presence || creator.keypair(type: :rsa) + options = { 'type' => 'RsaSignature2017', - 'creator' => ActivityPub::TagManager.instance.key_uri_for(creator), + 'creator' => keypair.full_uri, 'created' => Time.now.utc.iso8601, } options_hash = hash(options.without('type', 'id', 'signatureValue').merge('@context' => CONTEXT)) document_hash = hash(@json.without('signature')) to_be_signed = options_hash + document_hash - keypair = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : creator.keypair - signature = Base64.strict_encode64(keypair.sign(OpenSSL::Digest.new('SHA256'), to_be_signed)) + signature = Base64.strict_encode64(keypair.keypair.sign(OpenSSL::Digest.new('SHA256'), to_be_signed)) # Mastodon's context is either an array or a single URL context_with_security = Array(@json['@context']) diff --git a/app/lib/request.rb b/app/lib/request.rb index 5cbecd7b008..f7bbd104466 100644 --- a/app/lib/request.rb +++ b/app/lib/request.rb @@ -100,9 +100,8 @@ class Request def on_behalf_of(actor, sign_with: nil) raise ArgumentError, 'actor must not be nil' if actor.nil? - key_id = ActivityPub::TagManager.instance.key_uri_for(actor) - keypair = sign_with.present? ? OpenSSL::PKey::RSA.new(sign_with) : actor.keypair - @signing = HttpSignatureDraft.new(keypair, key_id) + keypair = sign_with.presence || actor.keypair(type: :rsa) + @signing = HttpSignatureDraft.new(keypair.keypair, keypair.full_uri) self end diff --git a/app/models/account.rb b/app/models/account.rb index 7550bb2a9d2..4400b8807ad 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -301,8 +301,22 @@ class Account < ApplicationRecord strikes.where(overruled_at: nil).count end - def keypair - @keypair ||= OpenSSL::PKey::RSA.new(private_key || public_key) + def keypair(type: nil) + # Pick the first (oldest) keypair matching the expected type, + # as we can expect our key rotation code to add stand-by keys with higher IDs + # before pruning older keys with lower IDs after some time. + + scope = keypairs.usable.order(id: :asc) + scope = scope.where(type: type) if type.present? + + case type + when :rsa, nil + # The legacy key is always RSA, so only fallback + # when no other type is requested + scope.first || Keypair.from_legacy_account(self) + else + scope.first + end end def tags_as_strings=(tag_names) diff --git a/app/models/keypair.rb b/app/models/keypair.rb index 35cd801c9d5..4e2f3ce6c52 100644 --- a/app/models/keypair.rb +++ b/app/models/keypair.rb @@ -50,6 +50,12 @@ class Keypair < ApplicationRecord alias actor account + def full_uri + return ActivityPub::TagManager.instance.uri_for(account) + local_fragment if local_fragment.present? + + uri + end + def keypair @keypair ||= begin case type @@ -87,4 +93,22 @@ class Keypair < ApplicationRecord type: :rsa ) end + + def self.from_worker_arg(account, private_key_pem_or_hash) + if private_key_pem_or_hash.is_a?(String) + account.keypairs.build( + private_key: private_key_pem_or_hash, + local_fragment: '#main-key', + type: :rsa + ) + else + account.keypairs.build( + private_key: private_key_pem_or_hash['private_key'], + public_key: private_key_pem_or_hash['public_key'], + uri: private_key_pem_or_hash['uri'], + local_fragment: private_key_pem_or_hash['local_fragment'], + type: private_key_pem_or_hash['type'] + ) + end + end end diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index 4244f4f1775..ceea4ff2b3c 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -24,7 +24,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer attribute :interaction_policy attribute :featured_collections - has_one :public_key, serializer: ActivityPub::PublicKeySerializer + has_one :keypair, key: :public_key, serializer: ActivityPub::PublicKeySerializer has_many :virtual_tags, key: :tag has_many :virtual_attachments, key: :attachment diff --git a/app/serializers/activitypub/public_key_serializer.rb b/app/serializers/activitypub/public_key_serializer.rb index 8621517e7cd..a3a15949cd7 100644 --- a/app/serializers/activitypub/public_key_serializer.rb +++ b/app/serializers/activitypub/public_key_serializer.rb @@ -6,11 +6,11 @@ class ActivityPub::PublicKeySerializer < ActivityPub::Serializer attributes :id, :owner, :public_key_pem def id - ActivityPub::TagManager.instance.key_uri_for(object) + object.full_uri end def owner - ActivityPub::TagManager.instance.uri_for(object) + ActivityPub::TagManager.instance.uri_for(object.actor) end def public_key_pem diff --git a/app/workers/activitypub/delivery_worker.rb b/app/workers/activitypub/delivery_worker.rb index 8cd39f700ca..26ecd0c4b3b 100644 --- a/app/workers/activitypub/delivery_worker.rb +++ b/app/workers/activitypub/delivery_worker.rb @@ -48,7 +48,7 @@ class ActivityPub::DeliveryWorker def build_request(http_client) Request.new(:post, @inbox_url, body: @json, http_client: http_client).tap do |request| - request.on_behalf_of(@source_account, sign_with: @options[:sign_with]) + request.on_behalf_of(@source_account, sign_with: sign_with) request.add_headers(HEADERS) request.add_headers({ 'Collection-Synchronization' => synchronization_header }) if ENV['DISABLE_FOLLOWERS_SYNCHRONIZATION'] != 'true' && @options[:synchronize_followers] end @@ -93,4 +93,8 @@ class ActivityPub::DeliveryWorker def request_pool RequestPool.current end + + def sign_with + @options[:sign_with].presence && Keypair.from_worker_arg(@source_account, @options[:sign_with]) + end end diff --git a/app/workers/activitypub/update_distribution_worker.rb b/app/workers/activitypub/update_distribution_worker.rb index 6b6c9056327..4c79133d909 100644 --- a/app/workers/activitypub/update_distribution_worker.rb +++ b/app/workers/activitypub/update_distribution_worker.rb @@ -23,6 +23,10 @@ class ActivityPub::UpdateDistributionWorker < ActivityPub::RawDistributionWorker end def payload - @payload ||= serialize_payload(@account, ActivityPub::UpdateActorSerializer, signer: @account, sign_with: @options[:sign_with]).to_json + @payload ||= serialize_payload(@account, ActivityPub::UpdateActorSerializer, signer: @account, sign_with: sign_with).to_json + end + + def sign_with + @options[:sign_with].presence && Keypair.from_worker_arg(@account, @options[:sign_with]) end end diff --git a/lib/mastodon/cli/accounts.rb b/lib/mastodon/cli/accounts.rb index e4c1aac1287..9f3fda27062 100644 --- a/lib/mastodon/cli/accounts.rb +++ b/lib/mastodon/cli/accounts.rb @@ -612,10 +612,22 @@ module Mastodon::CLI def rotate_keys_for_account(account, delay = 0) fail_with_message 'No such account' if account.nil? - old_key = account.private_key + old_key = account.keypair new_key = OpenSSL::PKey::RSA.new(2048) - account.update(private_key: new_key.to_pem, public_key: new_key.public_key.to_pem) - ActivityPub::UpdateDistributionWorker.perform_in(delay, account.id, { 'sign_with' => old_key }) + + account.update(private_key: nil, public_key: '', keypairs: [account.keypairs.build(local_fragment: '#main-key', type: :rsa, public_key: new_key.public_key.to_pem, private_key: new_key.to_pem)]) + + ActivityPub::UpdateDistributionWorker.perform_in( + delay, + account.id, + { + 'sign_with' => { + 'private_key' => old_key.private_key, + 'local_fragment' => old_key.local_fragment, + 'type' => old_key.type, + }, + } + ) end end end diff --git a/spec/lib/activitypub/linked_data_signature_spec.rb b/spec/lib/activitypub/linked_data_signature_spec.rb index 7aaff9680e3..113cfcea9bd 100644 --- a/spec/lib/activitypub/linked_data_signature_spec.rb +++ b/spec/lib/activitypub/linked_data_signature_spec.rb @@ -159,6 +159,6 @@ RSpec.describe ActivityPub::LinkedDataSignature do options_hash = Digest::SHA256.hexdigest(canonicalize(options.merge('@context' => ActivityPub::LinkedDataSignature::CONTEXT))) document_hash = Digest::SHA256.hexdigest(canonicalize(document)) to_be_verified = options_hash + document_hash - Base64.strict_encode64(from_actor.keypair.sign(OpenSSL::Digest.new('SHA256'), to_be_verified)) + Base64.strict_encode64(from_actor.keypair.keypair.sign(OpenSSL::Digest.new('SHA256'), to_be_verified)) end end diff --git a/spec/lib/mastodon/cli/accounts_spec.rb b/spec/lib/mastodon/cli/accounts_spec.rb index 927c6ca8deb..54cf4549f1f 100644 --- a/spec/lib/mastodon/cli/accounts_spec.rb +++ b/spec/lib/mastodon/cli/accounts_spec.rb @@ -951,15 +951,15 @@ RSpec.describe Mastodon::CLI::Accounts do let(:arguments) { [account.username] } it 'correctly rotates keys for the specified account' do - old_private_key = account.private_key - old_public_key = account.public_key + old_private_key = account.keypair.private_key + old_public_key = account.keypair.public_key expect { subject } .to output_results('OK') account.reload - expect(account.private_key).to_not eq(old_private_key) - expect(account.public_key).to_not eq(old_public_key) + expect(account.keypair.private_key).to_not eq(old_private_key) + expect(account.keypair.public_key).to_not eq(old_public_key) end it 'broadcasts the new keys for the specified account' do @@ -986,15 +986,15 @@ RSpec.describe Mastodon::CLI::Accounts do let(:options) { { all: true } } it 'correctly rotates keys for all local accounts' do - old_private_keys = accounts.map(&:private_key) - old_public_keys = accounts.map(&:public_key) + old_private_keys = accounts.map { |account| account.keypair.private_key } + old_public_keys = accounts.map { |account| account.keypair.public_key } expect { subject } .to output_results('rotated') accounts.each(&:reload) - expect(accounts.map(&:private_key)).to_not eq(old_private_keys) - expect(accounts.map(&:public_key)).to_not eq(old_public_keys) + expect(accounts.map { |account| account.keypair.private_key }).to_not eq(old_private_keys) + expect(accounts.map { |account| account.keypair.public_key }).to_not eq(old_public_keys) end it 'broadcasts the new keys for each account' do diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index b3f09161074..69842edf509 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -246,9 +246,10 @@ RSpec.describe Account do end describe '#keypair' do - it 'returns an RSA key pair' do + it 'returns a Keypair object with a RSA key pair' do account = Fabricate(:account) - expect(account.keypair).to be_instance_of OpenSSL::PKey::RSA + expect(account.keypair).to be_instance_of Keypair + expect(account.keypair.keypair).to be_instance_of OpenSSL::PKey::RSA end end @@ -738,7 +739,7 @@ RSpec.describe Account do expect(account) .to be_private_key .and be_public_key - expect(account.keypair) + expect(account.keypair.keypair) .to be_private .and be_public end @@ -748,7 +749,7 @@ RSpec.describe Account do it 'does not generate keys' do key = OpenSSL::PKey::RSA.new(1024).public_key account = described_class.create!(domain: 'remote', uri: 'https://remote/actor', username: 'remote_user_with_public', public_key: key.to_pem) - expect(account.keypair.params).to eq key.params + expect(account.keypair.keypair.params).to eq key.params end it 'normalizes domain' do diff --git a/spec/models/keypair_spec.rb b/spec/models/keypair_spec.rb index e7b18d8e68f..59482d3bcad 100644 --- a/spec/models/keypair_spec.rb +++ b/spec/models/keypair_spec.rb @@ -11,6 +11,29 @@ RSpec.describe Keypair do end end + describe '#full_uri' do + let(:keypair) { Fabricate(:keypair, account: account) } + + context 'with a remote account' do + let(:account) { Fabricate(:remote_account) } + + it 'returns an HTTP URI equals to the stored URI' do + expect(keypair.full_uri) + .to start_with('https://') + .and eq(keypair.uri) + end + end + + context 'with a local account' do + let(:account) { Fabricate(:account) } + + it 'returns an HTTP URI starting with the account URI' do + expect(keypair.full_uri) + .to start_with(ActivityPub::TagManager.instance.uri_for(account)) + end + end + end + describe 'from_keyid' do context 'when a key with the given key ID exists' do let(:account) { Fabricate(:account, domain: 'example.com') } diff --git a/spec/support/signed_request_helpers.rb b/spec/support/signed_request_helpers.rb index a4423af748f..25a71f5d5e3 100644 --- a/spec/support/signed_request_helpers.rb +++ b/spec/support/signed_request_helpers.rb @@ -9,10 +9,10 @@ module SignedRequestHelpers headers['Host'] = Rails.configuration.x.local_domain signed_headers = headers.merge('(request-target)' => "get #{path}").slice('(request-target)', 'Host', 'Date') - key_id = ActivityPub::TagManager.instance.key_uri_for(sign_with) keypair = sign_with.keypair + key_id = keypair.uri signed_string = signed_headers.map { |key, value| "#{key.downcase}: #{value}" }.join("\n") - signature = Base64.strict_encode64(keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) + signature = Base64.strict_encode64(keypair.keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) headers['Signature'] = "keyId=\"#{key_id}\",algorithm=\"rsa-sha256\",headers=\"#{signed_headers.keys.join(' ').downcase}\",signature=\"#{signature}\"" @@ -29,10 +29,10 @@ module SignedRequestHelpers signed_headers = headers.merge('(request-target)' => "post #{path}").slice('(request-target)', 'Host', 'Date', 'Digest') - key_id = ActivityPub::TagManager.instance.key_uri_for(sign_with) keypair = sign_with.keypair + key_id = keypair.uri signed_string = signed_headers.map { |key, value| "#{key.downcase}: #{value}" }.join("\n") - signature = Base64.strict_encode64(keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) + signature = Base64.strict_encode64(keypair.keypair.sign(OpenSSL::Digest.new('SHA256'), signed_string)) headers['Signature'] = "keyId=\"#{key_id}\",algorithm=\"rsa-sha256\",headers=\"#{signed_headers.keys.join(' ').downcase}\",signature=\"#{signature}\""