-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add an exportable
option to default-plugins
#109
Conversation
Addresses issue ucan-wg#108
The ts-ucan/packages/default-plugins/src/p256/keypair.ts Lines 73 to 76 in 4b1be87
Perhaps it makes sense to change it to |
It is weird that it wouldn't match the RSA export format though (which is also pkcs8). |
To me, it looks like ed25519 is the only plugin with a full / symmetric import-export support. The p256 looks asymmetric (import is jwk, export is pkcs8). RSA only looks to export and doesn't have an import. As part of this PR, I could revamp both p256 and RSA to have an import and export based on pkcs8. What do you think? |
Sounds good. I'll leave it up to you to whether it's going to be JWK or PCKS8 :) ✌️ |
Ok, I did a pass and got symmetric import / export functionality for all of the plugins. I tried to reuse where I could, but realized a lot of the import type functions are there for other purposes (usually in the process of signing or verifying a message). I went with JWK where it was available (p256 and rsa). Ed25519 is using a different library and doesn't appear to support JWK, but I it was the easiest to get working with the existing format. |
exportable
option to EcdsaKeyPairexportable
option to default-plugins
@kshinn is there a reason why you removed the |
Oops, that wasn't supposed to commit to this branch. I'll return the dependency. Long story short: I'm trying to make this library work in a constrained nodejs setting (total size needs to be as small as possible). I'm trying to eliminate some of the polyfills and work directly with node libs to do this. But this is unrelated to the changes I'm making here and I didn't see that they leaked in. I will undo those changes in this branch. |
[Fix] Normalize on uint8arrays
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting this on approve, because I trust you to fix the last nit before we merge :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work, this looks good to me :)
Addresses issue #108
This simply adds and exportable parameter to the EcdsaKeyPair class to allow the key to be exported.