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

SameSite still omitted when set to SameSiteMode.None, with patched .net core 2.1 #18779

Closed
smcl opened this issue Feb 4, 2020 · 10 comments
Closed
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.

Comments

@smcl
Copy link

smcl commented Feb 4, 2020

Describe the bug

When creating a cookie it still doesn't seem possible to have SameSite=None set even though this was supposed to be sorted in the 2.1.14 patch last year (i've also tried installing 2.1.15) - see issue #12125 and #13746. I originally observed this when testing out our IdentityServer4 app which uses OIDC, when we send users to an external Identity Provider an ".AspNetCore.Correlation." cookie is set without the specified SameSite even though I believed I had set this all up fine. I spent a while figuring out what I had configured incorrectly until I boiled it down into the example I've included below and realised that something outside my control may be going wrong.

I would be delighted to find out I'm wrong and that there's a simple fix that doesn't involve manually hacking Path as "/; SameSite=None"; because that's where I'm currently leaning ...

To Reproduce

Create a simple console application:

$ dotnet new console -f netcoreapp2.1 -n samesite.test
The template "Console Application" was created successfully.

Processing post-creation actions...
Running 'dotnet restore' on samesite.test\samesite.test.csproj...
  Restore completed in 254.28 ms for C:\Users\sean.mclemon\source\one-off\samesite.test\samesite.test.csproj.

Restore succeeded.

... then open up Program.cs and set it to the following

using Microsoft.Net.Http.Headers;
using System;

namespace samesite.test
{
    class Program
    {
        static void Main(string[] args)
        {
            var cookie = new SetCookieHeaderValue("some-key", "some-value")
            {
                Domain = "example.com",
                Path = "/path",
                Expires = DateTimeOffset.UtcNow.AddDays(1),
                MaxAge = TimeSpan.FromDays(1),
                Secure = true,
                SameSite = SameSiteMode.None,
                HttpOnly = true
            };

            Console.WriteLine(cookie.ToString());
        }
    }
}

next cd to the dir, add the Microsoft.AspNetCore.Http package, build and run

$ cd samesite.test

$ dotnet add package Microsoft.AspNetCore.Http --version 2.2.2
  Writing C:\Temp\sean.mclemon\tmpEEC1.tmp
info : Adding PackageReference for package 'Microsoft.AspNetCore.Http' into project 'C:\Users\sean.mclemon\source\one-off\samesite.test\samesite.test.csproj'.
info : Restoring packages for C:\Users\sean.mclemon\source\one-off\samesite.test\samesite.test.csproj...
info : Package 'Microsoft.AspNetCore.Http' is compatible with all the specified frameworks in project 'C:\Users\sean.mclemon\source\one-off\samesite.test\samesite.test.csproj'.
info : PackageReference for package 'Microsoft.AspNetCore.Http' version '2.2.2' added to file 'C:\Users\sean.mclemon\source\one-off\samesite.test\samesite.test.csproj'.
info : Committing restore...
info : Writing assets file to disk. Path: C:\Users\sean.mclemon\source\one-off\samesite.test\obj\project.assets.json
log  : Restore completed in 742.35 ms for C:\Users\sean.mclemon\source\one-off\samesite.test\samesite.test.csproj.

$ dotnet build
Microsoft (R) Build Engine version 16.4.0+e901037fe for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 37.94 ms for C:\Users\sean.mclemon\source\one-off\samesite.test\samesite.test.csproj.
  samesite.test -> C:\Users\sean.mclemon\source\one-off\samesite.test\bin\Debug\netcoreapp2.1\samesite.test.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:02.35

$ dotnet run
some-key=some-value; expires=Wed, 05 Feb 2020 14:45:39 GMT; max-age=86400; domain=example.com; path=/path; secure; httponly

Notice that there's no "SameSite" there. If we used SameSiteMode.Lax or SameSiteMode.Strict then we would see it.

Further technical details

$ dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.100
 Commit:    cd82f021f4

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18363
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.100\

Host (useful for support):
  Version: 3.1.0
  Commit:  65f04fb6db

.NET Core SDKs installed:
  1.1.14 [C:\Program Files\dotnet\sdk]
  2.1.803 [C:\Program Files\dotnet\sdk]
  3.0.100 [C:\Program Files\dotnet\sdk]
  3.1.100 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 1.0.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download
@javiercn javiercn added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Feb 4, 2020
@blowdart
Copy link
Contributor

blowdart commented Feb 4, 2020

  1. Are you targetting .NET Framework as your runtime? If so you need to specifically add two dependencies;
<PackageReference Include="Microsoft.Net.Http.Headers" Version="2.1.14" />
<PackageReference Include="Microsoft.AspNetCore.CookiePolicy" Version="2.1.14" />
  1. You also need to configure middlewares to use the SecurePolicy you wish; for example
services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
    .AddCookie(options =>
    {
        options.Cookie.SameSite = SameSiteMode.None;
        options.Cookie.SecurePolicy = CookieSecurePolicy.Always;
        options.Cookie.IsEssential = true;
    });

This is documented.

  1. Finally if you want (and you should want) to intercept cookies and remove the none value for browsers that don't support it then you need to use CookiePolicy as detailed in the docs.

@blowdart blowdart added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Feb 4, 2020
@smcl
Copy link
Author

smcl commented Feb 4, 2020

No I'm not targetting .NET Framework, I have configured middleware as per the documentation and I have a solution in place to strip out None for the some of the browsers which don't support it similar to that described in the docs.

I really appreciate the suggestions but in this issue I've got a little self-contained example that I think indicates SameSite=None is not correctly set when it really ought to be. It's possible that it's related my original problems but I think we should take one step at a time and understand if there are any issues with my cut-down example which strictly deals with SetCookieHeaderValue because it should be easy to figure out.

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Feb 4, 2020
@blowdart
Copy link
Contributor

blowdart commented Feb 4, 2020

An example would be great

@blowdart blowdart added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Feb 4, 2020
@smcl
Copy link
Author

smcl commented Feb 4, 2020

Please see the "To Reproduce" of the original comment, it's all there

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Feb 4, 2020
@blowdart
Copy link
Contributor

blowdart commented Feb 4, 2020

Oh wait, that's it?

What version of core 2.1 do you have installed?

(Also @Tratcher)

@blowdart
Copy link
Contributor

blowdart commented Feb 4, 2020

Oh wait, you're going through SetCookieHeaderValue? Why a console app, rather than a web app? The web app approach works in my demos.

var cookieOptions = new CookieOptions
{
    Secure = true,
    HttpOnly = true,
    SameSite = SameSiteMode.None
};
Response.Cookies.Append("CookieName", "cookieValue", cookieOptions);

@Tratcher
Copy link
Member

Tratcher commented Feb 4, 2020

@blowdart I'll take over.

@smcl nice repro steps. I'll give them a try quick.

@Tratcher
Copy link
Member

Tratcher commented Feb 4, 2020

dotnet add package Microsoft.AspNetCore.Http --version 2.2.2

Why did you add version 2.2.2 to a 2.1 project?

I replaced that step with this one and now it's working:
dotnet add package Microsoft.Net.Http.Headers --version 2.1.14

some-key=some-value; expires=Wed, 05 Feb 2020 20:55:42 GMT; max-age=86400; domain=example.com; path=/path; secure; samesite=none; httponly

Needing this direct dependency is expected when you're working only with packages. Normally that would only apply to .NET Framework apps, but in your repro it also applies because you're not referencing the ASP.NET Core shared runtime (Microsoft.AspNetCore.App) that contains the patch.

@smcl
Copy link
Author

smcl commented Feb 4, 2020

Ahhh I installed that library in my little example as I (mistakenly) believed that was the necessary package required to access SetCookieHeaderValue and experiment with the problem. OK so this suggests to me I'll need to dig through my dependencies in the original app that prompted me to create this and see if I can find anything funny.

Thanks heaps, this can be closed. My apologies for raising a non-issue!

@smcl smcl closed this as completed Feb 4, 2020
@blowdart
Copy link
Contributor

blowdart commented Feb 4, 2020

It wasn't a non-issue, it's good to get to the bottom of these things.

@dotnet dotnet locked as resolved and limited conversation to collaborators Mar 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update.
Projects
None yet
Development

No branches or pull requests

4 participants