refactor(ATL-6924): add support for other keys #834

EzequielPostan · 2024-04-17T01:05:28Z

Overview

Fix ATL-6707 and ATL-6924

Screenshots

Checklists

Pre-submit checklist:

Self-reviewed the diff
New code has proper comments/documentation/tests
Any changes not covered by tests have been tested manually
The README files are updated
If new libraries are included, they have licenses compatible with our project
If there is a db migration altering existing tables, there is a proper migration test

Pre-merge checklist:

Commits have useful messages
Review clarifications made it into the code

This commit replaces the used of ECPublicKey

github-actions · 2024-04-17T01:07:17Z

Unit Test Results

274 tests 273 ✔️ 24s ⏱️
  39 suites     1 💤
  39 files     0 ❌

Results for commit a41f5ee.

♻️ This comment has been updated with latest results.

atala-dev · 2024-04-17T01:50:58Z

🦙 MegaLinter status: ⚠️ WARNING

Descriptor	Linter	Files	Errors
✅ ACTION	actionlint	1	0
⚠️ MARKDOWN	markdownlint	5	90
⚠️ MARKDOWN	markdown-link-check	5	10
✅ MARKDOWN	markdown-table-formatter	5	0
⚠️ PROTOBUF	protolint	5	5
✅ REPOSITORY	dustilock	yes	no
✅ REPOSITORY	git_diff	yes	no
✅ REPOSITORY	grype	yes	no
⚠️ REPOSITORY	kics	yes	51
✅ REPOSITORY	syft	yes	no
✅ REPOSITORY	trivy-sbom	yes	no
⚠️ REPOSITORY	trufflehog	yes	1
✅ SQL	sql-lint	2	0
⚠️ YAML	prettier	2	1
✅ YAML	v8r	2	0
✅ YAML	yamllint	2	0

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by

EzequielPostan · 2024-04-17T02:09:22Z

node/src/main/scala/io/iohk/atala/prism/node/repositories/daos/package.scala

-        assert(curveId == ECConfig.getCURVE_NAME)
-        val javaPublicKey: ECPublicKey =
-          EC.toPublicKeyFromCompressed(compressed)
+        val publicKeyData: PublicKeyData = CompressedPublicKeyData(
+          curveId,
+          compressed
+        )


up to this point, we were always returning an ECPublicKey instance and assuming it was a Secp256k1 key
Now, we are taking the compressed key from the db and generating a wrapper of the curve name and compressed key

EzequielPostan · 2024-04-17T02:13:05Z

node/src/main/scala/io/iohk/atala/prism/node/grpc/ProtoCodecs.scala

-  def toECKeyData(key: ECPublicKey): node_models.ECKeyData = {
-    val point = key.getCurvePoint


before, the code assumed we had an ECPublicKey which could be decomposed into x and y coordinates. Now we assume it is always a compressed key

EzequielPostan · 2024-04-17T02:13:59Z

node/src/main/scala/io/iohk/atala/prism/node/grpc/ProtoCodecs.scala

-      ecKeyData: node_models.ECKeyData,
+      compressedEcKeyData: node_models.CompressedECKeyData,


as we now assume we always retrieve a compressed key, we request that the model to convert is a compressed one

does it apply to new key types and old key type as well? would that require a change in spec then?

IMO I wouldn't require change on the specs. Would just put in the spec that the compressed version is recommended.

this method is applied on data retrieved from the node DB, we store keys in compressed format, meaning that all keys we retrieve will be compressed indistinctly of how the user submitted it. So, no need for spec changes for this part

We do need to tell in the spec that Ed25519 and X25519 keys can only be sent in compressed format (which I understand is fine, please correct if I am wrong @FabioPinheiro ). Secp keys can be sent in any of the formats and the node will work fine

This commit simplifies models by deleting classes not needed at parsing time

FabioPinheiro · 2024-04-17T10:44:10Z

I checking your PR. I realize we are working or doing the same thing.

This commit replaces some uses of Array and uses Vector instead to allow more transparent equality checks

shotexa · 2024-04-18T10:22:50Z

node/src/main/scala/io/iohk/atala/prism/node/crypto/CryptoUtils.scala

+
+  Security.addProvider(provider)
+
+  def hash(bArray: Array[Byte]): Vector[Byte] = {


Would it make sense to specify the type of hash (sha256) in the name of the function? it does not matter that much in this codebase, but if we've been working on something we plan to extend, I'd prefer to name it more specific.

shotexa · 2024-04-18T10:26:11Z

node/src/main/scala/io/iohk/atala/prism/node/crypto/CryptoUtils.scala

+    bytes.map(byte => f"${byte & 0xff}%02x").mkString
+  }
+
+  def hexedHash(bArray: Array[Byte]): String = {


Can you also add an alternative to https://github.com/input-output-hk/atala-prism-sdk/blob/c5144fb9a078aef6ad4b54aa6290ee1797a23da2/prism-crypto/src/commonMain/kotlin/io/iohk/atala/prism/crypto/Sha256Digest.kt#L22 ? I need it in identity module as well

sounds good, let me create the type too so we remove Vector

This commit adds support to decode hex strings into Sha256Hash bytes

EzequielPostan · 2024-04-18T17:19:44Z

@shotexa , I added the functions you requested, let me know if we can merge

shotexa

LGTM!

This commit improves the organization of CryptoUtils methods It also delete the used of SDK methods related to Sha256 hashing

EzequielPostan · 2024-04-18T23:45:08Z

I added a commit to improve the crypto utilities @shotexa after you approved the PR. I think the new refactor will be better for your work, if possible, take a look at the last commit

* refactor: clean up migrations (#830) * Add everything Signed-off-by: Shota Jolbordi <[email protected]> * Remove migration tests Signed-off-by: Shota Jolbordi <[email protected]> * add ingnore invalid create option for postres mega linter Signed-off-by: Shota Jolbordi <[email protected]> * Try mega linter 7 Signed-off-by: Shota Jolbordi <[email protected]> * edit megalinter.yaml Signed-off-by: Shota Jolbordi <[email protected]> * Disable some linters Signed-off-by: Shota Jolbordi <[email protected]> --------- Signed-off-by: Shota Jolbordi <[email protected]> * refactor(node): remove legacy BE files (#832) * Start cleaning up docs Signed-off-by: Shota Jolbordi <[email protected]> * Remove more files Signed-off-by: Shota Jolbordi <[email protected]> * Fix compilation issue Signed-off-by: Shota Jolbordi <[email protected]> * Trim docs Signed-off-by: Shota Jolbordi <[email protected]> * Fix compilation errors Signed-off-by: Shota Jolbordi <[email protected]> * Remove some bitcoin test resources and old docs Signed-off-by: Shota Jolbordi <[email protected]> * Lint Signed-off-by: Shota Jolbordi <[email protected]> * Fix tests Signed-off-by: Shota Jolbordi <[email protected]> --------- Signed-off-by: Shota Jolbordi <[email protected]> * refactor: Delete legacy operation related to credentials (#831) * ATL-6668: Delete legacy operation related to credentials This commit deletes the IssueCredentialBatchOperation and RevokeCredentialsOperation. It does not delete node API, daos nor repositories related to credentials * ATL-6668: Delete more files related to VCs This commit deletes even more files related to VC operations. It also removes some dependencies on the old SDK * ATL-6668: Delete unused tables This commit deletes tables and indexes from VC legacy operations * ATL-6668: Fix formatting * ATL-6668: Address review comments This commit deletes references to legacy VC code in protobuf and sql definitions * refactor(ATL-6924): add support for other keys (#834) * ATL-6924: Replace users of ECPublicKey This commit replaces the used of ECPublicKey * ATL-6924: Simplify models This commit simplifies models by deleting classes not needed at parsing time * ATL-6924: Simplify types This commit replaces some uses of Array and uses Vector instead to allow more transparent equality checks * ATL-6924: Add support for encoding functions This commit adds support to decode hex strings into Sha256Hash bytes * ATL-6924: Refactor CryptoUtils and remove SHA256Digest This commit improves the organization of CryptoUtils methods It also delete the used of SDK methods related to Sha256 hashing * refactor(ATL-6926): remove crypto sdk (#838) * ATL-6926: Remove uses of EC classes This commit removes most uses of the legacy crypto SDK * ATL-6926: Fix missing implementations This commit adds missing implementations for key encoding algorithms * ATL-6926: Fix tests * ATL-6926: Clean up code and tests * ATL-6926: Correct typo and add tests This commit adds the final tests to validate the cryptography implementation that replaces the SDK * refactor(node): Remove prism identity (#840) * clean up source files, replace DID usages Signed-off-by: Shota Jolbordi <[email protected]> * Fix some tests Signed-off-by: Shota Jolbordi <[email protected]> * Linting Signed-off-by: Shota Jolbordi <[email protected]> * Fix test Signed-off-by: Shota Jolbordi <[email protected]> * Fix fromString on DID Signed-off-by: Shota Jolbordi <[email protected]> * formatting Signed-off-by: Shota Jolbordi <[email protected]> * Address some PR comments Signed-off-by: Shota Jolbordi <[email protected]> --------- Signed-off-by: Shota Jolbordi <[email protected]> * refactor: crypto utils and tests (#841) * Remove uses of the old SDK This commit removes the uses of the old SDK and update tests * Refactor CryptoUtils class This commit re-organizes and renames methods for better use * Remove last dependency on old SDK This commit removes the last dependencies on the old SDK * docs(ATL-6669): update readme (#833) * ATL-6669: Update README This commits updates the README to focus oly on the PRISM node. * ATL-6669: Add IDE notes in README This commit adds instructions to load the project in different IDEs. The commit also removes incorrect sentences * ATL-6669: Delete old README and complete new one * ATL-6669: Add pre-commit configuration This commit adds a pre-commit hook that runs scalafmt and instructions to configure it * ATL-6669: Refer to identus configuration This commit updates the README and refers users to read identus documentation in order to configure the node. * ATL-7040: Delete DID based authentication (#843) This commit deletes the legacy DID based authentication * refactor(node): build files and add default port for gRPC server (#844) * Change some build files Signed-off-by: Shota Jolbordi <[email protected]> * Rmove unused files Signed-off-by: Shota Jolbordi <[email protected]> * Remove unused files Signed-off-by: Shota Jolbordi <[email protected]> * Remove outer node folder Signed-off-by: Shota Jolbordi <[email protected]> * Revert version name Signed-off-by: Shota Jolbordi <[email protected]> * Remove unneeded resolver from the build Signed-off-by: Shota Jolbordi <[email protected]> * Update fs2 Signed-off-by: Shota Jolbordi <[email protected]> * Start adding tests Signed-off-by: Shota Jolbordi <[email protected]> * Add more tests Signed-off-by: Shota Jolbordi <[email protected]> * Fix 1 failing test Signed-off-by: Shota Jolbordi <[email protected]> * Minor adjustment in docs Signed-off-by: Shota Jolbordi <[email protected]> * Remove test report aggregation step Signed-off-by: Shota Jolbordi <[email protected]> --------- Signed-off-by: Shota Jolbordi <[email protected]> * fix: Add http scheme support and removed the constraint from public keys table (#848) * Add http scheme support and removed the contraint from publickeys table Signed-off-by: mineme0110 <[email protected]> * ran scalafmt Signed-off-by: mineme0110 <[email protected]> --------- Signed-off-by: mineme0110 <[email protected]> --------- Signed-off-by: Shota Jolbordi <[email protected]> Signed-off-by: mineme0110 <[email protected]> Co-authored-by: shotexa <[email protected]> Co-authored-by: Ezequiel Postan <[email protected]> Co-authored-by: Shailesh Patil <[email protected]>

ATL-6924: Replace users of ECPublicKey

49ca1be

This commit replaces the used of ECPublicKey

EzequielPostan changed the base branch from main to open-source-node April 17, 2024 01:06

EzequielPostan commented Apr 17, 2024

View reviewed changes

ATL-6924: Simplify models

01967cc

This commit simplifies models by deleting classes not needed at parsing time

FabioPinheiro mentioned this pull request Apr 17, 2024

NOT TO MERGE Crypto agility ATL-6707 #835

Open

EzequielPostan marked this pull request as ready for review April 17, 2024 17:19

yshyn-iohk approved these changes Apr 17, 2024

View reviewed changes

EzequielPostan force-pushed the ATL-6924-add-support-for-other-keys branch from 12c8ca7 to ba4bf22 Compare April 17, 2024 22:30

ATL-6924: Simplify types

a8afcd6

This commit replaces some uses of Array and uses Vector instead to allow more transparent equality checks

EzequielPostan force-pushed the ATL-6924-add-support-for-other-keys branch from ba4bf22 to a8afcd6 Compare April 17, 2024 22:40

EzequielPostan requested a review from shotexa April 17, 2024 23:28

shotexa reviewed Apr 18, 2024

View reviewed changes

ATL-6924: Add support for encoding functions

73ff642

This commit adds support to decode hex strings into Sha256Hash bytes

shotexa approved these changes Apr 18, 2024

View reviewed changes

ATL-6924: Refactor CryptoUtils and remove SHA256Digest

a41f5ee

This commit improves the organization of CryptoUtils methods It also delete the used of SDK methods related to Sha256 hashing

FabioPinheiro approved these changes Apr 19, 2024

View reviewed changes

shotexa approved these changes Apr 19, 2024

View reviewed changes

EzequielPostan merged commit fcb8b52 into open-source-node Apr 19, 2024
4 checks passed

EzequielPostan deleted the ATL-6924-add-support-for-other-keys branch April 19, 2024 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(ATL-6924): add support for other keys #834

refactor(ATL-6924): add support for other keys #834

EzequielPostan commented Apr 17, 2024 •

edited by mkbreuningIOHK

Loading

github-actions bot commented Apr 17, 2024 •

edited

Loading

atala-dev commented Apr 17, 2024 •

edited

Loading

EzequielPostan Apr 17, 2024

EzequielPostan Apr 17, 2024

EzequielPostan Apr 17, 2024

shotexa Apr 18, 2024

FabioPinheiro Apr 18, 2024

EzequielPostan Apr 18, 2024

FabioPinheiro commented Apr 17, 2024

shotexa Apr 18, 2024

shotexa Apr 18, 2024

EzequielPostan Apr 18, 2024

EzequielPostan commented Apr 18, 2024

shotexa left a comment

EzequielPostan commented Apr 18, 2024

		def toECKeyData(key: ECPublicKey): node_models.ECKeyData = {
		val point = key.getCurvePoint

		ecKeyData: node_models.ECKeyData,
		compressedEcKeyData: node_models.CompressedECKeyData,


		Security.addProvider(provider)

		def hash(bArray: Array[Byte]): Vector[Byte] = {

refactor(ATL-6924): add support for other keys #834

refactor(ATL-6924): add support for other keys #834

Conversation

EzequielPostan commented Apr 17, 2024 • edited by mkbreuningIOHK Loading

Overview

Screenshots

Checklists

github-actions bot commented Apr 17, 2024 • edited Loading

Unit Test Results

atala-dev commented Apr 17, 2024 • edited Loading

🦙 MegaLinter status: ⚠️ WARNING

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FabioPinheiro commented Apr 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EzequielPostan commented Apr 18, 2024

shotexa left a comment

Choose a reason for hiding this comment

EzequielPostan commented Apr 18, 2024

EzequielPostan commented Apr 17, 2024 •

edited by mkbreuningIOHK

Loading

github-actions bot commented Apr 17, 2024 •

edited

Loading

atala-dev commented Apr 17, 2024 •

edited

Loading