-
-
Notifications
You must be signed in to change notification settings - Fork 68
Add KMIP HSM signing support #540
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
Draft
ximon18
wants to merge
57
commits into
main
Choose a base branch
from
poc-kmip-crypto-impl
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(cherry picked from commit e2540cd)
(cherry picked from commit 8d28ca5)
And why they depend on OpenSSL.
…ranch. - Use latest kmip-protocol crate version. - Remove kmip_pool module and the dependency on r2d2, use the pool from kmip-protocol instead. - Extend SignError to take a String reason (and remove Copy from SigningError). - Add fn flags() to the SignRaw trait. - Add optional dependencies for KMIP on the url and uuid crates. - Make KMIP support depend on openssl (for some byte parsing helper fns). - Fetch the public key in kmip::PublicKey::new() to (a) fail early, and (b) keep `fn dnskey()` infallalible. - Fix public key byte parsing. - Fix ECDSAP256SHA256 signature parsing. - Add the kmip::KeyUrl type. - Add kmip::KeyPair::new_from_urls() to construct a key pair from KMIP public and private key URLs. - Add kmip::KeyPair::public_key_url() and private_key_url() to obtainURLs for KMIP key pairs. - Add kmip::KeyPair::sign_raw_enqueue() and sign_raw_submit_queue() for batch signing. - Split kmip::KeyPair::sign_raw() into sign_pre() and sign_post() to support batch signing. - Refactor dnssec::sign::signatures::rrsigs functions to allow preparing data for signing to done separately to packaging of the resulting signature into an Rrsig. - Split sign_sorted_rrset_in() into sign_sorted_rrset_in_pre(), a call to key.sign_raw(), and sign_sorted_rrset_in_post(). - Split sign_sorted_zone_records() out into sign_sorted_zone_records_with() which takes a signer_fn callback function. - Make sign_sorted_zone_records() use sign_sorted_rrset_in() as the signer_fn. - This allows a caller to invoke sign_sorted_rrset_in_with() passing a callback fn that only invokes sign_sorted_rrset_in_pre() to gather N prepared data chunks to be signed together in a batch, then invoke sign_sorted_rrset_in_post() on each of the resulting signatures when they are ready. See branch use-kmip-batching-support-of-nameshed for example usage. - Note: This design should be revisited when we add async signing support.
Member
Author
|
@Philip-NLnetLabs: I made the changes we discussed yesterday, including updating the PR description to note that the key deletion and KMIP 1.3 work should also be done in this PR. |
As sign::generate() only supports OpenSSL and Ring generation, not KMIP.
…rsions behind latest as per the requirement specified in Cargo.toml).
… 1.2+ with secure ciphers and as a bonus drop dependency on non-Rust OpenSSL.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Add a 3rd crypto backend for DNSSEC signing (in addition to the current OpenSSL and Ring backends) that creates key pairs and signs data by submitting OASIS KMIP (Key Management Interoperability Protocol) requests to a KMIP capable HSM.
For Cascade, this, combined with kmip2pkcs11 which converts KMIP requests into requests to a loaded PKCS#11 module, also enables use of PKCS#11 compatible HSMs.
Status
This PR is basically functional but needs cleaning up:
xxx_preandxxx_post.Add an integration test using PyKMIP? (given the age and limitations of PyKMIP and difficulty running it, and that actual KMIP communication is not the responsibility of this crate, unit tests or code level integration tests, verifying that the expected requests are made may be enough)Update: The switch to TLS 1.2+ with secure ciphers via RustLS (see below) precludes this as RustLS doesn't support the insecure TLS used by PyKMIP.Initial review feedback also concluded that the following additional changes should be made:
kmip-protocolandkmip-ttlvcrates haven't been verified/extended to work with KMIP 1.3 yet this may involve just stating that we only supportkmip2pkcs11as a KMIP server initially.Design
The actual KMIP TCP+TLS client connection pooling, communication, and KMIP TTLV request encoding and response decoding are delegated to the existing
kmip-protocolandkmip-ttlvcrates used by Krill (with upgrades to support the DNSSEC required operations that were not needed for the Krill RPKI use case).KMIP batching support for signing requests has been implemented in order to amortize the per-request TCP/TLS overhead of each signing request by grouping multiple requests together, e.g. one per RRSIG that must be generated. The existing signing mechanism requires that the signature for each signing operation be known before proceeding to the next signing operation. This is incompatible with batching and so the existing mechanism has been extended to enable signing preparation to be done separately so that many blocks of data to be signed can be prepared then sent as a batch, and the batch results to then be made into RRSIGs.
The underlying KMIP crates support async usage but only the sync variant is used by this PR. A client has to be careful to consider which threads signing will be done on and what that will block.
Missing functionality
domainand was deemed out of scope for the current work.kmip-ttlvcrate with a much simpler KMIP specific hand-coded deserializer in thekmip-protocolcrate.dnst keysetKMIP support and in Cascade.