Skip to content

Use SameSite everywhere #308

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

Merged
merged 5 commits into from
Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
bool domainHasValue = !string.IsNullOrEmpty(options.Domain);
bool pathHasValue = !string.IsNullOrEmpty(options.Path);
bool expiresHasValue = options.Expires.HasValue;
bool sameSiteHasValue = options.SameSite.HasValue && SystemWebCookieManager.IsSameSiteAvailable;

string escapedKey = Uri.EscapeDataString(key);
string prefix = escapedKey + "=";
Expand All @@ -173,9 +174,12 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
!pathHasValue ? null : "; path=",
!pathHasValue ? null : options.Path,
!expiresHasValue ? null : "; expires=",
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss ", CultureInfo.InvariantCulture) + "GMT",
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss \\G\\M\\T", CultureInfo.InvariantCulture),
!options.Secure ? null : "; secure",
!options.HttpOnly ? null : "; HttpOnly");
!options.HttpOnly ? null : "; HttpOnly",
!sameSiteHasValue ? null : "; SameSite=",
!sameSiteHasValue ? null : GetStringRepresentationOfSameSite(options.SameSite.Value)
);

value = value ?? string.Empty;
bool quoted = false;
Expand Down Expand Up @@ -273,6 +277,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
{
Path = options.Path,
Domain = options.Domain,
HttpOnly = options.HttpOnly,
SameSite = options.SameSite,
Secure = options.Secure,
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
});

Expand All @@ -286,6 +293,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
{
Path = options.Path,
Domain = options.Domain,
HttpOnly = options.HttpOnly,
SameSite = options.SameSite,
Secure = options.Secure,
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
});
}
Expand Down Expand Up @@ -313,6 +323,14 @@ private static void SetOptions(HttpCookie cookie, CookieOptions options, bool do
{
cookie.HttpOnly = true;
}

if (SystemWebCookieManager.IsSameSiteAvailable)
{
SystemWebCookieManager.SameSiteSetter.Invoke(cookie, new object[]
{
options.SameSite ?? (SameSiteMode)(-1) // Unspecified
});
}
}

private static bool IsQuoted(string value)
Expand All @@ -329,5 +347,21 @@ private static string Quote(string value)
{
return '"' + value + '"';
}

private static string GetStringRepresentationOfSameSite(SameSiteMode siteMode)
{
switch (siteMode)
{
case SameSiteMode.None:
return "None";
case SameSiteMode.Lax:
return "Lax";
case SameSiteMode.Strict:
return "Strict";
default:
throw new ArgumentOutOfRangeException("siteMode",
string.Format(CultureInfo.InvariantCulture, "Unexpected SameSiteMode value: {0}", siteMode));
}
}
}
}
26 changes: 26 additions & 0 deletions src/Microsoft.Owin.Host.SystemWeb/SystemWebCookieManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Reflection;
using System.Web;
using Microsoft.Owin.Infrastructure;

Expand All @@ -12,6 +13,21 @@ namespace Microsoft.Owin.Host.SystemWeb
/// </summary>
public class SystemWebCookieManager : ICookieManager
{
// .NET 4.7.2, but requries a patch to emit SameSite=None
internal static readonly bool IsSameSiteAvailable;
internal static readonly MethodInfo SameSiteSetter;

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Performance", "CA1810:InitializeReferenceTypeStaticFieldsInline")]
static SystemWebCookieManager()
{
var systemWeb = typeof(HttpContextBase).Assembly;
IsSameSiteAvailable = systemWeb.GetType("System.Web.SameSiteMode") != null;
if (IsSameSiteAvailable)
{
SameSiteSetter = typeof(HttpCookie).GetProperty("SameSite").SetMethod;
}
}

/// <summary>
/// Creates a new instance of SystemWebCookieManager.
/// </summary>
Expand Down Expand Up @@ -104,6 +120,13 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
{
cookie.HttpOnly = true;
}
if (IsSameSiteAvailable)
{
SameSiteSetter.Invoke(cookie, new object[]
{
options.SameSite ?? (SameSiteMode)(-1) // Unspecified
});
}

webContext.Response.AppendCookie(cookie);
}
Expand Down Expand Up @@ -133,6 +156,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
{
Path = options.Path,
Domain = options.Domain,
HttpOnly = options.HttpOnly,
Secure = options.Secure,
SameSite = options.SameSite,
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ protected override async Task ApplyResponseGrantAsync()
{
Domain = Options.CookieDomain,
HttpOnly = Options.CookieHttpOnly,
SameSite = Options.CookieSameSite,
Path = Options.CookiePath ?? "/",
};
if (Options.CookieSecure == CookieSecureOption.SameAsRequest)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ public string CookieName
/// </summary>
public bool CookieHttpOnly { get; set; }

/// <summary>
/// Determines if the browser should allow the cookie to be sent with requests initiated from other sites.
/// The default is 'null' to exclude the setting and let the browser choose the default behavior.
/// </summary>
public SameSiteMode? CookieSameSite { get; set; }

/// <summary>
/// Determines if the cookie should only be transmitted on HTTPS request. The default is to limit the cookie
/// to HTTPS requests if the page which is doing the SignIn is also HTTPS. If you have an HTTPS sign in page
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,7 @@ protected virtual void RememberNonce(OpenIdConnectMessage message, string nonce)
Convert.ToBase64String(Encoding.UTF8.GetBytes(Options.StateDataFormat.Protect(properties))),
new CookieOptions
{
SameSite = SameSiteMode.None,
HttpOnly = true,
Secure = Request.IsSecure,
Expires = DateTime.UtcNow + Options.ProtocolValidator.NonceLifetime
Expand Down Expand Up @@ -667,6 +668,7 @@ protected virtual string RetrieveNonce(OpenIdConnectMessage message)
{
var cookieOptions = new CookieOptions
{
SameSite = SameSiteMode.None,
HttpOnly = true,
Secure = Request.IsSecure
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ protected void GenerateCorrelationId(AuthenticationProperties properties)

var cookieOptions = new CookieOptions
{
SameSite = SameSiteMode.None,
HttpOnly = true,
Secure = Request.IsSecure
};
Expand Down Expand Up @@ -243,6 +244,7 @@ protected void GenerateCorrelationId(ICookieManager cookieManager, Authenticatio

var cookieOptions = new CookieOptions
{
SameSite = SameSiteMode.None,
HttpOnly = true,
Secure = Request.IsSecure
};
Expand Down Expand Up @@ -277,6 +279,7 @@ protected bool ValidateCorrelationId(AuthenticationProperties properties, ILogge

var cookieOptions = new CookieOptions
{
SameSite = SameSiteMode.None,
HttpOnly = true,
Secure = Request.IsSecure
};
Expand Down Expand Up @@ -332,6 +335,7 @@ protected bool ValidateCorrelationId(ICookieManager cookieManager, Authenticatio

var cookieOptions = new CookieOptions
{
SameSite = SameSiteMode.None,
HttpOnly = true,
Secure = Request.IsSecure
};
Expand Down
29 changes: 27 additions & 2 deletions src/Microsoft.Owin/Infrastructure/ChunkingCookieManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
bool domainHasValue = !string.IsNullOrEmpty(options.Domain);
bool pathHasValue = !string.IsNullOrEmpty(options.Path);
bool expiresHasValue = options.Expires.HasValue;
bool sameSiteHasValue = options.SameSite.HasValue;

string escapedKey = Uri.EscapeDataString(key);
string prefix = escapedKey + "=";
Expand All @@ -146,9 +147,11 @@ public void AppendResponseCookie(IOwinContext context, string key, string value,
!pathHasValue ? null : "; path=",
!pathHasValue ? null : options.Path,
!expiresHasValue ? null : "; expires=",
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss ", CultureInfo.InvariantCulture) + "GMT",
!expiresHasValue ? null : options.Expires.Value.ToString("ddd, dd-MMM-yyyy HH:mm:ss \\G\\M\\T", CultureInfo.InvariantCulture),
!options.Secure ? null : "; secure",
!options.HttpOnly ? null : "; HttpOnly");
!options.HttpOnly ? null : "; HttpOnly",
!sameSiteHasValue ? null : "; SameSite=",
!sameSiteHasValue ? null : GetStringRepresentationOfSameSite(options.SameSite.Value));

value = value ?? string.Empty;
bool quoted = false;
Expand Down Expand Up @@ -277,6 +280,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
{
Path = options.Path,
Domain = options.Domain,
HttpOnly = options.HttpOnly,
SameSite = options.SameSite,
Secure = options.Secure,
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
});

Expand All @@ -290,6 +296,9 @@ public void DeleteCookie(IOwinContext context, string key, CookieOptions options
{
Path = options.Path,
Domain = options.Domain,
HttpOnly = options.HttpOnly,
SameSite = options.SameSite,
Secure = options.Secure,
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
});
}
Expand All @@ -309,5 +318,21 @@ private static string Quote(string value)
{
return '"' + value + '"';
}

private static string GetStringRepresentationOfSameSite(SameSiteMode siteMode)
{
switch (siteMode)
{
case SameSiteMode.None:
return "None";
case SameSiteMode.Lax:
return "Lax";
case SameSiteMode.Strict:
return "Strict";
default:
throw new ArgumentOutOfRangeException("siteMode",
string.Format(CultureInfo.InvariantCulture, "Unexpected SameSiteMode value: {0}", siteMode));
}
}
}
}
7 changes: 3 additions & 4 deletions src/Microsoft.Owin/ResponseCookieCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,13 @@ public void Delete(string key, CookieOptions options)
{
Path = options.Path,
Domain = options.Domain,
HttpOnly = options.HttpOnly,
SameSite = options.SameSite,
Secure = options.Secure,
Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
});
}

/// <summary>
/// Analogous to ToString() but without boxing so
/// we can save a bit of memory.
/// </summary>
private static string GetStringRepresentationOfSameSite(SameSiteMode siteMode)
{
switch (siteMode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
<Compile Include="AspNetAuthSessionStore.cs" />
<Compile Include="InMemoryAuthSessionStore.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="SameSiteCookieManager.cs" />
<Compile Include="Startup.cs" />
</ItemGroup>
<ItemGroup>
Expand Down
62 changes: 62 additions & 0 deletions tests/Katana.Sandbox.WebServer/SameSiteCookieManager.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using Microsoft.Owin;
using Microsoft.Owin.Infrastructure;

namespace Katana.Sandbox.WebServer
{
public class SameSiteCookieManager : ICookieManager
{
private readonly ICookieManager _innerManager;

public SameSiteCookieManager()
: this(new CookieManager())
{
}

public SameSiteCookieManager(ICookieManager innerManager)
{
_innerManager = innerManager;
}

public void AppendResponseCookie(IOwinContext context, string key, string value, CookieOptions options)
{
CheckSameSite(context, options);
_innerManager.AppendResponseCookie(context, key, value, options);
}

public void DeleteCookie(IOwinContext context, string key, CookieOptions options)
{
CheckSameSite(context, options);
_innerManager.DeleteCookie(context, key, options);
}

public string GetRequestCookie(IOwinContext context, string key)
{
return _innerManager.GetRequestCookie(context, key);
}

private void CheckSameSite(IOwinContext context, CookieOptions options)
{
if (DisallowsSameSiteNone(context) && options.SameSite == SameSiteMode.None)
{
// IOS12 and Mac OS X 10.14 treat SameSite=None as SameSite=Strict. Exclude the option instead.
// https://bugs.webkit.org/show_bug.cgi?id=198181
options.SameSite = null;
}
}

// https://myip.ms/view/comp_browsers/8568/Safari_12.html
public static bool DisallowsSameSiteNone(IOwinContext context)
{
// TODO: Use your User Agent library of choice here.
var userAgent = context.Request.Headers["User-Agent"];
return userAgent.Contains("CPU iPhone OS 12") // Also covers iPod touch
|| userAgent.Contains("iPad; CPU OS 12")
// Safari 12 and 13 are both broken on Mojave
|| userAgent.Contains("Macintosh; Intel Mac OS X 10_14");
}
}
}
Loading