Skip to content

Commit

Permalink
Use modulepath in data protection purpose
Browse files Browse the repository at this point in the history
- Prevents reuse of protected data across instances/schemes/modules
- Fixes #713
  • Loading branch information
AndersAbel committed Sep 19, 2023
1 parent 855f94d commit d9e4ff8
Show file tree
Hide file tree
Showing 13 changed files with 69 additions and 43 deletions.
8 changes: 5 additions & 3 deletions Sustainsys.Saml2.AspNetCore2/Saml2Handler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ namespace Sustainsys.Saml2.AspNetCore2
public class Saml2Handler : IAuthenticationRequestHandler, IAuthenticationSignOutHandler
{
private readonly IOptionsMonitorCache<Saml2Options> optionsCache;
private readonly IDataProtectionProvider dataProtectorProvider;

// Internal to be visible to tests.
internal Saml2Options options;
HttpContext context;
private readonly IDataProtector dataProtector;
private IDataProtector dataProtector;
private readonly IOptionsFactory<Saml2Options> optionsFactory;
bool emitSameSiteNone;

Expand All @@ -43,10 +44,9 @@ public class Saml2Handler : IAuthenticationRequestHandler, IAuthenticationSignOu
throw new ArgumentNullException(nameof(dataProtectorProvider));
}

dataProtector = dataProtectorProvider.CreateProtector(GetType().FullName);

this.optionsFactory = optionsFactory;
this.optionsCache = optionsCache;
this.dataProtectorProvider = dataProtectorProvider;
}

/// <InheritDoc />
Expand All @@ -58,6 +58,8 @@ public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context)

options = optionsCache.GetOrAdd(scheme.Name, () => optionsFactory.Create(scheme.Name));

dataProtector = dataProtectorProvider.CreateProtector(GetType().FullName, options.SPOptions.ModulePath);

emitSameSiteNone = options.Notifications.EmitSameSiteNone(context.Request.GetUserAgent());

return Task.CompletedTask;
Expand Down
9 changes: 7 additions & 2 deletions Sustainsys.Saml2.HttpModule/CommandResultHttpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,13 @@ public static partial class CommandResultHttpExtensions
/// <param name="commandResult">The CommandResult that will update the HttpResponse.</param>
/// <param name="response">Http Response to write the result to.</param>
/// <param name="emitSameSiteNone">Include a SameSite=None attribute on any cookies set</param>
/// <param name="modulePath">Path of Sustainsys.Saml2 instance - used for isolation of data protection</param>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "HttpStatusCode")]
public static void Apply(this CommandResult commandResult, HttpResponseBase response, bool emitSameSiteNone)
public static void Apply(
this CommandResult commandResult,
HttpResponseBase response,
bool emitSameSiteNone,
string modulePath)
{
if (commandResult == null)
{
Expand All @@ -28,7 +33,7 @@ public static void Apply(this CommandResult commandResult, HttpResponseBase resp

response.Cache.SetCacheability((HttpCacheability)commandResult.Cacheability);

ApplyCookies(commandResult, response, emitSameSiteNone);
ApplyCookies(commandResult, response, emitSameSiteNone, modulePath);
ApplyHeaders(commandResult, response);

if (commandResult.HttpStatusCode == HttpStatusCode.SeeOther || commandResult.Location != null)
Expand Down
10 changes: 8 additions & 2 deletions Sustainsys.Saml2.HttpModule/CommandResultHttpExtensionsShared.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,12 @@ public static partial class CommandResultHttpExtensions
/// <param name="commandResult">Commandresult</param>
/// <param name="response">Response</param>
/// <param name="emitSameSiteNone">Include a SameSite=None attribute on any cookies set</param>
public static void ApplyCookies(this CommandResult commandResult, HttpResponseBase response, bool emitSameSiteNone)
/// <param name="modulePath">Path of Sustainsys.Saml2 instance - used for isolation of data protection</param>
public static void ApplyCookies(
this CommandResult commandResult,
HttpResponseBase response,
bool emitSameSiteNone,
string modulePath)
{
if(commandResult == null)
{
Expand All @@ -37,7 +42,8 @@ public static void ApplyCookies(this CommandResult commandResult, HttpResponseBa
var protectedData = HttpRequestData.ConvertBinaryData(
MachineKey.Protect(
commandResult.GetSerializedRequestState(),
HttpRequestBaseExtensions.ProtectionPurpose));
HttpRequestBaseExtensions.ProtectionPurpose,
modulePath));

response.SetCookie(new HttpCookie(
commandResult.SetCookieName,
Expand Down
13 changes: 8 additions & 5 deletions Sustainsys.Saml2.HttpModule/HttpRequestBaseExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,26 @@ public static class HttpRequestBaseExtensions
/// Extension method to convert a HttpRequestBase to a HttpRequestData.
/// </summary>
/// <param name="requestBase">The request object used to populate the <c>HttpRequestData</c>.</param>
/// <param name="modulePath">Path of Sustainsys.Saml2 instance - used for isolation of data protection</param>
/// <returns>The <c>HttpRequestData</c> object that has been populated by the request.</returns>
public static HttpRequestData ToHttpRequestData(this HttpRequestBase requestBase)
public static HttpRequestData ToHttpRequestData(this HttpRequestBase requestBase, string modulePath)
{
return requestBase.ToHttpRequestData(false);
return requestBase.ToHttpRequestData(false, modulePath);
}

/// <summary>
/// Extension method to convert a HttpRequestBase to a HttpRequestData.
/// </summary>
/// <param name="requestBase">The request object used to populate the <c>HttpRequestData</c>.</param>
/// <param name="ignoreCookies">Ignore cookies when extracting data.
/// This is useful for the stub idp that might see the relay state
/// and the requester's cookie, but shouldn't try to decrypt it.</param>
/// <param name="modulePath">Path of Sustainsys.Saml2 instance - used for isolation of data protection</param>
/// <returns>The <c>HttpRequestData</c> object that has been populated by the request.</returns>
public static HttpRequestData ToHttpRequestData(
this HttpRequestBase requestBase,
bool ignoreCookies)
bool ignoreCookies,
string modulePath)
{
if (requestBase == null)
{
Expand All @@ -58,7 +61,7 @@ public static HttpRequestData ToHttpRequestData(this HttpRequestBase requestBase
requestBase.Form.Cast<string>().Select((de, i) =>
new KeyValuePair<string, IEnumerable<string>>(de, ((string)requestBase.Form[i]).Split(','))),
cookies,
v => MachineKey.Unprotect(v, ProtectionPurpose),
v => MachineKey.Unprotect(v, ProtectionPurpose, modulePath),
ClaimsPrincipal.Current);
}

Expand Down
5 changes: 3 additions & 2 deletions Sustainsys.Saml2.HttpModule/Saml2AuthenticationModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,16 @@ protected void OnAuthenticateRequest(object sender, EventArgs e)

var command = CommandFactory.GetCommand(commandName);
var commandResult = command.Run(
new HttpRequestWrapper(application.Request).ToHttpRequestData(),
new HttpRequestWrapper(application.Request).ToHttpRequestData(Options.SPOptions.ModulePath),
Options);

if (!commandResult.HandledResult)
{
commandResult.SignInOrOutSessionAuthenticationModule();
commandResult.Apply(
new HttpResponseWrapper(application.Response),
Options.Notifications.EmitSameSiteNone(application.Request.UserAgent));
Options.Notifications.EmitSameSiteNone(application.Request.UserAgent),
Options.SPOptions.ModulePath);
}
}
}
Expand Down
23 changes: 16 additions & 7 deletions Sustainsys.Saml2.Mvc/Saml2Controller.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,18 @@ public class Saml2Controller : Controller
public ActionResult SignIn()
{
var result = CommandFactory.GetCommand(CommandFactory.SignInCommandName).Run(
Request.ToHttpRequestData(),
Request.ToHttpRequestData(Options.SPOptions.ModulePath),
Options);

if(result.HandledResult)
{
throw new NotSupportedException("The MVC controller doesn't support setting CommandResult.HandledResult.");
}

result.ApplyCookies(Response, Options.Notifications.EmitSameSiteNone(Request.UserAgent));
result.ApplyCookies(
Response,
Options.Notifications.EmitSameSiteNone(Request.UserAgent),
options.SPOptions.ModulePath);
return result.ToActionResult();
}

Expand All @@ -67,7 +70,7 @@ public ActionResult SignIn()
public ActionResult Acs()
{
var result = CommandFactory.GetCommand(CommandFactory.AcsCommandName).Run(
Request.ToHttpRequestData(),
Request.ToHttpRequestData(Options.SPOptions.ModulePath),
Options);

if(result.HandledResult)
Expand All @@ -76,7 +79,10 @@ public ActionResult Acs()
}

result.SignInOrOutSessionAuthenticationModule();
result.ApplyCookies(Response, Options.Notifications.EmitSameSiteNone(Request.UserAgent));
result.ApplyCookies(
Response,
Options.Notifications.EmitSameSiteNone(Request.UserAgent),
options.SPOptions.ModulePath);
return result.ToActionResult();
}

Expand All @@ -89,7 +95,7 @@ public ActionResult Acs()
public ActionResult Index()
{
var result = CommandFactory.GetCommand(CommandFactory.MetadataCommand).Run(
Request.ToHttpRequestData(),
Request.ToHttpRequestData(Options.SPOptions.ModulePath),
Options);

result.ApplyHeaders(Response);
Expand All @@ -112,15 +118,18 @@ public ActionResult Index()
public ActionResult Logout()
{
var result = CommandFactory.GetCommand(CommandFactory.LogoutCommandName)
.Run(Request.ToHttpRequestData(), Options);
.Run(Request.ToHttpRequestData(Options.SPOptions.ModulePath), Options);

if (result.HandledResult)
{
throw new NotSupportedException("The MVC controller doesn't support setting CommandResult.HandledResult.");
}

result.SignInOrOutSessionAuthenticationModule();
result.ApplyCookies(Response, Options.Notifications.EmitSameSiteNone(Request.UserAgent));
result.ApplyCookies(
Response,
Options.Notifications.EmitSameSiteNone(Request.UserAgent),
options.SPOptions.ModulePath);
return result.ToActionResult();
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sustainsys.Saml2.Owin/Saml2AuthenticationMiddleware.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class Saml2AuthenticationMiddleware
}

options.DataProtector = app.CreateDataProtector(
typeof(Saml2AuthenticationMiddleware).FullName);
typeof(Saml2AuthenticationMiddleware).FullName, options.SPOptions.ModulePath);

if(options.SPOptions.Logger == null)
{
Expand Down
2 changes: 1 addition & 1 deletion Sustainsys.Saml2.StubIdp/Controllers/HomeController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private void ReadCustomIdpConfig(Guid? idpId, HomePageModel model)

private bool HandleReceivedAuthnReqest(HomePageModel model)
{
var requestData = Request.ToHttpRequestData(true);
var requestData = Request.ToHttpRequestData(true, null);
var binding = Saml2Binding.Get(requestData);
if (binding != null)
{
Expand Down
2 changes: 1 addition & 1 deletion Sustainsys.Saml2.StubIdp/Controllers/LogoutController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class LogoutController : Controller
{
public ActionResult Index()
{
var requestData = Request.ToHttpRequestData(true);
var requestData = Request.ToHttpRequestData(true, null);

var binding = Saml2Binding.Get(requestData);

Expand Down
4 changes: 2 additions & 2 deletions Tests/HttpModule.Tests/CommandResultHttpSharedTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public partial class CommandResultHttpTests
public void CommandResultHttp_ApplyCookies_NullCheck_CommandResult()
{
((CommandResult)null)
.Invoking(cr => cr.ApplyCookies(Substitute.For<HttpResponseBase>(), true))
.Invoking(cr => cr.ApplyCookies(Substitute.For<HttpResponseBase>(), true, "/saml2"))
.Should().Throw<ArgumentNullException>()
.And.ParamName.Should().Be("commandResult");
}
Expand All @@ -24,7 +24,7 @@ public void CommandResultHttp_ApplyCookies_NullCheck_CommandResult()
public void CommandResultHttp_ApplyCookies_NullCheck_Response()
{
new CommandResult()
.Invoking(cr => cr.ApplyCookies(null, true))
.Invoking(cr => cr.ApplyCookies(null, true, "/saml2"))
.Should().Throw<ArgumentNullException>()
.And.ParamName.Should().Be("response");
}
Expand Down
26 changes: 13 additions & 13 deletions Tests/HttpModule.Tests/CommandResultHttpTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public partial class CommandResultHttpTests
[TestMethod]
public void CommandResultHttp_Apply_ChecksResponseNull()
{
Action a = () => new CommandResult().Apply(null, true);
Action a = () => new CommandResult().Apply(null, true, "/saml2");

a.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("response");
}
Expand All @@ -27,7 +27,7 @@ public void CommandResultHttp_Apply_ChecksResponseNull()
public void CommandResultHttp_Apply_NullShouldThrow()
{
CommandResult obj = null;
Action a = () => obj.Apply(null, true);
Action a = () => obj.Apply(null, true, "/saml2");
a.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("commandResult");
}

Expand All @@ -39,7 +39,7 @@ public void CommandResultHttp_Apply_HttpStatusCode()
new CommandResult()
{
HttpStatusCode = HttpStatusCode.PaymentRequired
}.Apply(response, true);
}.Apply(response, true, "/saml2");

response.Received().StatusCode = (int)HttpStatusCode.PaymentRequired;
}
Expand All @@ -58,7 +58,7 @@ public void CommandResultHttp_Apply_SetsCookie()
null,
null,
null)
}.Apply(response, true);
}.Apply(response, true, "/saml2");

response.Received().SetCookie(
Arg.Is<HttpCookie>(c =>
Expand All @@ -83,7 +83,7 @@ public void CommandResultHttp_Apply_SetsCookie_NoSameSite()
null,
null,
null)
}.Apply(response, false);
}.Apply(response, false, "/saml2");

response.Received().SetCookie(
Arg.Is<HttpCookie>(c => c.SameSite == (SameSiteMode)(-1)));
Expand All @@ -104,7 +104,7 @@ public void CommandResultHttp_Apply_ClearsCookie()
new CommandResult()
{
ClearCookieName = "CookieName",
}.Apply(response, true);
}.Apply(response, true, "/saml2");

response.Received().SetCookie(
Arg.Is<HttpCookie>(c =>
Expand All @@ -122,7 +122,7 @@ public void CommandResultHttp_Apply_ClearsCookie_SecureFlag()
{
ClearCookieName = "CookieName",
SetCookieSecureFlag = true
}.Apply(response, true);
}.Apply(response, true, "/saml2");

response.Received().SetCookie(
Arg.Is<HttpCookie>(c =>
Expand All @@ -141,7 +141,7 @@ public void CommandResultHttp_Apply_Cacheability()
new CommandResult()
{
Cacheability = Cacheability.ServerAndNoCache
}.Apply(response, true);
}.Apply(response, true, "/saml2");

cache.Received().SetCacheability(HttpCacheability.ServerAndNoCache);
}
Expand All @@ -156,7 +156,7 @@ public void CommandResultHttp_Apply_HandleRedirect()
{
Location = new Uri(redirectTarget),
HttpStatusCode = HttpStatusCode.SeeOther
}.Apply(response, true);
}.Apply(response, true, "/saml2");

response.Received().Redirect(redirectTarget);
}
Expand All @@ -170,7 +170,7 @@ public void CommandResultHttp_Apply_ThrowsOnMissingLocation()
new CommandResult()
{
HttpStatusCode = HttpStatusCode.SeeOther
}.Apply(response, true);
}.Apply(response, true, "/saml2");

a.Should().Throw<InvalidOperationException>();
}
Expand All @@ -184,7 +184,7 @@ public void CommandResultHttp_Apply_ThrowsOnLocationWithoutRedirect()
new CommandResult()
{
Location = new Uri("http://example.com")
}.Apply(response, true);
}.Apply(response, true, "/saml2");

a.Should().Throw<InvalidOperationException>();
}
Expand All @@ -200,7 +200,7 @@ public void CommandResultHttp_Apply_Content()
ContentType = "text"
};

commandResult.Apply(response, true);
commandResult.Apply(response, true, "/saml2");

response.Received().ContentType = "text";
response.Received().Write("Some Content!");
Expand All @@ -215,7 +215,7 @@ public void CommandResultHttp_Apply_Headers()

commandResult.Headers.Add("header", "value");

commandResult.Apply(response, true);
commandResult.Apply(response, true, "/saml2");

response.Received().AddHeader("header", "value");
}
Expand Down
Loading

0 comments on commit d9e4ff8

Please sign in to comment.