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] DIDExchange handlers should do more signature checking #1226 #1232

Merged

Conversation

gmulhearn-anonyome
Copy link
Contributor

@gmulhearn-anonyome gmulhearn-anonyome commented Jun 24, 2024

branched off #1230
diff: anonyome/aries-vcx@gm/1228-did-exch-1_1...anonyome:aries-vcx:gm/1226-didexchange-signature-checks

fixes part 2 of #1226.

adds JWS signature verification when receiving a response. The new approach mostly matches ACApy's implementation found here: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/protocols/didexchange/v1_0/manager.py#L942.

It is a relatively aggressive approach where it will fail if neither DIDDoc attachments nor DIDRotate attachment have signatures.

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
…ing)

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@gmulhearn-anonyome
Copy link
Contributor Author

gmulhearn-anonyome commented Jun 24, 2024

TODO - verify still working in Acapy aath

still works at standard of previous PR ✅

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@gmulhearn-anonyome
Copy link
Contributor Author

@JamesKEbert this is ready for review after #1230 is merged

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
…e-checks

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@gmulhearn-anonyome
Copy link
Contributor Author

gmulhearn-anonyome commented Jun 26, 2024

alright this should be good to review now

JamesKEbert
JamesKEbert previously approved these changes Jul 3, 2024
Copy link
Contributor

@JamesKEbert JamesKEbert left a comment

Choose a reason for hiding this comment

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

Looks good to me--good additions! I left some minor thoughts, but that may not need to hold up merging

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Copy link
Contributor

@JamesKEbert JamesKEbert left a comment

Choose a reason for hiding this comment

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

LGTM

@JamesKEbert JamesKEbert merged commit de24004 into hyperledger:main Jul 4, 2024
22 checks passed
lukewli-anonyome pushed a commit to lukewli-anonyome/aries-vcx that referenced this pull request Jul 25, 2024
…ger#1226 (hyperledger#1232)

* move existing into v1_0 section

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* duplicate types

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* static generic types for messages created and piped thru all layers

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* simplify generics

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* change approach to use runtime versioning rather than generics

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* v1_1 branch processing, and some clippy

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* remove old todos

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* fixes for aath with self for 4/7 performance on RFC0793 & 4/7 on 0023

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* smalls patches from acapy testing

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* fix up mimetype handling as a result of testing acapy (text/string)

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* handle multikey (acapy uses this)

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* make invite handshake 1.1

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* include invitation id

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* pthid in response (for acapy)

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* merge fix and add hack for local aath testing

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* fixes for didpeer2

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* improve VM handling to understand more DIDDoc styles (acapy AATH testing)

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* fmt

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* clean switcher

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* label pass, and some fixes

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* test fixes

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* response sign verification mostly impled

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* fix did rotate content

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* negative test

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* small patch re acapy testing

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* pass in handshake ver

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* clippy and rebase

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* unnecessary mockall dep

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* any-wrapper approach

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* lint

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* borrow registries instead

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* jws testing

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

---------

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: lli <lli@anonyome.com>
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.

3 participants