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

[Announcement][3.1.0-preview1] Reacting to browser SameSite changes, impacts OpenIdConnect #14996

Closed
Tratcher opened this issue Oct 14, 2019 · 12 comments
Labels
breaking-change This issue / pr will introduce a breaking change, when resolved / merged.
Milestone

Comments

@Tratcher
Copy link
Member

Tratcher commented Oct 14, 2019

Reacting to browser SameSite changes in 3.1.0-preview1, impacts OpenIdConnect

Browsers like Chrome and Firefox are making breaking changes to their implementations of SameSite for cookies. These changes impact remote authentication scenarios like OpenIdConnect and WsFederation which must now opt out by sending SameSite=None. However, SameSite=None breaks on iOS 12 and some older versions of other browsers. The application will need to sniff these versions and omit SameSite.

Version introduced

3.1.0-preview1

Old behavior

SameSite is a 2016 draft standard extension to HTTP cookies intended to mitigate cross site request forgery (CSRF). This was originally designed as a feature the servers would opt into by adding the new parameters. ASP.NET Core 2.0 added initial support for SameSite.

New behavior

Google is now pushing a new draft standard that is not backwards compatible. It changes the default mode to Lax and adds a new entry None to opt out. Lax is OK for most application cookies but breaks cross site scenarios like OpenIdConnect and WsFederation login. Most OAuth logins are not affected due to differences in how the request flows. The new None parameter causes compatibility problems with clients that implemented the prior draft standard (e.g. iOS 12). Chrome plans to go live with their changes in Chrome 80 in February 2020.

ASP.NET Core 3.1 has been updated to implement the new SameSite behavior. This includes redefining the behavior of SameSiteMode.None to emit “SameSite=None” and adding a new value SameSiteMode.Unspecified to omit the SameSite attribute. All cookies APIs now default to Unspecified, though some components that use cookies set values more specific to their scenarios such as the OpenIdConnect correlation and nonce cookies.

For other recent changes in this area see the 3.0 announcement where most defaults were changed from Lax to None (but still using the prior standard).

Reason for change

Browser and spec changes as outlined above.

Recommended action

Applications that interact with remote sites such as through 3rd party login will need to test those scenarios on multiple browsers, as well as apply the CookiePolicy browser sniffing mitigation discussed below. See below for testing and browser sniffing instructions.
How to test if you’re affected
Test your web application using a client version that can opt-in to the new behavior. Chrome, Firefox, and Chromium Edge all have new opt-in feature flags that can be used for testing.
You’ll also want to do compatibility testing with older client versions after you’ve applied the patches, especially Safari. See “Supporting older browsers” below.

Chrome:
Chrome 78+ will give you misleading test results as it has a temporary mitigation in place, allowing cookies less than two minutes old. Chrome 76 or 77 with the appropriate test flags turned on will give you more accurate results. To test the new behavior toggle chrome://flags/#same-site-by-default-cookies to enabled. Older versions of Chrome (75 and below) are reported to fail with the new None setting. See “Supporting older browsers” below.
Whilst Google does not make older chrome versions available you can download older versions of Chromium which will suffice for testing. Follow the instructions at https://www.chromium.org/getting-involved/download-chromium, don’t go downloading random assed installers from the internet that say they’re for older versions.
Chromium 76 Win64
Chromium 74 Win64

Safari:
Safari 12 strictly implemented the prior draft and will fail if it sees the new None value in cookies. This must be avoided via the browser sniffing code shown below. Ensure you test Safari 12 and Safari 13 as well as WebKit based OS style logins using MSAL, ADAL or whatever library you are using. Note that the problem is dependent on the underlying OS version, OSX Mojave (10.14) and iOS 12 are known to have compatibility problems with the new behavior. Upgrading the OS to OSX Catalina (10.15) or iOS 13 fixes the problem. Safari does not currently have an opt-in flag for testing the new spec behavior.

Firefox:
Firefox support for the new standard can be tested on version 68+ by opting in on the “about:config” page with the feature flag “network.cookie.sameSite.laxByDefault”. There have not been reports of compatibility issues with older versions of Firefox.

Edge:
While Edge supports the old SameSite standard, as of version 44 it did not have any compatibility problems with the new standard.

Edge (Chromium):
The feature flag is edge://flags/#same-site-by-default-cookies. No compatibility issues were observed when testing with Edge Chromium 78.

Electron:
Versions of electron will include older versions of Chromium, for example the version of Electron used by Teams is Chromium 66 which exhibits the older behavior. You must perform your own compatibility testing with the version of Electron your product uses. See “Supporting older browsers” below.

Supporting older browsers:
The 2016 SameSite standard mandated that unknown values must be treated as SameSite=Strict values, so any older browsers which support the original standard may break when they see a SameSite property with a value of None. Web applications must implement browser sniffing if they intend to support these old browsers. AspNetCore as a rule does not implement browser sniffing for you because User-Agents values are highly unstable and change on a weekly basis. What is available is an extension point in CookiePolicy allowing you to plug in User-Agent specific logic.

In Startup.cs add the following

private void CheckSameSite(HttpContext httpContext, CookieOptions options) 
{ 
    if (options.SameSite == SameSiteMode.None) 
    { 
        var userAgent = httpContext.Request.Headers["User-Agent"].ToString(); 
        // TODO: Use your User Agent library of choice here. 
        if (/* UserAgent doesn’t support new behavior */) 
        { 
               options.SameSite = SameSiteMode.Unspecified; 
        } 
    } 
}

public void ConfigureServices(IServiceCollection services) 
{ 
    services.Configure<CookiePolicyOptions>(options => 
    { 
        options.MinimumSameSitePolicy = SameSiteMode.Unspecified; 
        options.OnAppendCookie = cookieContext =>  
            CheckSameSite(cookieContext.Context, cookieContext.CookieOptions); 
        options.OnDeleteCookie = cookieContext =>  
            CheckSameSite(cookieContext.Context, cookieContext.CookieOptions); 
    }); 
} 
 
public void Configure(IApplicationBuilder app) 
{ 
    app.UseCookiePolicy(); // Before UseAuthentication or anything else that writes cookies. 
    app.UseAuthentication(); 
    // … 
}

Opt-out switches:
There is a compat switch which enables you to temporarily opt-out of the new ASP.NET Core cookie behavior. Add a runtimeconfig.template.json file to your project containing:

{ 
  "configProperties": { 
    "Microsoft.AspNetCore.SuppressSameSiteNone": "true" 
  } 
} 

This switch will be removed in the next major version.

Other Versions:
Related SameSite patches are forthcoming for ASP.NET Core 2.1, 2.2, 3.0, Microsoft.Owin 4.1, and System.Web (.NET 4.7.2+).

Category

ASP.NET

Affected APIs

SameSiteMode
CookieOptions.SameSite
CookieBuilder.SameSite
CookiePolicyOptions.MinimumSameSitePolicy

Microsoft.Net.Http.Headers:
SameSiteMode
SetCookieHeaderValue.SameSite


Issue metadata

  • Issue type: breaking-change
@Tratcher Tratcher added the breaking-change This issue / pr will introduce a breaking change, when resolved / merged. label Oct 14, 2019
@Tratcher Tratcher added this to the Discussions milestone Oct 14, 2019
@sandrinodimattia
Copy link

sandrinodimattia commented Oct 14, 2019

Using UA sniffing is advised against by Google. The only reliable way to mitigate SameSite issues is to use a double cookie approach.

GoogleChromeLabs/samesite-examples#2 (comment)

@Tratcher
Copy link
Member Author

Tratcher commented Oct 14, 2019

Using UA sniffing is advised against by Google. The only reliable way to mitigate SameSite issues is to use a double cookie approach.

GoogleChromeLabs/samesite-examples#2 (comment)

Except double issuing cookies is more likely to cause them to fail because browsers like Safari have strict cookie size limits per domain. Auth cookies already push these limits today.

There are also many components that internally issue cookies which you would not have any control over. Adapting them for a temporary mitigation like this is not feasible.

@slaneyrw
Copy link

This also has the potential to impact SAML SSO posts.

I know if I turn ON the chrome flag then i can't sign into the Azure portal

@slaneyrw
Copy link

When can we expect ASP.NET and OWIN updates ?

@Tratcher
Copy link
Member Author

Tratcher commented Oct 17, 2019

When can we expect ASP.NET and OWIN updates ?

The plan is for November.

Microsoft.Owin 4.1 nightly builds are available on the dev feed documented here:
https://github.com/aspnet/aspnetkatana#nightly-build-feeds
Be warned that they expect the ASP.NET patch to be installed or you may end up with SameSite=-1 in your headers if you use the SystemWebCookieManager.

@Tratcher
Copy link
Member Author

@valeriob
Copy link

valeriob commented Nov 5, 2019

Is there an issue to track for 2.1 and 2.2 ?

@Tratcher
Copy link
Member Author

Tratcher commented Nov 5, 2019

@valeriob All of the AspNetCore patches have been merged for the upcoming November patch release:
2.1 #13746
2.2 #13858
3.0 #13870

@Tratcher
Copy link
Member Author

Tratcher commented Nov 8, 2019

I've made a correction to the sample code above. It used to read:

private void CheckSameSite(HttpContext httpContext, CookieOptions options) 
{ 
    if (options.SameSite > SameSiteMode.Unspecified) 
    { 

But the corrected version is:

private void CheckSameSite(HttpContext httpContext, CookieOptions options) 
{ 
    if (options.SameSite == SameSiteMode.None) 
    { 

@Tratcher
Copy link
Member Author

Some clarifications for folks using ASP.NET Core 2.1 or 2.2 on .NET Framework:

  • ASP.NET Core and System.Web (ASP.NET Classic) have independent implementations of SameSite. The SameSite KB patches for .NET Framework are not required if using ASP.NET Core. Nor does the System.Web SameSite minimum framework version requirement (.NET 4.7.2) apply to ASP.NET Core.
  • ASP.NET Core on .NET requires updating nuget package dependencies to get the appropriate fixes. This includes lifting the transitively referenced Microsoft.Net.Http.Headers to the necessary version (2.1.14 or 2.2.8) by adding it as direct dependency.
    <PackageReference Include="Microsoft.Net.Http.Headers" Version="2.1.14" />

@Tratcher
Copy link
Member Author

Patches

See https://devblogs.microsoft.com/dotnet/net-core-November-2019/ for information on the November 2.1.14, 2.2.8, and 3.0.1 patches that included these changes.

@ghost
Copy link

ghost commented Nov 12, 2020

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Nov 12, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 12, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change This issue / pr will introduce a breaking change, when resolved / merged.
Projects
None yet
Development

No branches or pull requests

4 participants