Simplify CSP header for SWF asset embeds, to fix 502 for some assets

Fun little bug: viewing the "Engulfed in Flames Effect" item was
showing our "502 Bad Gateway" custom error page in the embed. This is
because the Rails app was providing a `Content-Security-Policy` header
value that was longer than nginx is configured by default to allow, so
it was refusing the response, and showing the same 502 error as if the
app hadn't responded at all. (We discovered this by opening
`/var/log/nginx/error.log`, which explained this very clearly, ty~!)

In this change, we no longer list every `images.neopets.com` asset,
instead marking the entire domain as a valid image source for the
SWF asset embed iframe. I don't _love_ this solution, I liked the
property of specifying literally exactly the assets we allow! But I
don't think there's any practical danger here, and it helps a *lot* for
making this more reliable.

(If we could have solved this reliably by increasing nginx's allowed
response header size, I probably would've done that? But I researched a
bit, and ultimately concluded that I don't trust other intermediary
software like firewalls not to have the same issue. Let's not be
pushing the limits of HTTP headers of all things!)
This commit is contained in:
Emi Matchu 2024-09-12 15:59:18 -07:00
parent 68b6f46939
commit 58d7c38523

View file

@ -12,6 +12,13 @@ class SwfAssetsController < ApplicationController
helpers.image_url("favicon.png"), helpers.image_url("favicon.png"),
@swf_asset.image_url, @swf_asset.image_url,
*@swf_asset.canvas_movie_sprite_urls, *@swf_asset.canvas_movie_sprite_urls,
# For images, `images.neopets.com` is a generally safe host to load
# from (shouldn't be a vulnerable site or exfiltration vector), and
# doing this can help make this header a *lot* shorter, which helps
# our nginx reverse proxy (and probably some clients) handle it. (For
# example, see asset `667993` for "Engulfed in Flames Effect".)
hosts: ["https://images.neopets.com"],
) )
} }
@ -38,7 +45,14 @@ class SwfAssetsController < ApplicationController
private private
def src_list(*urls) def src_list(*urls, hosts: [])
urls.filter(&:present?).map { |url| url.sub(/\?.*\z/, "") }.join(" ") urls.
# Ignore any `nil`s that might arise
filter(&:present?).
# Remove query strings from URLs (they're invalid in CSPs)
map { |url| url.sub(/\?.*\z/, "") }.
# For the given `hosts`, remove all their specific URLs, and just list
# the host itself.
reject { |url| hosts.any? { |h| url.start_with? h } } + hosts
end end
end end