jenkinsci / oic-auth-plugin

A Jenkins plugin which lets you login to Jenkins using your own, self-hosted or public openid connect server.
https://plugins.jenkins.io/oic-auth
MIT License
71 stars 88 forks source link

Login not working if alg field is missing in jwks_uri response #304

Closed czymstef closed 4 months ago

czymstef commented 4 months ago

Jenkins and plugins versions report

Environment ```text Paste the output here ```

What Operating System are you using (both controller, and any agents involved in the problem)?

Linux

Reproduction steps

I upgraded the plugin from version 2.6 to the latest version (4.250.v5a_d993226437) and our login with Microsoft broke. Apparently, the keys in the jwks_uri response do not contain the optional "alg" field, which breaks the IdTokenVerifier from com.google.oauth-client:google-oauth-client:1.35.0. In line 628 of com.google.api.client.auth.openidconnect.IdTokenVerifier, the method buildPublicKey is called, which returns null if the alg field is missing. Unfortunately, ImmutableMap from guava does not support null values and will throw a NullPointerException.

Expected Results

it should work, just as it did with version 2.6

Actual Results

Jenkins error while logging in

Anything else?

I'm not sure if this is the right place for this issue, as the underlying problem is caused by the used oauth library. But obviously it worked some versions ago just fine, so it could be seen as a regression.

Are you interested in contributing a fix?

No response

michael-doubez commented 4 months ago

Indeed, the underlying library is google oauth/openidconnect and, while it does a good job as a generic library, it tends to expect Google's layout.

You are definitely in the right place. I won't be able to adress the issue this week. In the meantime, you can disable token verification (in advanced parameters). You won't be more at risk than before.

I'll add a known issue note in the release.

michael-doubez commented 4 months ago

For reference: https://datatracker.ietf.org/doc/html/rfc7517#section-4.4

It is strange, I found drafts where alg is mandatory and it is marked as OPTIONAL. Well ... In any case, I'll try to fix that or handle it as a case where checks will not be applied :(

octaviodmz commented 4 months ago

I also updated to the latest version and facing the same problem. I am using Keycloak (24.0.3) in my environment. I do have this exception stack thrown

 java.lang.Exception: Token request failed: java.io.IOException: Error fetching public key from certificate location https://keycloak.mydoma.in/realms/MyRealm/protocol/openid-connect/certs"

But I do have alg present in the response 🤔 when I access the URL in the log

{
  "keys": [
    {
      "kid": "REDACTED",
      "kty": "RSA",
      "alg": "RS256",
      "use": "sig",
      "n": "REDACTED",
      "e": "AQAB",
      "x5c": [
        "REDACTED"
      ],
      "x5t": "REDACTED",
      "x5t#S256": "REDACTED"
    },
    {
      "kid": "REDACTED",
      "kty": "RSA",
      "alg": "RSA-OAEP",
      "use": "enc",
      "n": "REDACTED",
      "e": "AQAB",
      "x5c": [
        "REDACTED"
      ],
      "x5t": "REDACTED",
      "x5t#S256": "REDACTED"
    }
  ]
}

Changing disableTokenVerification to true in the config.xml makes it work again

czymstef commented 4 months ago

The top level exception is the same though. I used a debugger to get to the cause of this, not sure if there is an easier way.

commodity729 commented 4 months ago

I have got affected as well. Setting disableTokenVerification for security realm helped, but how the F people managed to break it in the first place?

thimo-seitz commented 4 months ago

I have got affected as well. Setting disableTokenVerification for security realm helped, but how the F people managed to break it in the first place?

Change the setting in the config.xml. There is disableTokenVerification inside securityRealm after the upgrade of the plugin.

lanmarti commented 4 months ago

I've got the same setup and problem as https://github.com/jenkinsci/oic-auth-plugin/issues/304#issuecomment-2070897941 Keycloak 24.0.3, 'alg' fields present in the JWKS_URI.

michael-doubez commented 4 months ago

I am sorry it broke so many setup. I won t be able to deliver a fix until wednesday, at best. And it won t be pretty, the logic in Google API is deeply embedded. It will be more likely a workaround disabling signature verification if jwks URI parsing fails.

Could you please send me a copy of jwks content ?

lanmarti commented 4 months ago

example JWKS endpoint content

{
    "keys": [
        {
            "kid": "14MuZmRLJtgqpRrcATyTeCUpaU3IHm8kwuCzRJbWnyU",
            "kty": "RSA",
            "alg": "RSA-OAEP",
            "use": "enc",
            "n": "hZFq7mB43U_5uW1qa-l7lI4thQJ9SVVWgcmdHCemX65s20Vn5Fv34TERdDxST1ZbOHLtcRG-7ykTjnb36KLWBEWUU4KIeYqLgltx_Yx-e_4hcxGyWP323xFu9kHH3ZWOpx3Yv99lscCxRBZ0b-bIfENaAWm9e63NPIVnDFbpt6WBGPHm1PNpYqw_sjEn5BGovH75KxTSqZdMPnT5f-jRveKmNO7-dBnZxYL2vpNu6iXfD_2sXhoBQ3P41-zbFTNfy4yXPvnMjRaMPhhp5OtwLH_LWKfJf-7tQ9jPsYFsch2EcMX-o-G42IJyN3GYxMr0XImVWWUB7ILsPrYRp2OZaQ",
            "e": "AQAB",
            "x5c": [
                "MIICnzCCAYcCBgGBbRMM2zANBgkqhkiG9w0BAQsFADATMREwDwYDVQQDDAhpbnRlcm5hbDAeFw0yMjA2MTYxNTExMTNaFw0zMjA2MTYxNTEyNTNaMBMxETAPBgNVBAMMCGludGVybmFsMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAhZFq7mB43U/5uW1qa+l7lI4thQJ9SVVWgcmdHCemX65s20Vn5Fv34TERdDxST1ZbOHLtcRG+7ykTjnb36KLWBEWUU4KIeYqLgltx/Yx+e/4hcxGyWP323xFu9kHH3ZWOpx3Yv99lscCxRBZ0b+bIfENaAWm9e63NPIVnDFbpt6WBGPHm1PNpYqw/sjEn5BGovH75KxTSqZdMPnT5f+jRveKmNO7+dBnZxYL2vpNu6iXfD/2sXhoBQ3P41+zbFTNfy4yXPvnMjRaMPhhp5OtwLH/LWKfJf+7tQ9jPsYFsch2EcMX+o+G42IJyN3GYxMr0XImVWWUB7ILsPrYRp2OZaQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBoDAA4UR4d+C6IxrOu1Hyz0eqg6XELT0ds8E9IsdK9k46xj99bAdJr9yPFaqXx/ZclMfTELHhg4NUevUB2iWMNt3lSjmjZRancNhQG/DhK6mR1/EImWa/4Uz5ss3pE5De1UZaY9UhHD+9hTbFFZwwNR0wILvKK84Ij1It6xbwz/wHwuAzJ76VhizgV5zI6Du6jxP4ifSlT2hnuMIJUdudsSxH1MSDAd7SDUSsbP1OjX2NPBuz+3cIEMJwDTosrUqR2sI9MBUpEmaHE5IcwxRkTGZvfOnDYm+fUj/uyhUwp34JP++BEVlE7bi8SzqwY5inFT8KKjosfTBTS7m4f8dGy"
            ],
            "x5t": "H8orvE9ANCBucfmMLMfY2VFQdS4",
            "x5t#S256": "vtuAx3OoWxUUlDxuthIyTLe7gXg9j0KcNOsrBFAREXE"
        },
        {
            "kid": "UByZvg8-ZmKSjIq5zLiMtmmNXd5ZJSAxb7OyDOcthfM",
            "kty": "RSA",
            "alg": "RS256",
            "use": "sig",
            "n": "kGGMwI8s7KH82NaID8sBvz6N5DwzsqgSXofhr6P77LkpCXi2vvOpLzyTY2OFz1f6Ecf0-hCEmGHLEji6gCxUk4URr73n-jprL0dXo29z7uODnfzuB_chvbw-IbjOOj6Z7GV7fgw428jhLboygjklbcymLltHaUMfJjj0KuP5vaCu2dlgiyFKh8Imde8NcCR9zX19_76YNqJbvezB9WPeOcMR2NX-Clm5kq-mGfklf1c57IWAVMSb3bufIU5BARKPdM2pZJYt2F4KRf0hbQVOHFJ6Z6JhJUq83yeBUaH6GTIyvCHqekd9Uz7obBolb4vwZzAu0_CUp3BYBATjuNmO1w",
            "e": "AQAB",
            "x5c": [
                "MIICnzCCAYcCBgGBbRMMOzANBgkqhkiG9w0BAQsFADATMREwDwYDVQQDDAhpbnRlcm5hbDAeFw0yMjA2MTYxNTExMTNaFw0zMjA2MTYxNTEyNTNaMBMxETAPBgNVBAMMCGludGVybmFsMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAkGGMwI8s7KH82NaID8sBvz6N5DwzsqgSXofhr6P77LkpCXi2vvOpLzyTY2OFz1f6Ecf0+hCEmGHLEji6gCxUk4URr73n+jprL0dXo29z7uODnfzuB/chvbw+IbjOOj6Z7GV7fgw428jhLboygjklbcymLltHaUMfJjj0KuP5vaCu2dlgiyFKh8Imde8NcCR9zX19/76YNqJbvezB9WPeOcMR2NX+Clm5kq+mGfklf1c57IWAVMSb3bufIU5BARKPdM2pZJYt2F4KRf0hbQVOHFJ6Z6JhJUq83yeBUaH6GTIyvCHqekd9Uz7obBolb4vwZzAu0/CUp3BYBATjuNmO1wIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQB659gocsQOlPGsY0NXtWRE9U4or37y7LiOdzX2cXPVmW95p58Mt5ehTLWvJe2P1vEtt3wyUBwKWy0WZHsoFSdrOvNI3YZOxkHObgmK9rE8/k1ySZPiFb9KLfmwZxsWvRCNY8gbr705N+HHKiPJEE77IsQfpiyTFzUnRL/BWiDlhU1lHFZnoKFTht6wGDc0F+MtrN6c+i/PRgW4wboDOLMotLhXHfUrOUatx6XONaG+n790FRput+Gf1UsgQnQquGdW2o1dJP6nEsexgeew1nRvzyWoJgQPpT/N5k9smXlUlH7fQmxfaUefrpgL0kkq/YOj0IG6envW+siTpp+8x1yq"
            ],
            "x5t": "DsEcchxw-CtObqxyfClb8wfyxKI",
            "x5t#S256": "phCKIqd1tOxxcbSpXvUS8P0Mdw42gu0CLGGWeHlomjs"
        }
    ]
}
michael-doubez commented 4 months ago

Workaround is available in v4.257. The verification of JWKS can be re-enabled.

As it is, the check of signature is disabled at the first failure. The backend client will be replaced at some point and, hopefully, a wider range of signature algo will be supported.

A unit test has been added for the cases that were reported here.

Please open another issue if there are still failing cases.