From fa929d8b81002c95f729517d3ce3985f090c5980 Mon Sep 17 00:00:00 2001 From: ThibG Date: Sun, 1 Nov 2020 23:38:31 +0100 Subject: [PATCH] Tweak signature verification (#15069) * Add more specific error message when request body digest is invalid This may help other implementors debug their implementation. * Relax Host parameter requirement to GET requests The only POST requests processed by Mastodon need objects/actors (including their host) to be explicitly mentioned in the request's body, so replaying a legitimate request to another host should not be a security issue. * Support Digest headers using multiple algorithms or lowercase alogirthm names --- .../concerns/signature_verification.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb index f69c62ec2..fc3978fbb 100644 --- a/app/controllers/concerns/signature_verification.rb +++ b/app/controllers/concerns/signature_verification.rb @@ -76,6 +76,7 @@ module SignatureVerification raise SignatureVerificationError, 'Signed request date outside acceptable time window' unless matches_time_window? verify_signature_strength! + verify_body_digest! account = account_from_key_id(signature_params['keyId']) @@ -126,10 +127,19 @@ module SignatureVerification def verify_signature_strength! raise SignatureVerificationError, 'Mastodon requires the Date header or (created) pseudo-header to be signed' unless signed_headers.include?('date') || signed_headers.include?('(created)') raise SignatureVerificationError, 'Mastodon requires the Digest header or (request-target) pseudo-header to be signed' unless signed_headers.include?(Request::REQUEST_TARGET) || signed_headers.include?('digest') - raise SignatureVerificationError, 'Mastodon requires the Host header to be signed' unless signed_headers.include?('host') + raise SignatureVerificationError, 'Mastodon requires the Host header to be signed when doing a GET request' if request.get? && !signed_headers.include?('host') raise SignatureVerificationError, 'Mastodon requires the Digest header to be signed when doing a POST request' if request.post? && !signed_headers.include?('digest') end + def verify_body_digest! + return unless signed_headers.include?('digest') + + digests = request.headers['Digest'].split(',').map { |digest| digest.split('=', 2) }.map { |key, value| [key.downcase, value] } + sha256 = digests.assoc('sha-256') + raise SignatureVerificationError, "Mastodon only supports SHA-256 in Digest header. Offered algorithms: #{digests.map(&:first).join(', ')}" if sha256.nil? + raise SignatureVerificationError, "Invalid Digest value. Computed SHA-256 digest: #{body_digest}; given: #{sha256[1]}" if body_digest != sha256[1] + end + def verify_signature(account, signature, compare_signed_string) if account.keypair.public_key.verify(OpenSSL::Digest.new('SHA256'), signature, compare_signed_string) @signed_request_account = account @@ -153,8 +163,6 @@ module SignatureVerification raise SignatureVerificationError, 'Pseudo-header (expires) used but corresponding argument missing' if signature_params['expires'].blank? "(expires): #{signature_params['expires']}" - elsif signed_header == 'digest' - "digest: #{body_digest}" else "#{signed_header}: #{request.headers[to_header_name(signed_header)]}" end @@ -187,7 +195,7 @@ module SignatureVerification end def body_digest - "SHA-256=#{Digest::SHA256.base64digest(request_body)}" + @body_digest ||= Digest::SHA256.base64digest(request_body) end def to_header_name(name)