Skip to content

Commit

Permalink
Sanitize URLs in API key signup e-mails.
Browse files Browse the repository at this point in the history
Ensure that the contact URL/e-mails and example API URL all point to a
known hostname (some host that is configured by an admin within the
system) to prevent linking to external/unknown/malicious URLs.
  • Loading branch information
GUI committed Nov 1, 2018
1 parent 0d50540 commit 89f7ec2
Show file tree
Hide file tree
Showing 7 changed files with 388 additions and 39 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ Layout/IndentArray:
Layout/IndentHash:
EnforcedStyle: consistent

Layout/MultilineOperationIndentation:
EnforcedStyle: indented

Layout/SpaceAroundKeyword:
Enabled: false

Expand Down
6 changes: 6 additions & 0 deletions src/api-umbrella/web-app/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ gem "font-awesome-rails", "~> 4.7.0"
# Table-less models
gem "active_attr", "~> 0.12.0"

# URI parsing
gem "addressable", "~> 2.5.2"

# Public suffix for hostnames.
gem "public_suffix", "~> 3.0.3"

# Bundle gems for the local environment. Make sure to
# put test-only gems in this group so their generators
# and rake tasks are available in development mode:
Expand Down
2 changes: 2 additions & 0 deletions src/api-umbrella/web-app/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ PLATFORMS

DEPENDENCIES
active_attr (~> 0.12.0)
addressable (~> 2.5.2)
awesome_print (~> 1.8.0)
bootstrap-sass (~> 3.3.7)
brakeman
Expand Down Expand Up @@ -341,6 +342,7 @@ DEPENDENCIES
omniauth-google-oauth2 (~> 0.5.3)
omniauth-ldap (~> 2.0.0)
premailer-rails (~> 1.10.2)
public_suffix (~> 3.0.3)
puma (~> 3.12.0)
pundit (~> 2.0.0)
rabl (~> 0.13.1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,22 @@ def create
end

if(send_welcome_email)
ApiUserMailer.signup_email(@api_user.id, params[:options] || {}).deliver_later
# The contact URLs or example API URLs to embed within the e-mail
# message are accepted via the form params. To prevent sending
# e-mails that may link to third-party or malicious sites, sanitize
# these URLs to ensure they point to a known domain that is
# configured within API Umbrella.
#
# TODO: It may be better to have admins more explicitly manage these
# settings within the admin tool, but to retain compatibility with
# the current approach, we'll use this domain-based sanitization.
known_hosts = KnownHosts.new
options = params[:options]&.deep_dup || {}
options[:example_api_url] = known_hosts.sanitized_api_url(options[:example_api_url])
options[:contact_url] = known_hosts.sanitized_url(options[:contact_url])
options[:email_from_address] = known_hosts.sanitized_email(options[:email_from_address])

ApiUserMailer.signup_email(@api_user.id, options).deliver_later
end
if(send_notify_email)
ApiUserMailer.notify_api_admin(@api_user.id).deliver_later
Expand Down
93 changes: 93 additions & 0 deletions src/api-umbrella/web-app/lib/known_hosts.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
class KnownHosts
def initialize
file_config = ApiUmbrellaConfig.deep_stringify_keys

api_backends = (file_config["internal_apis"] || []) +
(file_config["apis"] || []) +
(ConfigVersion.active_config["apis"] || [])
website_backends = (file_config["internal_website_backends"] || []) +
(file_config["website_backends"] || []) +
(ConfigVersion.active_config["website_backends"] || [])

# Compile a list of all the known API frontend hosts based on the file or
# database configuration.
@known_api_hosts = Set.new
@known_api_hosts << ApiUmbrellaConfig[:web][:default_host]
@known_api_hosts << ApiUmbrellaConfig[:router][:web_app_host]
if file_config["hosts"]
file_config["hosts"].each do |host|
@known_api_hosts << host["hostname"]
end
end
api_backends.each do |api|
@known_api_hosts << api["frontend_host"]
end

# Compile a list of all the known "root" hosts for all API and website
# backend hosts. These root hosts account for public suffixes, so the root
# domain for "api.example.com" would be "example.com", while
# "api.cloudfront.net" would still be "api.cloudfront.net" (since other
# subdomains under a public suffix list may not be owned by you).
@known_root_hosts = @known_api_hosts.dup
website_backends.each do |website_backend|
@known_root_hosts << website_backend["frontend_host"]
end
@known_root_hosts.map! { |domain| PublicSuffix.domain(domain) }
end

def sanitized_url(url)
host = url_host(url)
if allowed_host?(host)
url
else
Rails.logger.warn("Rejecting unknown url host: #{url}")
nil
end
end

def sanitized_api_url(url)
host = url_host(url)
allowed_api_host?(host) ? url : nil
end

def sanitized_email(email)
host = email_host(email)
if allowed_host?(host)
email
else
Rails.logger.warn("Rejecting unknown email host: #{email}")
nil
end
end

def allowed_host?(host)
root_host = PublicSuffix.domain(host)
@known_root_hosts.include?(root_host)
end

def allowed_api_host?(host)
@known_api_hosts.include?(host)
end

private

def url_host(url)
return nil if url.blank?

begin
Addressable::URI.parse(url).host
rescue Addressable::URI::InvalidURIError
nil
end
end

def email_host(email)
return nil if email.blank?

begin
Mail::Address.new(email).domain
rescue Mail::Field::IncompleteParseError
nil
end
end
end
88 changes: 50 additions & 38 deletions test/apis/v1/users/test_create_welcome_email.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,52 +116,64 @@ def test_default_content
end

def test_customized_content
response = Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/users.json", http_options.deep_merge(admin_token).deep_merge({
:headers => { "Content-Type" => "application/x-www-form-urlencoded" },
:body => {
:user => FactoryBot.attributes_for(:api_user),
:options => {
:send_welcome_email => true,
:site_name => "External Example",
:email_from_name => "Tester",
:email_from_address => "test@google.com",
:example_api_url => "https://example.com/api.json?api_key={{api_key}}&test=1",
:contact_url => "https://example.com/contact-us",
},
prepend_api_backends([
{
:frontend_host => "example.com",
:backend_host => "127.0.0.1",
:servers => [{ :host => "127.0.0.1", :port => 9444 }],
:url_matches => [{ :frontend_prefix => "/#{unique_test_id}/", :backend_prefix => "/" }],
},
}))
assert_response_code(201, response)
]) do
response = Typhoeus.post("https://127.0.0.1:9081/api-umbrella/v1/users.json", http_options.deep_merge(admin_token).deep_merge({
:headers => { "Content-Type" => "application/x-www-form-urlencoded" },
:body => {
:user => FactoryBot.attributes_for(:api_user),
:options => {
:send_welcome_email => true,
:site_name => "External Example",
:email_from_name => "Tester",
:email_from_address => "test@example.com",
:example_api_url => "https://example.com/api.json?api_key={{api_key}}&test=1",
:contact_url => "https://example.com/contact-us",
},
},
}))
assert_response_code(201, response)

messages = delayed_job_sent_messages
assert_equal(1, messages.length)
messages = delayed_job_sent_messages
assert_equal(1, messages.length)

data = MultiJson.load(response.body)
user = ApiUser.find(data["user"]["id"])
message = messages.first
data = MultiJson.load(response.body)
user = ApiUser.find(data["user"]["id"])
message = messages.first

# To
refute_nil(user.email)
assert_equal([user.email], message["Content"]["Headers"]["To"])
# To
refute_nil(user.email)
assert_equal([user.email], message["Content"]["Headers"]["To"])

# API key
refute_nil(user.api_key)
assert_match(user.api_key, message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match(user.api_key, message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])
# API key
refute_nil(user.api_key)
assert_match(user.api_key, message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match(user.api_key, message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])

# Subject
assert_equal(["Your External Example API key"], message["Content"]["Headers"]["Subject"])
# Subject
assert_equal(["Your External Example API key"], message["Content"]["Headers"]["Subject"])

# From
assert_equal(["Tester <test@google.com>"], message["Content"]["Headers"]["From"])
# From
assert_equal(["Tester <test@example.com>"], message["Content"]["Headers"]["From"])

# URL Example
assert_match("Here's an example", message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match("Here's an\r\nexample", message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])
assert_match(%(<a href="https://example.com/api.json?api_key=#{user.api_key}&amp;test=1">https://example.com/api.json?<strong>api_key=#{user.api_key}</strong>&amp;test=1</a>), message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match("https://example.com/api.json?api_key=#{user.api_key}&test=1\r\n\r\n( https://example.com/api.json?api_key=#{user.api_key}&test=1 )", message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])
# URL Example
assert_match("Here's an example", message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match("Here's an\r\nexample", message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])
assert_match(%(<a href="https://example.com/api.json?api_key=#{user.api_key}&amp;test=1">https://example.com/api.json?<strong>api_key=#{user.api_key}</strong>&amp;test=1</a>), message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match("https://example.com/api.json?api_key=#{user.api_key}&test=1\r\n\r\n( https://example.com/api.json?api_key=#{user.api_key}&test=1 )", message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])

# Contact URL
assert_match(%(<a href="https://example.com/contact-us">contact us</a>), message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match("contact us \r\n( https://example.com/contact-us )", message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])
# Contact URL
assert_match(%(<a href="https://example.com/contact-us">contact us</a>), message["_mime_parts"]["text/html; charset=UTF-8"]["Body"])
assert_match("contact us \r\n( https://example.com/contact-us )", message["_mime_parts"]["text/plain; charset=UTF-8"]["Body"])
end
end

def test_sanitize
end
end
Loading

0 comments on commit 89f7ec2

Please sign in to comment.