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

Refactoring and examples for key interface #123

Merged
merged 2 commits into from
Sep 8, 2022
Merged

Conversation

Xynnn007
Copy link
Member

@Xynnn007 Xynnn007 commented Sep 5, 2022

Summary

Due to #27, there is a need to refactor the key_interface to deal with the following current problems:

  • When users use the signing_key module, they must import the crates into the Cargo.toml, like p256, p384, signature, digestetc. It is better for users not to need to care about the underlying Elliptic Curves. They can directly use sigstore crate without import any other crates to do all the key related operations.
  • Now the module does not have a convenient call routine for a user to import a private key and convert it into a SigStoreSigner. Also SigStoreSigner is somehow overlapped functionally with Signer trait object. It is good to combine them, and give a more easy way for users to convert a PEM/DER encoded private key into an object who implements sign.

Thus this PR includes

  • Wrapped the generics trait object into enums.
  • Combined Signer trait object and SigStoreSigner struct into an enum named SigStoreSigner.
  • Example and docs were attached.

Release Note

Added the new following APIs:

  • SigStoreKeyPair enum
  • to_sigstore_signer() for SigStoreKeyPair
  • to_sigstore_keypair() for SigStoreSigner
  • ECDSAKeys enum, a wrapper for EcdsaKeys

Changed the following structs:

  • Converted SigStoreSigner into an enum

Changed the following APIs:

  • To create a new SigStoreSigner:SigStoreSigner::new(SigningScheme::ECDSA_P256_SHA256_ASN1) -> SigningScheme::ECDSA_P256_SHA256_ASN1.create_signer()

Added the following examples due to typical scenarios:

  • Key Pair Generation, Signing and Verification
  • Key Pair Generation and Exporting
  • Key Pair Importing

Deleted the origin example:

  • ecdsa_p256

To make it clear, an overview architecture of the implementation for key-interface was included.

Documentation

Please refer to the key_interface examples for the usages.

@lukehinds
Copy link
Member

Thanks @Xynnn007 , I do have this on my queue to review this week. Would be good to get @flavio to look over as well!

flavio
flavio previously approved these changes Sep 7, 2022
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

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

LGTM, I left some minor comments

src/crypto/signing_key/ecdsa/mod.rs Show resolved Hide resolved
src/crypto/signing_key/ed25519.rs Show resolved Hide resolved
src/crypto/signing_key/ecdsa/ec.rs Outdated Show resolved Hide resolved
Xynnn007 added 2 commits September 7, 2022 21:47
- Wrap the Signer and KeyPair trait object inside enums
- Wrap ECDSA keys inside enum

Signed-off-by: Xynnn007 <mading.ma@alibaba-inc.com>
- key pair generation and export
- key pair generation, signing and verification
- key pair import
- docs for all above

Signed-off-by: Xynnn007 <mading.ma@alibaba-inc.com>
@lukehinds lukehinds merged commit 7805f14 into sigstore:main Sep 8, 2022
flavio added a commit to flavio/sigstore-rs that referenced this pull request Oct 7, 2022
Enhancements
============

* update user-agent value to be specific to sigstore-rs (sigstore#122)
* remove /api/v1/version from client by (sigstore#121)
* crate async fulcio client (sigstore#132)
* Removed ring dependency (sigstore#127)

Others
======

* Update dependencies
* Refactoring and examples for key interface (sigstore#123)
* Fix doc test failures (sigstore#136)

Contributors
============

* Bob Callaway (@bobcallaway)
* Bob McWhirter (@bobmcwhirter)
* Flavio Castelli (@flavio)
* Luke Hinds (@lukehinds)
* Xynnn (@Xynnn007)

Signed-off-by: Flavio Castelli <fcastelli@suse.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.

None yet

3 participants