Use recommended algorithm in assertions #990
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bgavrilMS
reviewed
Sep 22, 2025
| * @return A ClientAssertion containing the signed JWT | ||
| * @throws Exception If JWT creation or signing fails | ||
| */ | ||
| private static ClientAssertion generateRs256Jwt(String clientId, ClientCertificate credential, |
There was a problem hiding this comment.
There seem to be a lot of code dup between thjis method and the ps256 version
bgavrilMS
reviewed
Sep 22, 2025
bgavrilMS
reviewed
Sep 22, 2025
| } | ||
|
|
||
| @Test | ||
| void JwtHelper_buildJwt_UsesPSS256WhenSupported() throws Exception { |
There was a problem hiding this comment.
The way it's implemented in MSAL .NET is:
- for AAD, use SHA2 and PSS
- for ADFS, use SHA1 and PKCS1
I see that the way to specify a certificate in MSAL Java is similar. So why not have some higher level tests, i.e. start from the public API and assert what gets put on the wire?
bgavrilMS
requested changes
Sep 22, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes the issue described in #958
Microsoft recommends PS256 for assertions and other MSALs follow this, however MSAL Java still used RS256. It seems like PSS support was only added in Java 11, but it was thankfully backported to Java 8.
This PR refactors JwtHelper to use the recommended algorithm and adjusts/expands tests in order to cover the new behavior.
In addition, while making the changes I discovered a weird edge case:
PrivateKeyPrivateKeyfrom the Windows-MY keystore the underlying type is "sun.security.mscapi.CPrivateKey"sun.security.mscapi.CPrivateKeytype was accepted by the Signature instance created with "SHA256withRSA", but for some reason the Signature created with "RSASSA-PSS" throws an error about it not specifically being anRSAPrivateKeyinstanceSince our integration tests use a cert stored in Windows-MY simply changing the assertion to use PS256 would've broken many of our tests. I'm sure plenty of our customers retrieve certs in the same way, and I'm not sure what other key types would cause this error.
So to avoid any breaking changes, this PR also implements a mechanism to fallback to the old RS256 style when we get a certain exception, and has extra tests covering this fallback behavior.