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

fix: incorrect parsing of public key coordinates and enable some tests again #1215

Merged
merged 5 commits into from
Jun 21, 2024

Conversation

shotexa
Copy link
Contributor

@shotexa shotexa commented Jun 21, 2024

Description:

This PR fixes an issue with JWK coordinate generation for EcdsaSecp256k1Signature2019Proof.

JVM represents big integers in two's complement, which means that the leading byte is a sign (0 for pos and 1 for negative), because of this, the following line of code yields either a 32-byte array of a 33-byte array

pk.getW.getAffineX.toByteArray

Since getAffineX returns BigInteger, which is represented in two's complement, if the number is positive and it starts with one, toByteArray function appends 0 at the beginning to signify that it is a positive number. Secp256k1 coordinates can't be negative anyway so seems like Apollo is expecting an unsigned integer in binary representation, and we ware generating a signed integer in two's complement representation, because of this, sometimes when X and Y are positive numbers that start with 1, we would get 33-byte arrays.

Alternatives Considered (optional):

Link to existing ADR (Architecture Decision Record), if any. If relevant, describe other approaches explored and the selected approach. Documenting why the methods were not selected will create a knowledge base for future reference, helping prevent others from revisiting less optimal ideas.

Checklist:

  • My PR follows the contribution guidelines of this project
  • My PR is free of third-party dependencies that don't comply with the Allowlist
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked the PR title to follow the conventional commit specification

@shotexa shotexa self-assigned this Jun 21, 2024
Signed-off-by: Hyperledger Bot <[email protected]>
Co-authored-by: Hyperledger Bot <[email protected]>
Copy link
Contributor

github-actions bot commented Jun 21, 2024

Integration Test Results

15 files  +1  15 suites  +1   2s ⏱️ ±0s
33 tests +4  33 ✅ +4  0 💤 ±0  0 ❌ ±0 
46 runs  +4  46 ✅ +4  0 💤 ±0  0 ❌ ±0 

Results for commit e2474e5. ± Comparison against base commit cb4d479.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jun 21, 2024

Unit Test Results

 96 files  ±0   96 suites  ±0   22m 42s ⏱️ -4s
826 tests ±0  818 ✅ +1  8 💤 ±0  0 ❌  - 1 
833 runs  ±0  825 ✅ +1  8 💤 ±0  0 ❌  - 1 

Results for commit e2474e5. ± Comparison against base commit cb4d479.

♻️ This comment has been updated with latest results.

@shotexa shotexa marked this pull request as ready for review June 21, 2024 16:01
@shotexa shotexa merged commit 5398a75 into main Jun 21, 2024
12 checks passed
@shotexa shotexa deleted the NO-JIRA-fix-inorrect-parsing-of-pk-cords branch June 21, 2024 17:07
hyperledger-bot added a commit that referenced this pull request Jul 1, 2024
# [1.37.0-snapshot.test.1](cloud-agent-v1.36.1...cloud-agent-v1.37.0-snapshot.test.1) (2024-07-01)

### Bug Fixes

* avoid name and operationId conflict when creating oas client ([#1233](#1233)) ([73e8e24](73e8e24))
* incorrect parsing of public key coordinates and enable some tests again ([#1215](#1215)) ([5398a75](5398a75))
* KeyID from String to Opaque Type and presentation job cleanup  ([#1190](#1190)) ([b813faf](b813faf))
* make init-script.sh idempotent [#1173](#1173) ([#1194](#1194)) ([1712062](1712062))
* migrate and repair in case of renaming issues ([#1211](#1211)) ([cb4d479](cb4d479))
* Present Error Handling (Part 1: Repo Changes) ([#1172](#1172)) ([13e2447](13e2447))
* Present Error Handling Job ([#1204](#1204)) ([3191d8b](3191d8b))
* present error handling Part 2 ([#1177](#1177)) ([9ac6e52](9ac6e52))
* Verification Policy Error Handling ([#1228](#1228)) ([6117a3c](6117a3c))

### Features

* add oidc4vci protocol MVP ([#1182](#1182)) ([3ae91dc](3ae91dc))
* add schemaId to the issuance session ([#1199](#1199)) ([97f5d83](97f5d83))
* add support for EcdsaSecp256k1Signature2019Proof and fix pk encoding for EddsaJcs2022Proof ([#1127](#1127)) ([e617ded](e617ded))
* **agent:** add browser fingerprint label to http metrics ([#1231](#1231)) ([f090554](f090554))
* ATL 6829 - Integrate ZIO failures and defects ADR in credential status list ([#1175](#1175)) ([dffad1d](dffad1d))
* ATL-6832 ZIO failures and defects in entity controller ([#1203](#1203)) ([9050094](9050094))
* ATL-6833 integrate ZIO failures and defects in wallet event controller ([#1186](#1186)) ([8bc2018](8bc2018))
* ATL-6834 Use ZIO Failures and Defects effectively in the Issue flow ([#1139](#1139)) ([ede7b77](ede7b77))
* Handle Error in Background Jobs - Improve the way we store errors and defects in DB ([#1218](#1218)) ([e3cadc9](e3cadc9))
* implement ADR Use ZIO Failures and Defects Effectively - Mercury should not throw exceptions ([#1192](#1192)) ([a4ce87f](a4ce87f))
* improve OpenAPI's PresentProof to make more consistent ([#1130](#1130)) ([bdc5d20](bdc5d20))
* Integrate ZIO failures and defects ADR in DID Registrar Controller and Mercury ([#1180](#1180)) ([d8e2120](d8e2120))
* SDJWT holder key binding ([#1185](#1185)) ([628f2f0](628f2f0))
* use kid in the jwt proof header of OID4VCI CredentialIssue request ([#1184](#1184)) ([ee53eda](ee53eda))
* use the compact format in SD-JWT ([#1169](#1169)) ([65da651](65da651))

Signed-off-by: Allain Magyar <[email protected]>
patlo-iog pushed a commit that referenced this pull request Jul 1, 2024
# [1.37.0-snapshot.test.1](cloud-agent-v1.36.1...cloud-agent-v1.37.0-snapshot.test.1) (2024-07-01)

### Bug Fixes

* avoid name and operationId conflict when creating oas client ([#1233](#1233)) ([73e8e24](73e8e24))
* incorrect parsing of public key coordinates and enable some tests again ([#1215](#1215)) ([5398a75](5398a75))
* KeyID from String to Opaque Type and presentation job cleanup  ([#1190](#1190)) ([b813faf](b813faf))
* make init-script.sh idempotent [#1173](#1173) ([#1194](#1194)) ([1712062](1712062))
* migrate and repair in case of renaming issues ([#1211](#1211)) ([cb4d479](cb4d479))
* Present Error Handling (Part 1: Repo Changes) ([#1172](#1172)) ([13e2447](13e2447))
* Present Error Handling Job ([#1204](#1204)) ([3191d8b](3191d8b))
* present error handling Part 2 ([#1177](#1177)) ([9ac6e52](9ac6e52))
* Verification Policy Error Handling ([#1228](#1228)) ([6117a3c](6117a3c))

### Features

* add oidc4vci protocol MVP ([#1182](#1182)) ([3ae91dc](3ae91dc))
* add schemaId to the issuance session ([#1199](#1199)) ([97f5d83](97f5d83))
* add support for EcdsaSecp256k1Signature2019Proof and fix pk encoding for EddsaJcs2022Proof ([#1127](#1127)) ([e617ded](e617ded))
* **agent:** add browser fingerprint label to http metrics ([#1231](#1231)) ([f090554](f090554))
* ATL 6829 - Integrate ZIO failures and defects ADR in credential status list ([#1175](#1175)) ([dffad1d](dffad1d))
* ATL-6832 ZIO failures and defects in entity controller ([#1203](#1203)) ([9050094](9050094))
* ATL-6833 integrate ZIO failures and defects in wallet event controller ([#1186](#1186)) ([8bc2018](8bc2018))
* ATL-6834 Use ZIO Failures and Defects effectively in the Issue flow ([#1139](#1139)) ([ede7b77](ede7b77))
* Handle Error in Background Jobs - Improve the way we store errors and defects in DB ([#1218](#1218)) ([e3cadc9](e3cadc9))
* implement ADR Use ZIO Failures and Defects Effectively - Mercury should not throw exceptions ([#1192](#1192)) ([a4ce87f](a4ce87f))
* improve OpenAPI's PresentProof to make more consistent ([#1130](#1130)) ([bdc5d20](bdc5d20))
* Integrate ZIO failures and defects ADR in DID Registrar Controller and Mercury ([#1180](#1180)) ([d8e2120](d8e2120))
* SDJWT holder key binding ([#1185](#1185)) ([628f2f0](628f2f0))
* use kid in the jwt proof header of OID4VCI CredentialIssue request ([#1184](#1184)) ([ee53eda](ee53eda))
* use the compact format in SD-JWT ([#1169](#1169)) ([65da651](65da651))

Signed-off-by: Allain Magyar <[email protected]>
Signed-off-by: Pat Losoponkul <[email protected]>
patlo-iog pushed a commit that referenced this pull request Jul 1, 2024
# [1.37.0-snapshot.test.1](cloud-agent-v1.36.1...cloud-agent-v1.37.0-snapshot.test.1) (2024-07-01)

### Bug Fixes

* avoid name and operationId conflict when creating oas client ([#1233](#1233)) ([73e8e24](73e8e24))
* incorrect parsing of public key coordinates and enable some tests again ([#1215](#1215)) ([5398a75](5398a75))
* KeyID from String to Opaque Type and presentation job cleanup  ([#1190](#1190)) ([b813faf](b813faf))
* make init-script.sh idempotent [#1173](#1173) ([#1194](#1194)) ([1712062](1712062))
* migrate and repair in case of renaming issues ([#1211](#1211)) ([cb4d479](cb4d479))
* Present Error Handling (Part 1: Repo Changes) ([#1172](#1172)) ([13e2447](13e2447))
* Present Error Handling Job ([#1204](#1204)) ([3191d8b](3191d8b))
* present error handling Part 2 ([#1177](#1177)) ([9ac6e52](9ac6e52))
* Verification Policy Error Handling ([#1228](#1228)) ([6117a3c](6117a3c))

### Features

* add oidc4vci protocol MVP ([#1182](#1182)) ([3ae91dc](3ae91dc))
* add schemaId to the issuance session ([#1199](#1199)) ([97f5d83](97f5d83))
* add support for EcdsaSecp256k1Signature2019Proof and fix pk encoding for EddsaJcs2022Proof ([#1127](#1127)) ([e617ded](e617ded))
* **agent:** add browser fingerprint label to http metrics ([#1231](#1231)) ([f090554](f090554))
* ATL 6829 - Integrate ZIO failures and defects ADR in credential status list ([#1175](#1175)) ([dffad1d](dffad1d))
* ATL-6832 ZIO failures and defects in entity controller ([#1203](#1203)) ([9050094](9050094))
* ATL-6833 integrate ZIO failures and defects in wallet event controller ([#1186](#1186)) ([8bc2018](8bc2018))
* ATL-6834 Use ZIO Failures and Defects effectively in the Issue flow ([#1139](#1139)) ([ede7b77](ede7b77))
* Handle Error in Background Jobs - Improve the way we store errors and defects in DB ([#1218](#1218)) ([e3cadc9](e3cadc9))
* implement ADR Use ZIO Failures and Defects Effectively - Mercury should not throw exceptions ([#1192](#1192)) ([a4ce87f](a4ce87f))
* improve OpenAPI's PresentProof to make more consistent ([#1130](#1130)) ([bdc5d20](bdc5d20))
* Integrate ZIO failures and defects ADR in DID Registrar Controller and Mercury ([#1180](#1180)) ([d8e2120](d8e2120))
* SDJWT holder key binding ([#1185](#1185)) ([628f2f0](628f2f0))
* use kid in the jwt proof header of OID4VCI CredentialIssue request ([#1184](#1184)) ([ee53eda](ee53eda))
* use the compact format in SD-JWT ([#1169](#1169)) ([65da651](65da651))

Signed-off-by: Allain Magyar <[email protected]>
Signed-off-by: Pat Losoponkul <[email protected]>
patlo-iog pushed a commit that referenced this pull request Jul 1, 2024
Signed-off-by: Pat Losoponkul <[email protected]>

test: remove println

Signed-off-by: Pat Losoponkul <[email protected]>

chore(release): cut Identus Cloud agent 1.37.0-snapshot.test.1 release

* avoid name and operationId conflict when creating oas client ([#1233](#1233)) ([73e8e24](73e8e24))
* incorrect parsing of public key coordinates and enable some tests again ([#1215](#1215)) ([5398a75](5398a75))
* KeyID from String to Opaque Type and presentation job cleanup  ([#1190](#1190)) ([b813faf](b813faf))
* make init-script.sh idempotent [#1173](#1173) ([#1194](#1194)) ([1712062](1712062))
* migrate and repair in case of renaming issues ([#1211](#1211)) ([cb4d479](cb4d479))
* Present Error Handling (Part 1: Repo Changes) ([#1172](#1172)) ([13e2447](13e2447))
* Present Error Handling Job ([#1204](#1204)) ([3191d8b](3191d8b))
* present error handling Part 2 ([#1177](#1177)) ([9ac6e52](9ac6e52))
* Verification Policy Error Handling ([#1228](#1228)) ([6117a3c](6117a3c))

* add oidc4vci protocol MVP ([#1182](#1182)) ([3ae91dc](3ae91dc))
* add schemaId to the issuance session ([#1199](#1199)) ([97f5d83](97f5d83))
* add support for EcdsaSecp256k1Signature2019Proof and fix pk encoding for EddsaJcs2022Proof ([#1127](#1127)) ([e617ded](e617ded))
* **agent:** add browser fingerprint label to http metrics ([#1231](#1231)) ([f090554](f090554))
* ATL 6829 - Integrate ZIO failures and defects ADR in credential status list ([#1175](#1175)) ([dffad1d](dffad1d))
* ATL-6832 ZIO failures and defects in entity controller ([#1203](#1203)) ([9050094](9050094))
* ATL-6833 integrate ZIO failures and defects in wallet event controller ([#1186](#1186)) ([8bc2018](8bc2018))
* ATL-6834 Use ZIO Failures and Defects effectively in the Issue flow ([#1139](#1139)) ([ede7b77](ede7b77))
* Handle Error in Background Jobs - Improve the way we store errors and defects in DB ([#1218](#1218)) ([e3cadc9](e3cadc9))
* implement ADR Use ZIO Failures and Defects Effectively - Mercury should not throw exceptions ([#1192](#1192)) ([a4ce87f](a4ce87f))
* improve OpenAPI's PresentProof to make more consistent ([#1130](#1130)) ([bdc5d20](bdc5d20))
* Integrate ZIO failures and defects ADR in DID Registrar Controller and Mercury ([#1180](#1180)) ([d8e2120](d8e2120))
* SDJWT holder key binding ([#1185](#1185)) ([628f2f0](628f2f0))
* use kid in the jwt proof header of OID4VCI CredentialIssue request ([#1184](#1184)) ([ee53eda](ee53eda))
* use the compact format in SD-JWT ([#1169](#1169)) ([65da651](65da651))

Signed-off-by: Allain Magyar <[email protected]>
Signed-off-by: Pat Losoponkul <[email protected]>
hyperledger-bot added a commit that referenced this pull request Jul 1, 2024
# [1.37.0](cloud-agent-v1.36.1...cloud-agent-v1.37.0) (2024-07-01)

### Bug Fixes

* avoid name and operationId conflict when creating oas client ([#1233](#1233)) ([73e8e24](73e8e24))
* incorrect parsing of public key coordinates and enable some tests again ([#1215](#1215)) ([5398a75](5398a75))
* KeyID from String to Opaque Type and presentation job cleanup  ([#1190](#1190)) ([b813faf](b813faf))
* make init-script.sh idempotent [#1173](#1173) ([#1194](#1194)) ([1712062](1712062))
* migrate and repair in case of renaming issues ([#1211](#1211)) ([cb4d479](cb4d479))
* Present Error Handling (Part 1: Repo Changes) ([#1172](#1172)) ([13e2447](13e2447))
* Present Error Handling Job ([#1204](#1204)) ([3191d8b](3191d8b))
* present error handling Part 2 ([#1177](#1177)) ([9ac6e52](9ac6e52))
* Verification Policy Error Handling ([#1228](#1228)) ([6117a3c](6117a3c))

### Features

* add oidc4vci protocol MVP ([#1182](#1182)) ([3ae91dc](3ae91dc))
* add schemaId to the issuance session ([#1199](#1199)) ([97f5d83](97f5d83))
* add support for EcdsaSecp256k1Signature2019Proof and fix pk encoding for EddsaJcs2022Proof ([#1127](#1127)) ([e617ded](e617ded))
* **agent:** add browser fingerprint label to http metrics ([#1231](#1231)) ([f090554](f090554))
* ATL 6829 - Integrate ZIO failures and defects ADR in credential status list ([#1175](#1175)) ([dffad1d](dffad1d))
* ATL-6832 ZIO failures and defects in entity controller ([#1203](#1203)) ([9050094](9050094))
* ATL-6833 integrate ZIO failures and defects in wallet event controller ([#1186](#1186)) ([8bc2018](8bc2018))
* ATL-6834 Use ZIO Failures and Defects effectively in the Issue flow ([#1139](#1139)) ([ede7b77](ede7b77))
* Handle Error in Background Jobs - Improve the way we store errors and defects in DB ([#1218](#1218)) ([e3cadc9](e3cadc9))
* implement ADR Use ZIO Failures and Defects Effectively - Mercury should not throw exceptions ([#1192](#1192)) ([a4ce87f](a4ce87f))
* improve OpenAPI's PresentProof to make more consistent ([#1130](#1130)) ([bdc5d20](bdc5d20))
* Integrate ZIO failures and defects ADR in DID Registrar Controller and Mercury ([#1180](#1180)) ([d8e2120](d8e2120))
* SDJWT holder key binding ([#1185](#1185)) ([628f2f0](628f2f0))
* use kid in the jwt proof header of OID4VCI CredentialIssue request ([#1184](#1184)) ([ee53eda](ee53eda))
* use the compact format in SD-JWT ([#1169](#1169)) ([65da651](65da651))

Signed-off-by: Allain Magyar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants