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

Refactor repository and CA code to support BGPsec CSRs. #210

Merged
merged 13 commits into from
Jun 16, 2022
Merged

Conversation

partim
Copy link
Member

@partim partim commented Jun 2, 2022

BGPsec CSRs are signed using ECDSA keys rather than the usual RSA. Supporting this requires a slew of changes to the repository and ca modules.

This PR

  • moves repository::crypto into its own top-level module and adds the crypto feature,
  • adds separate signature algorithm types for RPKI and BGPsec and adds a SignatureAlgorithm trait so the two can be used in parallel,
  • makes crypto::signature::Signature generic over the signature algorithm,
  • changes the Signer trait and soft-signer implementation to be able to deal with both signature algorithm types via an intermediary SigningAlgorithm enum,
  • makes repository::x509::SignedData (the type used for the outer, signed portion of certificates, CRLs, and CSRs) generic over the signature algorithm type so it can be used for both RPKI and BGPsec objects,
  • moves repository::oid into its own top-level module and makes it depend on the bcder feature,
  • moves repository::csr to ca::csr where it belongs,
  • changes the ca::csr’s types to be generic over the signature algorithms and CSR attributes,
  • changes the type of the Extended Key Usage attribute of certificates and CSRs into a newtype around the wrapping capture,
  • add missing functionality to TbsCert and CertBuilder to be able to generate router certificates, and
  • probably some more I forgot.

@partim partim requested a review from timbru June 2, 2022 12:32
@timbru
Copy link
Contributor

timbru commented Jun 2, 2022

I did not look at the code yet. Or well I did not give it a good look yet. But I have made krill branch that depends on this and after some sweat and tears that now works with these updates.

I did not yet get to the point of trying to sign BGPSec Router Certificates - but all the other stuff still works.

Copy link
Contributor

@timbru timbru 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. I now want to try to create a BGPSec Router Certificate from Krill code. Then again, perhaps it's best to look at any changes needed (if any) for that as a separate PR.

@@ -43,7 +43,8 @@ default = []

# Main components of the crate.
ca = [ "repository", "serde-support" ]
repository = [ "bcder", "ring", "untrusted", "routecore/bcder" ]
crypto = [ "bcder", "ring", "untrusted" ]
repository = [ "bcder", "crypto", "routecore/bcder" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

If repository includes crypto does it still need bcder? Also, I am not sure why it needs both bcder and routecore/bcder - though that is not related to this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, I wonder if we can make repository not require crypto.

I’m not sure if enabling bcder here also enables it for routecore, so I decided to play it safe. This all is going to change eventually with the new way of dealing with features and dependencies stablised in a recent Rust release, but I don’t want to jump MSRVs too quickly.

@partim partim merged commit b21879e into main Jun 16, 2022
@partim partim deleted the bgpsec-csr branch June 16, 2022 08:52
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.

2 participants