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.

Details

(see also additional notes below regarding changes to support batching)

  • Adds a kmip feature gate.
  • Adds a kmip feature gate dependent dependencies on bcder, url and uuid.
  • Adds a new crypto::kmip module.
    • Unlike the openssl and ring modules, the kmip module KeyPair struct has additional functions related to batching support.
  • Extends crypto::sign::SignError to take a String reason argument as I deemed that I needed to be able to report more information from KMIP signing failure than just failure occurred.
    • Extends crypto::openssl and crypto::ring to return SignError with reason strings.
    • Removes Copy from the derives for dnssec::sign::error::SigningError because of this change.
  • Extends crypto::sign::SignRaw with flags() fn.
  • Extends crypto::sign::KeyPair and impl SignRaw for KeyPair with Kmip variant support.
  • Extends dnssec::sign::signatures::rrsigs with batching support (more information on that below)
  • Adjusts cryptography related RustDocs to reflect the added KMIP support.
  • Extends some functions in dnssec::sign::test_util with the Send bound. (I don't recall why)

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.

Changes to signing to support batching

The changes to support batching were made in dnssec/sign/signatures/rrsigs.rs and are as follows:

  1. Extracted all of the logic of sign_sorted_rrset_in() except the invocation of sign_raw() into pre and post halves which it then calls like so:
  • sign_sorted_rrset_in_pre() (constructs ProtoRrsig and serializes it to wire format)
  • The original sign_raw() invocation.
  • sign_sorted_rrset_in_post() passing it the output of sign_raw() - (converts the ProtoRrsig into an Rrsig using the given signature and creates a Record of Rtype RRSIG with the Rrsig RDATA)
  1. Made sign_sorted_zone_records() generic over D instead of hard-coding ZoneRecordData<Octs, N>.
  • This is needed to be able to call sign_sorted_rrset_in() with the appropriate D which when called from sign_sorted_zone_records() is the type of data D used by the given RecordsIter while when called from sign_rrset() it is the type of data used by the given Rrset, thus we need to be able to separate out D from the type that defines it.
  1. Modified sign_sorted_zone_records() to no longer be hard-coded to invoke sign_sorted_rrset_in() but instead to invoke a user-supplied callback function signer_fn: F. This can be sign_sorted_rrset_in() as it does prior to this PR, or to be something else.

The Cascade implementation of batching in NLnetLabs/cascade#14 uses the above changes like so, so that its sign_chunk() function:

  • Passes its own callback fn prep_data_to_sign() to sign_sorted_zone_records() which only collects the output of sign_sorted_rrset_in_pre(), and doesn't call sign_raw()or sign_sorted_rrset_in_post().
  • Adds the generated wire format RRSET chunk to be signed to a KMIP batch signing request by calling sign_raw_enqueue() instead of sign_raw().
  • When enough signing requests have been accumulated it calls sign_raw_submit_queue() to actually submit the KMIP batch request to the KMIP server.
  • Processes the KMIP server response signatures invoking sign_sorted_rrset_in_post() to create the final signed records for this chunk one RRSET at a time.

@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 ximon18 marked this pull request as ready for review November 25, 2025 08:26
bal-e added a commit to NLnetLabs/domain-kmip that referenced this pull request Nov 25, 2025
This copies the 'src/crypto/kmip.rs' file from
the 'poc-kmip-crypto-impl' pull request on Domain:
<NLnetLabs/domain#540>.

The file has not been modified at all, to serve as a good base for
reviewing changes.  There are dozens of compilation failures that will
be addressed in the following commits.
bal-e added a commit that referenced this pull request Nov 25, 2025
These are miscellaneous changes from
<#540> that are not directly
related to KMIP.  They are relied upon by some downstream codebases.
bal-e added a commit to NLnetLabs/domain-kmip that referenced this pull request Dec 5, 2025
This copies the 'src/crypto/kmip.rs' file from
the 'poc-kmip-crypto-impl' pull request on Domain:
<NLnetLabs/domain#540>.

The file has not been modified at all, to serve as a good base for
reviewing changes.  There are dozens of compilation failures that will
be addressed in the following commits.
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