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

PYIC-5099: Enable RSA signature validation #2001

Merged
merged 8 commits into from
Jun 24, 2024
Merged

PYIC-5099: Enable RSA signature validation #2001

merged 8 commits into from
Jun 24, 2024

Conversation

MikeCollingwood
Copy link
Contributor

@MikeCollingwood MikeCollingwood commented Jun 11, 2024

Proposed changes

What changed

  • Derive signing algorithm from signing key: link
  • Use signing algorithm to verify with correct algorithm: link
  • Added test case for verifying with RSA: link
  • Updated the local running cri config: link

Why did it change

  • So we can validate RSA-signed VCs we expect from DWP

Issue tracking

Remaining to do:

  • Change the config manually for the DWP KBV stub to be an RSA key

@MikeCollingwood MikeCollingwood marked this pull request as ready for review June 13, 2024 13:58
@MikeCollingwood MikeCollingwood requested review from a team as code owners June 13, 2024 13:58
throws ParseException, JOSEException {
return switch (signingAlgorithm) {
case EC -> new ECDSAVerifier(ECKey.parse(signingKey));
case RSA -> new RSASSAVerifier(RSAKey.parse(signingKey));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Digging a bit further into our libraries, it looks like you might be able to do this completely automatically.

Have a look at DefaultJWSVerifierFactory.createJWSVerifier() and JWK.parse()

Copy link
Contributor Author

@MikeCollingwood MikeCollingwood Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw JWK.parse, and am now updating to use the key to determine the algorithm. Will also look at createJWSVerifier

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to play around with createJWSVerifier but it feels like it may not be as much of a simplification as wanted compared to the new version of the getVerifier method.

            var signingKeyJws = JWSObject.parse(signingKey);
            var keyFactory = KeyFactory.getInstance(signingKeyJws.getHeader().getAlgorithm().toString());
            keyFactory.generatePublic(new PKCS8EncodedKeySpec(signingKey.getBytes())
            );

            new DefaultJWSVerifierFactory().createJWSVerifier(signingKeyJws.getHeader(), Key);

            var formattedVc = EC.equals(signingKeyJws.getHeader().getAlgorithm()) ? transcodeSignatureIfDerFormat(vc) : vc;

@shivanshuit914 shivanshuit914 force-pushed the pyic-5099 branch 4 times, most recently from 9a83c92 to 5e5446c Compare June 20, 2024 09:14
Copy link

sonarcloud bot commented Jun 21, 2024

@shivanshuit914 shivanshuit914 merged commit 4b1cef1 into main Jun 24, 2024
6 checks passed
@shivanshuit914 shivanshuit914 deleted the pyic-5099 branch June 24, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants