Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server-side Blazor should use WebSocket compression #30745

Closed
benaadams opened this issue Mar 8, 2021 · 6 comments
Closed

Server-side Blazor should use WebSocket compression #30745

benaadams opened this issue Mar 8, 2021 · 6 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. Status: Resolved

Comments

@benaadams
Copy link
Member

Dependent on dotnet/runtime#49304

@mkArtakMSFT mkArtakMSFT added the area-blazor Includes: Blazor, Razor Components label Mar 8, 2021
@mkArtakMSFT
Copy link
Member

Thanks for contacting us.
@blowdart are there any security concerns with this? Can we do this at all?

@blowdart
Copy link
Contributor

blowdart commented Mar 8, 2021

The RFC acknowledges the usual compression concern,

  1. Security Considerations

There is a known exploit when history-based compression is combined
with a secure transport [CRIME]. Implementors should pay attention
to this point when integrating this extension with other extensions
or protocols.

So, if you do it, it should be off by default IMO, much like http compression when TLS is on.

@GrabYourPitchforks any thoughts here?

@blowdart blowdart assigned mkArtakMSFT and unassigned blowdart Mar 8, 2021
@mkArtakMSFT
Copy link
Member

Thanks @blowdart.
After some more discussion we've decided not to do this work as the risk involved is quite high.

@mkArtakMSFT mkArtakMSFT added the ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. label Mar 8, 2021
@ghost ghost added the Status: Resolved label Mar 8, 2021
@SteveSandersonMS
Copy link
Member

It's also worth noting that Blazor Server already deduplicates strings if we know for sure that they came from compile-time constants, and hence don't vary by user input. So it's already a kind of compression for the things we know are safe.

@benaadams
Copy link
Member Author

Using https://themesof.net/ as an example server-side Blazor app; its still very chonky

94.2 kB is the reload of server-side render (as it resends the starting state vs WebSocket); which is 7 kB when compressed

482 kB is clicking the open nodes button; which is super chonk

image

@SteveSandersonMS
Copy link
Member

I'd personally be open to enabling an option for this if it became a common customer request. In this specific example with themesof.net, I don't think the compression oracle attacks would be a viable attack vector, since there's very limited scope for an attacker inserting their own data into the stream. Though maybe the attacker could be someone with write access to issue titles/labels.

Generally the problem with having these kinds of options is that developers may turn them on because they understand the benefits but don't understand the risks. The risk level of compression oracle attacks is really hard to estimate or communicate.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components ✔️ Resolution: Won't Fix Resolved because we decided not to change the behavior reported in this issue. Status: Resolved
Projects
None yet
Development

No branches or pull requests

4 participants