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

Implement KecCak-256 and Secp256k1 utility functions #20640

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

carlpulley-da
Copy link
Contributor

@carlpulley-da carlpulley-da commented Jan 22, 2025

Work that is part of adding CCTP crypto primitives into the engine - ref: https://github.com/DACH-NY/canton-network-utilities/issues/2746

On this PR we add crypto utility functions. Follow PRs will implement engine BuiltIns and Daml-LF primitive constants.

  • implement KecCak-256 digest utility function
  • implement Secp256k1 sign/verify utility functions
  • basic testing for the above

@@ -23,6 +23,7 @@ da_scala_test(
srcs = glob(["src/test/scala/**/*.scala"]),
deps = [
":crypto",
"@maven//:org_bouncycastle_bcprov_jdk15on",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added for testing of crypto primitives

@@ -43,4 +43,7 @@ final class MessageDigestPrototype(val algorithm: String) {

object MessageDigestPrototype {
final val Sha256 = new MessageDigestPrototype("SHA-256")

// lazily load the Keccak-256 digest algorithm as the runtime needs a suitable JCE provider (e.g. such as BouncyCastle)
final lazy val KecCak256 = new MessageDigestPrototype("KECCAK-256")
Copy link
Contributor Author

@carlpulley-da carlpulley-da Jan 22, 2025

Choose a reason for hiding this comment

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

Doing this should allow code that doesn't utilise CCTP crypto primitives to run

behavior of MessageSignaturePrototype.getClass.getSimpleName

override def beforeAll(): Unit = {
val _ = Security.insertProviderAt(new BouncyCastleProvider, 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order for Secp256k1 curve to be available, we appear to need to force BouncyCastle to be at the start of the chain

Copy link
Contributor

Choose a reason for hiding this comment

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

Security.addProvider should also work if you later specify to use BC explicitly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I originally did that, I hit issues when I tried to load the Secp256k1 curve (e.b. with new ECGenParameterSpec("secp256k1")). IIRC, the typical exception message was along the lines of "curve not found".

Placing BC at the start of the provider chain side stepped this issue.

Maybe there's some additional configuration that I need to do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specific exception at test time is:

- should be able to sign and verify messages *** FAILED *** (117 milliseconds)
  java.security.SignatureException: Curve not supported: org.bouncycastle.jce.spec.ECNamedCurveSpec@64a8c844
  at jdk.crypto.ec/sun.security.ec.ECDSASignature.engineSign(ECDSASignature.java:486)
  at java.base/java.security.Signature$Delegate.engineSign(Signature.java:1423)
  at java.base/java.security.Signature.sign(Signature.java:712)
  at com.daml.crypto.MessageSignaturePrototypeUtil.sign(MessageSignaturePrototypeUtil.scala:15)
  at com.daml.crypto.MessageSignaturePrototypeSpec.$anonfun$new$2(MessageSignaturePrototypeSpec.scala:35)
  ...

Copy link
Contributor

Choose a reason for hiding this comment

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

even when you do KeyPairGenerator.getInstance("EC", "BC") to make sure the engine for key gen is the one from BC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, resolved the my issue above. When fetching the signatures, its necessary to specify the provider - so use: Signature.getInstance(algorithm, "BC")

@carlpulley-da carlpulley-da marked this pull request as ready for review January 22, 2025 15:18
behavior of MessageSignaturePrototype.getClass.getSimpleName

override def beforeAll(): Unit = {
val _ = Security.insertProviderAt(new BouncyCastleProvider, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Security.addProvider should also work if you later specify to use BC explicitly

MessageSignaturePrototype.Secp256k1.algorithm shouldBe "SHA256withECDSA"
}

it should "be able to sign and verify messages" in {
Copy link
Contributor

Choose a reason for hiding this comment

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

what about testing for negative cases, such as trying to verify a signature with a different public key and ensure that it fails verification

messageSign.sign()
}

def verify(signature: Array[Byte], message: Array[Byte], publicKey: PublicKey): Boolean = {
Copy link
Contributor

Choose a reason for hiding this comment

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

be aware that JCE tends to throw exceptions for example I see the following cases:

  • signature is not in DER format as expected by JCE
  • public key not in DER and SPKI format as expected by JCE
  • public key EC point not on the expected curve, e.g., secp256k1

make sure to handle those GeneralSecurityExceptions. for a prototype level I would not add explicit checks for the cases above, but make sure to handle the exceptions from here in higher levels.

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.

2 participants