From 28b482874ab4393639a77fdd895658096bcbfd57 Mon Sep 17 00:00:00 2001
From: ThibG <thib@sitedethib.com>
Date: Mon, 7 Jan 2019 21:45:13 +0100
Subject: Improvements to signature verification (#9667)

* Refactor signature verification a bit

* Rescue signature verification if recorded public key is invalid

Fixes #8822

* Always re-fetch AP signing key when HTTP Signature verification fails

But when the account is not marked as stale, avoid fetching collections and
media, and avoid webfinger round-trip.

* Apply stoplight to key/account update as well as initial key retrieval
---
 app/controllers/concerns/signature_verification.rb | 45 +++++++++++++++-------
 1 file changed, 31 insertions(+), 14 deletions(-)

(limited to 'app/controllers/concerns')

diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb
index 887096e8b..91566c4fa 100644
--- a/app/controllers/concerns/signature_verification.rb
+++ b/app/controllers/concerns/signature_verification.rb
@@ -60,23 +60,26 @@ module SignatureVerification
     signature             = Base64.decode64(signature_params['signature'])
     compare_signed_string = build_signed_string(signature_params['headers'])
 
-    if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string)
-      @signed_request_account = account
-      @signed_request_account
-    elsif account.possibly_stale?
-      account = account.refresh!
+    return account unless verify_signature(account, signature, compare_signed_string).nil?
 
-      if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string)
-        @signed_request_account = account
-        @signed_request_account
-      else
-        @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}"
-        @signed_request_account = nil
-      end
-    else
-      @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}"
+    account_stoplight = Stoplight("source:#{request.ip}") { account.possibly_stale? ? account.refresh! : account_refresh_key(account) }
+      .with_fallback { nil }
+      .with_threshold(1)
+      .with_cool_off_time(5.minutes.seconds)
+      .with_error_handler { |error, handle| error.is_a?(HTTP::Error) ? handle.call(error) : raise(error) }
+
+    account = account_stoplight.run
+
+    if account.nil?
+      @signature_verification_failure_reason = "Public key not found for key #{signature_params['keyId']}"
       @signed_request_account = nil
+      return
     end
+
+    return account unless verify_signature(account, signature, compare_signed_string).nil?
+
+    @signature_verification_failure_reason = "Verification failed for #{account.username}@#{account.domain} #{account.uri}"
+    @signed_request_account = nil
   end
 
   def request_body
@@ -85,6 +88,15 @@ module SignatureVerification
 
   private
 
+  def verify_signature(account, signature, compare_signed_string)
+    if account.keypair.public_key.verify(OpenSSL::Digest::SHA256.new, signature, compare_signed_string)
+      @signed_request_account = account
+      @signed_request_account
+    end
+  rescue OpenSSL::PKey::RSAError
+    nil
+  end
+
   def build_signed_string(signed_headers)
     signed_headers = 'date' if signed_headers.blank?
 
@@ -131,4 +143,9 @@ module SignatureVerification
       account
     end
   end
+
+  def account_refresh_key(account)
+    return if account.local? || !account.activitypub?
+    ActivityPub::FetchRemoteAccountService.new.call(account.uri, only_key: true)
+  end
 end
-- 
cgit