FEATURE: Verify email webhook signatures (#19690)

* FEATURE: Verify Sendgrid webhook signature

* FEATURE: Verify more webhook signatures

* DEV: Add test for AWS webhook

* FEATURE: Implement algorithm for Mandrill

* FEATURE: Add warning if webhooks are unsafe

app/controllers/webhooks_controller.rb

@@ -6,12 +6,20 @@ class WebhooksController < ActionController::Base
   skip_before_action :verify_authenticity_token
 
   def mailgun
-    return mailgun_failure if SiteSetting.mailgun_api_key.blank?
+    return signature_failure if SiteSetting.mailgun_api_key.blank?
 
     params["event-data"] ? handle_mailgun_new(params) : handle_mailgun_legacy(params)
   end
 
   def sendgrid
+    if SiteSetting.sendgrid_verification_key.present?
+      return signature_failure if !valid_sendgrid_signature?
+    else
+      Rails.logger.warn(
+        "Received a Sendgrid webhook, but no verification key has been configured. This is unsafe behaviour and will be disallowed in the future.",
+      )
+    end
+
     events = params["_json"] || [params]
     events.each do |event|
       message_id = Email::MessageIdService.message_id_clean((event["smtp-id"] || ""))
@@ -32,6 +40,14 @@ class WebhooksController < ActionController::Base
   end
 
   def mailjet
+    if SiteSetting.mailjet_webhook_token.present?
+      return signature_failure if !valid_mailjet_token?
+    else
+      Rails.logger.warn(
+        "Received a Mailjet webhook, but no token has been configured. This is unsafe behaviour and will be disallowed in the future.",
+      )
+    end
+
     events = params["_json"] || [params]
     events.each do |event|
       message_id = event["CustomID"]
@@ -49,8 +65,17 @@ class WebhooksController < ActionController::Base
   end
 
   def mandrill
-    events = JSON.parse(params["mandrill_events"])
+    if SiteSetting.mandrill_authentication_key.present?
-    events.each do |event|
+      return signature_failure if !valid_mandrill_signature?
+    else
+      Rails.logger.warn(
+        "Received a Mandrill webhook, but no authentication key has been configured. This is unsafe behaviour and will be disallowed in the future.",
+      )
+    end
+
+    JSON
+      .parse(params["mandrill_events"])
+      .each do |event|
         message_id = event.dig("msg", "metadata", "message_id")
         to_address = event.dig("msg", "email")
         error_code = event.dig("msg", "diag")
@@ -73,6 +98,14 @@ class WebhooksController < ActionController::Base
   end
 
   def postmark
+    if SiteSetting.postmark_webhook_token.present?
+      return signature_failure if !valid_postmark_token?
+    else
+      Rails.logger.warn(
+        "Received a Postmark webhook, but no token has been configured. This is unsafe behaviour and will be disallowed in the future.",
+      )
+    end
+
     # see https://postmarkapp.com/developer/webhooks/bounce-webhook#bounce-webhook-data
     # and https://postmarkapp.com/developer/api/bounce-api#bounce-types
 
@@ -90,6 +123,14 @@ class WebhooksController < ActionController::Base
   end
 
   def sparkpost
+    if SiteSetting.sparkpost_webhook_token.present?
+      return signature_failure if !valid_sparkpost_token?
+    else
+      Rails.logger.warn(
+        "Received a Sparkpost webhook, but no token has been configured. This is unsafe behaviour and will be disallowed in the future.",
+      )
+    end
+
     events = params["_json"] || [params]
     events.each do |event|
       message_event = event.dig("msys", "message_event")
@@ -131,7 +172,7 @@ class WebhooksController < ActionController::Base
 
   private
 
-  def mailgun_failure
+  def signature_failure
     render body: nil, status: 406
   end
 
@@ -158,7 +199,7 @@ class WebhooksController < ActionController::Base
 
   def handle_mailgun_legacy(params)
     unless valid_mailgun_signature?(params["token"], params["timestamp"], params["signature"])
-      return mailgun_failure
+      return signature_failure
     end
 
     event = params["event"]
@@ -185,7 +226,7 @@ class WebhooksController < ActionController::Base
              signature["timestamp"],
              signature["signature"],
            )
-      return mailgun_failure
+      return signature_failure
     end
 
     data = params["event-data"]
@@ -205,6 +246,58 @@ class WebhooksController < ActionController::Base
     success
   end
 
+  def valid_sendgrid_signature?
+    signature = request.headers["X-Twilio-Email-Event-Webhook-Signature"]
+    timestamp = request.headers["X-Twilio-Email-Event-Webhook-Timestamp"]
+    request.body.rewind
+    payload = request.body.read
+
+    hashed_payload = Digest::SHA256.digest("#{timestamp}#{payload}")
+    decoded_signature = Base64.decode64(signature)
+
+    begin
+      public_key = OpenSSL::PKey::EC.new(Base64.decode64(SiteSetting.sendgrid_verification_key))
+    rescue StandardError => err
+      Rails.logger.error("Invalid Sendgrid verification key")
+      return false
+    end
+
+    public_key.dsa_verify_asn1(hashed_payload, decoded_signature)
+  end
+
+  def valid_mailjet_token?
+    ActiveSupport::SecurityUtils.secure_compare(params[:t], SiteSetting.mailjet_webhook_token)
+  end
+
+  def valid_mandrill_signature?
+    signature = request.headers["X-Mandrill-Signature"]
+
+    payload = "#{Discourse.base_url}/webhooks/mandrill"
+    params
+      .permit(:mandrill_events)
+      .to_h
+      .sort_by(&:first)
+      .each do |key, value|
+        payload += key.to_s
+        payload += value
+      end
+
+    payload_signature =
+      OpenSSL::HMAC.digest("sha1", SiteSetting.mandrill_authentication_key, payload)
+    ActiveSupport::SecurityUtils.secure_compare(
+      signature,
+      Base64.strict_encode64(payload_signature),
+    )
+  end
+
+  def valid_postmark_token?
+    ActiveSupport::SecurityUtils.secure_compare(params[:t], SiteSetting.postmark_webhook_token)
+  end
+
+  def valid_sparkpost_token?
+    ActiveSupport::SecurityUtils.secure_compare(params[:t], SiteSetting.sparkpost_webhook_token)
+  end
+
   def process_bounce(message_id, to_address, bounce_score, bounce_error_code = nil)
     return if message_id.blank? || to_address.blank?
 

config/locales/server.en.yml

@@ -2040,6 +2040,11 @@ en:
     block_auto_generated_emails: "Block incoming emails identified as being auto generated."
     ignore_by_title: "Ignore incoming emails based on their title."
     mailgun_api_key: "Mailgun Secret API key used to verify webhook messages."
+    sendgrid_verification_key: "Sendgrid verification key used to verify webhook messages."
+    mailjet_webhook_token: "Token used to verify webhook payload. It must be passed as the 't' query parameter of the webhook, for example: https://example.com/webhook/mailjet?t=supersecret"
+    mandrill_authentication_key: "Mandrill authentication key used to verify webhook messages."
+    postmark_webhook_token: "Token used to verify webhook payload. It must be passed as the 't' query parameter of the webhook, for example: https://example.com/webhook/postmark?t=supersecret"
+    sparkpost_webhook_token: "Token used to verify webhook payload. It must be passed as the 't' query parameter of the webhook, for example: https://example.com/webhook/sparkpost?t=supersecret"
 
     soft_bounce_score: "Bounce score added to the user when a temporary bounce happens."
     hard_bounce_score: "Bounce score added to the user when a permanent bounce happens."

config/site_settings.yml

@@ -1246,6 +1246,21 @@ email:
     default: ""
     regex: '^((key-)?\h{32}|\h{32}-\h{8}-\h{8})$'
     secret: true
+  sendgrid_verification_key:
+    default: ""
+    secret: true
+  mailjet_webhook_token:
+    default: ""
+    secret: true
+  mandrill_authentication_key:
+    default: ""
+    secret: true
+  postmark_webhook_token:
+    default: ""
+    secret: true
+  sparkpost_webhook_token:
+    default: ""
+    secret: true
   bounce_score_threshold:
     client: true
     default: 4

spec/requests/webhooks_controller_spec.rb

@@ -105,6 +105,53 @@ RSpec.describe WebhooksController do
       expect(email_log.bounce_error_code).to eq("5.0.0")
       expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
+
+    it "verifies signatures" do
+      SiteSetting.sendgrid_verification_key =
+        "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE83T4O/n84iotIvIW4mdBgQ/7dAfSmpqIM8kF9mN1flpVKS3GRqe62gw+2fNNRaINXvVpiglSI8eNEc6wEA3F+g=="
+
+      post "/webhooks/sendgrid.json",
+           headers: {
+             "X-Twilio-Email-Event-Webhook-Signature" =>
+               "MEUCIGHQVtGj+Y3LkG9fLcxf3qfI10QysgDWmMOVmxG0u6ZUAiEAyBiXDWzM+uOe5W0JuG+luQAbPIqHh89M15TluLtEZtM=",
+             "X-Twilio-Email-Event-Webhook-Timestamp" => "1600112502",
+           },
+           params:
+             "[{\"email\":\"hello@world.com\",\"event\":\"dropped\",\"reason\":\"Bounced Address\",\"sg_event_id\":\"ZHJvcC0xMDk5NDkxOS1MUnpYbF9OSFN0T0doUTRrb2ZTbV9BLTA\",\"sg_message_id\":\"LRzXl_NHStOGhQ4kofSm_A.filterdrecv-p3mdw1-756b745b58-kmzbl-18-5F5FC76C-9.0\",\"smtp-id\":\"<LRzXl_NHStOGhQ4kofSm_A@ismtpd0039p1iad1.sendgrid.net>\",\"timestamp\":1600112492}]\r\n"
+
+      expect(response.status).to eq(200)
+    end
+
+    it "returns error if signature verification fails" do
+      SiteSetting.sendgrid_verification_key =
+        "MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE83T4O/n84iotIvIW4mdBgQ/7dAfSmpqIM8kF9mN1flpVKS3GRqe62gw+2fNNRaINXvVpiglSI8eNEc6wEA3F+g=="
+
+      post "/webhooks/sendgrid.json",
+           headers: {
+             "X-Twilio-Email-Event-Webhook-Signature" =>
+               "MEUCIQCtIHJeH93Y+qpYeWrySphQgpNGNr/U+UyUlBkU6n7RAwIgJTz2C+8a8xonZGi6BpSzoQsbVRamr2nlxFDWYNH3j/0=",
+             "X-Twilio-Email-Event-Webhook-Timestamp" => "1600112502",
+           },
+           params:
+             "[{\"email\":\"hello@world.com\",\"event\":\"dropped\",\"reason\":\"Bounced Address\",\"sg_event_id\":\"ZHJvcC0xMDk5NDkxOS1MUnpYbF9OSFN0T0doUTRrb2ZTbV9BLTA\",\"sg_message_id\":\"LRzXl_NHStOGhQ4kofSm_A.filterdrecv-p3mdw1-756b745b58-kmzbl-18-5F5FC76C-9.0\",\"smtp-id\":\"<LRzXl_NHStOGhQ4kofSm_A@ismtpd0039p1iad1.sendgrid.net>\",\"timestamp\":1600112492}]\r\n"
+
+      expect(response.status).to eq(406)
+    end
+
+    it "returns error if signature is invalid" do
+      SiteSetting.sendgrid_verification_key = "foo"
+
+      post "/webhooks/sendgrid.json",
+           headers: {
+             "X-Twilio-Email-Event-Webhook-Signature" =>
+               "MEUCIQCtIHJeH93Y+qpYeWrySphQgpNGNr/U+UyUlBkU6n7RAwIgJTz2C+8a8xonZGi6BpSzoQsbVRamr2nlxFDWYNH3j/0=",
+             "X-Twilio-Email-Event-Webhook-Timestamp" => "1600112502",
+           },
+           params:
+             "[{\"email\":\"hello@world.com\",\"event\":\"dropped\",\"reason\":\"Bounced Address\",\"sg_event_id\":\"ZHJvcC0xMDk5NDkxOS1MUnpYbF9OSFN0T0doUTRrb2ZTbV9BLTA\",\"sg_message_id\":\"LRzXl_NHStOGhQ4kofSm_A.filterdrecv-p3mdw1-756b745b58-kmzbl-18-5F5FC76C-9.0\",\"smtp-id\":\"<LRzXl_NHStOGhQ4kofSm_A@ismtpd0039p1iad1.sendgrid.net>\",\"timestamp\":1600112492}]\r\n"
+
+      expect(response.status).to eq(406)
+    end
   end
 
   describe "#mailjet" do
@@ -127,9 +174,47 @@ RSpec.describe WebhooksController do
       expect(email_log.bounce_error_code).to eq(nil) # mailjet doesn't give us this
       expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
+
+    it "verifies signatures" do
+      SiteSetting.mailjet_webhook_token = "foo"
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      post "/webhooks/mailjet.json?t=foo",
+           params: {
+             "event" => "bounce",
+             "email" => email,
+             "hard_bounce" => true,
+             "CustomID" => message_id,
+           }
+
+      expect(response.status).to eq(200)
+      expect(email_log.reload.bounced).to eq(true)
+    end
+
+    it "returns error if signature verification fails" do
+      SiteSetting.mailjet_webhook_token = "foo"
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      post "/webhooks/mailjet.json?t=bar",
+           params: {
+             "event" => "bounce",
+             "email" => email,
+             "hard_bounce" => true,
+             "CustomID" => message_id,
+           }
+
+      expect(response.status).to eq(406)
+      expect(email_log.reload.bounced).to eq(false)
+    end
   end
 
   describe "#mandrill" do
+    let(:payload) do
+      "mandrill_events=%5B%7B%22event%22%3A%22hard_bounce%22%2C%22msg%22%3A%7B%22email%22%3A%22em%40il.com%22%2C%22diag%22%3A%225.1.1%22%2C%22bounce_description%22%3A%22smtp%3B+550-5.1.1+The+email+account+that+you+tried+to+reach+does+not+exist.%22%2C%22metadata%22%3A%7B%22message_id%22%3A%2212345%40il.com%22%7D%7D%7D%5D"
+    end
+
     it "works" do
       user = Fabricate(:user, email: email)
       email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
@@ -159,6 +244,38 @@ RSpec.describe WebhooksController do
       expect(email_log.bounce_error_code).to eq("5.1.1")
       expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
+
+    it "verifies signatures" do
+      SiteSetting.mandrill_authentication_key = "wr_JeJNO9OI65RFDrvk3Zw"
+
+      post "/webhooks/mandrill.json",
+           headers: {
+             "X-Mandrill-Signature" => "Q5pCb903EjEqRZ99gZrlYKOfvIU=",
+           },
+           params: payload
+
+      expect(response.status).to eq(200)
+    end
+
+    it "returns error if signature verification fails" do
+      SiteSetting.mandrill_authentication_key = "wr_JeJNO9OI65RFDrvk3Zw"
+
+      post "/webhooks/mandrill.json", headers: { "X-Mandrill-Signature" => "foo" }, params: payload
+
+      expect(response.status).to eq(406)
+    end
+
+    it "returns error if signature is invalid" do
+      SiteSetting.mandrill_authentication_key = "foo"
+
+      post "/webhooks/mandrill.json",
+           headers: {
+             "X-Mandrill-Signature" => "Q5pCb903EjEqRZ99gZrlYKOfvIU=",
+           },
+           params: payload
+
+      expect(response.status).to eq(406)
+    end
   end
 
   describe "#mandrill_head" do
@@ -187,6 +304,7 @@ RSpec.describe WebhooksController do
       expect(email_log.bounce_error_code).to eq(nil) # postmark doesn't give us this
       expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
+
     it "soft bounces" do
       user = Fabricate(:user, email: email)
       email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
@@ -204,6 +322,38 @@ RSpec.describe WebhooksController do
       expect(email_log.bounce_error_code).to eq(nil) # postmark doesn't give us this
       expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.soft_bounce_score)
     end
+
+    it "verifies signatures" do
+      SiteSetting.postmark_webhook_token = "foo"
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      post "/webhooks/postmark.json?t=foo",
+           params: {
+             "Type" => "HardBounce",
+             "MessageID" => message_id,
+             "Email" => email,
+           }
+
+      expect(response.status).to eq(200)
+      expect(email_log.reload.bounced).to eq(true)
+    end
+
+    it "returns error if signature verification fails" do
+      SiteSetting.postmark_webhook_token = "foo"
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      post "/webhooks/postmark.json?t=bar",
+           params: {
+             "Type" => "HardBounce",
+             "MessageID" => message_id,
+             "Email" => email,
+           }
+
+      expect(response.status).to eq(406)
+      expect(email_log.reload.bounced).to eq(false)
+    end
   end
 
   describe "#sparkpost" do
@@ -235,5 +385,136 @@ RSpec.describe WebhooksController do
       expect(email_log.bounced).to eq(true)
       expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
+
+    it "verifies signatures" do
+      SiteSetting.sparkpost_webhook_token = "foo"
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      post "/webhooks/sparkpost.json?t=foo",
+           params: {
+             "_json" => [
+               {
+                 "msys" => {
+                   "message_event" => {
+                     "bounce_class" => 10,
+                     "error_code" => "554",
+                     "rcpt_to" => email,
+                     "rcpt_meta" => {
+                       "message_id" => message_id,
+                     },
+                   },
+                 },
+               },
+             ],
+           }
+
+      expect(response.status).to eq(200)
+      expect(email_log.reload.bounced).to eq(true)
+    end
+
+    it "returns error if signature verification fails" do
+      SiteSetting.sparkpost_webhook_token = "foo"
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      post "/webhooks/sparkpost.json?t=bar",
+           params: {
+             "_json" => [
+               {
+                 "msys" => {
+                   "message_event" => {
+                     "bounce_class" => 10,
+                     "error_code" => "554",
+                     "rcpt_to" => email,
+                     "rcpt_meta" => {
+                       "message_id" => message_id,
+                     },
+                   },
+                 },
+               },
+             ],
+           }
+
+      expect(response.status).to eq(406)
+      expect(email_log.reload.bounced).to eq(false)
+    end
+  end
+
+  describe "#aws" do
+    let(:payload) do
+      {
+        "Type" => "Notification",
+        "Message" => {
+          "notificationType" => "Bounce",
+          :"bounce" => {
+            "bounceType" => "Permanent",
+            "reportingMTA" => "dns; email.example.com",
+            :"bouncedRecipients" => [
+              {
+                "emailAddress" => email,
+                "status" => "5.1.1",
+                "action" => "failed",
+                "diagnosticCode" => "smtp; 550 5.1.1 <#{email}>... User",
+              },
+            ],
+            "bounceSubType" => "General",
+            "timestamp" => "2016-01-27T14:59:38.237Z",
+            "feedbackId" => "00000138111222aa-33322211-cccc-cccc-cccc-ddddaaaa068a-000000",
+            "remoteMtaIp" => "127.0.2.0",
+          },
+          :"mail" => {
+            "timestamp" => "2016-01-27T14:59:38.237Z",
+            "source" => "john@example.com",
+            "sourceArn" => "arn:aws:ses:us-east-1:888888888888:identity/example.com",
+            "sourceIp" => "127.0.3.0",
+            "sendingAccountId" => "123456789012",
+            "callerIdentity" => "IAM_user_or_role_name",
+            "messageId" => message_id,
+            "destination" => [email, "jane@example.com", "mary@example.com", "richard@example.com"],
+            "headersTruncated" => false,
+            "headers" => [
+              { "name" => "From", "value" => "\"John Doe\" <john@example.com>" },
+              {
+                "name" => "To",
+                "value" =>
+                  "\"Test\" <#{email}>, \"Jane Doe\" <jane@example.com>, \"Mary Doe\" <mary@example.com>, \"Richard Doe\" <richard@example.com>",
+              },
+              { "name" => "Message-ID", "value" => message_id },
+              { "name" => "Subject", "value" => "Hello" },
+              { "name" => "Content-Type", "value" => "text/plain; charset=\"UTF-8\"" },
+              { "name" => "Content-Transfer-Encoding", "value" => "base64" },
+              { "name" => "Date", "value" => "Wed, 27 Jan 2016 14:05:45 +0000" },
+            ],
+            "commonHeaders" => {
+              "from" => ["John Doe <john@example.com>"],
+              "date" => "Wed, 27 Jan 2016 14:05:45 +0000",
+              "to" => [
+                "\"Test\" <#{email}>, Jane Doe <jane@example.com>, Mary Doe <mary@example.com>, Richard Doe <richard@example.com>",
+              ],
+              "messageId" => message_id,
+              "subject" => "Hello",
+            },
+          },
+        }.to_json,
+      }.to_json
+    end
+
+    before { Jobs.run_immediately! }
+
+    it "works" do
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      require "aws-sdk-sns"
+      Aws::SNS::MessageVerifier.any_instance.stubs(:authentic?).with(payload).returns(true)
+
+      post "/webhooks/aws.json", headers: { "RAW_POST_DATA" => payload }
+      expect(response.status).to eq(200)
+
+      email_log.reload
+      expect(email_log.bounced).to eq(true)
+      expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
+    end
   end
 end