-
Notifications
You must be signed in to change notification settings - Fork 7.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Istio Origin Authentication doesn't work with some EC Keys #19424
Comments
https://discuss.istio.io/t/jwks-parse-error/4142/10 has some context on this issue. |
As mentioned in https://discuss.istio.io/t/jwks-parse-error/4142/4?u=yangminzhu, Istio switched to use the Envoy JWT filter in 1.3 and the the upstream JWT verify library used in Envoy doesn't support the EC key at that time. The Envoy and the upstream JWT library added the support in envoyproxy/envoy#8807 but this fix is not available in 1.3 (but should be in 1.4). Some options for now:
So probably the best option for the short term is to use the flag |
@yangminzhu I will try upgrading to 1.3 first using that flag, however I did also run into this issue in 1.2.7, but not 1.2.2. So I'm not sure it will work. |
@yangminzhu @JimmyCYJ I tested moving to all of the latest versions of 1.2, 1.3, and 1.4 and I'm unable to change my versions. If not resolved by next week, I'll have to abandon using istio. Version 1.2.10Both with the
Version 1.3.6For version
Version 1.4.2Even more concerning is moving to version
Note this is a really big deal for anyone using PingFederate as an IdP as we can't upgrade and current versions of envoy have security flaws. |
The USE_ISTIO_JWT_FILTER environment variable is introduced in istio 1.3 to fall back to use the Istio JWT filter, it doesn't exist in istio 1.2. It seems the JWT from pingfed is not recognised by either Istio JWT filter nor Envoy JWT filter, this really has to be fixed in Encoy JWT filter. By taking a quick look at the code here https://github.com/google/jwt_verify_lib/blob/9f10e2d60d42edeb6662e185707a7d6a4ebc5604/src/jwks.cc#L168, it looks like the error is caused because the JWT from pingfed doesn't set the alg field. The code uses ES256 if alg is not set and check if the crv is set to P-256. However the JWT from pingfed sometimes set the crv to P-521 or P-384 which in turn caused the error. @jammerful I'm OOO today and Thursday, I will do some more research of the standard to see what's the right thing to do here, I'm not sure should the JWT set alg field explicitly or should the code be more tolorent here. Also @qiwzhang could you take a look and let me know what do you think? Thanks |
@yangminzhu Thanks for the information. Regarding the change in the parsing library it looks like it was changed in the 1.2 branch, as I'm unable to move from 1.2.x to 1.2.10. I will also open an issue with the envoyproxy today. |
@yangminzhu @qiwzhang I've created issues in both envoy and google/jwt_verify_lib, you can see them linked above. Any further help would be appreciated. |
This should be fixed by google/jwt_verify_lib#32, I think @qiwzhang will have another PR to update the Envoy to pick it up. Once that is done, I will update Istio to pick up the latest Envoy with the fix. @jammerful, I think we may have this in Istio 1.4.3 or 1.5 depending on if we have another patch release for 1.4, either case, it will probably take a while for the release. |
@yangminzhu Thank you so much for resolving this, I look forward to upgrading istio.😀 |
@yangminzhu Should the latest envoy fix be pulled in now? Also is it looking likely that we'll have a 1.4.3 release? |
@jammerful I'm not sure about the 1.4.3 release, @howardjohn may know about this. I'm trying to pull in the upstream Envoy in envoyproxy/envoy-wasm#355, but there are some mysterious error related to the wasm, @kyessenov @PiotrSikora could you help here? Thanks. |
1.4.3 will be released next week, so this needs to be done asap if you want to get it in that release |
@howardjohn @yangminzhu It would be amazing if this could be make it into the 1.4.3 release. |
@jammerful I did some more investigation and it turns out we don't need to update envoy-wasm to cherrypick this fix to 1.4, we should actually do this in istio/envoy repo: istio/envoy#131, @howardjohn could you review the cherrypick for 1.4? thank you |
Thank you, hope this makes into 1.4.3 as it will be much easier to upgrade to. |
@jammerful This should be fixed in 1.4.3 with the merge of #19912. Is it possible for you to verify the fix? The 1.4.3 is not released yet so you have to build it manually from the release-1.4 branch. If you happen to have some JWT token that could be shared with me, I can also verify it locally. Thank you. |
@yangminzhu Awesome, thank you so much. Unfortunately, I can't easily build istio from a branch. I can however give you a JWKS to test:
Let me know if can help you in any other way, or if there is anything else you need. |
@jammerful I just tested the JWKS in my local build and I didn't see any errors in the envoy log. I do not have the JWT for testing with a real request but I think the log is enough to tell that the bug had been fixed. I'm closing this now and feel free to re-open if there is still any issues, thank you. |
Bug description
Istio is unable to parse JWKS' from PingFederate that contain EC keys. This is blocking me from upgrading istio versions, which is security issue with istio.
Expected behavior
Istio should parse the JWKS below as it is perfectly valid, the error that is being printed is incorrect--all EC keys have their required components.
Steps to reproduce the bug
{"keys":[{"kty":"EC","kid":"Ozxgl1WsWyQBF1lcbYyjYl","use":"sig","x":"Vq3vKJCeJyihZIauouQp3eFmuYLCsEcID_sGigAx2gs","y":"3UaPoda-HCrJeU4i960qFHMschTdmqjkitiIyYc5svA","crv":"P-256"},{"kty":"EC","kid":"RBsu7DAEUcrgx4X91TVrsY","use":"sig","x":"dQ_SlIctw4nWZEJ2rasiyrps7jXuxr1E81zw4X-aaY1LLRhcaUxpInytXeZK5mOm","y":"xrhQlZQZeDWm7VRHThevLTKqOkqfNwTmHL7P_f23BPn8SLlXd9p1jS4LzL0KK0rL","crv":"P-384"},{"kty":"RSA","kid":"X3oArm2sGh5pO0jWl5O41C","use":"sig","n":"lOTXC2Pfi0dajzDAOERQLTtT0_GbjAFaNflzV-0tWxban1CPEN0n5UG5z-c0KxKY6fhZshQ1Opr3VbQmE1MGSeYf3qEUD4Th3ZubVV_2Yhuio-UZXllz1EGgAh7sC9TzQi84jIYj_mhdno4l6Y3FVlvM6VtNYRGPDNgjRoVfd63vocXAqgUok6LpEcL9MbrvmK-hFPVNX7euGN_xm_qZM5-JrJMnKz6shnjrul7yZ-ZExzfFx_LSBqum-fkKv2FfEoJqyzyVlbabmDUZ81B9ZP0nfaP3e-IRSQECuXf52PfHqEgZbrax8hKAfpZKJX643gxSCnBWdmNO-BFSlho4pw","e":"AQAB"},{"kty":"EC","kid":"sxG_WeuLxIKXoVit-8vyQf","use":"sig","x":"AG3w2vYgVbn4E27rkxZPUVrzLWhMctY5GOP6xygLLFwNRaoOx2gnlQPwAsEXHxz80u5lfmOms0pJSjuDrNqs5pB4","y":"Ad0K-hbFmTVj3nMOw7jAdl21dlU35pG1g7h_Tswr0VYfxqg4ubIPyXrrtmlKH8q3c2Gqgq77Uq12qfcDE8zF2a4v","crv":"P-521"},{"kty":"EC","kid":"7uLnfLOhXPmOZ2BUaeUue-","use":"sig","x":"fR71QOze0q-0uDBuOyTdJANOA0Kz_WT0ykuuuLH94uCJzD13B5OBB1y4jtOvIYvX","y":"WwjH9z9kzSsGZmkl5BvXhyCO8udP9nNFuZM4WugT7C-kD0UNqojpCEkigWorRlin","crv":"P-384"},{"kty":"EC","kid":"FWwgHZozeidVHog8YEjhT-","use":"sig","x":"C2U_Vuv_t3VEOv-UaprJTir3L_SWq_gjwVSUcLTq5Bc","y":"U9F6c5qlnRLSR_KRiCyAzRk7YUpXrXbqsUJYYlTPw7A","crv":"P-256"},{"kty":"RSA","kid":"aNspNiifg5EiN-R1R-RO4O","use":"sig","n":"klTzMXRM83AWKoKRZQwI5XOdJ0tB0jr1ieo_uLT_W6w_hNY8jbFzN1xwxjyUZFBHQVDU2PlyjRZDKQ5tIRs05__DwCvYdDiN0i5f5ChpB66UkAUn1IuuGUQYP7OHVSdeDCFCq3q8UGrl2FCFRxyL9k9hu0VX7Pj06SNGr74_hbCXF6VJmcgDmoXrTXEC5hSBciauGmAxlIbhDbDx-3QHrPJ5P4OLCwH0kfW2RB50O6DMZmKhXeSqE0WiI9KbMz_Mq-TIMiPNkRC1Wsdv4sKoYHf1t6sdp6n9qconQRGwGZNaWcKk6nno-26a_8CxGAuHh8BIFIvFj5AohFFXu2-slw","e":"AQAB"},{"kty":"EC","kid":"ikc6qm-M_topQoGzydo8zI","use":"sig","x":"AE8C5SEixbzl9ez1NPsYOehpnPohIAMxucjZjw2E9aVCBdGo7t2hKv2Aa7ql4zpeTucrDP3ZyUKK6-D3m3C10ojx","y":"AXNjApqwEKFjFmR4bbxAf_8nS74zDuOtXsq1PnoU58ZcHON1ZBUOEXY0Y6IPw1Q8ngaLHg30I300L1ZL0aIX4ygv","crv":"P-521"},{"kty":"EC","kid":"403dlMPeSjSNp4cQ0GBSLL","use":"sig","x":"AI4FHJCcr7fMCb2BfCj9l6bptD513AVVSbOLP2wPM0Aq6CKOEWlN5Yc8qW1Z8NFOPkjeBsCagV02qNJFxH0oi9em","y":"AK3nxsAk3aMOwqIGU-9xXW2I_wwkRxbTR5cPPceWEzpvUHXqyfOXihVYPTJkbaYggyOWjlpw5RYJZGBTLkFUzdal","crv":"P-521"},{"kty":"EC","kid":"IcGz3uUD_EfbMQWQ-6SrmT","use":"sig","x":"10bDbsUH92XHCizkNtBzumyzro8aZypDTqG6ob1fMXk","y":"zfradNJdJ739STTO9vQmOtMl3r7XlnX2SNEusGsFtz0","crv":"P-256"},{"kty":"EC","kid":"oTrCYqttZBLTIv4R5lrfiZ","use":"sig","x":"kOxZSdBd2DPGjuf0lrV30Bc8LCj2EBMfmluPc3sV44fnsZWtTnQz4pCcDjz2hzOY","y":"587zK_ggKmzpYaKz8AaBKERnAsF0AfLjIo2dAu0BdCO0FzSJGyV_cDpNmAdx8ah4","crv":"P-384"},{"kty":"RSA","kid":"vty_MATEPV9warjejj_hef","use":"sig","n":"6TDzjPXHfjDygJ3wa-0BB4m028hatl-PnfT5BEFcIkkWhGRswQSpiGCGjb3DXQ4LxAwZ_XM5RuBYMOMZw9qEU01lhQycqYVOub7R0lli2oDETW4pOATa6JW7QyyXcbbcnYxqj6qfwKb_XfXBDfLpwT8K1_ylJxAymV1ZgfeXDexGBr7d4fLNzgGV7CjZcYmMftn3CktKrA2vy8fLQ2wfVQgfo1J9UqCmLoo6sorW2Sn23Vsx4sTN8OkcrYzpd8_0Gj0X4jXsnk05rfvAWfn8iBHxe6ERBuVh48SWkGjpRnLLoyFmZFg0d1aDlskWp2rYu0VeeUAfKDAeGAU3_1ihuQ","e":"AQAB"}]}
istio-proxy
sidecar and you'll see:Version (include the output of
istioctl version --remote
andkubectl version
andhelm version
if you used Helm)Istio version:
Kubernetes version:
Helm version:
How was Istio installed?
Through the helm chart, via templating with no tiller.
Environment where bug was observed (cloud vendor, OS, etc)
AWS EKS version 1.14.8
The text was updated successfully, but these errors were encountered: