Security review for Blazor

[mkArtakMSFT]

Threat Models

• Client
• Server

Documents will be made public when review is complete.

Client Action Items

• Investigate CSP for bootstrapping
• Investigate providing a default CORS configuration.
• Follow up on the need for CSRF once browser support is locked

Server Action Items

• Investigate turning on CSP in our default layouts. (require same-origin)
  • Investigation is done, pending a decission on whether we add it or not.
  • We decided not to use CSP due to usability concerns.
• Security review of embedded files.
  • Notes about security sent to the stakeholders.
• Provide documentation and guidance for responsible use of JS -> .NET interop in the server-side model. (JS interop is untrusted input)[Preview 8] Blazor security guidance and best practices AspNetCore.Docs#12995
• Security code review for JS interop.
• Add default timeouts for JS interop async calls.
  • JSInterop library work
  • Blazor server-side work
• Sanitize JS interop exceptions (generic errors).
  • JSInterop library work
  • Blazor server-side work
• Generate CircuitIds that are not guessable/tamperable.
• Validate the same user identity/browser session when allowing a client to join a Circuit.
  • Only concern here was scale out + cookie, but it should be doable thanks to the new signalr stickyness.
  • Follow up with tooling to enable signalr stickiness by default when deploying to Azure.
• Investigate securing websockets for cross-origin access.
  • We are not doing anything by default, simply providing guidance, covered as part of [Preview 8] Blazor security guidance and best practices AspNetCore.Docs#12995
• Security code review for Message Pack + Blazor Pack
• Security code review for render tree serialization
• Show and tell with Auth for @blowdart
  • @SteveSandersonMS is on point for this.
• Measure and mitigate risk of opening too many circuits
  • Add limits to the amount of queued pending renders. [Blazor server-side] Limit the amount of queued pending renders #11964
  • Add timeouts for idle circuits. [Blazor] Add a timeout for idle circuits #11965
  • Provide guidance on how to limit the number of opened circuits per user. [Blazor] Guidance on limiting circuits per user #11966
  • Prevent start circuit from creating multiple circuits under any circumstance. [Blazor] StartCircuit should check for an existing circuit. #12054
  • Ensure appropriate memory thresholds under heavy event loads. Decide if server-side Blazor needs limits on how many events can be delivered and the max size for events #12003
• Sanitize unhandled exceptions on ComponentHub [Blazor] Sanitize unhandled exceptions on Componenthub #12055
• Incorrect batch acks should not produce warnings. [Blazor] Incorrect batch acks should not produce warnings #12056
• Passing invalid parameters to StartCircuit should not produce exceptions. [Blazor] Harden start circuit #12057
• Convert ids for events and components into long. [Blazor] Convert event ids to longs #12058

[javiercn]

We've done all the work we wanted to do here. We've met several times with the appropriate people to look at the different security aspects involved in server side blazor.

The remaining action items are tracked in separate issues and the last piece is to put together the meeting notes we collected during the reviews and put them in a consistent way as part of the security guidance doc for blazor server-side applications dotnet/AspNetCore.Docs#12995

@mkArtakMSFT I'm not sure if we should just close this issue at this point.

[rynowak]

There's a bunch of items that are not checked off. Check them off if they are done, or strike them out if a note if we didn't do it 😆

Did you update the threat model with the additional stuff?

[mkArtakMSFT]

Looks like all the unchecked items except one have additional tracking items. Please either create one more tracking item for the following one or simply strike it through (to be explicit that we didn't do it, neither we plan to):

Provide documentation and guidance for responsible use of JS -> .NET interop in the server-side model. (JS interop is untrusted input)

And yes, as @rynowak said, update teh threat model with additional stuff, based on all the findings. Then this can be closed.