AzureAD / microsoft-authentication-library-for-java

Microsoft Authentication Library (MSAL) for Java http://aka.ms/aadv2
MIT License
282 stars 137 forks source link

only demand RSAKey from PrivateKey implementaton #747

Closed JoergBudi closed 7 months ago

JoergBudi commented 8 months ago

MSAL4J adds a 2048 private key bitlength check.

It demands the key to implement java.security.interfaces.RSAPrivateKey to perform the bitlength check, but that is too demanding as java.security.interfaces.RSAKey (which is a sub interface of RSAPrivateKey) provides the bitlength() method . Some hsm-box implementations implement RSAKey only thus MSAL4J is not useable with these hsmbox providers. The later JDK signing algorithm runs fine without RSAPrivateKey being implemented.

JoergBudi commented 8 months ago

@microsoft-github-policy-service agree

JoergBudi commented 8 months ago

Hi, can I do anything to let the missing "MSAL Java CI Build" check run ?

bgavrilMS commented 8 months ago

@Avery-Dunn will provide an update. Current thinking is to remove that check altogether, although if tests pass this is ok too.

Avery-Dunn commented 8 months ago

@JoergBudi : Those tests are run internally only (to prevent any code injection), but I tested your changes and it all ran fine. However, as @bgavrilMS mentioned we talked about this a bit and decided that it'd be better if MSAL Java doesn't enforce key size limits at all (some MSALs do, others don't). I've just created a PR to remove the requirement (https://github.com/AzureAD/microsoft-authentication-library-for-java/pull/749), and once we're sure there aren't any unexpected side effects we'll merge that one and get it in our next release.

Avery-Dunn commented 7 months ago

As of https://github.com/AzureAD/microsoft-authentication-library-for-java/pull/749 in the latest 1.14.1 release, we no longer check the key size so this PR is no longer needed. Thanks for bringing this to our attention, and feel free to let us know about any other issues.