OIDC: too short a secret breaks the self-created JWT process

[FroMage]

Describe the bug

For Github and Facebook we have to create our own JWT token because they don't include an ID Token. If the secret is too short like SECRET then I get the following exception during that token creation:

io.smallrye.jwt.build.JwtSignatureException: SRJWT05012: Failure to create a signed JWT token: A key of the same size as the hash output (i.e. 256 bits for HS256) or larger MUST be used with the HMAC SHA algorithms but this key is only 64 bits]

I am not sure we should really be throwing if the secret is short, unless there's a real requirement of OIDC to have long secrets.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[quarkus-bot]

/cc @pedroigor, @sberyozkin

[sberyozkin]

@FroMage It is https://datatracker.ietf.org/doc/html/rfc7518#section-8.8 recommendation which Jose4j enforces by default which is sound.
Is it a problem with facebook ? AFAIK, github allocates at least a 32 char password, If it is a wiremock test issue then I'd not worry about it at all :-), you can use KeyUtils from smallrye-jwt/common which will generate a correct size key for the given signing algorithm

[sberyozkin]

See also https://datatracker.ietf.org/doc/html/rfc7518#section-3.2, and specifically A key of the same size as the hash output (for instance, 256 bits for "HS256") or larger MUST be used with this algorithm. etc

[FroMage]

Well, you're quoting JWT specs, not OIDC ones, so unless there's something that forces OIDC secrets to be of that size, we might need to not throw when making our own internal JWT token based on that secret.

[sberyozkin]

@FroMage Steph, OIDC is built on top of JWA (algorithms) among other things, things like RSA keys must be at least 2048 bits is also coming from there, so yes, indirectly it is a requirement. I can introduce a system property in smallrye-jwt-build to optionally relax the key length verification. But my question is - is it a test issue or production issue ? If it is a test issue then IMHO we should not spend time on getting this property added and supporting unnecessarily the weaker signature or encryption, if the key length is less than a recommended length for a specific signature algorithm than the attack vector increases.
We have more important issues to deal with to support your various cases than this one IMHO, so please let me know if it is a test issue or not...

[sberyozkin]

@FroMage I think I'll introduce that system property anyway as there could be other cases where relaxing may be required, ex, some legacy provider expects RSA keys with 1024 bits, etc; as far as quarkus-oidc is concerned I still think we should not do anything specific about it - but if really needed then that system property will be around to relax it

[FroMage]

It's during testing I've noticed this, but I thought that it may apply to prod in some cases. This is not about key lengths, I think, this is about how we generate our own internal JWT by creating a SecretKey based on the client secret:

    public static SecretKey createSecretKeyFromSecret(String secret) {
        byte[] secretBytes = secret.getBytes(StandardCharsets.UTF_8);
        return new SecretKeySpec(secretBytes, "AES");
    }

What I mean, is that if the OIDC spec (or JWA indirectly) forces those client secrets to be of a minimum length, then fine, we're safe. But if not, it's entirely possible that we find someone in the future using short client secrets and this code will blow up for them.

[sberyozkin]

@FroMage yeah, this code is fine, I believe even Jose4J uses AES, the HMAC qualifier apparently makes no difference (see for ex this comment in Jose4j), bu in any case, it is entirely about the key lengths, and indeed, as you say, there might be cases, where a provider can not allocate a correct length password, so I'm about to add a property to let users relax the key length verification - it is already possible for the smallrye-jwt token verification but not creation...

[sberyozkin]

@FroMage FYI, smallrye/smallrye-jwt#553