Double check emulator path when skipping endorsement check #6704

boydc2014 · 2023-11-06T05:37:07Z

Fixes a potential security risk. #minor

Description

This PR ensures we skip endorsement check only when we are certain about this is emulator path, instead of assuming it is emulator based on the endorsement list get from open id endpoint.

Specific Changes

Add a double check after skip the endorsement check
Add the check afterwards instead of doing a pre-check is to minimize the cost and change size

Testing

Tested locally with positive and negative tests

tracyboehrer · 2023-11-06T13:58:29Z

This is failing on:

Error Message:
at System.IdentityModel.Tokens.Jwt.JwtSecurityTokenHandler.ValidateSignature(String token, TokenValidationParameters validationParameters)
at System.IdentityModel.Tokens.Jwt.JwtSecurityTokenHandler.ValidateToken(String token, TokenValidationParameters validationParameters, SecurityToken& validatedToken)
at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.ValidateTokenAsync(String jwtToken, String channelId, String[] requiredEndorsements) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs:line 259
at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String scheme, String parameter, String channelId, String[] requiredEndorsements) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs:line 197
at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String authorizationHeader, String channelId, String[] requiredEndorsements) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs:line 150
at Microsoft.Bot.Connector.Authentication.EmulatorValidation.AuthenticateEmulatorToken(String authHeader, ICredentialProvider credentials, IChannelProvider channelProvider, HttpClient httpClient, String channelId, AuthenticationConfiguration authConfig) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\EmulatorValidation.cs:line 143
at Microsoft.Bot.Connector.Authentication.JwtTokenValidation.AuthenticateTokenAsync(String authHeader, ICredentialProvider credentials, IChannelProvider channelProvider, String channelId, AuthenticationConfiguration authConfig, String serviceUrl, HttpClient httpClient) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenValidation.cs:line 252
at Microsoft.Bot.Connector.Authentication.JwtTokenValidation.ValidateAuthHeader(String authHeader, ICredentialProvider credentials, IChannelProvider channelProvider, String channelId, AuthenticationConfiguration authConfig, String serviceUrl, HttpClient httpClient) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenValidation.cs:line 138
at Microsoft.Bot.Connector.Authentication.JwtTokenValidation.AuthenticateRequest(IActivity activity, String authHeader, ICredentialProvider credentials, IChannelProvider provider, AuthenticationConfiguration authConfig, HttpClient httpClient) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenValidation.cs:line 86
at Microsoft.Bot.Connector.Authentication.BuiltinBotFrameworkAuthentication.AuthenticateRequestAsync(Activity activity, String authHeader, CancellationToken cancellationToken) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\BuiltinBotFrameworkAuthentication.cs:line 95
at Microsoft.Bot.Builder.CloudAdapterBase.ProcessActivityAsync(String authHeader, Activity activity, BotCallbackHandler callback, CancellationToken cancellationToken) in D:\a\1\s\libraries\Microsoft.Bot.Builder\CloudAdapterBase.cs:line 298
at Microsoft.Bot.Builder.Integration.AspNet.Core.CloudAdapter.ProcessAsync(HttpRequest httpRequest, HttpResponse httpResponse, IBot bot, CancellationToken cancellationToken) in D:\a\1\s\libraries\integration\Microsoft.Bot.Builder.Integration.AspNet.Core\CloudAdapter.cs:line 105
at Microsoft.Bot.Builder.Integration.AspNet.Core.Tests.CloudAdapterTests.ExpiredTokenShouldThrowUnauthorizedAccessException() in D:\a\1\s\tests\integration\Microsoft.Bot.Builder.Integration.AspNet.Core.Tests\CloudAdapterTests.cs:line 846

tracyboehrer · 2023-11-06T14:29:37Z

Scratch that. This one:

[xUnit.net 00:00:18.58] Microsoft.Bot.Connector.Tests.Authentication.JwtTokenExtractorTests.JwtTokenExtractor_WithValidCert_ShouldNotAllowCertSigningKey [FAIL]
[xUnit.net 00:00:18.58] System.UnauthorizedAccessException : Could not validate endorsement key.
[xUnit.net 00:00:18.58] Stack Trace:
[xUnit.net 00:00:18.58] D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs(295,0): at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.ValidateTokenAsync(String jwtToken, String channelId, String[] requiredEndorsements)
[xUnit.net 00:00:18.58] D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs(197,0): at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String scheme, String parameter, String channelId, String[] requiredEndorsements)
[xUnit.net 00:00:18.58] D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs(150,0): at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String authorizationHeader, String channelId, String[] requiredEndorsements)
[xUnit.net 00:00:18.58] D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs(130,0): at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String authorizationHeader, String channelId)
[xUnit.net 00:00:18.58] D:\a\1\s\tests\Microsoft.Bot.Connector.Tests\Authentication\JwtTokenExtractorTests.cs(88,0): at Microsoft.Bot.Connector.Tests.Authentication.JwtTokenExtractorTests.JwtTokenExtractor_WithValidCert_ShouldNotAllowCertSigningKey()
[xUnit.net 00:00:18.58] --- End of stack trace from previous location ---
Failed Microsoft.Bot.Connector.Tests.Authentication.JwtTokenExtractorTests.JwtTokenExtractor_WithValidCert_ShouldNotAllowCertSigningKey [503 ms]
Error Message:
System.UnauthorizedAccessException : Could not validate endorsement key.
Stack Trace:
at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.ValidateTokenAsync(String jwtToken, String channelId, String[] requiredEndorsements) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs:line 295
at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String scheme, String parameter, String channelId, String[] requiredEndorsements) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs:line 197
at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String authorizationHeader, String channelId, String[] requiredEndorsements) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs:line 150
at Microsoft.Bot.Connector.Authentication.JwtTokenExtractor.GetIdentityAsync(String authorizationHeader, String channelId) in D:\a\1\s\libraries\Microsoft.Bot.Connector\Authentication\JwtTokenExtractor.cs:line 130
at Microsoft.Bot.Connector.Tests.Authentication.JwtTokenExtractorTests.JwtTokenExtractor_WithValidCert_ShouldNotAllowCertSigningKey() in D:\a\1\s\tests\Microsoft.Bot.Connector.Tests\Authentication\JwtTokenExtractorTests.cs:line 88

tracyboehrer · 2023-11-06T14:34:39Z

I believe the test should be:

public async Task JwtTokenExtractor_WithValidCert_ShouldNotAllowCertSigningKey()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
var now = DateTimeOffset.UtcNow;
var cn = "test.cert.botframework.com";

    // Create valid self-signed certificate
    var cert = CreateSelfSignedCertificate(cn, from: now.AddDays(-10), to: now.AddDays(9));

    // Build token extractor and use it to validate a token created from the cert
    await Assert.ThrowsAnyAsync<UnauthorizedAccessException>(() => BuildExtractorAndValidateToken(cert));

    DeleteKeyContainer(cn);
}

}

tracyboehrer · 2024-02-01T21:01:06Z

@boydc2014 Is this still a thing?

coveralls · 2024-04-23T05:57:52Z

Pull Request Test Coverage Report for Build 388172

Details

0 of 0 changed or added relevant lines in 0 files are covered.
20 unchanged lines in 6 files lost coverage.
Overall coverage decreased (-0.01%) to 78.159%

Files with Coverage Reduction	New Missed Lines	%
/libraries/integration/Microsoft.Bot.Builder.Integration.AspNet.Core/ServiceCollectionExtensions.cs	1	93.55%
/libraries/AdaptiveExpressions/BuiltinFunctions/GetPreviousViableTime.cs	1	90.91%
/libraries/Microsoft.Bot.Streaming/Payloads/StreamManager.cs	1	90.0%
/libraries/AdaptiveExpressions/BuiltinFunctions/GetNextViableTime.cs	1	90.91%
/libraries/Microsoft.Bot.Connector.Streaming/Session/StreamingSession.cs	3	90.09%
/libraries/Microsoft.Bot.Connector/Authentication/JwtTokenExtractor.cs	13	47.66%

Totals
Change from base Build 387708:	-0.01%
Covered Lines:	26184
Relevant Lines:	33501

💛 - Coveralls

BruceHaley · 2024-04-23T06:01:57Z

✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll

…#6704) * Skip endorsement check based on explicit token validation * fix styles * Fixed failing test which now expects an Unauthorized exception --------- Co-authored-by: Rakesh <[email protected]>

boydc2014 added 2 commits November 6, 2023 08:21

Skip endorsement check based on explicit token validation

2a00709

fix styles

5e0cb76

boydc2014 requested a review from a team as a code owner November 6, 2023 05:37

tracyboehrer added the Automation: Parity with js The PR needs to be ported to JS label Nov 6, 2023

tracyboehrer approved these changes Nov 6, 2023

View reviewed changes

rakeshchandran8182 added 2 commits April 23, 2024 10:59

Merge branch 'main' into donglei/fix-endorsement-check

50dfe2e

Fixed failing test which now expects an Unauthorized exception

c3fcebc

boydc2014 merged commit f5cee3e into main Apr 23, 2024
10 of 11 checks passed

boydc2014 deleted the donglei/fix-endorsement-check branch April 23, 2024 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Double check emulator path when skipping endorsement check #6704

Double check emulator path when skipping endorsement check #6704

boydc2014 commented Nov 6, 2023 •

edited

Loading

tracyboehrer commented Nov 6, 2023 •

edited

Loading

tracyboehrer commented Nov 6, 2023

tracyboehrer commented Nov 6, 2023

tracyboehrer commented Feb 1, 2024

coveralls commented Apr 23, 2024

BruceHaley commented Apr 23, 2024

Double check emulator path when skipping endorsement check #6704

Double check emulator path when skipping endorsement check #6704

Conversation

boydc2014 commented Nov 6, 2023 • edited Loading

Description

Specific Changes

Testing

tracyboehrer commented Nov 6, 2023 • edited Loading

tracyboehrer commented Nov 6, 2023

tracyboehrer commented Nov 6, 2023

tracyboehrer commented Feb 1, 2024

coveralls commented Apr 23, 2024

Pull Request Test Coverage Report for Build 388172

Details

💛 - Coveralls

BruceHaley commented Apr 23, 2024

boydc2014 commented Nov 6, 2023 •

edited

Loading

tracyboehrer commented Nov 6, 2023 •

edited

Loading