diff --git a/app/controllers/webhooks_controller.rb b/app/controllers/webhooks_controller.rb index b84d59c269e..6044ef83bb8 100644 --- a/app/controllers/webhooks_controller.rb +++ b/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,20 +65,29 @@ class WebhooksController < ActionController::Base end def mandrill - events = JSON.parse(params["mandrill_events"]) - events.each do |event| - message_id = event.dig("msg", "metadata", "message_id") - to_address = event.dig("msg", "email") - error_code = event.dig("msg", "diag") - - case event["event"] - when "hard_bounce" - process_bounce(message_id, to_address, SiteSetting.hard_bounce_score, error_code) - when "soft_bounce" - process_bounce(message_id, to_address, SiteSetting.soft_bounce_score, error_code) - end + if SiteSetting.mandrill_authentication_key.present? + 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") + + case event["event"] + when "hard_bounce" + process_bounce(message_id, to_address, SiteSetting.hard_bounce_score, error_code) + when "soft_bounce" + process_bounce(message_id, to_address, SiteSetting.soft_bounce_score, error_code) + end + end + success end @@ -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? diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8a3eb8cbb2c..2b656f5ec02 100644 --- a/config/locales/server.en.yml +++ b/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." diff --git a/config/site_settings.yml b/config/site_settings.yml index e842264f559..5e917c9d9b7 100644 --- a/config/site_settings.yml +++ b/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 diff --git a/spec/requests/webhooks_controller_spec.rb b/spec/requests/webhooks_controller_spec.rb index 8d6050096c6..452774d7737 100644 --- a/spec/requests/webhooks_controller_spec.rb +++ b/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\":\"\",\"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\":\"\",\"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\":\"\",\"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\" " }, + { + "name" => "To", + "value" => + "\"Test\" <#{email}>, \"Jane Doe\" , \"Mary Doe\" , \"Richard Doe\" ", + }, + { "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 "], + "date" => "Wed, 27 Jan 2016 14:05:45 +0000", + "to" => [ + "\"Test\" <#{email}>, Jane Doe , Mary Doe , Richard Doe ", + ], + "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