Skip to content

Commit e986e22

Browse files
George KrecharGeorge Krechar
George Krechar
authored and
George Krechar
committed
Merged PR 10198: Don't resolve jku claim by default
#### AI-Generated Description This pull request introduces the following changes: - It adds a new constructor for the `SignedHttpRequestHandler` class that sets the default timeout for the `_defaultHttpClient` field to 10 seconds. - It changes the `_defaultHttpClient` field from private to internal for testing purposes. - It adds a new property `AllowResolvingPopKeyFromJku` to the `SignedHttpRequestValidationParameters` class that indicates whether PoP key can be resolved from the 'jku' claim. - It adds a new property `AllowedDomainsForJkuRetrieval` to the `SignedHttpRequestValidationParameters` class that specifies a list of allowed domains for 'jku' claim retrieval. - It adds logic to the `ResolvePopKeyFromJkuAsync` method in the `SignedHttpRequestHandler` class to check the `AllowResolvingPopKeyFromJku` and `AllowedDomainsForJkuRetrieval` properties before resolving a PoP key from the 'jku' claim. - It adds a new method `IsJkuUriInListOfAllowedDomains` to the `SignedHttpRequestHandler` class that checks if a given 'jku' URI belongs to one of the allowed domains. - It adds new unit tests for the `SignedHttpRequestHandler` constructor, the `IsJkuUriInListOfAllowedDomains` method, and the `ResolvePopKeyFromJkuAsync` method in the `SignedHttpRequestHandler` class. - It adds new unit tests for the `SignedHttpRequestCtorTests` and the `SignedHttpRequestUtilityTests` classes. - It adds new exception messages to the `LogMessages` class related to the 'jku' claim validation.
1 parent a62cd3b commit e986e22

File tree

5 files changed

+178
-5
lines changed

5 files changed

+178
-5
lines changed

‎src/Microsoft.IdentityModel.Protocols.SignedHttpRequest/LogMessages.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,5 +46,7 @@ internal static class LogMessages
4646
public const string IDX23034 = "IDX23034: Signed http request signature validation failed. SignedHttpRequest: '{0}'";
4747
public const string IDX23035 = "IDX23035: Unable to resolve a PoP key from the 'jku' claim. Multiple keys are found in the referenced JWK Set document and the 'cnf' claim doesn't contain a 'kid' value.";
4848
public const string IDX23036 = "IDX23036: Signed http request nonce validation failed. Exceptions caught: '{0}'.";
49+
public const string IDX23037 = "IDX23037: Resolving a PoP key from the 'jku' claim is not allowed. To allow it, set AllowResolvingPopKeyFromJku property on SignedHttpRequestValidationParameters to true and provide a list of trusted domains via AllowedDomainsForJkuRetrieval.";
50+
public const string IDX23038 = "IDX23038: Resolving a PoP key from the 'jku' claim is not allowed as '{0}' is not present in the list of allowed domains for 'jku' retrieval: '{1}'. If '{0}' belongs to a trusted domain, add it to AllowedDomainsForJkuRetrieval property on SignedHttpRequestValidationParameters.";
4951
}
5052
}

‎src/Microsoft.IdentityModel.Protocols.SignedHttpRequest/SignedHttpRequestHandler.cs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,15 @@ public class SignedHttpRequestHandler
3737
};
3838

3939
private readonly Uri _baseUriHelper = new Uri("http://localhost", UriKind.Absolute);
40-
private readonly HttpClient _defaultHttpClient = new HttpClient();
40+
internal readonly HttpClient _defaultHttpClient = new HttpClient();
41+
42+
/// <summary>
43+
/// Initializes a new instance of <see cref="SignedHttpRequestHandler"/>.
44+
/// </summary>
45+
public SignedHttpRequestHandler()
46+
{
47+
_defaultHttpClient.Timeout = TimeSpan.FromSeconds(10);
48+
}
4149

4250
#region SignedHttpRequest creation
4351
/// <summary>
@@ -1121,6 +1129,17 @@ internal virtual async Task<SecurityKey> ResolvePopKeyFromJweAsync(string jwe, S
11211129
/// <returns>A resolved PoP <see cref="SecurityKey"/>.</returns>
11221130
internal virtual async Task<SecurityKey> ResolvePopKeyFromJkuAsync(string jkuSetUrl, Cnf cnf, SignedHttpRequestValidationContext signedHttpRequestValidationContext, CancellationToken cancellationToken)
11231131
{
1132+
if (signedHttpRequestValidationContext.SignedHttpRequestValidationParameters.AllowResolvingPopKeyFromJku == false)
1133+
{
1134+
throw LogHelper.LogExceptionMessage(new SignedHttpRequestInvalidPopKeyException(LogHelper.FormatInvariant(LogMessages.IDX23037)));
1135+
}
1136+
1137+
if (!IsJkuUriInListOfAllowedDomains(jkuSetUrl, signedHttpRequestValidationContext))
1138+
{
1139+
var allowedDomains = string.Join(", ", signedHttpRequestValidationContext.SignedHttpRequestValidationParameters.AllowedDomainsForJkuRetrieval ?? new List<string>());
1140+
throw LogHelper.LogExceptionMessage(new SignedHttpRequestInvalidPopKeyException(LogHelper.FormatInvariant(LogMessages.IDX23038, jkuSetUrl, allowedDomains)));
1141+
}
1142+
11241143
var popKeys = await GetPopKeysFromJkuAsync(jkuSetUrl, signedHttpRequestValidationContext, cancellationToken).ConfigureAwait(false);
11251144

11261145
if (popKeys == null || popKeys.Count == 0)
@@ -1281,6 +1300,18 @@ private Uri EnsureAbsoluteUri(Uri uri)
12811300
}
12821301
}
12831302

1303+
private static bool IsJkuUriInListOfAllowedDomains(string jkuSetUrl, SignedHttpRequestValidationContext signedHttpRequestValidationContext)
1304+
{
1305+
if (string.IsNullOrEmpty(jkuSetUrl))
1306+
return false;
1307+
1308+
if (signedHttpRequestValidationContext.SignedHttpRequestValidationParameters.AllowedDomainsForJkuRetrieval.Count == 0)
1309+
return false;
1310+
1311+
var uri = new Uri(jkuSetUrl, UriKind.RelativeOrAbsolute);
1312+
return signedHttpRequestValidationContext.SignedHttpRequestValidationParameters.AllowedDomainsForJkuRetrieval.Any(domain => uri.Host.EndsWith(domain, StringComparison.OrdinalIgnoreCase));
1313+
}
1314+
12841315
/// <summary>
12851316
/// Sanitizes the query params to comply with the specification.
12861317
/// </summary>

‎src/Microsoft.IdentityModel.Protocols.SignedHttpRequest/SignedHttpRequestValidationParameters.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ public class SignedHttpRequestValidationParameters
8484
{
8585
private TimeSpan _signedHttpRequestLifetime = DefaultSignedHttpRequestLifetime;
8686
private TokenHandler _tokenHandler = new JsonWebTokenHandler();
87+
private ICollection<string> _allowedDomainsForJkuRetrieval;
8788

8889
/// <summary>
8990
/// Gets or sets a value indicating whether the unsigned query parameters are accepted or not.
@@ -97,6 +98,26 @@ public class SignedHttpRequestValidationParameters
9798
/// <remarks>https://datatracker.ietf.org/doc/html/draft-ietf-oauth-signed-http-request-03#section-5.1</remarks>
9899
public bool AcceptUnsignedHeaders { get; set; } = true;
99100

101+
/// <summary>
102+
/// Gets or sets a value indicating whether PoP key can be resolved from 'jku' claim.
103+
/// If you set this property to true, you must set values in <see cref="AllowedDomainsForJkuRetrieval"/>.
104+
/// </summary>
105+
/// <remarks>https://datatracker.ietf.org/doc/html/rfc7800#section-3.5</remarks>
106+
public bool AllowResolvingPopKeyFromJku { get; set; } = false;
107+
108+
/// <summary>
109+
/// Gets or sets a list of allowed domains for 'jku' claim retrieval.
110+
/// The domains are not directly compared with the 'jku' claim. Allowed domain would be
111+
/// deemed valid if the host specified in the 'jku' claim ends with the domain value.
112+
/// </summary>
113+
/// <remarks>
114+
/// Domains should be provided in the following format:
115+
/// "contoso.com"
116+
/// "abc.fabrikam.com"
117+
/// </remarks>
118+
public ICollection<string> AllowedDomainsForJkuRetrieval => _allowedDomainsForJkuRetrieval ??
119+
Interlocked.CompareExchange(ref _allowedDomainsForJkuRetrieval, new List<string>(), null) ?? _allowedDomainsForJkuRetrieval;
120+
100121
/// <summary>
101122
/// Gets or sets the claims to validate if present.
102123
/// </summary>

‎test/Microsoft.IdentityModel.Protocols.SignedHttpRequest.Tests/PopKeyResolvingTests.cs

Lines changed: 115 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ namespace Microsoft.IdentityModel.Protocols.SignedHttpRequest.Tests
1818
{
1919
public class PopKeyResolvingTests
2020
{
21+
internal const string _defaultJkuDomain = "contoso.com";
22+
2123
[Theory, MemberData(nameof(ResolvePopKeyFromCnfClaimAsyncTheoryData))]
2224
public async Task ResolvePopKeyFromCnfClaimAsync(ResolvePopKeyTheoryData theoryData)
2325
{
@@ -398,7 +400,7 @@ public async Task ResolvePopKeyFromJkuAsync(ResolvePopKeyTheoryData theoryData)
398400
{
399401
var signedHttpRequestValidationContext = theoryData.BuildSignedHttpRequestValidationContext();
400402
var handler = new SignedHttpRequestHandlerPublic();
401-
var popKey = await handler.ResolvePopKeyFromJkuAsync(string.Empty, null, signedHttpRequestValidationContext, CancellationToken.None).ConfigureAwait(false);
403+
var popKey = await handler.ResolvePopKeyFromJkuAsync(theoryData.JkuSetUrl, null, signedHttpRequestValidationContext, CancellationToken.None).ConfigureAwait(false);
402404

403405
if (popKey == null)
404406
context.AddDiff("Resolved Pop key is null.");
@@ -429,6 +431,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuTheoryData
429431
{"mockGetPopKeysFromJkuAsync_return0Keys", null }
430432
}
431433
},
434+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
432435
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), "IDX23031"),
433436
TestId = "InvalidZeroKeysReturned",
434437
},
@@ -441,6 +444,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuTheoryData
441444
{"mockGetPopKeysFromJkuAsync_returnNull", null }
442445
}
443446
},
447+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
444448
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), "IDX23031"),
445449
TestId = "InvalidNullReturned",
446450
},
@@ -453,8 +457,106 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuTheoryData
453457
{"mockGetPopKeysFromJkuAsync_return1Key", null }
454458
}
455459
},
460+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
456461
TestId = "ValidOneKeyReturned",
457462
},
463+
new ResolvePopKeyTheoryData
464+
{
465+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = false },
466+
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), "IDX23037"),
467+
TestId = "JkuTurnedOff",
468+
},
469+
new ResolvePopKeyTheoryData
470+
{
471+
JkuSetUrl = "",
472+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true },
473+
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), string.Format(LogMessages.IDX23038, "" , "")),
474+
TestId = "JkuTurnedOnEmptyUrl"
475+
},
476+
new ResolvePopKeyTheoryData
477+
{
478+
JkuSetUrl = "https://www.contoso.com",
479+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true },
480+
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), string.Format(LogMessages.IDX23038, "https://www.contoso.com" , "")),
481+
TestId = "JkuTurnedOnNullDomains"
482+
},
483+
new ResolvePopKeyTheoryData
484+
{
485+
JkuSetUrl = "https://www.contoso.com",
486+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true, AllowedDomainsForJkuRetrieval = { "contoso1.com", "test.contoso.com" }},
487+
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), string.Format(LogMessages.IDX23038, "https://www.contoso.com" , "contoso1.com, test.contoso.com")),
488+
TestId = "JkuTurnedOnDomainsMissmatch"
489+
},
490+
new ResolvePopKeyTheoryData
491+
{
492+
CallContext = new CallContext()
493+
{
494+
PropertyBag = new Dictionary<string, object>()
495+
{
496+
// to simulate http call and satisfy test requirements
497+
{"mockGetPopKeysFromJkuAsync_return1Key", null }
498+
}
499+
},
500+
JkuSetUrl = "https://www.contoso.com",
501+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true, AllowedDomainsForJkuRetrieval = { ".com" }},
502+
TestId = "JkuTurnedOnTopLevelDomainMatch"
503+
},
504+
new ResolvePopKeyTheoryData
505+
{
506+
CallContext = new CallContext()
507+
{
508+
PropertyBag = new Dictionary<string, object>()
509+
{
510+
// to simulate http call and satisfy test requirements
511+
{"mockGetPopKeysFromJkuAsync_return1Key", null }
512+
}
513+
},
514+
JkuSetUrl = "https://www.contoso.com",
515+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true, AllowedDomainsForJkuRetrieval = { "contoso.com" }},
516+
TestId = "JkuTurnedOnDomainsMatch"
517+
},
518+
new ResolvePopKeyTheoryData
519+
{
520+
CallContext = new CallContext()
521+
{
522+
PropertyBag = new Dictionary<string, object>()
523+
{
524+
// to simulate http call and satisfy test requirements
525+
{"mockGetPopKeysFromJkuAsync_return1Key", null }
526+
}
527+
},
528+
JkuSetUrl = "https://www.contoso.com",
529+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true, AllowedDomainsForJkuRetrieval = { "Contoso.com" }},
530+
TestId = "JkuTurnedOnDomainsMatchCaseInsensitive"
531+
},
532+
new ResolvePopKeyTheoryData
533+
{
534+
CallContext = new CallContext()
535+
{
536+
PropertyBag = new Dictionary<string, object>()
537+
{
538+
// to simulate http call and satisfy test requirements
539+
{"mockGetPopKeysFromJkuAsync_return1Key", null }
540+
}
541+
},
542+
JkuSetUrl = "https://contoso.com/mykeys/key/1?test=true",
543+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true, AllowedDomainsForJkuRetrieval = { "Contoso.com" }},
544+
TestId = "JkuTurnedOnUrlWithPathAndQueryParam"
545+
},
546+
new ResolvePopKeyTheoryData
547+
{
548+
CallContext = new CallContext()
549+
{
550+
PropertyBag = new Dictionary<string, object>()
551+
{
552+
// to simulate http call and satisfy test requirements
553+
{"mockGetPopKeysFromJkuAsync_return1Key", null }
554+
}
555+
},
556+
JkuSetUrl = "https://localhost/keys",
557+
SignedHttpRequestValidationParameters = { AllowResolvingPopKeyFromJku = true, AllowedDomainsForJkuRetrieval = { "localhost" }},
558+
TestId = "JkuTurnedOnLocalUrl"
559+
}
458560
};
459561
}
460562
}
@@ -531,7 +633,7 @@ public async Task ResolvePopKeyFromJkuKidAsync(ResolvePopKeyTheoryData theoryDat
531633
var signedHttpRequestValidationContext = theoryData.BuildSignedHttpRequestValidationContext();
532634
var handler = new SignedHttpRequestHandlerPublic();
533635
Cnf cnf = new Cnf { Kid = theoryData.Kid };
534-
var popKey = await handler.ResolvePopKeyFromJkuAsync(string.Empty, cnf, signedHttpRequestValidationContext, CancellationToken.None).ConfigureAwait(false);
636+
var popKey = await handler.ResolvePopKeyFromJkuAsync(theoryData.JkuSetUrl, cnf, signedHttpRequestValidationContext, CancellationToken.None).ConfigureAwait(false);
535637

536638
if (popKey == null)
537639
context.AddDiff("Resolved Pop key is null.");
@@ -562,6 +664,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuKidTheoryD
562664
{"mockGetPopKeysFromJkuAsync_return2Keys", null }
563665
}
564666
},
667+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
565668
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), "IDX23021"),
566669
TestId = "InvalidNoKidMatch",
567670
},
@@ -576,6 +679,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuKidTheoryD
576679
{"mockGetPopKeysFromJkuAsync_return0Keys", null }
577680
}
578681
},
682+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
579683
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), "IDX23031"),
580684
TestId = "InvalidZeroKeysReturned",
581685
},
@@ -589,6 +693,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuKidTheoryD
589693
{"mockGetPopKeysFromJkuAsync_returnNull", null }
590694
}
591695
},
696+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
592697
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), "IDX23031"),
593698
TestId = "InvalidNullReturned",
594699
},
@@ -602,6 +707,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuKidTheoryD
602707
{"mockGetPopKeysFromJkuAsync_return2Keys", null }
603708
}
604709
},
710+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
605711
ExpectedException = new ExpectedException(typeof(SignedHttpRequestInvalidPopKeyException), "IDX23021"),
606712
TestId = "InvalidNoKidMatch",
607713
},
@@ -615,6 +721,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuKidTheoryD
615721
{"mockGetPopKeysFromJkuAsync_return2Keys", null }
616722
}
617723
},
724+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
618725
TestId = "ValidOneKidMatch",
619726
},
620727
new ResolvePopKeyTheoryData
@@ -627,6 +734,7 @@ public static TheoryData<ResolvePopKeyTheoryData> ResolvePopKeyFromJkuKidTheoryD
627734
{"mockGetPopKeysFromJkuAsync_return1Key", null }
628735
}
629736
},
737+
SignedHttpRequestValidationParameters = { AllowedDomainsForJkuRetrieval = { _defaultJkuDomain } },
630738
TestId = "ValidKidMatch",
631739
},
632740
};
@@ -861,6 +969,8 @@ public SignedHttpRequestValidationContext BuildSignedHttpRequestValidationContex
861969
return new SignedHttpRequestValidationContext(SignedHttpRequestToken is JsonWebToken jwt ? jwt.EncodedToken : "dummy", httpRequestData, SignedHttpRequestTestUtils.DefaultTokenValidationParameters, SignedHttpRequestValidationParameters, callContext);
862970
}
863971

972+
internal const string _defaultJkuUri = "https://contoso.com/jku";
973+
864974
internal JObject ConfirmationClaim { get; set; }
865975

866976
public string MethodToCall { get; set; }
@@ -881,7 +991,8 @@ public SignedHttpRequestValidationContext BuildSignedHttpRequestValidationContex
881991
ValidateP = true,
882992
ValidateQ = true,
883993
ValidateTs = true,
884-
ValidateU = true
994+
ValidateU = true,
995+
AllowResolvingPopKeyFromJku = true
885996
};
886997

887998
public SigningCredentials SigningCredentials { get; set; } = SignedHttpRequestTestUtils.DefaultSigningCredentials;
@@ -896,7 +1007,7 @@ public SignedHttpRequestValidationContext BuildSignedHttpRequestValidationContex
8961007

8971008
public string Kid { get; set; }
8981009

899-
public string JkuSetUrl { get; set; }
1010+
public string JkuSetUrl { get; set; } = _defaultJkuUri;
9001011

9011012
public int ExpectedNumberOfPopKeysReturned { get; set; }
9021013
}

0 commit comments

Comments
 (0)