From e117c9fb6d6e9e185b84996ad15d24e8a5b63103 Mon Sep 17 00:00:00 2001 From: Brandur Date: Fri, 24 Apr 2020 10:58:42 -0700 Subject: [PATCH] Expose `Stripe::Webhook.compute_signature` (#915) Exposes the `.compute_signature` method, which may be useful when testing webhook signing in test suites. I change the API slightly so that a caller isn't forced to do as much string mangling, and to match the one that we already have in stripe-go: ``` go func ComputeSignature(t time.Time, payload []byte, secret string) []byte { ``` Add basic documentation and test case. I also change a few things around so that we send `Time` objects around more often where applicable, and don't change then to Unix integers until the last moment that we need to. The one other alternative API I considered is this one, which would default the timestamp to the current time to allow the method to be called with one fewer arg: ``` ruby def self.compute_signature(payload, secret: timestamp: Time.now) ``` I decided against it in the end though because it does remove some explicitness, and it's not a big deal to just pass in `Time.now`, especially given that this is not expected to be a commonly used method. Fixes #912. --- lib/stripe/webhook.rb | 28 +++++++++++++++++++++------- test/stripe/webhook_test.rb | 27 ++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/lib/stripe/webhook.rb b/lib/stripe/webhook.rb index ef06ccafa..e11eace26 100644 --- a/lib/stripe/webhook.rb +++ b/lib/stripe/webhook.rb @@ -24,10 +24,20 @@ def self.construct_event(payload, sig_header, secret, module Signature EXPECTED_SCHEME = "v1" - def self.compute_signature(payload, secret) - OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) + # Computes a webhook signature given a time (probably the current time), + # a payload, and a signing secret. + def self.compute_signature(timestamp, payload, secret) + raise ArgumentError, "timestamp should be an instance of Time" \ + unless timestamp.is_a?(Time) + raise ArgumentError, "payload should be a string" \ + unless payload.is_a?(String) + raise ArgumentError, "secret should be a string" \ + unless secret.is_a?(String) + + timestamped_payload = "#{timestamp.to_i}.#{payload}" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, + timestamped_payload) end - private_class_method :compute_signature # Extracts the timestamp and the signature(s) with the desired scheme # from the header @@ -35,7 +45,7 @@ def self.get_timestamp_and_signatures(header, scheme) list_items = header.split(/,\s*/).map { |i| i.split("=", 2) } timestamp = Integer(list_items.select { |i| i[0] == "t" }[0][1]) signatures = list_items.select { |i| i[0] == scheme }.map { |i| i[1] } - [timestamp, signatures] + [Time.at(timestamp), signatures] end private_class_method :get_timestamp_and_signatures @@ -53,6 +63,11 @@ def self.verify_header(payload, header, secret, tolerance: nil) begin timestamp, signatures = get_timestamp_and_signatures(header, EXPECTED_SCHEME) + + # TODO: Try to knock over this blanket rescue as it can unintentionally + # swallow many valid errors. Instead, try to validate an incoming + # header one piece at a time, and error with a known exception class if + # any part is found to be invalid. Rescue that class here. rescue StandardError raise SignatureVerificationError.new( "Unable to extract timestamp and signatures from header", @@ -67,8 +82,7 @@ def self.verify_header(payload, header, secret, tolerance: nil) ) end - signed_payload = "#{timestamp}.#{payload}" - expected_sig = compute_signature(signed_payload, secret) + expected_sig = compute_signature(timestamp, payload, secret) unless signatures.any? { |s| Util.secure_compare(expected_sig, s) } raise SignatureVerificationError.new( "No signatures found matching the expected signature for payload", @@ -76,7 +90,7 @@ def self.verify_header(payload, header, secret, tolerance: nil) ) end - if tolerance && timestamp < Time.now.to_f - tolerance + if tolerance && timestamp < Time.now - tolerance raise SignatureVerificationError.new( "Timestamp outside the tolerance zone (#{Time.at(timestamp)})", header, http_body: payload diff --git a/test/stripe/webhook_test.rb b/test/stripe/webhook_test.rb index 42ed1258e..f22dd13d5 100644 --- a/test/stripe/webhook_test.rb +++ b/test/stripe/webhook_test.rb @@ -13,12 +13,29 @@ class WebhookTest < Test::Unit::TestCase SECRET = "whsec_test_secret" def generate_header(opts = {}) - opts[:timestamp] ||= Time.now.to_i + opts[:timestamp] ||= Time.now opts[:payload] ||= EVENT_PAYLOAD opts[:secret] ||= SECRET opts[:scheme] ||= Stripe::Webhook::Signature::EXPECTED_SCHEME - opts[:signature] ||= Stripe::Webhook::Signature.send(:compute_signature, "#{opts[:timestamp]}.#{opts[:payload]}", opts[:secret]) - "t=#{opts[:timestamp]},#{opts[:scheme]}=#{opts[:signature]}" + opts[:signature] ||= Stripe::Webhook::Signature.compute_signature( + opts[:timestamp], + opts[:payload], + opts[:secret] + ) + "t=#{opts[:timestamp].to_i},#{opts[:scheme]}=#{opts[:signature]}" + end + + context ".compute_signature" do + should "compute a signature which can then be verified" do + timestamp = Time.now + signature = Stripe::Webhook::Signature.compute_signature( + timestamp, + EVENT_PAYLOAD, + SECRET + ) + header = generate_header(timestamp: timestamp, signature: signature) + assert(Stripe::Webhook::Signature.verify_header(EVENT_PAYLOAD, header, SECRET)) + end end context ".construct_event" do @@ -70,7 +87,7 @@ def generate_header(opts = {}) end should "raise a SignatureVerificationError when the timestamp is not within the tolerance" do - header = generate_header(timestamp: Time.now.to_i - 15) + header = generate_header(timestamp: Time.now - 15) e = assert_raises(Stripe::SignatureVerificationError) do Stripe::Webhook::Signature.verify_header(EVENT_PAYLOAD, header, SECRET, tolerance: 10) end @@ -88,7 +105,7 @@ def generate_header(opts = {}) end should "return true when the header contains a valid signature and the timestamp is off but no tolerance is provided" do - header = generate_header(timestamp: 12_345) + header = generate_header(timestamp: Time.at(12_345)) assert(Stripe::Webhook::Signature.verify_header(EVENT_PAYLOAD, header, SECRET)) end end