diff --git a/lib/hooks/plugins/auth/hmac.rb b/lib/hooks/plugins/auth/hmac.rb index 8394752..ad3cbc0 100644 --- a/lib/hooks/plugins/auth/hmac.rb +++ b/lib/hooks/plugins/auth/hmac.rb @@ -202,80 +202,84 @@ def self.valid_timestamp?(headers, config) return false if timestamp_value.strip.empty? parsed_timestamp = parse_timestamp(timestamp_value.strip) - return false unless parsed_timestamp + return false unless parsed_timestamp.is_a?(Integer) - # parsed_timestamp is a Time object - now = Time.now.utc + now = Time.now.utc.to_i (now - parsed_timestamp).abs <= tolerance end # Parse timestamp value supporting both ISO 8601 UTC and Unix formats # - # Attempts to parse the timestamp in the following order: - # 1. ISO 8601 UTC format (e.g., "2025-06-12T10:30:00Z") - # 2. Unix timestamp (e.g., "1609459200") - # # @param timestamp_value [String] The timestamp string to parse - # @return [Time, nil] Time object if parsing succeeds, nil otherwise + # @return [Integer, nil] Epoch seconds if parsing succeeds, nil otherwise # @note Security: Strict validation prevents various injection attacks # @api private def self.parse_timestamp(timestamp_value) - # Try ISO 8601 first, then Unix + # Reject if contains any control characters, whitespace, or null bytes + if timestamp_value =~ /[\u0000-\u001F\u007F-\u009F]/ + log.warn("Auth::HMAC validation failed: Timestamp contains invalid characters") + return nil + end ts = parse_iso8601_timestamp(timestamp_value) return ts if ts ts = parse_unix_timestamp(timestamp_value) return ts if ts - nil + + # If neither format matches, return nil + log.warn("Auth::HMAC validation failed: Timestamp (#{timestamp_value}) is not valid ISO 8601 UTC or Unix format") + return nil end - # Check if timestamp string looks like ISO 8601 UTC format + # Check if timestamp string looks like ISO 8601 UTC format (must have UTC indicator) # # @param timestamp_value [String] The timestamp string to check - # @return [Boolean] true if it appears to be ISO 8601 format + # @return [Boolean] true if it appears to be ISO 8601 format (with or without UTC indicator) # @api private def self.iso8601_timestamp?(timestamp_value) - # Accepts Z, +00:00, or +0000, and T or space as separator - !!(timestamp_value =~ /\A\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}(?:\.\d+)?(Z|\+00:00|\+0000)\z/) + !!(timestamp_value =~ /\A\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}(?:\.\d+)?(Z|\+00:00|\+0000)?\z/) end - # Parse ISO 8601 UTC timestamp string + # Parse ISO 8601 UTC timestamp string (must have UTC indicator) # # @param timestamp_value [String] ISO 8601 timestamp string - # @return [Time, nil] Time object if parsing succeeds, nil otherwise - # @note Only accepts UTC timestamps (ending with 'Z' or explicit UTC) + # @return [Integer, nil] Epoch seconds if parsing succeeds, nil otherwise + # @note Only accepts UTC timestamps (ending with 'Z', '+00:00', '+0000') # @api private def self.parse_iso8601_timestamp(timestamp_value) - # Normalize 'YYYY-MM-DD HH:MM:SS(.fraction)? +0000' to 'YYYY-MM-DDTHH:MM:SS(.fraction)?+00:00' if timestamp_value =~ /\A(\d{4}-\d{2}-\d{2}) (\d{2}:\d{2}:\d{2}(?:\.\d+)?)(?: )\+0000\z/ timestamp_value = "#{$1}T#{$2}+00:00" end + # Ensure the timestamp explicitly includes a UTC indicator + return nil unless timestamp_value =~ /(Z|\+00:00|\+0000)\z/ return nil unless iso8601_timestamp?(timestamp_value) - t = Time.iso8601(timestamp_value) rescue nil + t = Time.parse(timestamp_value) rescue nil return nil unless t - # Only accept UTC (Z, +00:00, or +0000) - return t if t.utc? || t.utc_offset == 0 - nil + # The check for UTC indicator in regex makes this t.utc? or t.utc_offset == 0 redundant + # but kept for safety, though it should always be true now if Time.parse succeeds. + (t.utc? || t.utc_offset == 0) ? t.to_i : nil end - # Parse Unix timestamp string + # Parse Unix timestamp string (must be positive integer, no leading zeros except for "0") # # @param timestamp_value [String] Unix timestamp string - # @return [Time, nil] Time object if parsing succeeds, nil otherwise + # @return [Integer, nil] Epoch seconds if parsing succeeds, nil otherwise + # @note Only accepts positive integer values, no leading zeros except for "0" # @api private def self.parse_unix_timestamp(timestamp_value) return nil unless unix_timestamp?(timestamp_value) ts = timestamp_value.to_i return nil if ts <= 0 - Time.at(ts).utc + ts end - # Check if timestamp string looks like Unix timestamp format + # Check if timestamp string looks like Unix timestamp format (no leading zeros except "0") # # @param timestamp_value [String] The timestamp string to check # @return [Boolean] true if it appears to be Unix timestamp format # @api private def self.unix_timestamp?(timestamp_value) - !!(timestamp_value =~ /\A\d+\z/) || timestamp_value == "0" + return true if timestamp_value == "0" + !!(timestamp_value =~ /\A[1-9]\d*\z/) end # Compute HMAC signature based on configuration requirements @@ -325,7 +329,7 @@ def self.compute_signature(payload:, headers:, secret:, config:) # - {body}: Replaced with the raw payload # @example Template usage # template: "{version}:{timestamp}:{body}" - # result: "v0:1609459200:{"event":"push"}" + # result: "v0:1609459200:{\"event\":\"push\"}" # @api private def self.build_signing_payload(payload:, headers:, config:) template = config[:payload_template] @@ -355,7 +359,7 @@ def self.build_signing_payload(payload:, headers:, config:) # - :algorithm_prefixed: "sha256=abc123..." (GitHub style) # - :hash_only: "abc123..." (Shopify style) # - :version_prefixed: "v0=abc123..." (Slack style) - # @note Defaults to algorithm_prefixed format for unknown format styles + # @note Defaults to algorithm-prefixed format for unknown format styles # @api private def self.format_signature(hash, config) format_style = FORMATS[config[:format]] diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 1ba48b5..34116d6 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -116,6 +116,162 @@ end end + describe "hmac_with_timestamp" do + it "successfully processes a valid POST request with HMAC signature and timestamp" do + payload = { text: "Hello, World!" } + timestamp = Time.now.utc.iso8601 + body = payload.to_json + signing_payload = "#{timestamp}:#{body}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}", + "X-HMAC-Timestamp" => timestamp + } + response = http.post("/webhooks/hmac_with_timestamp", body, headers) + expect(response).to be_a(Net::HTTPSuccess) + body = JSON.parse(response.body) + expect(body["status"]).to eq("success") + end + + it "successfully processes a valid POST request with HMAC signature and timestamp and an empty payload" do + payload = {} + timestamp = Time.now.utc.iso8601 + body = payload.to_json + signing_payload = "#{timestamp}:#{body}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}", + "X-HMAC-Timestamp" => timestamp + } + response = http.post("/webhooks/hmac_with_timestamp", body, headers) + expect(response).to be_a(Net::HTTPSuccess) + body = JSON.parse(response.body) + expect(body["status"]).to eq("success") + end + + it "successfully processes a valid POST request with HMAC signature and the POST has no body" do + timestamp = Time.now.utc.iso8601 + signing_payload = "#{timestamp}:" # Empty body + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}", + "X-HMAC-Timestamp" => timestamp + } + response = http.post("/webhooks/hmac_with_timestamp", nil, headers) + expect(response).to be_a(Net::HTTPSuccess) + body = JSON.parse(response.body) + expect(body["status"]).to eq("success") + end + + it "fails due to using the wrong HMAC secret" do + payload = { text: "Hello, World!" } + timestamp = Time.now.utc.iso8601 + body = payload.to_json + signing_payload = "#{timestamp}:#{body}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), "bad-hmac-secret", signing_payload) + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}", + "X-HMAC-Timestamp" => timestamp + } + response = http.post("/webhooks/hmac_with_timestamp", body, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("authentication failed") + end + + it "fails due to missing timestamp header" do + payload = { text: "Hello, World!" } + body = payload.to_json + signing_payload = "#{Time.now.utc.iso8601}:#{body}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}" + # Missing X-HMAC-Timestamp header + } + response = http.post("/webhooks/hmac_with_timestamp", body, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("authentication failed") + end + + it "fails due to invalid timestamp format" do + payload = { text: "Hello, World!" } + invalid_timestamp = "not-a-timestamp" + body = payload.to_json + signing_payload = "#{invalid_timestamp}:#{body}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}", + "X-HMAC-Timestamp" => invalid_timestamp + } + response = http.post("/webhooks/hmac_with_timestamp", body, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("authentication failed") + end + + it "rejects request with timestamp manipulation attack" do + payload = { text: "Hello, World!" } + original_timestamp = Time.now.utc.iso8601 + manipulated_timestamp = (Time.now.utc + 100).iso8601 # Future timestamp + + # Create signature with original timestamp + signing_payload = "#{original_timestamp}:#{payload.to_json}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + + # But send manipulated timestamp in header + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}", + "X-HMAC-Timestamp" => manipulated_timestamp + } + + response = http.post("/webhooks/hmac_with_timestamp", payload.to_json, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("authentication failed") + end + + it "fails because the timestamp is too old" do + payload = { text: "Hello, World!" } + # Use timestamp that's 10 minutes old (beyond the tolerance) + expired_timestamp = (Time.now.utc - 600).iso8601 + + signing_payload = "#{expired_timestamp}:#{payload.to_json}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha256=#{digest}", + "X-HMAC-Timestamp" => expired_timestamp + } + + response = http.post("/webhooks/hmac_with_timestamp", payload.to_json, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("authentication failed") + end + + it "fails because the wrong HMAC algorithm is used" do + payload = { text: "Hello, World!" } + timestamp = Time.now.utc.iso8601 + body = payload.to_json + signing_payload = "#{timestamp}:#{body}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha512"), FAKE_ALT_HMAC_SECRET, signing_payload) + + headers = { + "Content-Type" => "application/json", + "X-HMAC-Signature" => "sha512=#{digest}", + "X-HMAC-Timestamp" => timestamp + } + + response = http.post("/webhooks/hmac_with_timestamp", body, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("authentication failed") + end + end + describe "slack" do it "successfully processes a valid POST request with HMAC signature and timestamp" do payload = { text: "Hello, Slack!" } @@ -207,6 +363,23 @@ expect(body["status"]).to eq("success") end + it "successfully processes request with ISO 8601 UTC timestamp (ruby default method)" do + payload = { text: "Hello, Slack!" } + iso_timestamp = Time.now.utc.iso8601 + body = payload.to_json + signing_payload = "v0:#{iso_timestamp}:#{body}" + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, signing_payload) + headers = { + "Content-Type" => "application/json", + "Signature-256" => "v0=#{digest}", + "X-Timestamp" => iso_timestamp + } + response = http.post("/webhooks/slack", body, headers) + expect(response).to be_a(Net::HTTPSuccess) + body = JSON.parse(response.body) + expect(body["status"]).to eq("success") + end + it "successfully processes request with ISO 8601 UTC timestamp using +00:00 format" do payload = { text: "Hello, Slack!" } iso_timestamp = Time.now.utc.strftime("%Y-%m-%dT%H:%M:%S+00:00") diff --git a/spec/acceptance/config/endpoints/hmac_with_timestamp.yml b/spec/acceptance/config/endpoints/hmac_with_timestamp.yml new file mode 100644 index 0000000..8727466 --- /dev/null +++ b/spec/acceptance/config/endpoints/hmac_with_timestamp.yml @@ -0,0 +1,12 @@ +path: /hmac_with_timestamp +handler: Hello + +auth: + type: hmac + secret_env_key: ALT_WEBHOOK_SECRET + header: X-HMAC-Signature + timestamp_header: X-HMAC-Timestamp + timestamp_tolerance: 60 # 1 minute + algorithm: sha256 + format: "algorithm=signature" # produces "sha256=abc123..." + payload_template: "{timestamp}:{body}"