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.

Sign up for GitHub

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

Jump to bottom

Clarification on signer methods and return types #78

Closed
mirceanis opened this issue Apr 9, 2020 · 6 comments · Fixed by #149
Closed

Clarification on signer methods and return types #78

mirceanis opened this issue Apr 9, 2020 · 6 comments · Fixed by #149
Assignees
@mirceanis
Labels
documentation enhancement New feature or request

Comments

@mirceanis
Copy link
Member

signer params are very confusing at the moment.

ES256K signers are expected to produce ECDSASignature objects while Ed25519 signers are expected to produce strings.

The developer is notified of this by an error.
See SignerAlgorithm.ts#L28 and SignerAlgorithm.ts#L41

This should be improved.
Signers should only produce strings since that way, signing can be externalized and there would no longer be a need to understand all algorithms locally, thus allowing support for many more crypto suites.
@mirceanis mirceanis added enhancement New feature or request documentation labels Apr 9, 2020
@oed
Copy link
Contributor

oed commented Apr 9, 2020

Any thoughts on using a more generic underlying library such as jose? Would extend the number of supported algorithms :)
Don't know how difficult it would be to migrate, but would be nice with wider support!

@mirceanis
Copy link
Member Author

Yes, this relates directly to that approach.

If this lib makes assumptions about the structure of signatures then it essentially prohibits the use of libs like jose

@oed
Copy link
Contributor

oed commented Apr 9, 2020

Is that because of the use of ES256K-R? Basically because of ethereum addresses being valid signature keys in this library?

@mirceanis
Copy link
Member Author

Yes, so there would still be special handling of non-standard algorithms; at least for a while.
The point of this issue is to relax the need of this library to know signature formats and expect signers to always produce strings.

Regarding ES256K-R, in the long run it should not even be a thing since, even for ethereumAddress as keys, it can be verified almost the same as ES256K.

In parallel, I made a proposal to ethr-did-resolver to use public keys as identifiers instead of ethereum addresses. That would alleviate the need to have special handling here.

@oed
Copy link
Contributor

oed commented Apr 9, 2020

That sounds great :) FYI, we are likely to start using the jose library to do IPLD object signatures and encryption in Ceramic. But afaik, it should still be possible to verify signatures generated from that with the did-jwt library!

@oed oed mentioned this issue Apr 28, 2020
Closed
mirceanis added a commit to decentralized-identity/veramo that referenced this issue Jan 28, 2021
@awoie @mirceanis
feat(did-provider-key): add did:key provider; fixes #335 (#351)
42cd2b0 
* feat: add did:key provider creator
* feat: add support for NaclSigner with Ed25519 issuer key
* fix: use base64 encoding for private keys with NaclSigner
  see [decentralized-identity/did-jwt#78](decentralized-identity/did-jwt#78)
* docs: add did-key-provider to docsconfig.json
* style: remove unreachable code

Co-authored-by: Oliver Terbu <[email protected]>
Co-authored-by: Mircea Nistor <[email protected]>
@mirceanis mirceanis self-assigned this Feb 1, 2021
mirceanis added a commit that referenced this issue Feb 9, 2021
@mirceanis
feat: add ES256KSigner and EdDSASigner with uniform APIs
99cd161 
fixes #78
This deprecates `SimpleSigner`, `NaclSigner`, and `EllipticSigner`.
They will be removed in the next major release.
@mirceanis mirceanis mentioned this issue Feb 9, 2021
Merged
mirceanis added a commit that referenced this issue Feb 10, 2021
@mirceanis
feat: add ES256KSigner and EdDSASigner with uniform APIs (#149)(#78)
cdd3c0f 
This deprecates `SimpleSigner`, `NaclSigner`, and `EllipticSigner`.
They will be removed in the next major release.

fixes #78
@uport-automation-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 4.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mirceanis mirceanis

Labels
documentation enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add ES256KSigner and EdDSASigner with uniform APIs decentralized-identity/did-jwt
3 participants
@mirceanis @oed @uport-automation-bot