HSM 1: KMIP walking skeleton (#566) #679

ximon18 · 2021-09-28T14:49:52Z

Introduces a KMIP signer, disabled by default.

The scope for this PR is deliberately limited. The KMIP signer itself is fully functional but it is not yet fully wired into Krill. It is not configurable, it has no metrics, it has no persistent mapping of Krill key identifiers to HSM key identifiers (update: added in PR #686 for that), it doesn't support async operation (update: async will not be added at the moment), there's no extensions to the API or CLI to see which Signer owns a particular key, etc. This PR is just about showing that the Krill test suite passes when forcing Krill to use a PyKMIP server via the new KMIP signer.

KMIP support can only be enabled currently by using BOTH the hsm and hsm-tests feature flags. In normal usage we may want to use a local signer for some operations even when using a KMIP signer for others. The hsm-tests feature flag forces Krill to use KMIP for ALL operations. This is used by a new GitHub Actions hsmtest job in the CI workflow to test Krill against PyKMIP. If you look at the test log output you can see lines like the following:

2021-10-05 11:32:09 [DEBUG] [krill::commons::crypto::signers::kmip::internal] Probing server at 127.0.0.1:5696
2021-10-05 11:32:09 [WARN] [krill::commons::crypto::signers::kmip::internal] KMIP server lacks support for one or more required operations: ModifyAttribute
2021-10-05 11:32:09 [INFO] [krill::commons::crypto::signers::kmip::internal] Using KMIP server 'PyKMIP 0.10.0 Software Server' at 127.0.0.1:5696

To be discussed.

OpenSSL is used as it was difficult to make RustLS connect to PyKMIP, and while eventually that was solved it may be unhelpful to customers to force the strict "no unsafe features (e.g. self-signed certificates) and no obsolete crpytography" opinions of RustLS on them as it may be too strict for real world HSM deployments.

Other notes:

Uses newly released bcder crate DER unsigned integer support.
Lacks tests of its own though is tested by the Krill test suite and the kmip-protocol and kmip-ttlv crates have tests, not exhaustive but also quite a bit more than nothing.
The target server is hard-coded to local host with certificates for testing with PyKMIP. In fact there are no changes anywhere in Krill other than adding support for this signer as a replacement for the existing OpenSSL based "soft signer".

Details:

Adds a dependency on the kmip-protocol crate for TCP+TLS high level KMIP client support based on the underlying kmip-ttlv crate, neither of which has been reviewed yet.
Adds a dependency on the backoff crate for retry support.
Adds a dependency on the r2d2 crate for connection pooling support.
Refactor signers to crypto::signers and replace the Dummy signer with a KMIP signer.
Added a "hsmtest" job to the GitHub Actions CI workflow that runs all Krill tests using the KMIP signer against PyKMIP.
Added a "hsm-tests" Cargo feature flag for configuring Krill to use ONLY KMIP as a signer, not OpenSSL at all.
Currently building without the "hsm-tests" feature flag set will fail if the "hsm" feature flag is set.
Krill isn't ready to be used in "hsm" mode yet.
Introduces SignerRouter between KrillSigner and SignerProvider, to implement the Signer trait so that it can be passed to builders so that their invocation of a signer also goes via SignerRouter thus dispatching to the correct signer.

This PR doesn't offer any way to see that the keys have actually been created. However, the tests pass, keys are created and signing is done, the KMIP signer is shown connecting to the PyKMIP server in the logs, and the OpenSSL signer is disabled by the feature flags used by the hsmtest job. If you watch the PyKMIP log while the test is running you can see that it is connecting and executing KMIP operations.

Update: I've updated the GH Actions CI workflow in this PR branch to always dump the PyKMIP log file, not only on error, which should make it easier for reviewers to see that there is indeed some activity.

Using a custom local KMIP client when I inspected the keys inside my local PyKMIP instance before and after running the Krill test suite, before it was empty, and after it looked like something this:

Key: 1787, Name: 'krill-public-key-2a5d9ec24d8b8fe7a17b24d15a381c01c0b47c7b'
Key: 1788, Name: 'krill-private-key-2a5d9ec24d8b8fe7a17b24d15a381c01c0b47c7b'
Key: 1765, Name: 'krill-public-key-d46af8f74d79055a0d0da6ff806f089c8f18eb30'
Key: 1766, Name: 'krill-private-key-d46af8f74d79055a0d0da6ff806f089c8f18eb30'
Key: 1755, Name: 'krill-public-key-cd27dfee6834107489e2eebb54899b8c9038d1a8'
Key: 1756, Name: 'krill-private-key-cd27dfee6834107489e2eebb54899b8c9038d1a8'
...
Key: 3, Name: 'krill-public-key-a3897008c85f8ad7ad2658c89e1dcdbca6301485'
Key: 4, Name: 'krill-private-key-a3897008c85f8ad7ad2658c89e1dcdbca6301485'
Key: 5, Name: 'krill-public-key-7809b44da1d6be664db7cabd7deb9009c94ff5c3'
Key: 6, Name: 'krill-private-key-7809b44da1d6be664db7cabd7deb9009c94ff5c3'
Key: 1, Name: 'krill-public-key-0daebf72d4b7aecf00da86db43ed220a3df78c59'
Key: 2, Name: 'krill-private-key-0daebf72d4b7aecf00da86db43ed220a3df78c59'

If you run Krill with trace level logging enabled you will then see a stripped diagnostic representation of the KMIP requests and responses being sent and received, e.g.:

2021-10-05 17:10:47 [TRACE] [kmip_protocol::client::client] KMIP TTLV request: 78[77[69[6Ai6Bi]0Di]0F[5Ce2:79[1F[08[0At0Be4:]08[0At0Bi]]65[08[0At0B[55t54e1:]]08[0At0Bi]]6E[08[0At0B[55t54e1:]]08[0At0Bi]]]]]
2021-10-05 17:10:47 [TRACE] [kmip_protocol::client::client] KMIP TTLV response: 7B[7A[69[6Ai6Bi]92d0Di]0F[5Ce2:7Fe0:7C[66t6Ft]]]

The diag_to_txt example included with the kmip-protocol crate can render these diagnostic strings in slightly more readable form, e.g. the first line from the log example above renders like so:

 cargo run --example diag_to_txt /tmp/r
   Compiling kmip-protocol v0.3.3-dev (/home/ximon/src/krill-kmip-protocol)
    Finished dev [unoptimized + debuginfo] target(s) in 4.37s
     Running `target/debug/examples/diag_to_txt /tmp/r`
Tag: Request Message (0x420078), Type: Structure (0x01), Data: 
  Tag: Request Header (0x420077), Type: Structure (0x01), Data: 
    Tag: Protocol Version (0x420069), Type: Structure (0x01), Data: 
      Tag: Protocol Version Major (0x42006A), Type: Integer (0x02), Data: <redacted>
      Tag: Protocol Version Minor (0x42006B), Type: Integer (0x02), Data: <redacted>
    Tag: Batch Count (0x42000D), Type: Integer (0x02), Data: <redacted>
  Tag: Batch Item (0x42000F), Type: Structure (0x01), Data: 
    Tag: Operation (0x42005C), Type: Enumeration (0x05), Data: 2
    Tag: Request Payload (0x420079), Type: Structure (0x01), Data: 
      Tag: Common Template-Attribute (0x42001F), Type: Structure (0x01), Data: 
        Tag: Attribute (0x420008), Type: Structure (0x01), Data: 
          Tag: Attribute Name (0x42000A), Type: TextString (0x07), Data: <redacted>
          Tag: Attribute Value (0x42000B), Type: Enumeration (0x05), Data: 4
        Tag: Attribute (0x420008), Type: Structure (0x01), Data: 
          Tag: Attribute Name (0x42000A), Type: TextString (0x07), Data: <redacted>
          Tag: Attribute Value (0x42000B), Type: Integer (0x02), Data: <redacted>
      Tag: Private Key Template-Attribute (0x420065), Type: Structure (0x01), Data: 
        Tag: Attribute (0x420008), Type: Structure (0x01), Data: 
          Tag: Attribute Name (0x42000A), Type: TextString (0x07), Data: <redacted>
          Tag: Attribute Value (0x42000B), Type: Structure (0x01), Data: 
            Tag: Name Value (0x420055), Type: TextString (0x07), Data: <redacted>
            Tag: Name Type (0x420054), Type: Enumeration (0x05), Data: 1
        Tag: Attribute (0x420008), Type: Structure (0x01), Data: 
          Tag: Attribute Name (0x42000A), Type: TextString (0x07), Data: <redacted>
          Tag: Attribute Value (0x42000B), Type: Integer (0x02), Data: <redacted>
      Tag: Public Key Template-Attribute (0x42006E), Type: Structure (0x01), Data: 
        Tag: Attribute (0x420008), Type: Structure (0x01), Data: 
          Tag: Attribute Name (0x42000A), Type: TextString (0x07), Data: <redacted>
          Tag: Attribute Value (0x42000B), Type: Structure (0x01), Data: 
            Tag: Name Value (0x420055), Type: TextString (0x07), Data: <redacted>
            Tag: Name Type (0x420054), Type: Enumeration (0x05), Data: 1
        Tag: Attribute (0x420008), Type: Structure (0x01), Data: 
          Tag: Attribute Name (0x42000A), Type: TextString (0x07), Data: <redacted>
          Tag: Attribute Value (0x42000B), Type: Integer (0x02), Data: <redacted>

Operation code 2 is Create Key Pair according to the KMIP specification (see KMIP 1.0 section 9.1.3.2.26 Operation Enumeration).

The second log line, the response, renders like this:

❯ cargo run --example diag_to_txt /tmp/r2
    Finished dev [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/examples/diag_to_txt /tmp/r2`
Tag: Response Message (0x42007B), Type: Structure (0x01), Data: 
  Tag: Response Header (0x42007A), Type: Structure (0x01), Data: 
    Tag: Protocol Version (0x420069), Type: Structure (0x01), Data: 
      Tag: Protocol Version Major (0x42006A), Type: Integer (0x02), Data: <redacted>
      Tag: Protocol Version Minor (0x42006B), Type: Integer (0x02), Data: <redacted>
    Tag: Time Stamp (0x420092), Type: DateTime (0x09), Data: <redacted>
    Tag: Batch Count (0x42000D), Type: Integer (0x02), Data: <redacted>
  Tag: Batch Item (0x42000F), Type: Structure (0x01), Data: 
    Tag: Operation (0x42005C), Type: Enumeration (0x05), Data: 2
    Tag: Result Status (0x42007F), Type: Enumeration (0x05), Data: 0
    Tag: Response Payload (0x42007C), Type: Structure (0x01), Data: 
      Tag: Private Key Unique Identifier (0x420066), Type: TextString (0x07), Data: <redacted>
      Tag: Public Key Unique Identifier (0x42006F), Type: TextString (0x07), Data: <redacted>

This shows a successful response that includes the internal PyKMIP identifiers for the created keys (which are redacted in the diagnostic output).

…, and support in principle selecting which signer to use for which purpose. (#539)

- Add a dependency on the backoff crate for retry support. - Add a dependency on the r2d2 crate for connection pooling support. - Uses GitHub versions of the bcder and rpki crates for the DER Unsigned Integer support needed by the KMIP signer. - Refactor signers to crypto::signers and replace the Dummy signer with a KMIP signer. - Added a "hsmtest" job to the GitHub Actions CI workflow that runs all Krill tests using the KMIP signer against PyKMIP. - Added a "hsm-tests" Cargo feature flag for configuring Krill to use ONLY KMIP as a signer, not OpenSSL at all. Currently building without the "hsm-tests" feature flag set will fail if the "hsm" feature flag is set. Krill isn't ready to be used in "hsm" mode yet. - Changes SignerProvider to implement the Signer trait so that it can be passed to builders so that their invocation of a signer also goes via SignerProvider dispatching to the correct signer.

… the write lock while switching to using the server as part of finishing a successful probe.

…mplementation

… PyKMIP.

…er. Previously some Krill logic when invoked was given the same signer as handling the current purpose to invoke later even if for a different purpose. If the initial purpose required the KMIP signer as the key owning signer but the later purpose was one-off signing then that should be able to be routed if desired to the OpenSslSigner, for example. Introduces another layer of indirection: RouterSigner.

src/commons/crypto/signing.rs

"router", not "dispatcher", and don't lock the entire SignerRouter for create/delete key operations.

…mplementation

…cessor PR #688 which is now redundant because those changes are now present in the 'dev' branch which this PR targets.

…mplementation

…in this case).

…mplementation

src/commons/crypto/signers/kmip/internal.rs

src/commons/crypto/signers/kmip/signer.rs

src/commons/crypto/signing.rs

doc/development/hsm/architecture.md

src/commons/crypto/signers/kmip/internal.rs

…trait.

ximon18 · 2021-11-15T13:49:20Z

@timbru : Am I right in thinking that your review didn't including looking at the kmip-protocol crate dependency code or the kmip-ttlv crate that it depends on, both of which were written by me and whose code is also in NLnet Labs GitHub repositories?

timbru · 2021-11-15T13:52:10Z

@timbru : Am I right in thinking that your review didn't including looking at the kmip-protocol crate dependency code or the kmip-ttlv crate that it depends on, both of which were written by me and whose code is also in NLnet Labs GitHub repositories?

Indeed. I am just focussing on krill itself here - I am happy to look at the kmip libs as a separate effort.

…creation.

ximon18 added 3 commits September 23, 2021 02:27

Support multiple signers of different types behind a HSM feature flag…

5571271

…, and support in principle selecting which signer to use for which purpose. (#539)

Added some developer docs.

1f2c9ee

Merge branch 'dev' into issue-539-support-multiple-signers

0fb9c91

ximon18 changed the base branch from main to dev September 28, 2021 14:50

ximon18 changed the title ~~Issue 566 implement krill kmip based signer implementation~~ KMIP walking skeleton (#566) Sep 28, 2021

ximon18 linked an issue Sep 28, 2021 that may be closed by this pull request

Implement Krill KMIP based Signer implementation #566

Closed

ximon18 mentioned this pull request Sep 30, 2021

HSM: Support multiple signers (#539) #674

Merged

ximon18 force-pushed the issue-566-implement-krill-kmip-based-signer-implementation branch from 575745d to 870a102 Compare October 5, 2021 11:26

ximon18 added 3 commits October 5, 2021 14:48

Avoid potential race conditions: Check for expected state, and retain…

b849f66

… the write lock while switching to using the server as part of finishing a successful probe.

Merge branch 'dev' into issue-566-implement-krill-kmip-based-signer-i…

4e759aa

…mplementation

Merge branch 'dev' into issue-566-implement-krill-kmip-based-signer-i…

431b54d

…mplementation

ximon18 requested a review from timbru October 5, 2021 15:00

Always dump the PyKMIP log so that we can see evidence of Krill using…

9b788ae

… PyKMIP.

ximon18 marked this pull request as ready for review October 5, 2021 15:03

ximon18 added 3 commits October 5, 2021 22:55

Additional architecture docs regarding signing.

5e359b3

Remove commented out code.

369d00a

ximon18 mentioned this pull request Oct 6, 2021

Deploy SoftHSM and/or PyKMIP for the Krill functional test #683

Closed

This was linked to issues Oct 6, 2021

Deploy SoftHSM and/or PyKMIP for the Krill functional test #683

Closed

Decide how to handle HSM related failures #542

Closed

ximon18 mentioned this pull request Oct 6, 2021

Decide how to handle HSM related failures #542

Closed

ximon18 linked an issue Oct 6, 2021 that may be closed by this pull request

Time out signer requests #643

Open

ximon18 mentioned this pull request Oct 6, 2021

Time out signer requests #643

Open

ximon18 commented Oct 6, 2021

View reviewed changes

src/commons/crypto/signing.rs Outdated Show resolved Hide resolved

Remove unnecessary Arc<RwLock<..>> and naming cleanup.

280ccc4

"router", not "dispatcher", and don't lock the entire SignerRouter for create/delete key operations.

ximon18 added the hsm Relates to adding HSM support to Krill label Oct 14, 2021

ximon18 requested a review from partim October 25, 2021 15:29

ximon18 mentioned this pull request Oct 25, 2021

HSM 2: Persistent signer key mappings #686

Merged

Merge branch 'dev' into issue-566-implement-krill-kmip-based-signer-i…

1548efd

…mplementation

Merge branch 'dev' into issue-566-implement-krill-kmip-based-signer-i…

dd705df

…mplementation

ximon18 added 5 commits November 3, 2021 11:28

Apply crate upgrade and signer 'de-mut' changes that were made in suc…

3e6c0cb

…cessor PR #688 which is now redundant because those changes are now present in the 'dev' branch which this PR targets.

Merge branch 'dev' into issue-566-implement-krill-kmip-based-signer-i…

90de083

…mplementation

Merge fix.

00e93ce

Merge branch 'dev' into issue-566-implement-krill-kmip-based-signer-i…

a1c39f7

…mplementation

Wait longer to "fix" brittle test when using a slower signer (PyKMIP …

e217717

…in this case).

ximon18 changed the title ~~KMIP walking skeleton (#566)~~ HSM 1: KMIP walking skeleton (#566) Nov 10, 2021

Merge branch 'dev' into issue-566-implement-krill-kmip-based-signer-i…

5ceba33

…mplementation

timbru approved these changes Nov 11, 2021

View reviewed changes

ximon18 added 3 commits November 14, 2021 23:19

Review feedback: KmipSigner doesn't needt o implement the Signer trait.

7feffe1

Review feedback: SignerProvider doesn't need to implement the Signer …

40bafc3

…trait.

OpenSslSigner no longer needs to implement the Signer trait.

8e9cda8

Review feedback: Spend more words on why KMIP keys are renamed after …

0782791

…creation.

ximon18 changed the base branch from dev to hsm November 22, 2021 21:46

ximon18 merged commit 936dbda into hsm Nov 23, 2021

ximon18 deleted the issue-566-implement-krill-kmip-based-signer-implementation branch November 23, 2021 10:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HSM 1: KMIP walking skeleton (#566) #679

HSM 1: KMIP walking skeleton (#566) #679

ximon18 commented Sep 28, 2021 •

edited

Loading

ximon18 commented Nov 15, 2021

timbru commented Nov 15, 2021

HSM 1: KMIP walking skeleton (#566) #679

HSM 1: KMIP walking skeleton (#566) #679

Conversation

ximon18 commented Sep 28, 2021 • edited Loading

ximon18 commented Nov 15, 2021

timbru commented Nov 15, 2021

ximon18 commented Sep 28, 2021 •

edited

Loading