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

Add Support for secp256k1 algorithms (AKA ES256K) #439

Merged
merged 4 commits into from
Sep 17, 2020
Merged

Conversation

jimmyjames
Copy link
Contributor

@jimmyjames jimmyjames commented Sep 14, 2020

Changes

This replaces #428 to add support for the secp256k1 algorithm. The only changes from that original PR are the ones requested on the review:

  • Remove the addition of a deprecated method
  • Whitespace fixes

It also removes the README txt file.

For review purposes, see the commits for this PR; the changes to the original PR have been captured as separate commits.

References

#428

Testing

  • This change adds test coverage
  • This change has been tested on the latest version of Java or why not

How to create a secp256k1 key to be used with ES256K algorithm:

Generate the key

openssl ecparam -genkey -name secp256k1 -out KEY.pem -text

  • The file is split into 2 parts a curve specification (EC PARAMETERS) and the key value
  • This format is not suitable to be read by the PermUtils PEM reader so we have to convert it into PCKS8 format:

openssl pkcs8 -topk8 -inform PEM -outform PEM -nocrypt -in KEY.pem -out ec256k-key.private.pem

The public key is then calculated as follows:

openssl ec -pubout -in KEY.pem -out ec256k-key-public.pem

How to get DER signature:

ECDSAAlgorithm algorithm256 = (ECDSAAlgorithm) Algorithm.ECDSA256((ECPublicKey) readPublicKeyFromFile(PUBLIC_KEY_FILE_256K, "EC"), (ECPrivateKey) readPrivateKeyFromFile(PRIVATE_KEY_FILE_256K, "EC"));
String[] parts = ES256K_JWT.split("\\.");

byte[] derSignature = algorithm256.JOSEToDER(Base64.decodeBase64(parts[2]));
String jwt=parts[0]+"."+parts[1]+"."+Base64.encodeBase64URLSafeString(derSignature);

Checklist

@jimmyjames jimmyjames added CH: Added medium This PR may require moderate effort to action, or contains many changes to review labels Sep 14, 2020
@jimmyjames jimmyjames added this to the v3-Next milestone Sep 14, 2020
@jimmyjames jimmyjames requested review from a team and lbalmaceda September 14, 2020 22:38
lbalmaceda
lbalmaceda previously approved these changes Sep 14, 2020

@Test
public void shouldAddKeyIdIfAvailableFromECDSAKAlgorithms() throws Exception {
ECPrivateKey privateKey = (ECPrivateKey) PemUtils.readPrivateKeyFromFile(PRIVATE_KEY_FILE_EC_256K
Copy link
Contributor

Choose a reason for hiding this comment

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

weird formatting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed a commit to fix running the formatter.


String signed = JWT.create().sign(Algorithm.ECDSA256K(publicKey, privateKey) );
Copy link
Contributor

Choose a reason for hiding this comment

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

might be good to run the formatter shortcut option+command+L to update all these changes

Suggested change
String signed = JWT.create().sign(Algorithm.ECDSA256K(publicKey, privateKey) );
String signed = JWT.create().sign(Algorithm.ECDSA256K(publicKey, privateKey));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, added a commit to do just that.

import java.security.spec.InvalidKeySpecException;
import java.security.spec.PKCS8EncodedKeySpec;
import java.security.spec.X509EncodedKeySpec;
import java.security.spec.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the other approach but this is a test so it's fine. However, try running the import organizer shortcut control+option+O

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@lbalmaceda
Copy link
Contributor

Good to merge 👍

@lbalmaceda lbalmaceda merged commit 4a55c30 into master Sep 17, 2020
@jimmyjames jimmyjames deleted the secp256k1-redux branch September 18, 2020 21:40
@jimmyjames jimmyjames modified the milestones: v3-Next, 3.11.0 Sep 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added medium This PR may require moderate effort to action, or contains many changes to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants