Skip to content

Conversation

@ximon18
Copy link
Member

@ximon18 ximon18 commented Jun 6, 2025

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:

  • Adding RustDoc & other comments.
  • Addressing Clippy lints and other CI failures.
  • Choose better fn names than xxx_pre and xxx_post.
  • Replace any leftover unwraps with error handling.
  • Check if all currently "supported" algorithms actually work, or make them fail? RSASHA256 and ECDSAP256SHA256 have been tested but other algorithms have not. Update: Using unsupported algorithms will fail.
  • Add unit tests.
  • 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.
  • Secret protection: are there sensitive values that should be protected in-memory using special data types?

Initial review feedback also concluded that the following additional changes should be made:

  • Key deletion, while not provided for OpenSSL or Ring keys, should be added alongside `generate() for KMIP keys because it is non-triivial for callers to implement this themselves. The code in dnst keyset to do this should be moved into domain.
  • Use RustLS and KMIP 1.3 instead of OpenSSL and KMIP 1.2 because KMIP 1.2 mandates support for insecure cryptographic protocols which we do not want to support and also allows us to use RustLS instead of OpenSSL As the code in the kmip-protocol and kmip-ttlv crates haven't been verified/extended to work with KMIP 1.3 yet this may involve just stating that we only support kmip2pkcs11 as 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-protocol and kmip-ttlv crates 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

  • Batching for non-signing requests: I'm not convinced this is actually needed.
  • Async signing: This would be ideal for handling the potential network & signing delays and the request batching, but would be a considerable change to domain and was deemed out of scope for the current work.
  • Performance tuning (memory, CPU, how threads are used and where blocking occurs): This could likely do with more work. The largest overhead seen in ad-hoc flame graph monitoring was actually in KMIP response deserialization, for which I would like to replace the current Serde based generic TTLV deserializer in the kmip-ttlv crate with a much simpler KMIP specific hand-coded deserializer in the kmip-protocol crate.
  • Configuration tuning: some configurability exists but more may be required or better defaults / computed settings e.g. to neither overload the application host or KMIP server, nor under-utilize the KMIP server. However, it may be that most if not all of that is an application concern and not part of this library code.
  • An example showing how to do KMIP signing, and how to do batched signing. Currently usage of this functionality can only be seen in the dnst keyset KMIP support and in Cascade.

@ximon18 ximon18 requested a review from a team June 6, 2025 09:35
ximon18 added 18 commits June 6, 2025 13:30
(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.
@ximon18 ximon18 changed the title KMIP crypto support (WIP) Add support for KMIP Aug 12, 2025
@ximon18 ximon18 changed the title Add support for KMIP Add KMIP HSM signing support Aug 12, 2025
@ximon18
Copy link
Member Author

ximon18 commented Aug 14, 2025

@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.

ximon18 added 25 commits August 14, 2025 10:22
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants