Transmit EPK and use as public key during decrypt (#2120)

* Propagate EPK and KID as defined by RFC7518

* Guard new behavior behind an app-compat switch

* Encode EPK as a JSON object and not an escaped string

---------

Co-authored-by: Greg Domzalski <greg@yubico.com>

src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.CreateToken.cs

@@ -13,12 +13,17 @@
 using Microsoft.IdentityModel.Abstractions;
 using Microsoft.IdentityModel.Logging;
 using Microsoft.IdentityModel.Tokens;
+using Microsoft.IdentityModel.Tokens.Json;
 using JsonPrimitives = Microsoft.IdentityModel.Tokens.Json.JsonSerializerPrimitives;
 using TokenLogMessages = Microsoft.IdentityModel.Tokens.LogMessages;
 
 namespace Microsoft.IdentityModel.JsonWebTokens
 {
-    /// <remarks>This partial class contains methods and logic related to the creation of tokens.</remarks>
+    /// <summary>
+    /// A <see cref="SecurityTokenHandler"/> designed for creating and validating Json Web Tokens.
+    /// See: https://datatracker.ietf.org/doc/html/rfc7519 and http://www.rfc-editor.org/info/rfc7515.
+    /// </summary>
+    /// <remarks>This partial class is focused on TokenCreation.</remarks>
     public partial class JsonWebTokenHandler : TokenHandler
     {
         /// <summary>
@@ -1055,8 +1060,32 @@ internal static byte[] WriteJweHeader(
                     writer.WriteString(JwtHeaderUtf8Bytes.Alg, encryptingCredentials.Alg);
                     writer.WriteString(JwtHeaderUtf8Bytes.Enc, encryptingCredentials.Enc);
 
-                    if (encryptingCredentials.Key.KeyId != null)
-                        writer.WriteString(JwtHeaderUtf8Bytes.Kid, encryptingCredentials.Key.KeyId);
+                    // Since developers may have already worked around this issue, implicitly taking a dependency on the
+                    // old behavior, we guard the new behavior behind an AppContext switch. The new/RFC-conforming behavior
+                    // is treated as opt-in. When the library is at the point where it is able to make breaking changes
+                    // (such as the next major version update) we should consider whether or not this app-compat switch
+                    // needs to be maintained.
+                    if (AppContext.TryGetSwitch(AppCompatSwitches.UseRfcDefinitionOfEpkAndKid, out bool isEnabled) && isEnabled)
+                    {
+                        if (encryptingCredentials.KeyExchangePublicKey.KeyId != null)
+                            writer.WriteString(JwtHeaderUtf8Bytes.Kid, encryptingCredentials.KeyExchangePublicKey.KeyId);
+
+                        if (SupportedAlgorithms.EcdsaWrapAlgorithms.Contains(encryptingCredentials.Alg))
+                        {
+                            writer.WritePropertyName(JwtHeaderUtf8Bytes.Epk);
+                            string publicJwk = JsonWebKeyConverter.ConvertFromSecurityKey(encryptingCredentials.Key).RepresentAsAsymmetricPublicJwk();
+#if NET6_0_OR_GREATER
+                            writer.WriteRawValue(publicJwk);
+#else
+                            JsonPrimitives.WriteAsJsonElement(ref writer, publicJwk);
+#endif
+                        }
+                    }
+                    else
+                    {
+                        if (encryptingCredentials.Key.KeyId != null)
+                            writer.WriteString(JwtHeaderUtf8Bytes.Kid, encryptingCredentials.Key.KeyId);
+                    }
 
                     if (!string.IsNullOrEmpty(compressionAlgorithm))
                         writer.WriteString(JwtHeaderUtf8Bytes.Zip, compressionAlgorithm);
@@ -1360,10 +1389,27 @@ internal IEnumerable<SecurityKey> GetContentEncryptionKeys(JsonWebToken jwtToken
 #if NET472 || NET6_0_OR_GREATER
                     if (SupportedAlgorithms.EcdsaWrapAlgorithms.Contains(jwtToken.Alg))
                     {
-                        // on decryption we get the public key from the EPK value see: https://datatracker.ietf.org/doc/html/rfc7518#appendix-C
+                        ECDsaSecurityKey publicKey;
+
+                        // Since developers may have already worked around this issue, implicitly taking a dependency on the
+                        // old behavior, we guard the new behavior behind an AppContext switch. The new/RFC-conforming behavior
+                        // is treated as opt-in. When the library is at the point where it is able to make breaking changes
+                        // (such as the next major version update) we should consider whether or not this app-compat switch
+                        // needs to be maintained.
+                        if (AppContext.TryGetSwitch(AppCompatSwitches.UseRfcDefinitionOfEpkAndKid, out bool isEnabled) && isEnabled)
+                        {
+                            // on decryption we get the public key from the EPK value see: https://datatracker.ietf.org/doc/html/rfc7518#appendix-C
+                            jwtToken.TryGetHeaderValue(JwtHeaderParameterNames.Epk, out string epk);
+                            publicKey = new ECDsaSecurityKey(new JsonWebKey(epk), false);
+                        }
+                        else
+                        {
+                            publicKey = validationParameters.TokenDecryptionKey as ECDsaSecurityKey;
+                        }
+
                         var ecdhKeyExchangeProvider = new EcdhKeyExchangeProvider(
                             key as ECDsaSecurityKey,
-                            validationParameters.TokenDecryptionKey as ECDsaSecurityKey,
+                            publicKey,
                             jwtToken.Alg,
                             jwtToken.Enc);
                         jwtToken.TryGetHeaderValue(JwtHeaderParameterNames.Apu, out string apu);

src/Microsoft.IdentityModel.Tokens/AppCompatSwitches.cs

@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+namespace Microsoft.IdentityModel.Tokens;
+
+/// <summary>
+/// Identifiers used for switching between different app compat behaviors within the Microsoft.IdentityModel libraries.
+/// </summary>
+/// <remarks>
+/// The Microsoft.IdentityModel libraries use <see cref="System.AppContext" /> to turn on or off certain API behavioral
+/// changes that might have an effect on application compatibility. This class defines the set of switches that are
+/// available to modify library behavior. Application compatibility is favored as the default - so if your application
+/// needs to rely on the new behavior, you will need to enable the switch manually. Setting a switch's value can be
+/// done programmatically through the <see cref="System.AppContext.SetSwitch" /> method, or through other means such as
+/// setting it through MSBuild, app configuration, or registry settings. These alternate methods are described in the
+/// <see cref="System.AppContext.SetSwitch" /> documentation.
+/// </remarks>
+public static class AppCompatSwitches
+{
+    /// <summary>
+    /// Uses <see cref="EncryptingCredentials.KeyExchangePublicKey"/> for the token's `kid` header parameter. When using
+    /// ECDH-based key wrap algorithms the public key portion of <see cref="EncryptingCredentials.Key" /> is also written
+    /// to the token's `epk` header parameter.
+    /// </summary>
+    /// <remarks>
+    /// Enabling this switch improves the library's conformance to RFC 7518 with regards to how the header values for
+    /// `kid` and `epk` are set in ECDH key wrap scenarios. The previous behavior erroneously used key ID of
+    /// <see cref="EncryptingCredentials.Key"/> as the `kid` parameter, and did not automatically set `epk` as the spec
+    /// defines. This switch enables the intended behavior where <see cref="EncryptingCredentials.KeyExchangePublicKey"/>
+    /// is used for `kid` and the public portion of <see cref="EncryptingCredentials.Key"/> is used for `epk`.
+    /// </remarks>
+    public const string UseRfcDefinitionOfEpkAndKid = "Switch.Microsoft.IdentityModel.UseRfcDefinitionOfEpkAndKid";
+}

src/System.IdentityModel.Tokens.Jwt/JwtHeader.cs

@@ -15,7 +15,7 @@
 namespace System.IdentityModel.Tokens.Jwt
 {
     /// <summary>
-    /// Initializes a new instance of <see cref="JwtHeader"/> which contains JSON objects representing the cryptographic operations applied to the JWT and optionally any additional properties of the JWT. 
+    /// Initializes a new instance of <see cref="JwtHeader"/> which contains JSON objects representing the cryptographic operations applied to the JWT and optionally any additional properties of the JWT.
     /// The member names within the JWT Header are referred to as Header Parameter Names.
     /// <para>These names MUST be unique and the values must be <see cref="string"/>(s). The corresponding values are referred to as Header Parameter Values.</para>
     /// </summary>
@@ -214,8 +214,25 @@ public JwtHeader(EncryptingCredentials encryptingCredentials, IDictionary<string
             else
                 Enc = encryptingCredentials.Enc;
 
-            if (!string.IsNullOrEmpty(encryptingCredentials.Key.KeyId))
-                Kid = encryptingCredentials.Key.KeyId;
+            // Since developers may have already worked around this issue, implicitly taking a dependency on the
+            // old behavior, we guard the new behavior behind an AppContext switch. The new/RFC-conforming behavior
+            // is treated as opt-in. When the library is at the point where it is able to make breaking changes
+            // (such as the next major version update) we should consider whether or not this app-compat switch
+            // needs to be maintained.
+            if (AppContext.TryGetSwitch(AppCompatSwitches.UseRfcDefinitionOfEpkAndKid, out bool isEnabled) && isEnabled)
+            {
+                if (!string.IsNullOrEmpty(encryptingCredentials.KeyExchangePublicKey.KeyId))
+                    Kid = encryptingCredentials.KeyExchangePublicKey.KeyId;
+
+                // Parameter MUST be present [...] when [key agreement] algorithms are used: https://www.rfc-editor.org/rfc/rfc7518#section-4.6.1.1
+                if (SupportedAlgorithms.EcdsaWrapAlgorithms.Contains(encryptingCredentials.Alg))
+                    Add(JwtHeaderParameterNames.Epk, JsonWebKeyConverter.ConvertFromSecurityKey(encryptingCredentials.Key).RepresentAsAsymmetricPublicJwk());
+            }
+            else
+            {
+                if (!string.IsNullOrEmpty(encryptingCredentials.Key.KeyId))
+                    Kid = encryptingCredentials.Key.KeyId;
+            }
 
             if (string.IsNullOrEmpty(tokenType))
                 Typ = JwtConstants.HeaderType;
@@ -346,19 +363,19 @@ public string X5t
                 return GetStandardClaim(JwtHeaderParameterNames.X5t);
             }
         }
-        
+
         /// <summary>
         /// Gets the certificate used to sign the token
         /// </summary>
-        /// <remarks>If the 'x5c' claim is not found, null is returned.</remarks>   
+        /// <remarks>If the 'x5c' claim is not found, null is returned.</remarks>
         public string X5c => GetStandardClaim(JwtHeaderParameterNames.X5c);
 
         /// <summary>
         /// Gets the 'value' of the 'zip' claim { zip, 'value' }.
         /// </summary>
-        /// <remarks>If the 'zip' claim is not found, null is returned.</remarks>   
+        /// <remarks>If the 'zip' claim is not found, null is returned.</remarks>
         public string Zip => GetStandardClaim(JwtHeaderParameterNames.Zip);
-         
+
         /// <summary>
         /// Deserializes Base64UrlEncoded JSON into a <see cref="JwtHeader"/> instance.
         /// </summary>

src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs

@@ -95,7 +95,7 @@ public JwtSecurityTokenHandler()
         }
 
         /// <summary>
-        /// Gets or sets the <see cref="MapInboundClaims"/> property which is used when determining whether or not to map claim types that are extracted when validating a <see cref="JwtSecurityToken"/>. 
+        /// Gets or sets the <see cref="MapInboundClaims"/> property which is used when determining whether or not to map claim types that are extracted when validating a <see cref="JwtSecurityToken"/>.
         /// <para>If this is set to true, the <see cref="Claim.Type"/> is set to the JSON claim 'name' after translating using this mapping. Otherwise, no mapping occurs.</para>
         /// <para>The default value is true.</para>
         /// </summary>
@@ -112,12 +112,12 @@ public bool MapInboundClaims
                 if (!_mapInboundClaims && value && _inboundClaimTypeMap.Count == 0)
                     _inboundClaimTypeMap = new Dictionary<string, string>(DefaultInboundClaimTypeMap);
 
-                _mapInboundClaims = value;            
+                _mapInboundClaims = value;
             }
-        } 
+        }
 
         /// <summary>
-        /// Gets or sets the <see cref="InboundClaimTypeMap"/> which is used when setting the <see cref="Claim.Type"/> for claims in the <see cref="ClaimsPrincipal"/> extracted when validating a <see cref="JwtSecurityToken"/>. 
+        /// Gets or sets the <see cref="InboundClaimTypeMap"/> which is used when setting the <see cref="Claim.Type"/> for claims in the <see cref="ClaimsPrincipal"/> extracted when validating a <see cref="JwtSecurityToken"/>.
         /// <para>The <see cref="Claim.Type"/> is set to the JSON claim 'name' after translating using this mapping.</para>
         /// <para>The default value is ClaimTypeMapping.InboundClaimTypeMap.</para>
         /// </summary>
@@ -823,7 +823,7 @@ public override SecurityToken ReadToken(string token)
         {
             return ReadJwtToken(token);
         }
-        
+
         /// <summary>
         /// Deserializes token with the provided <see cref="TokenValidationParameters"/>.
         /// </summary>
@@ -861,7 +861,7 @@ public override SecurityToken ReadToken(XmlReader reader, TokenValidationParamet
         /// <exception cref="SecurityTokenReplayAddFailedException"><paramref name="token"/> could not be added to the <see cref="TokenValidationParameters.TokenReplayCache"/>.</exception>
         /// <exception cref="SecurityTokenReplayDetectedException"><paramref name="token"/> is found in the cache.</exception>
         /// <returns> A <see cref="ClaimsPrincipal"/> from the JWT. Does not include claims found in the JWT header.</returns>
-        /// <remarks> 
+        /// <remarks>
         /// Many of the exceptions listed above are not thrown directly from this method. See <see cref="Validators"/> to examine the call graph.
         /// </remarks>
         public override ClaimsPrincipal ValidateToken(string token, TokenValidationParameters validationParameters, out SecurityToken validatedToken)
@@ -1483,7 +1483,7 @@ private JwtSecurityToken ValidateSignature(string token, JwtSecurityToken jwtTok
                         (object)exceptionStrings ?? "",
                         jwtToken)));
                 }
-            }         
+            }
 
             throw LogHelper.LogExceptionMessage(new SecurityTokenSignatureKeyNotFoundException(TokenLogMessages.IDX10500));
         }
@@ -1521,7 +1521,7 @@ protected virtual ClaimsIdentity CreateClaimsIdentity(JwtSecurityToken jwtToken,
 
                 actualIssuer = ClaimsIdentity.DefaultIssuer;
             }
-            
+
             return MapInboundClaims ? CreateClaimsIdentityWithMapping(jwtToken, actualIssuer, validationParameters) : CreateClaimsIdentityWithoutMapping(jwtToken, actualIssuer, validationParameters);
         }
 
@@ -1613,8 +1613,8 @@ private ClaimsIdentity CreateClaimsIdentityWithoutMapping(JwtSecurityToken jwtTo
         /// <remarks>If <see cref="ClaimsIdentity.BootstrapContext"/> is not null:
         /// <para>&#160;&#160;If 'type' is 'string', return as string.</para>
         /// <para>&#160;&#160;if 'type' is 'BootstrapContext' and 'BootstrapContext.SecurityToken' is 'JwtSecurityToken'</para>
-        /// <para>&#160;&#160;&#160;&#160;if 'JwtSecurityToken.RawData' != null, return RawData.</para>        
-        /// <para>&#160;&#160;&#160;&#160;else return <see cref="JwtSecurityTokenHandler.WriteToken( SecurityToken )"/>.</para>        
+        /// <para>&#160;&#160;&#160;&#160;if 'JwtSecurityToken.RawData' != null, return RawData.</para>
+        /// <para>&#160;&#160;&#160;&#160;else return <see cref="JwtSecurityTokenHandler.WriteToken( SecurityToken )"/>.</para>
         /// <para>&#160;&#160;if 'BootstrapContext.Token' != null, return 'Token'.</para>
         /// <para>default: <see cref="JwtSecurityTokenHandler.WriteToken(SecurityToken)"/> new ( <see cref="JwtSecurityToken"/>( actor.Claims ).</para>
         /// </remarks>
@@ -1741,7 +1741,7 @@ protected virtual SecurityKey ResolveTokenDecryptionKey(string token, JwtSecurit
 
             if (!string.IsNullOrEmpty(jwtToken.Header.Kid))
             {
-                if (validationParameters.TokenDecryptionKey != null 
+                if (validationParameters.TokenDecryptionKey != null
                     && string.Equals(validationParameters.TokenDecryptionKey.KeyId, jwtToken.Header.Kid, validationParameters.TokenDecryptionKey is X509SecurityKey ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal))
                     return validationParameters.TokenDecryptionKey;
 
@@ -1785,7 +1785,7 @@ protected virtual SecurityKey ResolveTokenDecryptionKey(string token, JwtSecurit
         }
 
         /// <summary>
-        /// Decrypts a JWE and returns the clear text 
+        /// Decrypts a JWE and returns the clear text
         /// </summary>
         /// <param name="jwtToken">the JWE that contains the cypher text.</param>
         /// <param name="validationParameters">contains crypto material.</param>
@@ -1858,10 +1858,27 @@ internal IEnumerable<SecurityKey> GetContentEncryptionKeys(JwtSecurityToken jwtT
 #if NET472 || NET6_0_OR_GREATER
                     if (SupportedAlgorithms.EcdsaWrapAlgorithms.Contains(jwtToken.Header.Alg))
                     {
-                        //// on decryption we get the public key from the EPK value see: https://datatracker.ietf.org/doc/html/rfc7518#appendix-C
+                        ECDsaSecurityKey publicKey;
+
+                        // Since developers may have already worked around this issue, implicitly taking a dependency on the
+                        // old behavior, we guard the new behavior behind an AppContext switch. The new/RFC-conforming behavior
+                        // is treated as opt-in. When the library is at the point where it is able to make breaking changes
+                        // (such as the next major version update) we should consider whether or not this app-compat switch
+                        // needs to be maintained.
+                        if (AppContext.TryGetSwitch(AppCompatSwitches.UseRfcDefinitionOfEpkAndKid, out bool isEnabled) && isEnabled)
+                        {
+                            //// on decryption we get the public key from the EPK value see: https://datatracker.ietf.org/doc/html/rfc7518#appendix-C
+                            string epk = jwtToken.Header.GetStandardClaim(JwtHeaderParameterNames.Epk);
+                            publicKey = new ECDsaSecurityKey(new JsonWebKey(epk), false);
+                        }
+                        else
+                        {
+                            publicKey = validationParameters.TokenDecryptionKey as ECDsaSecurityKey;
+                        }
+
                         var ecdhKeyExchangeProvider = new EcdhKeyExchangeProvider(
                             key as ECDsaSecurityKey,
-                            validationParameters.TokenDecryptionKey as ECDsaSecurityKey,
+                            publicKey,
                             jwtToken.Header.Alg,
                             jwtToken.Header.Enc);
                         string apu = jwtToken.Header.GetStandardClaim(JwtHeaderParameterNames.Apu);