Base64-encode HTML markup over the wire #4859
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
swissspidy
added
Type: Bug
|
Size Change: +9.63 kB (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
Neat but seems kinda brittle. Did we reach out or receive any guidance from Cloudflare or other WAF providers about this? |
|
Not yet. But read #4805 for some context. The landscape is quite broad with many different vendors. And getting changes into something like the OWASP ruleset is going to take a long time. Whatever some vendor would do, it would end up being some cat and mouse game. |
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## main #4859 +/- ##
==========================================
+ Coverage 75.75% 76.16% +0.41%
==========================================
Files 911 903 -8
Lines 16223 16024 -199
==========================================
- Hits 12289 12205 -84
+ Misses 3934 3819 -115
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
@swissspidy This PR is still in draft. But I think it is good to go. Is it blocked by anything? Like #4898? |
spacedmonkey
approved these changes
Oct 21, 2020
This was referenced Oct 27, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This is an attempt to solve conflicts with Web Application Firewalls that block requests to due HTML being sent. WAFs like the one from Cloudflare or Sucuri even scan JSON objects to find HTML in there, and of course they treat a full blown HTML document (the AMP markup we create) as suspicious due to containing script tags etc.
So my thought here was simple: if no HTML is found, nothing can be blocked.
And the easiest way that came to mind here was simple base64-encoding the markup before sending it in the client, and then decoding it again on the server-side.
Relevant Technical Choices
__WEB_STORIES_ENCODED__so that we can easily detect this in PHP.It might not be needed because we decode it right away upon receival, but I think it's safer to keep.
To-do
User-facing changes
N/A
Testing Instructions
Using templates and publishing stories should still work as expected. Especially when behind a WAF.
Fixes #4805