Mailgunインバウンドメール署名の不正パラメータを適切に拒否する修正

Mailgunイングレスにおいて、signatureパラメータが文字列以外の型（配列など）で送信された場合、401 Unauthorizedを返す代わりにNoMethodErrorが発生していたバグを修正しました。1行の型チェック追加により、不正なパラメータ形状を認証失敗として正しく扱えるようになります。

背景

Mailgunイングレスのsigned?メソッドは、リクエストパラメータから取得したsignature値をActiveSupport::SecurityUtils.secure_compareに直接渡していました。この実装はsignatureが常に文字列であることを前提としており、配列などの非文字列型が渡された場合の考慮がありませんでした。

HTTPリクエストのパラメータ解析では、同名パラメータが複数送信された場合や特定のエンコード形式によって、値が文字列ではなく配列として解釈されることがあります。Issue #57484で報告されたように、signatureが配列として渡されるとsecure_compare内部で.bytesizeを呼び出す際に以下のエラーが発生していました：

NoMethodError: undefined method `bytesize' for an instance of Array
    activesupport/lib/active_support/security_utils.rb:34:in `secure_compare'
    app/controllers/action_mailbox/ingresses/mailgun/inbound_emails_controller.rb:98:in `signed?'

このエラーはコントローラーが401 Unauthorizedを返す前に発生するため、不正なリクエストが認証失敗として処理されるべきところを、アプリケーションエラーとして露出させてしまっていました。

技術的な変更

修正はactionmailbox/app/controllers/action_mailbox/ingresses/mailgun/inbound_emails_controller.rbのsigned?メソッドに対して、1行の変更のみで行われています。

変更前:

def signed?
  ActiveSupport::SecurityUtils.secure_compare signature, expected_signature
end

変更後:

def signed?
  signature.is_a?(String) && ActiveSupport::SecurityUtils.secure_compare(signature, expected_signature)
end

signature.is_a?(String)によるガード節を&&の短絡評価で先行させることで、文字列以外の型が渡された場合はsecure_compareの呼び出しをスキップし、falseを返します。signed?がfalseを返すとauthenticated?もfalseとなり、コントローラーは401 Unauthorizedを返します。

リグレッションテストとして、配列型のsignatureパラメータを含むリクエストが401 Unauthorizedを返し、ActionMailbox::InboundEmailが作成されないことを検証するテストケースも追加されています：

test "rejecting an inbound email from Mailgun with a malformed signature" do
  assert_no_difference -> { ActionMailbox::InboundEmail.count } do
    travel_to "2018-10-09 15:15:00 EDT"
    post rails_mailgun_inbound_emails_url, params: {
      timestamp: 1539112500,
      token: "7VwW7k6Ak7zcTwoSoNm7aTtbk1g67MKAnsYLfUB7PdszbgR5Xi",
      signature: [ "ef24c5225322217bb065b80bb54eb4f9206d764e3e16abab07f0a64d1cf477cc" ],
      "body-mime" => file_fixture("../files/welcome.eml").read
    }
  end

  assert_response :unauthorized
end

設計判断

secure_compareの呼び出し前に型チェックを置く設計が採用されました。

secure_compareは定数時間比較（タイミング攻撃対策）を目的とした関数であり、その内部で型チェックを行うのは関数の責務を超えます。呼び出し側で事前に型を保証することで、セキュリティユーティリティの責務境界を明確に保っています。また、is_a?(String)はStringのサブクラスも許容するため、必要以上に制限的ではありません。

ショートサーキット評価（&&）を使うことで、型チェックが失敗した場合はsecure_compareの実行コストも発生しない点も合理的です。セキュリティの文脈では「不正な入力は早期に拒否する」原則に沿った実装といえます。

まとめ

本PRは、外部からの不正なパラメータ形状がアプリケーション内部のエラーとして漏出するという防御的設計の欠陥を、最小限の変更で修正しています。型チェックによるガード節の追加という単純な手法で、セキュリティ的にクリーンな失敗（401 Unauthorized）を保証しています。