Support EcdsaSecp256r1Signature2019 linked data proof

[gmulhearn]

NOTE: branched from #3442 (must merge first)

easier diff view: anonyome#4

• adds native support for verifying and signing with EcdsaSecp256r1Signature2019 (mostly a clone of Ed25519 2020 suite)

[gmulhearn]

update:
seems like i might be able to make this a plugin instead, i.e. via https://github.com/dbluhm/acapy-ld-signer , however i believe the ExternalSuiteProvider is only used in the signing flows, and not in the verifying flows. The verify w3c ldp cred/pres flows seem to use _get_all_proof_suites which returns the list of pre-existing suites (not utilizing ExternalSuiteProvider plugin). Does that seem correct? @dbluhm

If this is right, then I suppose an open question is whether EcdsaSecp256r1Signature2019 should be apart of acapy (which this draft PR does), or if ExternalSuiteProvider can be architectured to allowed suites for verification to be passed in aswell.

An argument to not include EcdsaSecp256r1Signature2019 is that it could be a considered as a step sideways, rather than a step forwards towards DataIntegrityProofs in VCDM2.0

[dbluhm]

seems like i might be able to make this a plugin instead, i.e. via https://github.com/dbluhm/acapy-ld-signer , however i believe the ExternalSuiteProvider is only used in the signing flows, and not in the verifying flows. The verify w3c ldp cred/pres flows seem to use _get_all_proof_suites which returns the list of pre-existing suites (not utilizing ExternalSuiteProvider plugin). Does that seem correct? @dbluhm

The original intent of the ExternalSuiteProvider was to make it possible to use something like a remote KMS to do signatures, which is why it's not used in the verification process; it wasn't necessarily intended to add support for additional signature types. I am not against the idea of enabling it to also provide hooks for permitting a plugin to handle cred/pres verification as well.

That being said, I am in favor of enabling ACA-Py to handle EcdsaSecp256r1Signature2019 natively, even if it's not a "forward-looking" option for Data Integrity Proofs.

Any thoughts on VCDM 2.0 and DI, @PatStLouis?

[dbluhm]

The "security hotspots" reported by SonarCloud are not really an issue but it does bring up the question: should those urls be HTTPS?

[gmulhearn]

@dbluhm hmmm yea it is interesting. ofcourse i've just copy pasted the contents from the context URL: https://w3id.org/security/multikey/v1 , which has them in http. the links in question auto redirect to https, so in theory i could change them to https. however it feels dishonest to have a context which doesn't match the real source.

but yea, definitely a question for upstream (i.e. in w3c).

Also on the sonarqube condition about duplicate code, should i try solve this? it's mostly duplicate code of acapy_agent/vc/ld_proofs/suites/ed25519_signature_2020.py, and IMO since it's relatively small it's more useful to have it be duplicated instead of refactored into some MulitbaseProofValueSignatureBase class which Ed25519Signature2020 & EcdsaSecp256r1Signature2019 inherit from. But what's the acapy convention for situations like this?

[jamshale]

@gmulhearn Don't worry about the duplicated code. Sometimes it's a good indicator for a refactor but it's small enough here we can ignore it.

[jamshale]

Looks good to me.

Will wait to approve to give others a chance to review.

[ff137]

Looks good to me!
Didn't really review the tests, so that's the main thing I'd ask someone else to double check, that they are indeed meaningful for this feature. I'm kind of out of the loop with this new signature type.

I just left a comment about the maintainability / readability of the present_proof dif handler ... wanted to ask for some minor refactoring, but I realise it's best left for a major rework! And I'll be happy to give that a shot sometime.

[dbluhm]

To complement @ff137's review, I focused on reviewing tests and am happy with them. Nice work!

[jamshale]

@gmulhearn Can you merge with main one more time? Doesn't allow me to do it.

[gmulhearn]

@jamshale moving target! re-merged main 👍

[sonarqubecloud]

Quality Gate failed

Failed conditions
9.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud