wso2 / product-is

Welcome to the WSO2 Identity Server source code! For info on working with the WSO2 Identity Server repository and contributing code, click the link below.
http://wso2.github.io/
Apache License 2.0
748 stars 729 forks source link

JWKS endpoint returns x5t#s256 value incompatible with the RFC specification #18817

Closed tharakawijekoon closed 10 months ago

tharakawijekoon commented 10 months ago

Describe the issue: This issue[1] is still not fixed, the initial fix done for this issue is correct[2] but since then an incorrect fix[3] has been added.

Currently, the x5t#S256 parameter in the JWKS response is not as per the specifications[4]. 3rd party libraries parse that the Identity server's JWKS response are failing due to this.

When the x5t#S256 parameter is calculated we get the sha256 hash/digest of the certificate[5] which is a 32 byte array but we are converting this to a string with the hexify[6] and it is this string that is being base64 url encoded.

Digging further into this to find whether it is the string hash that should be encoded or whether it is the 32 byte array(representing 64 characters), I came across the following example in a specification[7] which confirms that the 32 byte array needs to be base64 url encoded instead of hexified string.

So as mentioned in the specification if the certificate is as follows(default cert in the IS-5.10.0):

-----BEGIN CERTIFICATE----- MIIDqTCCApGgAwIBAgIEXbABozANBgkqhkiG9w0BAQsFADBkMQswCQYDVQQGEwJV UzELMAkGA1UECAwCQ0ExFjAUBgNVBAcMDU1vdW50YWluIFZpZXcxDTALBgNVBAoM BFdTTzIxDTALBgNVBAsMBFdTTzIxEjAQBgNVBAMMCWxvY2FsaG9zdDAeFw0xOTEw MjMwNzMwNDNaFw0yMjAxMjUwNzMwNDNaMGQxCzAJBgNVBAYTAlVTMQswCQYDVQQI DAJDQTEWMBQGA1UEBwwNTW91bnRhaW4gVmlldzENMAsGA1UECgwEV1NPMjENMAsG A1UECwwEV1NPMjESMBAGA1UEAwwJbG9jYWxob3N0MIIBIjANBgkqhkiG9w0BAQEF AAOCAQ8AMIIBCgKCAQEAxeqoZYbQ/Sr8DOFQ+/qbEbCp6Vzb5hzH7oa3hf2FZxRK F0H6b8COMzz8+0mvEdYVvb/31jMEL2CIQhkQRol1IruD6nBOmkjuXJSBficklMaJ ZORhuCrB4roHxzoG19aWmscA0gnfBKo2oGXSjJmnZxIh+2X6syHCfyMZZ00LzDyr goXWQXyFvCA2ax54s7sKiHOM3P4A9W4QUwmoEi4HQmPgJjIM4eGVPh0GtIANN+BO Q1KkUI7OzteHCTLu3VjxM0sw8QRayZdhniPF+U9n3fa1mO4KLBsW4mDLjg8R/JuA GTX/SEEGj0B5HWQAP6myxKFz2xwDaCGvT+rdvkktOwIDAQABo2MwYTAUBgNVHREE DTALgglsb2NhbGhvc3QwHQYDVR0OBBYEFEDpLB4PDgzsdxD2FV3rVnOr/A0DMB0G A1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjALBgNVHQ8EBAMCBPAwDQYJKoZI hvcNAQELBQADggEBAE8H/axAgXjt93HGCYGumULW2lKkgqEvXryP2QkRpbyQSsTY cL7ZLSVB7MVVHtIsHh8f1C4Xq6Qu8NUrqu5ZLC1pUByaqR2ZIzcj/OWLGYRjSTHS VmVIq9QqBq1j7r6f3BWqaOIiknmTzEuqIVlOTY0gO+SHdS62vr2FCz4yOrBEulGA vomsU8sqg4PhFnkhxI4M912Ly+2RgN9L7AkhzK+EzXY1/QtlI/VysNfS6zrHasKz 6CrKKCGqQnBnSvSTyF9OR5KFHnkAwE995IZrcSQicMxsLhTMUHDLQ/gRyy7V/ZpD MfAWR+5OeQiNAp/bG4fjJoTdoqkul51+2bHHVrU= -----END CERTIFICATE-----

it should return x5t#S256": "NhKtjwG07PRxTwvI4HG2QD3TTE3eYtgd1LkdGqNW3uY"

[1]https://github.com/wso2/product-is/issues/14899 [2]https://github.com/wso2-extensions/identity-inbound-auth-oauth/pull/2067 [3]https://github.com/wso2-extensions/identity-inbound-auth-oauth/pull/2124 [4]https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.8 [5]https://github.com/wso2-extensions/identity-inbound-auth-oauth/blob/474ca2a61d098f5dea1c925bae2b917409115288/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java#L3279 [6]https://github.com/wso2-extensions/identity-inbound-auth-oauth/blob/474ca2a61d098f5dea1c925bae2b917409115288/components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/oauth2/util/OAuth2Util.java#L3280 [7]https://datatracker.ietf.org/doc/html/rfc8705#section-appendix.a

tharakawijekoon commented 10 months ago

With the hexify method, It seems like we are converting the thumbprint/hash to human-readable format. which seems to be an unnecessary step.

A SHA-256 hash has 64 hexadecimal characters and these 64 hexadecimal characters can be contained in the 32 byte array(4 bits per character), we do not need to convert it to human-readable format.

ThaminduDilshan commented 10 months ago

We have raised this in the OAuth working group and got responded that the hexifying step is wrong [1]. Hence we should fix this. We need to treat this as a high priority issue for IS 7.0.0.

cc: @madurangasiriwardena / @DMHP

[1] Mail: [OAUTH-WG] Query on correct approach of calculating the "x5t#S256" parameter in the JWKS response (available at https://www.mail-archive.com/oauth@ietf.org/msg23004.html)

ThaminduDilshan commented 10 months ago

We need to add a test case for this (unit tests)