From 936dbdab0553bd4468c33e19e997377dc81fc555 Mon Sep 17 00:00:00 2001 From: Ximon Eighteen <3304436+ximon18@users.noreply.github.com> Date: Tue, 23 Nov 2021 09:40:40 +0100 Subject: [PATCH] Initial KMIP signer support (#566) * Support multiple signers of different types behind a `hsm` feature flag, and support in principle selecting which signer to use for which purpose (#539). Note: Currently only usable in combination with a new`hsm-tests` feature flag due to lack of any actual means to select an alternate signer via code or config. * Replaces the dummy signer with a KMIP signer (#566) and supporting dependencies `kmip-protocol` (#557, #558, #559), `backoff` (retry support), `r2d2` (connection pooling support). Adds a `hsm-tests` feature flag for testing exclusively with KMIP, i.e. not using the OpenSSL signer at all. * Adds a GitHub Actions `hsmtest` CI job that tests Krill integration with a co-installed PyKMIP instance (#560, #561, #683). --- .github/workflows/ci.yml | 40 + Cargo.lock | 140 +++- Cargo.toml | 6 +- doc/development/hsm/architecture.md | 101 ++- src/commons/api/ca.rs | 5 +- src/commons/crypto/mod.rs | 2 + src/commons/crypto/signers/error.rs | 48 ++ src/commons/crypto/signers/kmip/connpool.rs | 125 +++ src/commons/crypto/signers/kmip/internal.rs | 781 ++++++++++++++++++ src/commons/crypto/signers/kmip/keymap.rs | 13 + src/commons/crypto/signers/kmip/mod.rs | 21 + src/commons/crypto/signers/kmip/signer.rs | 84 ++ src/commons/crypto/signers/mod.rs | 7 + .../{util => crypto/signers}/softsigner.rs | 77 +- src/commons/crypto/signers/util.rs | 17 + src/commons/crypto/signing.rs | 384 +++++---- src/commons/error.rs | 3 +- src/commons/util/dummysigner.rs | 48 -- src/commons/util/mod.rs | 3 - test-resources/pykmip/README.md | 48 ++ test-resources/pykmip/ca.crt | 14 + test-resources/pykmip/run-server.py | 10 + test-resources/pykmip/server.conf | 11 + test-resources/pykmip/server.crt | 41 + test-resources/pykmip/server.key | 5 + tests/suspend.rs | 2 +- 26 files changed, 1729 insertions(+), 307 deletions(-) create mode 100644 src/commons/crypto/signers/error.rs create mode 100644 src/commons/crypto/signers/kmip/connpool.rs create mode 100644 src/commons/crypto/signers/kmip/internal.rs create mode 100644 src/commons/crypto/signers/kmip/keymap.rs create mode 100644 src/commons/crypto/signers/kmip/mod.rs create mode 100644 src/commons/crypto/signers/kmip/signer.rs create mode 100644 src/commons/crypto/signers/mod.rs rename src/commons/{util => crypto/signers}/softsigner.rs (77%) create mode 100644 src/commons/crypto/signers/util.rs delete mode 100644 src/commons/util/dummysigner.rs create mode 100644 test-resources/pykmip/README.md create mode 100644 test-resources/pykmip/ca.crt create mode 100755 test-resources/pykmip/run-server.py create mode 100644 test-resources/pykmip/server.conf create mode 100644 test-resources/pykmip/server.crt create mode 100644 test-resources/pykmip/server.key diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b8b07977b..ca95acd50 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,3 +67,43 @@ jobs: name: cypress-ui-test-captures ${{ matrix.os }} ${{ matrix.rust }} path: target/ui/ if-no-files-found: ignore + + hsmtest: + name: hsmtest + runs-on: ubuntu-18.04 + steps: + - name: Checkout repository + uses: actions/checkout@v1 + + - name: Install Rust + uses: hecrj/setup-rust-action@v1 + with: + rust-version: stable + + - uses: actions/setup-python@v2 + with: + python-version: '3.x' + + - name: Install PyKMIP + uses: BSFishy/pip-action@v1 + with: + packages: pykmip + + - name: Compile the tests + run: | + cargo build --tests --no-default-features --features hsm,hsm-tests + + - name: Run the tests against the PyKMIP server + run: | + cd test-resources/pykmip + python run-server.py & + sleep 5s + openssl s_client -connect 127.0.0.1:5696 || true + cd - + cargo test --no-default-features --features hsm,hsm-tests -- --test-threads=1 2>&1 + + - name: Dump PyKMIP log + working-directory: test-resources/pykmip + run: | + ls -la + cat server.log \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 33aeb4259..d55641c53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,17 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a" +[[package]] +name = "backoff" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9fe17f59a06fe8b87a6fc8bf53bb70b3aba76d7685f432487a68cd5552853625" +dependencies = [ + "getrandom 0.2.3", + "instant", + "rand 0.8.4", +] + [[package]] name = "backtrace" version = "0.3.61" @@ -364,6 +375,28 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "enum-display-derive" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f16ef37b2a9b242295d61a154ee91ae884afff6b8b933b486b12481cc58310ca" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "enum-flags" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3682d2328e61f5529088a02cd20bb0a9aeaeeeb2f26597436dd7d75d1340f8f5" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "error-chain" version = "0.11.0" @@ -777,10 +810,45 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "kmip-protocol" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef3e9e144b27f0e12e027f8eae5f174656c5c5fae9a45f3a06c2b866ce6e4f02" +dependencies = [ + "cfg-if", + "enum-display-derive", + "enum-flags", + "kmip-ttlv", + "log", + "maybe-async", + "openssl", + "rustc_version 0.4.0", + "serde", + "serde_bytes", + "serde_derive", + "trait-set", +] + +[[package]] +name = "kmip-ttlv" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4829d1eaf141b0fd93402d9e219a2453984106931de6e0cdbd8ca02bec58f2bc" +dependencies = [ + "cfg-if", + "hex", + "maybe-async", + "rustc_version 0.4.0", + "serde", + "trait-set", +] + [[package]] name = "krill" version = "0.9.3-rc3" dependencies = [ + "backoff", "base64 0.13.0", "basic-cookies", "bcder", @@ -796,18 +864,20 @@ dependencies = [ "hyper", "intervaltree", "jmespatch", + "kmip-protocol", "libc", "libflate", "log", "openidconnect", "openssl", "oso", + "r2d2", "rand 0.8.4", "regex", "reqwest", "rpassword", "rpki", - "rustc_version", + "rustc_version 0.2.3", "scrypt", "serde", "serde_json", @@ -925,6 +995,17 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a3e378b66a060d48947b590737b30a1be76706c8dd7b8ba0f2fe3989c68a853f" +[[package]] +name = "maybe-async" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6007f9dad048e0a224f27ca599d669fca8cfa0dac804725aab542b2eb032bce6" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "memchr" version = "2.4.1" @@ -1334,6 +1415,17 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "r2d2" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "545c5bc2b880973c9c10e4067418407a0ccaa3091781d1671d46eb35107cb26f" +dependencies = [ + "log", + "parking_lot", + "scheduled-thread-pool", +] + [[package]] name = "rand" version = "0.7.3" @@ -1565,7 +1657,16 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" dependencies = [ - "semver", + "semver 0.9.0", +] + +[[package]] +name = "rustc_version" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa0f585226d2e68097d4f95d113b15b83a82e819ab25717ec0590d9584ef366" +dependencies = [ + "semver 1.0.4", ] [[package]] @@ -1612,6 +1713,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "scheduled-thread-pool" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc6f74fd1204073fa02d5d5d68bec8021be4c38690b61264b2fdb48083d0e7d7" +dependencies = [ + "parking_lot", +] + [[package]] name = "scopeguard" version = "1.1.0" @@ -1672,6 +1782,12 @@ dependencies = [ "semver-parser", ] +[[package]] +name = "semver" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "568a8e6258aa33c13358f81fd834adb854c6f7c9468520910a9b1e8fac068012" + [[package]] name = "semver-parser" version = "0.7.0" @@ -1697,6 +1813,15 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_bytes" +version = "0.11.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16ae07dd2f88a366f15bd0632ba725227018c69a1c8550a927324f8eb8368bb9" +dependencies = [ + "serde", +] + [[package]] name = "serde_derive" version = "1.0.130" @@ -2118,6 +2243,17 @@ dependencies = [ "tracing-serde", ] +[[package]] +name = "trait-set" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "875c4c873cc824e362fa9a9419ffa59807244824275a44ad06fec9684fff08f2" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "try-lock" version = "0.2.3" diff --git a/Cargo.toml b/Cargo.toml index bbaea7dd6..d248ffb7c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ exclude = [ build = "build.rs" [dependencies] +backoff = { version = "0.3.0", optional = true } base64 = "^0.13" basic-cookies = { version = "^0.1", optional = true } bcder = "0.6.1" @@ -35,11 +36,13 @@ hex = "^0.4" hyper = { version = "^0.14", features = ["server"] } intervaltree = "0.2.6" jmespatch = { version = "^0.3", features = ["sync"], optional = true } +kmip = { version = "0.3.1", package = "kmip-protocol", features = ["tls-with-openssl"], optional = true } libflate = "^1" log = "^0.4" openidconnect = { version = "^2.0.0", optional = true, default_features = false } openssl = { version = "^0.10", features = ["v110"] } oso = { version = "^0.12", optional = true, default_features = false } +r2d2 = { version = "0.8.9", optional = true } rand = "^0.8" regex = { version = "^1.4", optional = true, default_features = false, features = ["std"] } reqwest = { version = "0.11", features = ["json"] } @@ -73,7 +76,8 @@ ui-tests = [] extra-debug = [ "rpki/extra-debug" ] static-openssl = [ "openssl/vendored" ] all-except-ui-tests = [ "multi-user", "rta", "static-openssl", "aspa" ] -hsm = [] +hsm = ["backoff", "kmip", "r2d2"] +hsm-tests = [] # Make sure that Krill crashes on panics, rather than losing threads and # limping on in a bad state. diff --git a/doc/development/hsm/architecture.md b/doc/development/hsm/architecture.md index 76a8d2d8a..ef392cabf 100644 --- a/doc/development/hsm/architecture.md +++ b/doc/development/hsm/architecture.md @@ -1,22 +1,97 @@ # HSM: Architecture -## Why create new KMIP crates? +## Key data -No well or actively maintained Rust support for KMIP with sufficient functionality for Krill -existed at the time of writing. +- Krill `KeyIdentifier`: SHA-1 of the binary DER encoding of the X.509 ASN.1 SubjectPublicKeyInfo data structure) used + by Krill. Cannot be changed as it is stored in certificates as per standards and then later the certificate is the + only information available to the calling code, i.e. it doesn't have anything else to identify the key involved. -The closest candidate, https://github.com/visa/kmip, was used to explore KMIP support in the -Krill HSM prototype code. However, it was decided to create our own KMIP library because the -visa crate: +## Key architectural decisions + +- Create our own KMIP Rust library as no actively maintained Rust support for KMIP with sufficient functionality for + Krill existed at the time of writing. The closest candidate, https://github.com/visa/kmip, was used to explore KMIP + support in the Krill HSM prototype code. However, it was decided to create our own KMIP library because the visa + crate: - Was not published on crates.io. - Had only rudimentary error reporting (as it targets no-std environments). - - Lacks documentation. - - Lacked TCP+TLS client support. - - Lacked support for KMIP operations that Krill requires. - - Did not appear to be actively maintained or intended for use by others. + - Lacked documentation, support for needed KMIP operations & TCP+TLS client support. + + Don't include the KMIP library in the main Krill code base as it is orthogonal to and independently useful outside of + Krill, and the Krill codebase is already quite large & slow to compile. + +- Support multiple concurrently active signers to support rollover to a new signer (creation of new keys with the new + signer while continued use of keys created by the previous signer), fallback to the OpenSSL signer for random number + generation whne the chosen doesn't support this capability, generation of one-time keys using OpenSSL as doing this + with an HSM would be slow (create, activate, sign, deactivate, destroy, potentially each being a network round trip + plus relatively slow execution of operations compared to local OpenSSL) and because the security benefits of an HSM + are not needed for one-time signing keys. + +- Maintain a mapping of Krill `KeyIdentifier` to signer identifier so that we can dispatch signing requests to the + correct signer. + +- Maintain a mapping of Krill `KeyIdentifier` to signer specific key identifiers. + +- Support generation of the `KeyIdentifier` from component parts (RSA modulus and exponent) for cases where the signer + doesn't (guarantee) support for exposing that for us itself but only provides access to the component parts. + +- Don't fail to start Krill if a signer backend is not reachable or lacks required capabilities. Preventing Krill from + operating won't fix the problem and prevents Krill from doing anything else useful with keys from other signers or + offering its API or UI. + +## Design + +- KrillSigner acts as the central hub aware of all signers and key mappings and dispatches requests to the correct + signer based on the action being performed and/or the `KeyIdentifier` involved. + +- Persist key mappings to: TBD + +- Permit signers to be async: TBD + +- Be robust in case of network delays and errors and problems in external signing services. Retry requests that fail + due to issues potentially caused by transient network degradation. Re-use TCP+TLS sessions to avoid costly TCP+TLS + setup and teardown costs per request to the signer service. + +- The signer can be in one of three statuses: + - Proving - Rather than constantly attempt to "probe" (contact the server and check its capabilities) we track in + this state when we last probed the server so that we can limit how often we try probing the server. + - Unusable - Probing was able to contact the server and found it unusable. + - Usable - Probing was able to contact the server and found it usable. + +## Implementation details + +- Connection pooling is handled by the `r2d2` crate. +- Retry and backoff is handled by the `backoff` crate. +- TLS is handled by the `openssl` crate as `rustls` may impose stricter limits on outbound connectivity to HSMs than + can be pragmatically expected to work in all customer environments. +- TBD: Switch to `tokio-native-tls` to keep the benefits of and control of local O/S native TLS providers and avoid the + 'modern security' limitations of `rustls` while switching to an `async` model. + +## Control flow + +Prior to the addition of HSM support there was only ever a single Signer and control flow looked like this: + +``` +Krill calling code -> KrillSigner -> OpenSslSigner +``` + +With the addition of HSM support there may be multiple concurrently active Signers and the control flow becomes this: + +``` +Krill calling code -> KrillSigner -> SignerRouter -> SignerProvider -> One of: OpenSslSigner, KmipSigner or Pkcs11Signer. + | | + +--------> KeyMapper <---------+ +``` + +_(note: in the current version of the code there is only an in-memory KeyMap used only by the KmipSigner and only ever + one active Signer impl)_ -## Why not add KMIP code to Krill directly? +The SignerRouter either: +- Uses the KeyMapper to determine which Signer owns the KeyIdentifier being used, OR +- Uses internal rules to decide which Signer to invoke (e.g. always generate random values and do one-off signing using + the OpenSslSigner for example) -The Krill code base is already large enough and slow enough to compile. KMIP support may also -be of interest to others. It thus seemed a good candidate for separation from Krill itself. \ No newline at end of file +The Signers: +- Update the KeyMapper when new keys are created to indicate that this Signer owns the key. +- Store KeyIdentifier to HSM internal key identifier mappings in the KeyMapper. +- Retrieve HSM internal key identifiers from the KeyMapper based on the KeyIdentifier being used. \ No newline at end of file diff --git a/src/commons/api/ca.rs b/src/commons/api/ca.rs index 094a1ed4c..c962afd8b 100644 --- a/src/commons/api/ca.rs +++ b/src/commons/api/ca.rs @@ -2665,9 +2665,10 @@ impl ResourceSetError { mod test { use bytes::Bytes; - use rpki::repository::crypto::{signer::Signer, PublicKeyFormat}; + use rpki::repository::crypto::PublicKeyFormat; + + use crate::commons::crypto::signers::softsigner::OpenSslSigner; - use crate::commons::util::softsigner::OpenSslSigner; use crate::test; use super::*; diff --git a/src/commons/crypto/mod.rs b/src/commons/crypto/mod.rs index 53d41d61c..c69fdcea9 100644 --- a/src/commons/crypto/mod.rs +++ b/src/commons/crypto/mod.rs @@ -10,4 +10,6 @@ pub use self::error::*; mod signing; pub use self::signing::*; +pub mod signers; + pub type CryptoResult = std::result::Result; diff --git a/src/commons/crypto/signers/error.rs b/src/commons/crypto/signers/error.rs new file mode 100644 index 000000000..327454913 --- /dev/null +++ b/src/commons/crypto/signers/error.rs @@ -0,0 +1,48 @@ +use std::{fmt, path::PathBuf}; + +use openssl::error::ErrorStack; + +use crate::commons::error::KrillIoError; + +#[derive(Debug)] +pub enum SignerError { + KmipError(String), + OpenSslError(ErrorStack), + JsonError(serde_json::Error), + InvalidWorkDir(PathBuf), + IoError(KrillIoError), + KeyNotFound, + DecodeError, +} + +impl fmt::Display for SignerError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + SignerError::KmipError(e) => write!(f, "KMIP Error: {}", e), + SignerError::OpenSslError(e) => write!(f, "OpenSsl Error: {}", e), + SignerError::JsonError(e) => write!(f, "Could not decode public key info: {}", e), + SignerError::InvalidWorkDir(path) => write!(f, "Invalid base path: {}", path.to_string_lossy()), + SignerError::IoError(e) => e.fmt(f), + SignerError::KeyNotFound => write!(f, "Could not find key"), + SignerError::DecodeError => write!(f, "Could not decode key"), + } + } +} + +impl From for SignerError { + fn from(e: ErrorStack) -> Self { + SignerError::OpenSslError(e) + } +} + +impl From for SignerError { + fn from(e: serde_json::Error) -> Self { + SignerError::JsonError(e) + } +} + +impl From for SignerError { + fn from(e: KrillIoError) -> Self { + SignerError::IoError(e) + } +} diff --git a/src/commons/crypto/signers/kmip/connpool.rs b/src/commons/crypto/signers/kmip/connpool.rs new file mode 100644 index 000000000..629b9bcc9 --- /dev/null +++ b/src/commons/crypto/signers/kmip/connpool.rs @@ -0,0 +1,125 @@ +/// KMIP TLS connection pool +/// +/// Used to: +/// - Avoid repeated TCP connection setup and TLS session establishment for mutiple KMIP requests made close together +/// in time. +/// - Handle loss of connectivity by re-creating the connection when an existing connection is considered to be +/// "broken" at the network level. +use std::time::Duration; + +use kmip::client::ConnectionSettings; + +use crate::commons::crypto::signers::{error::SignerError, kmip::internal::KmipTlsClient}; + +/// Manages KMIP TCP + TLS connection creation. +/// +/// Uses the [r2d2] crate to manage a pool of connections. +/// +/// [r2d2]: https://crates.io/crates/r2d2/ +#[derive(Debug)] +pub struct ConnectionManager { + conn_settings: ConnectionSettings, +} + +impl ConnectionManager { + /// Create a pool of up-to N TCP + TLS connections to the KMIP server. + #[rustfmt::skip] + pub fn create_connection_pool( + conn_settings: ConnectionSettings, + max_size: u32, + ) -> Result, SignerError> { + let pool = r2d2::Pool::builder() + // Don't pre-create idle connections to the KMIP server + .min_idle(Some(0)) + + // Create at most this many concurrent connections to the KMIP server + .max_size(max_size) + + // Don't verify that a connection is usable when fetching it from the pool (as doing so requires sending a + // request to the server and we might as well just try the actual request that we want the connection for) + .test_on_check_out(false) + + // Don't use the default logging behaviour as `[ERROR] [r2d2] Server error: ...` is a bit confusing for end + // users who shouldn't know or care that we use the r2d2 crate. + .error_handler(Box::new(ErrorLoggingHandler)) + + // Don't keep using the same connection for longer than around 30 minutes (unless in use in which case it + // will wait until the connection is returned to the pool before closing it) - maybe long held connections + // would run into problems with some firewalls. + .max_lifetime(Some(Duration::from_secs(60*30))) + + // Don't keep connections open that were not used in the last 10 minutes. + .idle_timeout(Some(Duration::from_secs(60*10))) + + // Don't wait longer than 30 seconds for a new connection to be established, instead try again to connect. + .connection_timeout(Duration::from_secs(30)) + + // Use our connection manager to create connections in the pool and to verify their health + .build(ConnectionManager { conn_settings })?; + + Ok(pool) + } + + /// Connect using the given connection settings to a KMIP server. + /// + /// This function creates a new connection to the server. The connection is NOT taken from the connection pool. + pub fn connect_one_off(settings: &ConnectionSettings) -> Result { + let conn = kmip::client::tls::openssl::connect(settings)?; + Ok(conn) + } +} + +impl r2d2::ManageConnection for ConnectionManager { + type Connection = KmipTlsClient; + + type Error = kmip::client::Error; + + /// Establishes a KMIP server connection which will be added to the connection pool. + fn connect(&self) -> Result { + Self::connect_one_off(&self.conn_settings) + } + + /// This function is never used because the [r2d2] `test_on_check_out` flag is set to false when the connection + /// pool is created. + /// + /// [r2d2]: https://crates.io/crates/r2d2/ + fn is_valid(&self, _conn: &mut Self::Connection) -> Result<(), Self::Error> { + unreachable!() + } + + /// Quickly verify if an existing connection is broken. + /// + /// Used to discard and re-create connections that encounter multiple connection related errors. + fn has_broken(&self, conn: &mut Self::Connection) -> bool { + conn.connection_error_count() > 1 + } +} + +/// A Krill specific [r2d2] error logging handler. +/// +/// Logs connection pool related connection error messages using the format `"[] Pool error: ..."` instead of +/// the default [r2d2] `"[ERROR] [r2d2] Server error: ..."` format. Assumes that the logging framework will include the +/// logging module context in the logged message, i.e. `xxx::kmip::xxx` and thus we don't need to mention KMIP in the +/// logged message content. +/// +/// Rationale: +/// - The use of the [r2d2] crate is an internal detail which of no use to end users consulting the logs and which we +/// may change at any time. +/// - Krill should be the one to determine the appropriate level to log a connection issue at, not [r2d2]. +#[derive(Debug)] +struct ErrorLoggingHandler; + +impl r2d2::HandleError for ErrorLoggingHandler +where + E: std::fmt::Display, +{ + fn handle_error(&self, err: E) { + warn!("Pool error: {}", err) + } +} + +impl From for SignerError { + fn from(err: r2d2::Error) -> Self { + SignerError::KmipError(format!("{}", err)) + } +} diff --git a/src/commons/crypto/signers/kmip/internal.rs b/src/commons/crypto/signers/kmip/internal.rs new file mode 100644 index 000000000..25a5785e7 --- /dev/null +++ b/src/commons/crypto/signers/kmip/internal.rs @@ -0,0 +1,781 @@ +use std::{ + net::TcpStream, + ops::Deref, + sync::{Arc, RwLock, RwLockReadGuard}, + time::{Duration, Instant}, +}; + +use backoff::ExponentialBackoff; +use bcder::encode::{PrimitiveContent, Values}; +use bytes::Bytes; +use kmip::{ + client::{Client, ClientCertificate, ConnectionSettings}, + types::{ + common::{KeyMaterial, ObjectType, Operation}, + response::{ManagedObject, RNGRetrieveResponsePayload}, + }, +}; +use openssl::ssl::SslStream; +use r2d2::PooledConnection; +use rpki::repository::crypto::{ + signer::KeyError, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, +}; + +use crate::commons::{ + api::Timestamp, + crypto::signers::{ + error::SignerError, + kmip::{ + connpool::ConnectionManager, + keymap::{KeyMap, KmipKeyPairIds}, + }, + util, + }, +}; + +//------------ Types and constants ------------------------------------------------------------------------------------ + +/// The time to wait between attempts to initially connect to the KMIP server to verify our connection settings and the +/// server capabilities. +const RETRY_INIT_EVERY: Duration = Duration::from_secs(30); + +/// The time to wait between an initial and subsequent attempt at sending a request to the KMIP server. +const RETRY_REQ_AFTER: Duration = Duration::from_secs(2); + +/// How much longer should we wait from one request attempt to the next compared to the previous wait? +const RETRY_REQ_AFTER_MULTIPLIER: f64 = 1.5; + +/// The maximum amount of time to keep retrying a failed request. +const RETRY_REQ_UNTIL_MAX: Duration = Duration::from_secs(30); + +/// The maximum number of concurrent connections to the KMIP server to pool. +const MAX_CONCURRENT_SERVER_CONNECTIONS: u32 = 5; + +/// TODO: Make this a configuration setting. For now set it to false because PyKMIP 0.10.0 says it doesn't support the +/// `ModifyAttribute` but sending a modify attribute request succeeds. +const IGNORE_MISSING_CAPABILITIES: bool = false; + +/// A KMIP client that uses a specific TLS and TCP stream implementation. Currently set to [SslStream] from the +/// [openssl] crate. This will be a different type if we switch to different TCP and/or TLS implementations or to an +/// async implementation, but the client interface will remain the same. +pub type KmipTlsClient = Client>; + +//------------ The KMIP signer management interface ------------------------------------------------------------------- + +#[derive(Clone, Debug)] +pub struct KmipSigner { + /// A probe dependent interface to the KMIP server. + server: Arc>, +} + +impl KmipSigner { + /// Creates a new instance of KmipSigner. + pub fn build() -> Result { + let conn_settings = Self::get_test_connection_settings(); + let server = Arc::new(RwLock::new(ProbingServerConnector::new(conn_settings))); + Ok(KmipSigner { server }) + } + + /// Returns true if the KMIP server supports generation of random numbers, false otherwise. + pub fn supports_random(&self) -> bool { + match self.server() { + Ok(status) => status.state().supports_rng_retrieve, + Err(_) => false, + } + } + + // TODO: Remove me once we support passing configuration in to `fn build()`. + fn get_test_connection_settings() -> ConnectionSettings { + let client_cert = ClientCertificate::SeparatePem { + cert_bytes: include_bytes!("../../../../../test-resources/pykmip/server.crt").to_vec(), + key_bytes: Some(include_bytes!("../../../../../test-resources/pykmip/server.key").to_vec()), + }; + let server_cert = include_bytes!("../../../../../test-resources/pykmip/server.crt").to_vec(); + let ca_cert = include_bytes!("../../../../../test-resources/pykmip/ca.crt").to_vec(); + + ConnectionSettings { + host: "127.0.0.1".to_string(), + port: 5696, + username: None, + password: None, + insecure: true, + client_cert: Some(client_cert), + server_cert: Some(server_cert), + ca_cert: Some(ca_cert), + connect_timeout: Some(Duration::from_secs(5)), + read_timeout: Some(Duration::from_secs(5)), + write_timeout: Some(Duration::from_secs(5)), + max_response_bytes: Some(64 * 1024), + } + } +} + +//------------ Probe based server access ------------------------------------------------------------------------------ + +/// Probe status based access to the KMIP server. +/// +/// To avoid blocking Krill startup due to HSM connection timeout or failure we start in a `Pending` status which +/// signifies that we haven't yet verified that we can connect to the HSM or that it supports the capabilities that we +/// require. +/// +/// At some point later once an initial connection has been established the KMIP signer changes status to either +/// `Usable` or `Unusable` based on what was discovered about the KMIP server. +#[derive(Clone, Debug)] +enum ProbingServerConnector { + /// We haven't yet been able to connect to the HSM using the TCP+TLS+KMIP protocol. If there was already a failed + /// attempt to connect the timestamp of the attempt is remembered so that we can choose to space out connection + /// attempts rather than attempt to connect every time Krill tries to use the signer. + Probing { + // The connection settings are not optional but are stored in an Option so that we can "take" them out when + // moving from the Probing status for use in the Usable status. + conn_settings: Option, + last_probe_time: Option, + }, + + /// The HSM was successfully probed but found to be lacking required capabilities and is thus unusable by Krill. + Unusable, + + /// The HSM was successfully probed and confirmed to have the required capabilities. + /// + /// Note that this does not mean that the HSM is currently contactable, only that we were able to contact it at + /// least once since Krill was started. If the domain name/IP address used to connect to Krill now point to a + /// different HSM instance the previously determined conclusion that the HSM is usable may no longer be valid. + /// + /// In this status we keep state concerning our relationship with the HSM. + Usable(UsableServerState), +} + +impl ProbingServerConnector { + /// Create a new connector to a KMIP server that hasn't been probed yet. + pub fn new(conn_settings: ConnectionSettings) -> Self { + ProbingServerConnector::Probing { + conn_settings: Some(conn_settings), + last_probe_time: None, + } + } + + /// Marks now as the last probe attempt timestamp. + /// + /// Calling this function while not in the Probing state will result in a panic. + pub fn mark(&self) -> Result<(), SignerError> { + match self { + #[rustfmt::skip] + ProbingServerConnector::Probing { mut last_probe_time, .. } => { + last_probe_time.replace(Instant::now()); + Ok(()) + } + _ => Err(SignerError::KmipError( + "Internal error: cannot mark last probe time as probing has already finished.".into(), + )), + } + } + + pub fn conn_settings(&self) -> &ConnectionSettings { + match self { + ProbingServerConnector::Probing { conn_settings, .. } => conn_settings.as_ref().unwrap(), + _ => unreachable!(), + } + } + + pub fn take_conn_settings(&mut self) -> ConnectionSettings { + match self { + ProbingServerConnector::Probing { conn_settings, .. } => conn_settings.take().unwrap(), + _ => unreachable!(), + } + } + + /// Helper function to retrieve the state associated with status Usable. Only called when in status `Usable`. + /// Calling this function while in another state will result in a panic. + pub fn state(&self) -> &UsableServerState { + match self { + ProbingServerConnector::Usable(state) => state, + _ => unreachable!(), + } + } +} + +/// The details needed to interact with a usable KMIP server. +#[derive(Clone, Debug)] +struct UsableServerState { + /// A pool of TCP + TLS clients for connecting to the KMIP server + pool: r2d2::Pool, + + /// TODO: Replace me by a persistent mechanism shared by all signers, not just the KMIP signer. + key_map: Arc>, + + /// Does the KMIP server support the RNG Retrieve operation (for generating random values)? + supports_rng_retrieve: bool, +} + +impl UsableServerState { + pub fn new(pool: r2d2::Pool, supports_rng_retrieve: bool) -> UsableServerState { + let key_map = Arc::new(RwLock::new(KeyMap::new())); + + UsableServerState { + pool, + key_map, + supports_rng_retrieve, + } + } +} + +impl KmipSigner { + /// Get a read lock on the Usable server status, if the server is usable. + /// + /// Returns `Ok` with the status read lock if the KMIP server is usable, otherwise returns an `Err` because the + /// server is unusable or we haven't yet been able to establish if it is usable or not. + /// + /// Will try probing again if we didn't already manage to connect to the server and the delay period between probes + /// has elapsed. + fn server(&self) -> Result, SignerError> { + fn get_server_if_usable( + status: RwLockReadGuard, + ) -> Option, SignerError>> { + // Check the status through the unlocked read lock + match &*status { + ProbingServerConnector::Usable(_) => { + // KMIP server has been confirmed as usable, return the read-lock granting access to the current + // status and via it the current state of our relationship with the KMIP server. + Some(Ok(status)) + } + + ProbingServerConnector::Unusable => { + // KMIP server has been confirmed as unusable, fail. + Some(Err(SignerError::KmipError("KMIP server is unusable".into()))) + } + + ProbingServerConnector::Probing { last_probe_time, .. } => { + // We haven't yet established whether the KMIP server is usable or not. If we haven't yet checked or we + // haven't tried checking again for a while, then try contacting it again. If we can't establish + // whether or not the server is usable, return an error. + if !is_time_to_check(RETRY_INIT_EVERY, *last_probe_time) { + Some(Err(SignerError::KmipError("KMIP server is not yet available".into()))) + } else { + None + } + } + } + } + + // Return the current status or attempt to set it by probing the server + let status = self.server.read().expect("KMIP status lock is poisoned"); + get_server_if_usable(status).unwrap_or_else(|| { + self.probe_server() + .and_then(|_| Ok(self.server.read().expect("KMIP status lock is poisoned"))) + .map_err(|err| SignerError::KmipError(format!("KMIP server is not yet available: {}", err))) + }) + } + + /// Verify if the configured KMIP server is contactable and supports the required capabilities. + fn probe_server(&self) -> Result<(), SignerError> { + // Hold a write lock for the duration of our attempt to verify the KMIP server so that no other attempt occurs + // at the same time. Bail out if another thread is performing a probe and has the lock. This is the same result + // as when attempting to use the KMIP server between probe retries. + let mut status = self + .server + .try_write() + .map_err(|_| SignerError::KmipError("KMIP server is not yet available".into()))?; + + // Update the timestamp of our last attempt to contact the KMIP server. This is used above to know when we have + // waited long enough before attempting to contact the server again. This also guards against attempts to probe + // when probing has already finished as mark() will fail in that case. + status.mark()?; + + let conn_settings = status.conn_settings(); + debug!("Probing server at {}:{}", conn_settings.host, conn_settings.port); + + // Attempt a one-off connection to check if we should abort due to a configuration error (e.g. unusable + // certificate) that will never work, and to determine the capabilities of the server (which may affect our + // behaviour). + let conn = ConnectionManager::connect_one_off(conn_settings).map_err(|err| { + let reason = match err { + // Fatal error + kmip::client::Error::ConfigurationError(err) => { + format!("Failed to connect KMIP server: Configuration error: {}", err) + } + + // Impossible errors: we didn't yet try to send a request or receive a response + kmip::client::Error::SerializeError(err) + | kmip::client::Error::RequestWriteError(err) + | kmip::client::Error::ResponseReadError(err) + | kmip::client::Error::DeserializeError(err) + | kmip::client::Error::InternalError(err) + | kmip::client::Error::Unknown(err) => { + format!("Failed to connect KMIP server: Unexpected error: {}", err) + } + + // I/O error attempting to contact the server or a problem on an internal problem at the server, not + // necessarily fatal or a reason to abort creating the pool. + kmip::client::Error::ServerError(err) => { + format!("Failed to connect KMIP server: Server error: {}", err) + } + }; + + SignerError::KmipError(reason) + })?; + + // We managed to establish a TCP+TLS connection to the KMIP server. Send it a Query request to discover how + // it calls itself and which KMIP operations it supports. + let server_properties = conn.query().map_err(|err| SignerError::KmipError(err.to_string()))?; + let supported_operations = server_properties.operations.unwrap_or_default(); + + // Check whether or not the KMIP operations that we require are supported by the server + let mut unsupported_operations = Vec::new(); + for required_op in &[ + Operation::CreateKeyPair, + Operation::Activate, + Operation::Sign, + Operation::Revoke, + Operation::Destroy, + Operation::Get, + Operation::ModifyAttribute, + ] { + if !supported_operations.contains(required_op) { + unsupported_operations.push(required_op.to_string()); + } + } + + // Warn about and (optionally) fail due to the lack of any unsupported operations. + if !unsupported_operations.is_empty() { + warn!( + "KMIP server lacks support for one or more required operations: {}", + unsupported_operations.join(",") + ); + + // Hard fail due to unsupported operations, unless our configuration tells us to try using this server + // anyway. For example, PyKMIP 0.10.0 does not include the ModifyAttribute operation in the set of + // supported operations even though it does support it. Without this flag we would not be able to use + // PyKMIP with Krill! + if IGNORE_MISSING_CAPABILITIES { + *status = ProbingServerConnector::Unusable; + return Err(SignerError::KmipError("KMIP server is unusable".to_string())); + } + } + + // Switch from probing the server to using it. + // ------------------------------------------- + + let server_identification = server_properties.vendor_identification.unwrap_or("Unknown".into()); + + // Take the ConnectionSettings out of the Probing status so that we can move it to the Usable status. (we + // could clone it but it potentially contains a lot of certificate and key byte data and is about to get + // dropped when we change status which is silly when we still need it, instead take it with us to the new + // status) + let conn_settings = status.take_conn_settings(); + + // Success! We can use this server. Announce it and switch our status to KmipSignerStatus::Usable. + info!( + "Using KMIP server '{}' at {}:{}", + server_identification, conn_settings.host, conn_settings.port + ); + + let supports_rng_retrieve = supported_operations.contains(&Operation::RNGRetrieve); + let pool = ConnectionManager::create_connection_pool(conn_settings, MAX_CONCURRENT_SERVER_CONNECTIONS)?; + let state = UsableServerState::new(pool, supports_rng_retrieve); + + *status = ProbingServerConnector::Usable(state); + Ok(()) + } +} + +//------------ Connection related functions --------------------------------------------------------------------------- + +impl KmipSigner { + /// Get a connection to the KMIP server from the pool, if the server is usable. + fn connect(&self) -> Result, SignerError> { + let conn = self.server()?.state().pool.get()?; + Ok(conn) + } + + /// Perform some operation using a KMIP server pool connection. + /// + /// Fails if the KMIP server is not [KmipSignerStatus::Usable]. If the operation fails due to a transient + /// connection error, retry with backoff upto a defined retry limit. + fn with_conn(&self, desc: &str, do_something_with_conn: F) -> Result + where + F: FnOnce(&KmipTlsClient) -> Result + Copy, + { + // Define the backoff policy to use + let backoff_policy = ExponentialBackoff { + initial_interval: RETRY_REQ_AFTER, + multiplier: RETRY_REQ_AFTER_MULTIPLIER, + max_elapsed_time: Some(RETRY_REQ_UNTIL_MAX), + ..Default::default() + }; + + // Define a notify callback to customize messages written to the logger + let notify = |err, next: Duration| { + warn!("{} failed, retrying in {} seconds: {}", desc, next.as_secs(), err); + }; + + // Define an operation to (re)try + let op = || { + // First get a (possibly already existing) connection from the pool + let conn = self.connect()?; + + // Next, try to execute the callers operation using the connection. If it fails, examine the cause of + // failure to determine if it should be a hard-fail (no more retries) or if we should try again. + Ok((do_something_with_conn)(conn.deref()).map_err(retry_on_connection_error)?) + }; + + // Don't even bother going round the retry loop if we haven't yet successfully connected to the KMIP server + // and verified its capabilities: + let _ = self.server()?; + + // Try (and retry if needed) the requested operation. + Ok(backoff::retry_notify(backoff_policy, op, notify)?) + } +} + +/// The status of a key. +/// +/// KMIP servers require that a key be activated before it can be used for signing and be inactive (revoked) before it +/// can be deleted. +#[derive(Debug, PartialEq)] +pub enum KeyStatus { + /// The key is inactive. + Inactive, + + /// The key was activated. + Active, +} + +// High level helper functions for use by the public Signer interface implementation +impl KmipSigner { + /// Remember that the given KMIP public and private key pair IDs correspond to the given KeyIdentifier. + pub fn remember_kmip_key_ids( + &self, + key_id: &KeyIdentifier, + kmip_key_ids: KmipKeyPairIds, + ) -> Result<(), SignerError> { + let _ = self + .server()? + .state() + .key_map + .write() + .expect("KMIP key map lock is poisoned") + .insert(key_id.clone(), kmip_key_ids); + Ok(()) + } + + /// Given a KeyIdentifier lookup the corresponding KMIP public and private key pair IDs. + pub fn lookup_kmip_key_ids(&self, key_id: &KeyIdentifier) -> Result> { + Ok(self + .server()? + .state() + .key_map + .read() + .expect("KMIP key map lock is poisoned") + .get(key_id) + .ok_or(KeyError::KeyNotFound)? + .clone()) + } + + /// Create a key pair in the KMIP server in the requested format and make it ready for use by Krill. + pub fn build_key(&self, algorithm: PublicKeyFormat) -> Result<(PublicKey, KmipKeyPairIds), SignerError> { + if !matches!(algorithm, PublicKeyFormat::Rsa) { + return Err(SignerError::KmipError(format!( + "Algorithm {:?} not supported while creating key", + &algorithm + ))); + } + + // Give keys a Krill specific but random name initially. Once we have created them we can determine the SHA-1 + // of their X.509 SubjectPublicKeyInfo aka the Krill KeyIdentifier and use that in the name instead of the + // random component. + + // The name given to a key is purely for our own use, the KMIP server doesn't care about it. We give keys a + // name that clearly indicates they relate to Krill as this may be helpful to the KMIP server operator. Once + // the key is created we rename it to include its Krill KeyIdentifier (aka the SHA-1 of the X.509 + // SubjectPublicKeyInfo) so that we can relate the key back to its usage in Krill. We include the Unix seconds + // since 1970-01-01 timestamp in the name initially just as some rough at-a-glance indication of when it was + // created and to differentiate it from other keys with the same name (of which there should be none as they + // key should either be renamed after creation or should have been deleted at some point). + let prefix = format!("krill_new_key_{}", Timestamp::now()); + let private_key_name = format!("{}_priv", prefix); + let public_key_name = format!("{}_pub", prefix); + + // Create the RSA key pair + let kmip_key_pair_ids = self.create_rsa_key_pair(private_key_name, public_key_name)?; + + // Prepare the new keys for use, and attempt to destroy them if anything goes wrong + let public_key = self + .prepare_keypair_for_use(&kmip_key_pair_ids.private_key_id, &kmip_key_pair_ids.public_key_id) + .or_else(|err| { + let _ = self.destroy_key_pair(&kmip_key_pair_ids, KeyStatus::Inactive); + Err(SignerError::KmipError(err.to_string())) + })?; + + Ok((public_key, kmip_key_pair_ids)) + } + + /// Create an RSA key pair in the KMIP server. + fn create_rsa_key_pair( + &self, + private_key_name: String, + public_key_name: String, + ) -> Result { + let (private_key_id, public_key_id) = self + .with_conn("create key pair", |conn| { + conn.create_rsa_key_pair(2048, private_key_name.clone(), public_key_name.clone()) + }) + .map_err(|err| SignerError::KmipError(format!("Failed to create RSA key pair: {}", err)))?; + + let kmip_key_ids = KmipKeyPairIds { + public_key_id, + private_key_id, + }; + + Ok(kmip_key_ids) + } + + /// Make the given KMIP private and public key pair ready for use by Krill. + /// + /// Note that this function renames the created keys but this is not needed for correct functioning of Krill, it + /// is rather done to aid the KMIP server operator when administering the HSM. + /// + /// It also activates the private key. Without this the key cannot be used for signing. An alternate approach could + /// be to set the activation date of the key when creating it thereby avoiding the extra activation step, or to + /// perform the activation operation as part of a bulk request also containing the create key operation, thereby + /// reducing the number of round trips to the server. + fn prepare_keypair_for_use(&self, private_key_id: &str, public_key_id: &str) -> Result { + // Create a public key object for the public key + let public_key = self.get_public_key_from_id(&public_key_id)?; + + // Determine names for the public and private key that allow them to be related back to their usage in Krill + // TODO: Give even more helpful names to the keys such as the name of the CA they were created for? + let hex_key_id = hex::encode(public_key.key_identifier()); + let new_public_key_name = format!("krill-public-key-{}", hex_key_id); + let new_private_key_name = format!("krill-private-key-{}", hex_key_id); + + // Rename the keys to their new names + self.with_conn("rename key", |conn| { + conn.rename_key(public_key_id, new_public_key_name.clone()) + }) + .map_err(|err| { + SignerError::KmipError(format!( + "Failed to set name on new public key '{}': {}", + public_key_id, err + )) + })?; + + self.with_conn("rename key", |conn| { + conn.rename_key(private_key_id, new_private_key_name.clone()) + }) + .map_err(|err| { + SignerError::KmipError(format!( + "Failed to set name on new private key '{}': {}", + private_key_id, err + )) + })?; + + // Activate the private key so that it can be used for signing. Do this last otherwise if there is a problem + // with preparing the key pair for use we have to deactivate the private key before we can destroy it. + self.with_conn("activate key", |conn| conn.activate_key(&private_key_id)) + .map_err(|err| { + SignerError::KmipError(format!( + "Failed to activate new private key '{}': {}", + private_key_id, err + )) + })?; + + Ok(public_key) + } + + /// Get the RSA public bytes for the given KMIP server public key. + fn get_rsa_public_key_bytes(&self, public_key_id: &str) -> Result { + let response_payload = self + .with_conn("get key", |conn| conn.get_key(public_key_id)) + .map_err(|err| { + SignerError::KmipError(format!( + "Failed to get key material for public key with ID '{}': {:?}", + public_key_id, err + )) + })?; + + if response_payload.object_type != ObjectType::PublicKey { + return Err(SignerError::KmipError(format!( + "Failed to get key material: unsupported object type '{:?}' returned by KMIP Get operation for public key with ID '{}'", + response_payload.object_type, public_key_id))); + } + + let key_material = match response_payload.cryptographic_object { + ManagedObject::PublicKey(public_key) => public_key.key_block.key_value.key_material, + _ => { + return Err(SignerError::KmipError(format!( + "Failed to get key material: unsupported cryptographic object type returned by KMIP Get operation for public key with ID '{}'", + public_key_id))); + } + }; + + let rsa_public_key_bytes = match key_material { + KeyMaterial::Bytes(bytes) => bytes::Bytes::from(bytes), + KeyMaterial::TransparentRSAPublicKey(pub_key) => { + util::rsa_public_key_bytes_from_parts(&pub_key.modulus, &pub_key.public_exponent)? + } + KeyMaterial::TransparentRSAPrivateKey(priv_key) => { + if let Some(public_exponent) = priv_key.public_exponent { + util::rsa_public_key_bytes_from_parts(&priv_key.modulus, &public_exponent)? + } else { + return Err(SignerError::KmipError(format!( + "Failed to get key material: missing exponent in transparent RSA private key returned by KMIP Get operation for public key with ID '{}'", + public_key_id))); + } + } + _ => { + return Err(SignerError::KmipError(format!( + "Failed to get key material: unsupported key material type {:?} returned by KMIP Get operation for public key with ID '{}'", + key_material, public_key_id))); + } + }; + + Ok(rsa_public_key_bytes) + } + + pub fn get_public_key_from_id(&self, public_key_id: &str) -> Result { + let rsa_public_key_bytes = self.get_rsa_public_key_bytes(public_key_id)?; + + let subject_public_key = bcder::BitString::new(0, rsa_public_key_bytes); + + let subject_public_key_info = + bcder::encode::sequence((PublicKeyFormat::Rsa.encode(), subject_public_key.encode())); + + let mut subject_public_key_info_source: Vec = Vec::new(); + subject_public_key_info + .write_encoded(bcder::Mode::Der, &mut subject_public_key_info_source) + .map_err(|err| { + SignerError::KmipError(format!( + "Failed to create DER encoded SubjectPublicKeyInfo from constituent parts: {}", + err + )) + })?; + + let public_key = PublicKey::decode(subject_public_key_info_source.as_slice()).map_err(|err| { + SignerError::KmipError(format!( + "Failed to create public key from the DER encoded SubjectPublicKeyInfo: {}", + err + )) + })?; + + Ok(public_key) + } + + pub fn sign_with_key( + &self, + private_key_id: &str, + algorithm: SignatureAlgorithm, + data: &[u8], + ) -> Result { + if algorithm.public_key_format() != PublicKeyFormat::Rsa { + return Err(SignerError::KmipError(format!( + "Algorithm '{:?}' not supported", + algorithm.public_key_format() + ))); + } + + let signed = self + .with_conn("sign", |conn| conn.sign(&private_key_id, data)) + .map_err(|err| SignerError::KmipError(format!("Signing failed: {}", err)))?; + + let sig = Signature::new(SignatureAlgorithm::default(), Bytes::from(signed.signature_data)); + + Ok(sig) + } + + pub fn destroy_key_pair(&self, kmip_key_pair_ids: &KmipKeyPairIds, mode: KeyStatus) -> Result { + let mut success = true; + + if let Err(err) = self.with_conn("destroy key", |conn| conn.destroy_key(&kmip_key_pair_ids.public_key_id)) { + success = false; + warn!( + "Failed to destroy KMIP public key '{}': {}", + &kmip_key_pair_ids.public_key_id, err + ); + } + + let mut deactivated = true; + if mode == KeyStatus::Active { + // TODO: it's unclear from the KMIP 1.2 specification if this can fail because the key is already revoked. + // If that is a possible failure scenario we should not abort here but instead continue to delete the key. + if let Err(err) = self.with_conn("revoke key", |conn| conn.revoke_key(&kmip_key_pair_ids.private_key_id)) { + success = false; + deactivated = false; + warn!( + "Failed to revoke KMIP private key '{}': {}", + &kmip_key_pair_ids.private_key_id, err + ); + } + } + + if deactivated { + // TODO: This can fail if the key is not in the correct state, e.g. one cause can is if the key is not + // revoked. We don't expect this because we assume we know whether we activated or revoked the key or not + // but if for some reason the key exists, we think it does not require revocation but actually it does, + // then we would fail here. In such a case we could attempt to revoke and retry, but that assumes we can + // detect that specific failure scenario. + if let Err(err) = self.with_conn("destroy key", |conn| { + conn.destroy_key(&kmip_key_pair_ids.private_key_id) + }) { + success = false; + warn!( + "Failed to destroy KMIP private key '{}': {}", + &kmip_key_pair_ids.private_key_id, err + ); + } + } + + Ok(success) + } + + pub fn get_random_bytes(&self, num_bytes_wanted: usize) -> Result, SignerError> { + if !self.supports_random() { + return Err(SignerError::KmipError( + "The KMIP server does not support random number generation".to_string(), + )); + } + let res: RNGRetrieveResponsePayload = self + .with_conn("rng retrieve", |conn: &KmipTlsClient| { + conn.rng_retrieve(num_bytes_wanted as i32) + }) + .map_err(|err| SignerError::KmipError(format!("Failed to retrieve random bytes: {:?}", err)))?; + + Ok(res.data) + } +} + +fn is_time_to_check(time_between_checks: Duration, possible_lack_check_time: Option) -> bool { + match possible_lack_check_time { + None => true, + Some(instant) => Instant::now().saturating_duration_since(instant) > time_between_checks, + } +} + +// -------------------------------------------------------------------------------------------------------------------- +// Retry with backoff related helper impls/fns: +// -------------------------------------------------------------------------------------------------------------------- + +impl From for SignerError { + fn from(err: kmip::client::Error) -> Self { + SignerError::KmipError(format!("Client error: {}", err)) + } +} + +impl From> for SignerError { + fn from(err: backoff::Error) -> Self { + match err { + backoff::Error::Permanent(err) => err, + backoff::Error::Transient(err) => err, + } + } +} + +fn retry_on_connection_error(err: kmip::client::Error) -> backoff::Error +where + E: From, +{ + if err.is_connection_error() { + backoff::Error::Transient(err.into()) + } else { + backoff::Error::Permanent(err.into()) + } +} diff --git a/src/commons/crypto/signers/kmip/keymap.rs b/src/commons/crypto/signers/kmip/keymap.rs new file mode 100644 index 000000000..abf58554e --- /dev/null +++ b/src/commons/crypto/signers/kmip/keymap.rs @@ -0,0 +1,13 @@ +use std::collections::HashMap; + +use rpki::repository::crypto::KeyIdentifier; + +/// KMIP key identifiers needed to use keys stored in the HSM. +#[derive(Clone, Debug)] +pub struct KmipKeyPairIds { + pub public_key_id: String, + pub private_key_id: String, +} + +/// An in-memory mapping of Krill [KeyIdentifier] to KMIP server internal key pair identifiers. +pub type KeyMap = HashMap; diff --git a/src/commons/crypto/signers/kmip/mod.rs b/src/commons/crypto/signers/kmip/mod.rs new file mode 100644 index 000000000..754f952dc --- /dev/null +++ b/src/commons/crypto/signers/kmip/mod.rs @@ -0,0 +1,21 @@ +//! Support for signing things using an external KMIP compliant cryptographic token. +//! +//! Currently only intended for sanity checking the use of KMIP with Krill by running as the only signer in place of +//! the usual [OpenSslSigner]. Assumes that the KMIP server is a [PyKMIP] instance that is created for and destroyed +//! after the Krill tests have run. Uses hard-coded connection details and in-memory storage of key identifiers issued +//! by the KMIP server. +//! +//! The current implementation splits the KMIP signer into four Rust modules: +//! - `connpool`: Connection pooling related functionality. +//! - `internal`: KMIP server interaction, including probing and retry/backoff logic. +//! - `keymap`: In-memory mapping of `KeyIdentifier` to KMIP key identifiers. +//! - `signer`: The public signer trait implementation. Delegates to `internal`. +//! +//! [OpenSslSigner]: crate::commons::util::softsigner::OpenSslSigner +//! [PyKMIP]: https://github.com/OpenKMIP/PyKMIP +pub mod connpool; +pub mod internal; +pub mod keymap; +pub mod signer; + +pub use internal::KmipSigner; diff --git a/src/commons/crypto/signers/kmip/signer.rs b/src/commons/crypto/signers/kmip/signer.rs new file mode 100644 index 000000000..4f3570280 --- /dev/null +++ b/src/commons/crypto/signers/kmip/signer.rs @@ -0,0 +1,84 @@ +use rpki::repository::crypto::{ + signer::KeyError, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, SigningError, +}; + +use crate::commons::crypto::signers::{ + error::SignerError, + kmip::{internal::KeyStatus, KmipSigner}, +}; + +// Implement the functions defined by the `Signer` trait because `SignerProvider` expects to invoke them, but as the +// dispatching is not trait based we don't actually have to implement the `Signer` trait. +impl KmipSigner { + pub fn create_key(&self, algorithm: PublicKeyFormat) -> Result { + let (key, kmip_key_pair_ids) = self.build_key(algorithm)?; + let key_id = key.key_identifier(); + self.remember_kmip_key_ids(&key_id, kmip_key_pair_ids)?; + Ok(key_id) + } + + pub fn get_key_info(&self, key_id: &KeyIdentifier) -> Result> { + let kmip_key_pair_ids = self.lookup_kmip_key_ids(key_id)?; + self.get_public_key_from_id(&kmip_key_pair_ids.public_key_id) + .map_err(|err| KeyError::Signer(err)) + } + + pub fn destroy_key(&self, key_id: &KeyIdentifier) -> Result<(), KeyError> { + let kmip_key_pair_ids = self.lookup_kmip_key_ids(key_id)?; + match self.destroy_key_pair(&kmip_key_pair_ids, KeyStatus::Active)? { + true => Ok(()), + false => Err( + SignerError::KmipError( + format!("Failed to completely destroy KMIP key pair for Krill KeyIdentifier '{:?}', KMIP public key '{}' and KMIP private key '{}'", + key_id, kmip_key_pair_ids.public_key_id, kmip_key_pair_ids.private_key_id)))?, + } + } + + pub fn sign + ?Sized>( + &self, + key_id: &KeyIdentifier, + algorithm: SignatureAlgorithm, + data: &D, + ) -> Result> { + let kmip_key_pair_ids = self.lookup_kmip_key_ids(key_id)?; + + let signature = self + .sign_with_key(&kmip_key_pair_ids.private_key_id, algorithm, data.as_ref()) + .map_err(|err| { + SigningError::Signer(SignerError::KmipError(format!( + "Signing data failed for Krill KeyIdentifier '{}' and KMIP private key id '{}': {}", + key_id, kmip_key_pair_ids.private_key_id, err + ))) + })?; + + Ok(signature) + } + + pub fn sign_one_off + ?Sized>( + &self, + algorithm: SignatureAlgorithm, + data: &D, + ) -> Result<(Signature, PublicKey), SignerError> { + // TODO: Is it possible to use a KMIP batch request to implement the create, activate, sign, deactivate, delete + // in one round-trip to the server? + let (key, kmip_key_pair_ids) = self.build_key(PublicKeyFormat::Rsa)?; + + let signature_res = self + .sign_with_key(&kmip_key_pair_ids.private_key_id, algorithm, data.as_ref()) + .map_err(|err| SignerError::KmipError(format!("One-off signing of data failed: {}", err))); + + let _ = self.destroy_key_pair(&kmip_key_pair_ids, KeyStatus::Active); + + let signature = signature_res?; + + Ok((signature, key)) + } + + pub fn rand(&self, data: &mut [u8]) -> Result<(), SignerError> { + let random_bytes = self.get_random_bytes(data.len())?; + + data.copy_from_slice(&random_bytes); + + Ok(()) + } +} diff --git a/src/commons/crypto/signers/mod.rs b/src/commons/crypto/signers/mod.rs new file mode 100644 index 000000000..29ba11639 --- /dev/null +++ b/src/commons/crypto/signers/mod.rs @@ -0,0 +1,7 @@ +pub mod error; + +#[cfg(feature = "hsm")] +pub mod kmip; + +pub mod softsigner; +pub mod util; diff --git a/src/commons/util/softsigner.rs b/src/commons/crypto/signers/softsigner.rs similarity index 77% rename from src/commons/util/softsigner.rs rename to src/commons/crypto/signers/softsigner.rs index 6f7ce0501..64c505207 100644 --- a/src/commons/util/softsigner.rs +++ b/src/commons/crypto/signers/softsigner.rs @@ -1,7 +1,7 @@ //! Support for signing things using software keys (through openssl) and //! storing them unencrypted on disk. use std::{ - fmt, fs, + fs, fs::File, io::Write, path::{Path, PathBuf}, @@ -12,17 +12,16 @@ use bytes::Bytes; use serde::{de, ser, Deserialize, Deserializer, Serialize, Serializer}; use openssl::{ - error::ErrorStack, hash::MessageDigest, pkey::{PKey, PKeyRef, Private}, rsa::Rsa, }; use rpki::repository::crypto::{ - signer::KeyError, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, Signer, SigningError, + signer::KeyError, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, SigningError, }; -use crate::commons::error::KrillIoError; +use crate::commons::{crypto::signers::error::SignerError, error::KrillIoError}; //------------ OpenSslSigner ------------------------------------------------- @@ -62,6 +61,10 @@ impl OpenSslSigner { Err(SignerError::InvalidWorkDir(work_dir.to_path_buf())) } } + + pub fn supports_random(&self) -> bool { + true + } } impl OpenSslSigner { @@ -93,11 +96,10 @@ impl OpenSslSigner { } } -impl Signer for OpenSslSigner { - type KeyId = KeyIdentifier; - type Error = SignerError; - - fn create_key(&self, _algorithm: PublicKeyFormat) -> Result { +// Implement the functions defined by the `Signer` trait because `SignerProvider` expects to invoke them, but as the +// dispatching is not trait based we don't actually have to implement the `Signer` trait. +impl OpenSslSigner { + pub fn create_key(&self, _algorithm: PublicKeyFormat) -> Result { let kp = OpenSslKeyPair::build()?; let pk = &kp.subject_public_key_info()?; @@ -114,12 +116,12 @@ impl Signer for OpenSslSigner { Ok(key_id) } - fn get_key_info(&self, key_id: &Self::KeyId) -> Result> { + pub fn get_key_info(&self, key_id: &KeyIdentifier) -> Result> { let key_pair = self.load_key(key_id)?; Ok(key_pair.subject_public_key_info()?) } - fn destroy_key(&self, key_id: &Self::KeyId) -> Result<(), KeyError> { + pub fn destroy_key(&self, key_id: &KeyIdentifier) -> Result<(), KeyError> { let path = self.key_path(key_id); if path.exists() { fs::remove_file(&path).map_err(|e| { @@ -132,17 +134,17 @@ impl Signer for OpenSslSigner { Ok(()) } - fn sign + ?Sized>( + pub fn sign + ?Sized>( &self, - key_id: &Self::KeyId, + key_id: &KeyIdentifier, _algorithm: SignatureAlgorithm, data: &D, - ) -> Result> { + ) -> Result> { let key_pair = self.load_key(key_id)?; Self::sign_with_key(key_pair.pkey.as_ref(), data).map_err(SigningError::Signer) } - fn sign_one_off + ?Sized>( + pub fn sign_one_off + ?Sized>( &self, _algorithm: SignatureAlgorithm, data: &D, @@ -156,7 +158,7 @@ impl Signer for OpenSslSigner { Ok((signature, key)) } - fn rand(&self, target: &mut [u8]) -> Result<(), SignerError> { + pub fn rand(&self, target: &mut [u8]) -> Result<(), SignerError> { openssl::rand::rand_bytes(target).map_err(SignerError::OpenSslError) } } @@ -214,49 +216,6 @@ impl OpenSslKeyPair { } } -//------------ OpenSslKeyError ----------------------------------------------- - -#[derive(Debug)] -pub enum SignerError { - OpenSslError(ErrorStack), - JsonError(serde_json::Error), - InvalidWorkDir(PathBuf), - IoError(KrillIoError), - KeyNotFound, - DecodeError, -} - -impl fmt::Display for SignerError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - SignerError::OpenSslError(e) => write!(f, "OpenSsl Error: {}", e), - SignerError::JsonError(e) => write!(f, "Could not decode public key info: {}", e), - SignerError::InvalidWorkDir(path) => write!(f, "Invalid base path: {}", path.to_string_lossy()), - SignerError::IoError(e) => e.fmt(f), - SignerError::KeyNotFound => write!(f, "Could not find key"), - SignerError::DecodeError => write!(f, "Could not decode key"), - } - } -} - -impl From for SignerError { - fn from(e: ErrorStack) -> Self { - SignerError::OpenSslError(e) - } -} - -impl From for SignerError { - fn from(e: serde_json::Error) -> Self { - SignerError::JsonError(e) - } -} - -impl From for SignerError { - fn from(e: KrillIoError) -> Self { - SignerError::IoError(e) - } -} - //------------ Tests --------------------------------------------------------- #[cfg(test)] diff --git a/src/commons/crypto/signers/util.rs b/src/commons/crypto/signers/util.rs new file mode 100644 index 000000000..56534cdea --- /dev/null +++ b/src/commons/crypto/signers/util.rs @@ -0,0 +1,17 @@ +use bcder::encode::{PrimitiveContent, Values}; + +use crate::commons::crypto::signers::error::SignerError; + +/// Helper function to create X.509 RSA Public Key bytes from a given RSA modulus and exponent. +pub fn rsa_public_key_bytes_from_parts(modulus: &[u8], public_exponent: &[u8]) -> Result { + let modulus = bcder::Unsigned::from_slice(modulus).map_err(|_| SignerError::DecodeError)?; + let public_exp = bcder::Unsigned::from_slice(public_exponent).map_err(|_| SignerError::DecodeError)?; + let rsa_public_key = bcder::encode::sequence((modulus.encode(), public_exp.encode())); + + let mut bytes: Vec = Vec::new(); + rsa_public_key + .write_encoded(bcder::Mode::Der, &mut bytes) + .map_err(|_| SignerError::DecodeError)?; + + Ok(bytes::Bytes::from(bytes)) +} diff --git a/src/commons/crypto/signing.rs b/src/commons/crypto/signing.rs index 0a2f4c43b..0a3ad020d 100644 --- a/src/commons/crypto/signing.rs +++ b/src/commons/crypto/signing.rs @@ -1,12 +1,10 @@ //! Support for signing mft, crl, certificates, roas.. //! Common objects for TAs and CAs use std::{ - ops::Deref, sync::{Arc, RwLock}, {convert::TryFrom, path::Path}, }; -use bcder::Captured; use bytes::Bytes; use rpki::{ @@ -14,7 +12,10 @@ use rpki::{ aspa::{Aspa, AspaBuilder}, cert::{Cert, KeyUsage, Overclaim, TbsCert}, crl::{Crl, CrlEntry, TbsCertList}, - crypto::{DigestAlgorithm, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, Signer}, + crypto::{ + signer::KeyError, DigestAlgorithm, KeyIdentifier, PublicKey, PublicKeyFormat, Signature, + SignatureAlgorithm, Signer, SigningError, + }, csr::Csr, manifest::{FileAndHash, Manifest, ManifestContent}, roa::{Roa, RoaBuilder}, @@ -25,274 +26,307 @@ use rpki::{ uri, }; -#[cfg(feature = "hsm")] -use crate::commons::util::dummysigner::DummySigner; - use crate::{ commons::{ api::{IssuedCert, RcvdCert, ReplacedObject, RepoInfo, RequestResourceLimit, ResourceSet}, - crypto::{self, CryptoResult}, + crypto::{ + self, + signers::{error::SignerError, softsigner::OpenSslSigner}, + CryptoResult, + }, error::Error, - util::{softsigner::OpenSslSigner, AllowedUri}, + util::AllowedUri, KrillResult, }, daemon::ca::CertifiedKey, }; -//------------ Signer -------------------------------------------------------- +#[cfg(feature = "hsm")] +use crate::commons::crypto::signers::kmip::KmipSigner; + +//------------ SignerProvider ------------------------------------------------ +/// Dispatchers Signer requests to a particular implementation of the Signer trait. +/// +/// Named and modelled after the similar AuthProvider concept that already exists in Krill. +#[allow(dead_code)] // Needed as we currently only ever construct one variant #[derive(Clone, Debug)] enum SignerProvider { OpenSsl(OpenSslSigner), #[cfg(feature = "hsm")] - #[allow(dead_code)] - Dummy(DummySigner), + Kmip(KmipSigner), } impl SignerProvider { - pub fn create_key(&mut self) -> CryptoResult { + pub fn supports_random(&self) -> bool { match self { - SignerProvider::OpenSsl(signer) => signer.create_key(PublicKeyFormat::Rsa), + SignerProvider::OpenSsl(signer) => signer.supports_random(), #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => signer.create_key(PublicKeyFormat::Rsa), + SignerProvider::Kmip(signer) => signer.supports_random(), } - .map_err(crypto::Error::signer) } +} - pub fn destroy_key(&mut self, key_id: &KeyIdentifier) -> CryptoResult<()> { +// Implement the functions defined by the `Signer` trait because `SignerRouter` expects to invoke them, but as the +// dispatching is not trait based we don't actually have to implement the `Signer` trait. +impl SignerProvider { + fn create_key(&self, algorithm: PublicKeyFormat) -> Result { match self { - SignerProvider::OpenSsl(signer) => signer.destroy_key(key_id), + SignerProvider::OpenSsl(signer) => signer.create_key(algorithm), #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => signer.destroy_key(key_id), + SignerProvider::Kmip(signer) => signer.create_key(algorithm), } - .map_err(crypto::Error::key_error) } - pub fn get_key_info(&self, key_id: &KeyIdentifier) -> CryptoResult { + fn get_key_info(&self, key: &KeyIdentifier) -> Result> { match self { - SignerProvider::OpenSsl(signer) => signer.get_key_info(key_id), + SignerProvider::OpenSsl(signer) => signer.get_key_info(key), #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => signer.get_key_info(key_id), + SignerProvider::Kmip(signer) => signer.get_key_info(key), } - .map_err(crypto::Error::key_error) } - pub fn random_serial(&self) -> CryptoResult { + fn destroy_key(&self, key: &KeyIdentifier) -> Result<(), KeyError> { match self { - SignerProvider::OpenSsl(signer) => Serial::random(signer.deref()), + SignerProvider::OpenSsl(signer) => signer.destroy_key(key), #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => Serial::random(signer.deref()), + SignerProvider::Kmip(signer) => signer.destroy_key(key), } - .map_err(crypto::Error::signer) } - pub fn sign + ?Sized>( + fn sign + ?Sized>( &self, - key_id: &KeyIdentifier, - sig_alg: SignatureAlgorithm, + key: &KeyIdentifier, + algorithm: SignatureAlgorithm, data: &D, - ) -> CryptoResult { + ) -> Result> { match self { - SignerProvider::OpenSsl(signer) => signer.sign(key_id, sig_alg, data), + SignerProvider::OpenSsl(signer) => signer.sign(key, algorithm, data), #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => signer.sign(key_id, sig_alg, data), + SignerProvider::Kmip(signer) => signer.sign(key, algorithm, data), } - .map_err(crypto::Error::signing) } - pub fn sign_one_off + ?Sized>( + fn sign_one_off + ?Sized>( &self, - sig_alg: SignatureAlgorithm, + algorithm: SignatureAlgorithm, data: &D, - ) -> CryptoResult<(Signature, PublicKey)> { + ) -> Result<(Signature, PublicKey), SignerError> { match self { - SignerProvider::OpenSsl(signer) => signer.sign_one_off(sig_alg, data), + SignerProvider::OpenSsl(signer) => signer.sign_one_off(algorithm, data), #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => signer.sign_one_off(sig_alg, data), + SignerProvider::Kmip(signer) => signer.sign_one_off(algorithm, data), } - .map_err(crypto::Error::signer) } - pub fn sign_csr(&self, base_repo: &RepoInfo, name_space: &str, key: &KeyIdentifier) -> CryptoResult { - fn func(signer: &T, base_repo: &RepoInfo, name_space: &str, key: &KeyIdentifier) -> CryptoResult - where - T: Signer, + fn rand(&self, target: &mut [u8]) -> Result<(), SignerError> { + match self { + SignerProvider::OpenSsl(signer) => signer.rand(target), + #[cfg(feature = "hsm")] + SignerProvider::Kmip(signer) => signer.rand(target), + } + } +} + +//------------ SignerRouter -------------------------------------------------- + +/// Manages multiple Signers and decides which Signer should handle which request. +#[derive(Clone, Debug)] +struct SignerRouter { + // The general signer is used for all signing operations except one-off signing. + general_signer: Arc>, + + // As the security of a HSM isn't needed for one-off keys, and HSMs are slow, by default this should be an instance + // of OpenSslSigner. However, if users think the perceived extra security is warranted let them use a different + // Signer for one-off keys if that's what they want. + one_off_signer: Arc>, + + // The signer to use when a configured signer doesn't support generation of random numbers. + rand_fallback_signer: Arc>, +} + +impl SignerRouter { + pub fn build(work_dir: &Path) -> KrillResult { + // The types of signer to initialize, the details needed to initialize them and the intended purpose for each + // signer (e.g. signer for past keys, currently used signer, signer to use for a key roll, etc.) should come + // from the configuration file. SignerRouter combines that input with its own rules, e.g. to dispatch a signing + // request to the correct signer we will need to determine which signer possesses the signing key, and the + // signer to use to create a new key depends on whether the key is one-off or not and whether or not it is + // being created for a key roll. For now the capability for different signers for different purposes exists but + // is not yet used. + + // TODO: Once it becomes possible to configure how an HSM is used by Krill we need to decide what the + // defaults should be and what should be configurable or not concerning HSM usage, and to document why, if + // permitted, it is acceptable to use local keys, signing & random number genration instead of the more + // secure HSM based alternatives (if available). + + // We always need an OpenSSL signer, either for keys created by a previous instance of Krill, or as a fallback + // random number generator for HSMs that don't support random number generation, or for creating one-off short + // lived signing keys. + let openssl_signer = Arc::new(RwLock::new(SignerProvider::OpenSsl(OpenSslSigner::build(work_dir)?))); + + #[cfg(not(feature = "hsm"))] { - let pub_key = signer.get_key_info(key).map_err(crypto::Error::key_error)?; - Csr::construct( - signer.deref(), - key, - &base_repo.ca_repository(name_space).join(&[]).unwrap(), // force trailing slash - &base_repo.rpki_manifest(name_space, &pub_key.key_identifier()), - Some(&base_repo.rpki_notify()), - ) - .map_err(crypto::Error::signing) + // For backward compatibility with currently released Krill, use OpenSSL for everything. + Ok(SignerRouter { + general_signer: openssl_signer.clone(), + one_off_signer: openssl_signer.clone(), + rand_fallback_signer: openssl_signer, + }) } - let enc = match self { - SignerProvider::OpenSsl(signer) => func(signer.deref(), base_repo, name_space, key), - #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => func(signer.deref(), base_repo, name_space, key), - }?; + #[cfg(all(feature = "hsm", not(feature = "hsm-tests")))] + { + // Currently the behaviour is the same with the hsm-tests feature enabled or disabled. This is because with + // it disabled the Krill 'functional' test fails with error 'Krill failed to start: Signing issue: Could + // not find key'. Implementing the mapping of keys to signers is beyond the scope of the current task and + // so this has for now been commented out. + compile_error!("The 'hsm' feature can only be used in combination with the 'hsm-tests' feature at present"); + unreachable!(); + // // When the HSM feature is activated but we are not in test mode: + // // - Use the HSM for key creation, signing, deletion, except for one-off keys. + // // - Use the HSM for random number generation, if supported, else use the OpenSSL signer. + // // - Use the OpenSSL signer for one-off keys. + // let kmip_signer = Arc::new(RwLock::new(SignerProvider::Kmip(KmipSigner::build()?))); + + // Ok(SignerRouter { + // general_signer: kmip_signer.clone(), + // one_off_signer: openssl_signer.clone(), + // rand_fallback_signer: openssl_signer, + // }) + } - Ok(Csr::decode(enc.as_slice())?) + #[cfg(all(feature = "hsm", feature = "hsm-tests"))] + { + // When the HSM feature is activated AND test mode is activated: + // - Use the HSM for as much as possible to depend on it as broadly as possible in the Krill test suite.. + // - Fallback to OpenSSL for random number generation if the HSM doesn't support it. + let kmip_signer = Arc::new(RwLock::new(SignerProvider::Kmip(KmipSigner::build()?))); + + Ok(SignerRouter { + general_signer: kmip_signer.clone(), + one_off_signer: kmip_signer.clone(), + rand_fallback_signer: openssl_signer, + }) + } } +} - pub fn sign_cert(&self, tbs: TbsCert, key_id: &KeyIdentifier) -> CryptoResult { - match self { - SignerProvider::OpenSsl(signer) => tbs.into_cert(signer.deref(), key_id), - #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => tbs.into_cert(signer.deref(), key_id), - } - .map_err(crypto::Error::signing) +impl Signer for SignerRouter { + type KeyId = KeyIdentifier; + type Error = SignerError; + + fn create_key(&self, algorithm: PublicKeyFormat) -> Result { + self.general_signer.write().unwrap().create_key(algorithm) } - pub fn sign_crl(&self, tbs: TbsCertList>, key_id: &KeyIdentifier) -> CryptoResult { - match self { - SignerProvider::OpenSsl(signer) => tbs.into_crl(signer.deref(), key_id), - #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => tbs.into_crl(signer.deref(), key_id), - } - .map_err(crypto::Error::signing) + fn get_key_info(&self, key_id: &KeyIdentifier) -> Result> { + self.general_signer.read().unwrap().get_key_info(key_id) } - pub fn sign_manifest( - &self, - content: ManifestContent, - builder: SignedObjectBuilder, - key_id: &KeyIdentifier, - ) -> CryptoResult { - match self { - SignerProvider::OpenSsl(signer) => content.into_manifest(builder, signer.deref(), key_id), - #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => content.into_manifest(builder, signer.deref(), key_id), - } - .map_err(crypto::Error::signing) + fn destroy_key(&self, key_id: &KeyIdentifier) -> Result<(), KeyError> { + self.general_signer.write().unwrap().destroy_key(key_id) } - pub fn sign_roa( + fn sign + ?Sized>( &self, - roa_builder: RoaBuilder, - object_builder: SignedObjectBuilder, key_id: &KeyIdentifier, - ) -> CryptoResult { - match self { - SignerProvider::OpenSsl(signer) => roa_builder.finalize(object_builder, signer.deref(), key_id), - #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => roa_builder.finalize(object_builder, signer.deref(), key_id), - } - .map_err(crypto::Error::signing) + algorithm: SignatureAlgorithm, + data: &D, + ) -> Result> { + self.general_signer.read().unwrap().sign(key_id, algorithm, data) } - pub fn sign_aspa( + fn sign_one_off + ?Sized>( &self, - aspa_builder: AspaBuilder, - object_builder: SignedObjectBuilder, - key_id: &KeyIdentifier, - ) -> CryptoResult { - match self { - SignerProvider::OpenSsl(signer) => aspa_builder.finalize(object_builder, signer.deref(), key_id), - #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => aspa_builder.finalize(object_builder, signer.deref(), key_id), - } - .map_err(crypto::Error::signing) + algorithm: SignatureAlgorithm, + data: &D, + ) -> Result<(Signature, PublicKey), SignerError> { + self.one_off_signer.read().unwrap().sign_one_off(algorithm, data) } - pub fn sign_rta(&self, rta_builder: &mut rta::RtaBuilder, ee: Cert) -> CryptoResult<()> { - let key = ee.subject_key_identifier(); - rta_builder.push_cert(ee); - - match self { - SignerProvider::OpenSsl(signer) => rta_builder.sign(signer.deref(), &key, None, None), - #[cfg(feature = "hsm")] - SignerProvider::Dummy(signer) => rta_builder.sign(signer.deref(), &key, None, None), + fn rand(&self, target: &mut [u8]) -> Result<(), SignerError> { + let signer = self.general_signer.read().unwrap(); + if signer.supports_random() { + signer.rand(target) + } else { + self.rand_fallback_signer.read().unwrap().rand(target) } - .map_err(crypto::Error::signing) } } +//------------ KrillSigner --------------------------------------------------- + +/// High level signing interface between Krill and the Signer backends. +/// +/// KrillSigner: +/// - Is configured via the Krill configuration file. +/// - Maps Result to KrillResult. +/// - Directs signers to use the RPKI standard key format (RSA). +/// - Directs signers to use the RPKI standard signature algorithm (RSA PKCS #1 v1.5 with SHA-256). +/// - Offers a higher level interface than the Signer trait. #[derive(Clone, Debug)] pub struct KrillSigner { - // KrillSigner chooses which signer to use when. The noise of handling the enum based dispatch is handled by the - // SignerProvider type defined above, patterned after the existing AuthProvider enum based approach. - // - // Use Arc references so that we can use refer to the same signer instance more than once if that signer should be - // used for multiple purposes, e.g. as both general_signer and one_off_signer in this case. - // - // Use an RwLock because the Signer trait from the rpki-rs crate uses &mut self for create_key() and destroy_key() - // operations. In future we might move the responsibility for locking into the signer so that it can lock only what - // actually needs to be locked raet - - // The general signer is used for all signing operations except one off signing. - general_signer: Arc>, - - // As the security of a HSM isn't needed for one off keys, and HSMs are slow, by default this should be an instance - // of OpenSslSigner. However, if users think the perceived extra security is warranted let them use a different - // Signer for one off keys if that's what they want. - one_off_signer: Arc>, + router: SignerRouter, } impl KrillSigner { pub fn build(work_dir: &Path) -> KrillResult { - // The types of signer to initialize, the details needed to initialize them and the intended purpose for each - // signer (e.g. signer for past keys, currently used signer, signer to use for a key roll, etc.) should come - // from the configuration file. KrillSigner should combine that input its own rules, e.g. to dispatch a signing - // request to the correct signer we will need to determine which signer possesses the signing key, and the - // signer to use to create a new key depends on whether the key is one-off or not and whether or not it is - // being created for a key roll. For now the capability for different signers for different purposes exists but - // is not yet used. - - let openssl_signer = OpenSslSigner::build(work_dir)?; - let openssl_signer = Arc::new(RwLock::new(SignerProvider::OpenSsl(openssl_signer))); - let general_signer = openssl_signer.clone(); - let one_off_signer = openssl_signer; Ok(KrillSigner { - general_signer, - one_off_signer, + router: SignerRouter::build(work_dir)?, }) } pub fn create_key(&self) -> CryptoResult { - self.general_signer.write().unwrap().create_key() + self.router + .create_key(PublicKeyFormat::Rsa) + .map_err(crypto::Error::signer) } pub fn destroy_key(&self, key_id: &KeyIdentifier) -> CryptoResult<()> { - self.general_signer.write().unwrap().destroy_key(key_id) + self.router.destroy_key(key_id).map_err(crypto::Error::key_error) } pub fn get_key_info(&self, key_id: &KeyIdentifier) -> CryptoResult { - self.general_signer.read().unwrap().get_key_info(key_id) + self.router.get_key_info(key_id).map_err(crypto::Error::key_error) } pub fn random_serial(&self) -> CryptoResult { - self.general_signer.read().unwrap().random_serial() + Serial::random(&self.router).map_err(crypto::Error::signer) } pub fn sign + ?Sized>(&self, key_id: &KeyIdentifier, data: &D) -> CryptoResult { - self.general_signer - .read() - .unwrap() + self.router .sign(key_id, SignatureAlgorithm::default(), data) + .map_err(crypto::Error::signing) } pub fn sign_one_off + ?Sized>(&self, data: &D) -> CryptoResult<(Signature, PublicKey)> { - self.one_off_signer - .read() - .unwrap() + self.router .sign_one_off(SignatureAlgorithm::default(), data) + .map_err(crypto::Error::signer) } pub fn sign_csr(&self, base_repo: &RepoInfo, name_space: &str, key: &KeyIdentifier) -> CryptoResult { - self.general_signer.read().unwrap().sign_csr(base_repo, name_space, key) + let pub_key = self.router.get_key_info(key).map_err(crypto::Error::key_error)?; + let enc = Csr::construct( + &self.router, + key, + &base_repo.ca_repository(name_space).join(&[]).unwrap(), // force trailing slash + &base_repo.rpki_manifest(name_space, &pub_key.key_identifier()), + Some(&base_repo.rpki_notify()), + ) + .map_err(crypto::Error::signing)?; + Ok(Csr::decode(enc.as_slice())?) } pub fn sign_cert(&self, tbs: TbsCert, key_id: &KeyIdentifier) -> CryptoResult { - self.general_signer.read().unwrap().sign_cert(tbs, key_id) + tbs.into_cert(&self.router, key_id).map_err(crypto::Error::signing) } pub fn sign_crl(&self, tbs: TbsCertList>, key_id: &KeyIdentifier) -> CryptoResult { - self.general_signer.read().unwrap().sign_crl(tbs, key_id) + tbs.into_crl(&self.router, key_id).map_err(crypto::Error::signing) } pub fn sign_manifest( @@ -301,10 +335,9 @@ impl KrillSigner { builder: SignedObjectBuilder, key_id: &KeyIdentifier, ) -> CryptoResult { - self.general_signer - .read() - .unwrap() - .sign_manifest(content, builder, key_id) + content + .into_manifest(builder, &self.router, key_id) + .map_err(crypto::Error::signing) } pub fn sign_roa( @@ -313,10 +346,9 @@ impl KrillSigner { object_builder: SignedObjectBuilder, key_id: &KeyIdentifier, ) -> CryptoResult { - self.general_signer - .read() - .unwrap() - .sign_roa(roa_builder, object_builder, key_id) + roa_builder + .finalize(object_builder, &self.router, key_id) + .map_err(crypto::Error::signing) } pub fn sign_aspa( @@ -325,22 +357,20 @@ impl KrillSigner { object_builder: SignedObjectBuilder, key_id: &KeyIdentifier, ) -> CryptoResult { - self.general_signer - .read() - .unwrap() - .sign_aspa(aspa_builder, object_builder, key_id) + aspa_builder + .finalize(object_builder, &self.router, key_id) + .map_err(crypto::Error::signing) } pub fn sign_rta(&self, rta_builder: &mut rta::RtaBuilder, ee: Cert) -> CryptoResult<()> { - self.general_signer.read().unwrap().sign_rta(rta_builder, ee) + let key = ee.subject_key_identifier(); + rta_builder.push_cert(ee); + rta_builder + .sign(&self.router, &key, None, None) + .map_err(crypto::Error::signing) } } -// //------------ Signer -------------------------------------------------------- -// -// pub trait Signer: crypto::Signer + Clone + Sized + Sync + Send + 'static {} -// impl + Clone + Sized + Sync + Send + 'static> Signer for T {} - //------------ CsrInfo ------------------------------------------------------- pub type CaRepository = uri::Rsync; diff --git a/src/commons/error.rs b/src/commons/error.rs index c378379bd..ee8fff4c7 100644 --- a/src/commons/error.rs +++ b/src/commons/error.rs @@ -15,12 +15,13 @@ use crate::{ rrdp::PublicationDeltaError, AspaCustomer, AspaProvidersUpdateConflict, ChildHandle, ErrorResponse, Handle, ParentHandle, PublisherHandle, ResourceClassName, ResourceSetError, RoaDefinition, }, + crypto::signers::error::SignerError, eventsourcing::{AggregateStoreError, KeyValueError}, remote::{ rfc6492::{self, NotPerformedResponse}, rfc8181::{self, ReportErrorCode}, }, - util::{httpclient, softsigner::SignerError}, + util::httpclient, }, daemon::{ca::RouteAuthorization, http::tls_keys}, upgrades::UpgradeError, diff --git a/src/commons/util/dummysigner.rs b/src/commons/util/dummysigner.rs deleted file mode 100644 index c9fe53d48..000000000 --- a/src/commons/util/dummysigner.rs +++ /dev/null @@ -1,48 +0,0 @@ -use rpki::repository::crypto::{KeyIdentifier, PublicKey, PublicKeyFormat, Signature, SignatureAlgorithm, Signer}; - -use super::softsigner::SignerError; - -/// A dummy signer to prove that compilation with two different Signer implementations works -#[derive(Clone, Debug)] -pub struct DummySigner; - -impl Signer for DummySigner { - type KeyId = KeyIdentifier; - type Error = SignerError; - - fn create_key(&self, _: PublicKeyFormat) -> Result { - unreachable!() - } - - fn get_key_info( - &self, - _: &Self::KeyId, - ) -> Result> { - unreachable!() - } - - fn destroy_key(&self, _: &Self::KeyId) -> Result<(), rpki::repository::crypto::signer::KeyError> { - unreachable!() - } - - fn sign + ?Sized>( - &self, - _: &Self::KeyId, - _: SignatureAlgorithm, - _: &D, - ) -> Result> { - unreachable!() - } - - fn sign_one_off + ?Sized>( - &self, - _: SignatureAlgorithm, - _: &D, - ) -> Result<(Signature, PublicKey), Self::Error> { - unreachable!() - } - - fn rand(&self, _: &mut [u8]) -> Result<(), Self::Error> { - unreachable!() - } -} diff --git a/src/commons/util/mod.rs b/src/commons/util/mod.rs index 65138a303..cacf3d876 100644 --- a/src/commons/util/mod.rs +++ b/src/commons/util/mod.rs @@ -11,12 +11,9 @@ use rpki::{ use crate::constants::KRILL_VERSION; -#[cfg(feature = "hsm")] -pub mod dummysigner; pub mod ext_serde; pub mod file; pub mod httpclient; -pub mod softsigner; pub mod xml; //------------ KrillVersion -------------------------------------------------- diff --git a/test-resources/pykmip/README.md b/test-resources/pykmip/README.md new file mode 100644 index 000000000..f028bb526 --- /dev/null +++ b/test-resources/pykmip/README.md @@ -0,0 +1,48 @@ +The files created in this directory were created and can be used like so: + +Tested on Ubuntu 18:04 + +``` +apt update +apt install -y python3-pip +pip3 install pykmip + +mkdir /etc/pykmip +cd /etc/pykmip +cat <san.cnf +[ext] +subjectAltName = DNS:localhost +EOF + +mkdir demoCA +touch demoCA/index.txt +echo 01 > demoCA/serial +openssl ecparam -out ca.key -name secp256r1 -genkey +openssl req -x509 -new -key ca.key -out ca.crt -outform PEM -days 3650 -subj "/C=NL/ST=Noord Holland/L=Amsterdam/O=NLnet Labs/CN=localhost" + +openssl ecparam -out server.key -name secp256r1 -genkey + +openssl req -new -nodes -key server.key -outform pem -out server.csr -subj "/C=NL/ST=Noord Holland/L=Amsterdam/O=NLnet Labs/CN=localhost" + +openssl ca -keyfile ca.key -cert ca.crt -in server.csr -out server.crt -outdir . -batch -noemailDN -extfile san.cnf -extensions ext + +openssl pkcs8 -topk8 -nocrypt -in server.key -out server.pkcs8.key + +mv server.pkcs8.key server.key + +cat <server.conf +[server] +hostname=localhost +port=5696 +certificate_path=/etc/pykmip/server.crt +key_path=/etc/pykmip/server.key +ca_path=/etc/pykmip/ca.crt +auth_suite=TLS1.2 +enable_tls_client_auth=False +tls_cipher_suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 +logging_level=DEBUG +database_path=/tmp/pykmip.db +EOF + +pykmip-server +``` diff --git a/test-resources/pykmip/ca.crt b/test-resources/pykmip/ca.crt new file mode 100644 index 000000000..3c23e8f95 --- /dev/null +++ b/test-resources/pykmip/ca.crt @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICGTCCAb+gAwIBAgIUf8251O2yXLWfeanKzrtm1TonKpYwCgYIKoZIzj0EAwIw +YjELMAkGA1UEBhMCTkwxFjAUBgNVBAgMDU5vb3JkIEhvbGxhbmQxEjAQBgNVBAcM +CUFtc3RlcmRhbTETMBEGA1UECgwKTkxuZXQgTGFiczESMBAGA1UEAwwJbG9jYWxo +b3N0MB4XDTIxMDkyODE5NTkyNloXDTMxMDkyNjE5NTkyNlowYjELMAkGA1UEBhMC +TkwxFjAUBgNVBAgMDU5vb3JkIEhvbGxhbmQxEjAQBgNVBAcMCUFtc3RlcmRhbTET +MBEGA1UECgwKTkxuZXQgTGFiczESMBAGA1UEAwwJbG9jYWxob3N0MFkwEwYHKoZI +zj0CAQYIKoZIzj0DAQcDQgAEqe0CtnUnyNNcoVGZkNBSsB1eboeMy2469zzh8PJz +vnH6M1o7eEEnX2q9jW2ATzwCy48Zc3V5yMq7+1Ppc5FFEqNTMFEwHQYDVR0OBBYE +FPONAMHT4HJ+QDHWJaGdS37/WcpiMB8GA1UdIwQYMBaAFPONAMHT4HJ+QDHWJaGd +S37/WcpiMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwIDSAAwRQIhAKldVHNF +yLWv1AvTy7mTnDuu2oBgpRJfx94Roig36OsEAiBIlHuX4b9YDXQpvn/W8ypZudcZ +GkhXkkl8izmT6ymsOA== +-----END CERTIFICATE----- diff --git a/test-resources/pykmip/run-server.py b/test-resources/pykmip/run-server.py new file mode 100755 index 000000000..295e07789 --- /dev/null +++ b/test-resources/pykmip/run-server.py @@ -0,0 +1,10 @@ +#!/usr/bin/python3 +from kmip.services.server import KmipServer + +server = KmipServer( + config_path='./server.conf', + log_path='./server.log' +) + +with server: + server.serve() diff --git a/test-resources/pykmip/server.conf b/test-resources/pykmip/server.conf new file mode 100644 index 000000000..0bd216394 --- /dev/null +++ b/test-resources/pykmip/server.conf @@ -0,0 +1,11 @@ +[server] +hostname=127.0.0.1 +port=5696 +certificate_path=./server.crt +key_path=./server.key +ca_path=./ca.crt +auth_suite=TLS1.2 +enable_tls_client_auth=False +tls_cipher_suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 +logging_level=DEBUG +database_path=/tmp/pykmip.db diff --git a/test-resources/pykmip/server.crt b/test-resources/pykmip/server.crt new file mode 100644 index 000000000..e1136f9fb --- /dev/null +++ b/test-resources/pykmip/server.crt @@ -0,0 +1,41 @@ +Certificate: + Data: + Version: 3 (0x2) + Serial Number: 1 (0x1) + Signature Algorithm: ecdsa-with-SHA256 + Issuer: C=NL, ST=Noord Holland, L=Amsterdam, O=NLnet Labs, CN=localhost + Validity + Not Before: Sep 28 19:59:26 2021 GMT + Not After : Sep 28 19:59:26 2022 GMT + Subject: C=NL, ST=Noord Holland, O=NLnet Labs, CN=localhost + Subject Public Key Info: + Public Key Algorithm: id-ecPublicKey + Public-Key: (256 bit) + pub: + 04:74:94:a6:97:4b:18:33:71:bb:29:40:75:b0:8a: + 66:ee:75:09:a8:fb:b0:83:71:a7:82:b5:4b:15:76: + d8:bd:2e:16:5c:b3:26:d8:bc:73:9b:fe:10:8b:95: + 4f:41:06:4a:ee:ac:f6:d7:bc:96:a7:ff:ef:39:52: + 0b:ea:25:c7:1d + ASN1 OID: prime256v1 + NIST CURVE: P-256 + X509v3 extensions: + X509v3 Subject Alternative Name: + DNS:localhost + Signature Algorithm: ecdsa-with-SHA256 + 30:45:02:21:00:d8:e6:2e:5b:40:77:dd:3f:1f:9f:5f:79:69: + 63:6c:93:db:c7:84:04:83:21:ed:68:b2:9d:eb:96:00:3e:d9: + db:02:20:50:3d:9a:e1:e3:37:db:94:d5:8d:e6:9f:ba:1f:15: + 18:f3:04:84:96:8e:c4:79:f4:15:90:3e:e5:e5:f0:b6:f9 +-----BEGIN CERTIFICATE----- +MIIBtzCCAV2gAwIBAgIBATAKBggqhkjOPQQDAjBiMQswCQYDVQQGEwJOTDEWMBQG +A1UECAwNTm9vcmQgSG9sbGFuZDESMBAGA1UEBwwJQW1zdGVyZGFtMRMwEQYDVQQK +DApOTG5ldCBMYWJzMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMjEwOTI4MTk1OTI2 +WhcNMjIwOTI4MTk1OTI2WjBOMQswCQYDVQQGEwJOTDEWMBQGA1UECAwNTm9vcmQg +SG9sbGFuZDETMBEGA1UECgwKTkxuZXQgTGFiczESMBAGA1UEAwwJbG9jYWxob3N0 +MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEdJSml0sYM3G7KUB1sIpm7nUJqPuw +g3GngrVLFXbYvS4WXLMm2Lxzm/4Qi5VPQQZK7qz217yWp//vOVIL6iXHHaMYMBYw +FAYDVR0RBA0wC4IJbG9jYWxob3N0MAoGCCqGSM49BAMCA0gAMEUCIQDY5i5bQHfd +Px+fX3lpY2yT28eEBIMh7WiyneuWAD7Z2wIgUD2a4eM325TVjeafuh8VGPMEhJaO +xHn0FZA+5eXwtvk= +-----END CERTIFICATE----- diff --git a/test-resources/pykmip/server.key b/test-resources/pykmip/server.key new file mode 100644 index 000000000..3a9c140cd --- /dev/null +++ b/test-resources/pykmip/server.key @@ -0,0 +1,5 @@ +-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgt8aDv687Trr5Dsfm +bpJIPM5B9VCGCKyv8XiVIUHZGeehRANCAAR0lKaXSxgzcbspQHWwimbudQmo+7CD +caeCtUsVdti9LhZcsybYvHOb/hCLlU9BBkrurPbXvJan/+85UgvqJccd +-----END PRIVATE KEY----- diff --git a/tests/suspend.rs b/tests/suspend.rs index 102eeb4f2..4d126e974 100644 --- a/tests/suspend.rs +++ b/tests/suspend.rs @@ -71,7 +71,7 @@ async fn test_suspension() { // Wait a bit, and then refresh testbed only, it should find that // the child 'CA' has not been updating, and will suspend it. { - sleep_seconds(5).await; + sleep_seconds(15).await; cas_refresh_single(&testbed).await; expect_suspended(&testbed, &ca).await;