Skip to content
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

bug: Unexpected JWT token generated by usage of TokenRequest class #1661

Closed
jgomer2001 opened this issue Apr 7, 2022 · 10 comments
Closed

bug: Unexpected JWT token generated by usage of TokenRequest class #1661

jgomer2001 opened this issue Apr 7, 2022 · 10 comments
Assignees
Labels
bug bug in code
Milestone

Comments

@jgomer2001
Copy link
Contributor

jgomer2001 commented Apr 7, 2022

When trying to figure out why scim tests are all failing in 4.4 I concluded there are some things off in certain underlying oxAuth classes that scim client employs.

In this call, the algorithm is being set to null. I checked the actual key in the keystore. It exists and has the following attributes:

id: 90161505-78e2-4927-9239-80ac1defbef2_sig_ps256
algorithm: RSA
size: 2048

Later when the assertion is extracted from the tokenRequest, this method assumes the alg is HS256 (is that assumption fine?). This provokes an NPE here, which simply logs the error and continues returning a JWT without signature.

I confirmed this by checking the HTTP request generated by tokenService.requestJwtAuthorizationRpt in UmaScimClient. It looks this way:

POST /oxauth/restv1/token HTTP/1.1[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "Accept: application/json[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "Content-Type: application/x-www-form-urlencoded[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "Content-Length: 628[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "Host: jans.kt.co[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "Connection: Keep-Alive[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "User-Agent: Apache-HttpClient/4.5.13 (Java/11.0.12)[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "Accept-Encoding: gzip,deflate[\r][\n]"
14:08:17 DEBUG Wire.java:73 - http-outgoing-1 >> "[\r][\n]"
14:08:17 DEBUG Wire.java:87 - http-outgoing-1 >> "client_assertion_type=urn%3Aietf%3Aparams%3Aoauth%3Aclient-assertion-type%3Ajwt-bearer&client_assertion=eyJraWQiOiI5MDE2MTUwNS03OGUyLTQ5MjctOTIzOS04MGFjMWRlZmJlZjJfc2lnX3BzMjU2IiwidHlwIjoiSldUIiwiYWxnIjoiSFMyNTYifQ.eyJzdWIiOiIxMjAyLjY2ZWQ4YWNmLTk1ZjMtNGZkNy1iN2Q4LTFiOTliZDlhZmM1MyIsImF1ZCI6Imh0dHBzOi8vamFucy5rdC5jby9veGF1dGgvcmVzdHYxL3Rva2VuIiwiaXNzIjoiMTIwMi42NmVkOGFjZi05NWYzLTRmZDctYjdkOC0xYjk5YmQ5YWZjNTMiLCJleHAiOjE2NDkzNTg3OTcsImlhdCI6MTY0OTM1ODQ5NywianRpIjoiZTljY2RlYzctNmEzYy00NTA2LWI3ZmQtZjIwNmQ5N2Q2Mjk5In0.&grant_type=urn%3Aietf%3Aparams%3Aoauth%3Agrant-type%3Auma-ticket&ticket=ef0ad601-ed9b-491b-bdd0-17d8890b3f69"

So the resulting (non sense) header is:

{
  "kid": "90161505-78e2-4927-9239-80ac1defbef2_sig_ps256",
  "typ": "JWT",
  "alg": "HS256"
}

Note how client_assertion parameter ends with . (dot).

Extra details:

  • Env = Gluu 4.4 (latest snapshot wars).
  • SCIM tests fail both in jenkins and local dev environment (applies to UMA mode only)
@jgomer2001 jgomer2001 added the bug bug in code label Apr 7, 2022
@jgomer2001 jgomer2001 added this to the 4.4.0 milestone Apr 7, 2022
@yuriyz
Copy link
Contributor

yuriyz commented Apr 7, 2022

If you set alg to SignatureAlgorithm.PS256 then it works, right?
Signature algorithm identification didn't change since 2016 as far as I can see.

image

Sounds like keys in jks does not have certificate or certificate refers to algorithms which are not what SignatureAlgorithm recognizes. (In this particular case SHA256withRSAandMGF1.)

re: HS256, it is fallback algorithm which assumes that shared key is used instead of asymetric keys. I think code can be better at this place and maybe we should aggresively fail:

  • if asymetric key is set with alg HS*
  • in opposite case when shared key is set with alg that is not HS* (RS*, PS* etc.)

Indeed fallback to HS with asymetric key does not make sense.

So correct algorithm should solve problem. (Check certificate algorithm or take it out of keyId or simply add configuration property ).

I will open improvement ticket on jans (trying to minimize impact on client code in 4.x ).

@jgomer2001
Copy link
Contributor Author

@yuriyz

Sounds like keys in jks does not have certificate or certificate refers to algorithms

key seems fine:

Screenshot from 2022-04-07 17-03-20

@jgomer2001
Copy link
Contributor Author

@yuriyz

If you set alg to SignatureAlgorithm.PS256 then it works, right?

hardcoding the alg works, yes

take it out of keyId or simply add configuration property

the key id is a parameter in the scim client API and the keystore contains a mix of keys that use different algorithms. Are you suggesting the use of a mechanism other than cryptoProvider to get the alg?

@yuriyz
Copy link
Contributor

yuriyz commented Apr 8, 2022

From kid 90161505-78e2-4927-9239-80ac1defbef2_sig_ps256 we know exactly that it is key generated for signature (sig) and algorithm is ps256. I propose get it out of kid or if we know we will have some generator another then our then add configuration parameter where we can specify algorithm explicitly.

@yurem
Copy link
Contributor

yurem commented Apr 8, 2022

@jgomer2001 I checked same scim issue few days ago. I find that signature verification not works with xyz_enc_rsa1_5 keys. As result this led to random scim test suite failures.

@jgomer2001
Copy link
Contributor Author

It's not exactly the same issue, however in both cases scim tests are going to fail.

Here I was complaining why algorithm identification not always working. It's a public method in oxauth-model that is not only used by oxauth itself but potentially in other projects.

Ofc signature verification is a more critical problem.

I can update docs so scim developers pick a kid ending in _rsXYZ for safety

@yuriyz
Copy link
Contributor

yuriyz commented Apr 8, 2022

@jgomer2001 attach or PM me jks file and password to that file. I will check.

@yuriyz
Copy link
Contributor

yuriyz commented Apr 8, 2022

Strange thing happens, for same key from jks UI tool shows one alg name but code shows another (it does not match).

image

@yuriyz
Copy link
Contributor

yuriyz commented Apr 11, 2022

This is pretty interesting but it sounds as we can't trust UI. I suspect it has hard-codes and does not show to us real value.

Since java 11 algorithm is set to RSASSA-PSS for all PS*, means we can't figure it out on algorithm name alone.
https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html#signature-algorithms

However we can do it by algorithm parameters stored as DER-encoded bytes in certificate. I've looked to implementation of some jwt libraries open sourced on github and think that I can solve it. Stay tuned :).

@yuriyz
Copy link
Contributor

yuriyz commented Apr 12, 2022

Issue is fixed in 4.4, master and jans.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bug in code
Projects
Development

No branches or pull requests

3 participants