From 198dc6d14f995ed517cfaeae16582ec42a9a8411 Mon Sep 17 00:00:00 2001 From: Marcel Guzik Date: Tue, 4 Nov 2025 14:54:45 +0000 Subject: [PATCH 1/2] refactor: extract cryptoki module from certificate lib.rs Signed-off-by: Marcel Guzik --- crates/common/certificate/src/cryptoki.rs | 158 ++++++++++++++++++++++ crates/common/certificate/src/lib.rs | 150 +------------------- 2 files changed, 164 insertions(+), 144 deletions(-) create mode 100644 crates/common/certificate/src/cryptoki.rs diff --git a/crates/common/certificate/src/cryptoki.rs b/crates/common/certificate/src/cryptoki.rs new file mode 100644 index 00000000000..9123fe30cdc --- /dev/null +++ b/crates/common/certificate/src/cryptoki.rs @@ -0,0 +1,158 @@ +//! Signing self-signed certs and CSRs using private keys stored on a PKCS #11 cryptographic tokens, +//! accessible via Cryptoki. + +use crate::{CertificateError, PemCertificate, SignatureAlgorithm}; +use anyhow::Context; +use camino::Utf8Path; +use tedge_p11_server::{service::ChooseSchemeRequest, CryptokiConfig}; +use tracing::trace; +use x509_parser::public_key::PublicKey; + +/// A key pair using a remote private key. +/// +/// To generate a CSR we need: +/// - the public key, because the public key is a part of the certificate (subject public key info) +/// - the private key, to sign the CSR to prove that the public key is ours +/// +/// With private key in the HSM, we can't access its private parts, but we can still use it to sign. +/// For the public key, instead of deriving it from the private key, which needs some additions to +/// our PKCS11 code, we can just reuse the SPKI section from an existing certificate if we have it, +/// i.e. we're renewing and not getting a brand new cert. +/// +/// An alternative, looking at how it's done in gnutls (_pkcs11_privkey_get_pubkey and +/// pkcs11_read_pubkey functions), seems to be: +/// - if the key is RSA, a public key can be trivially derived from the public properties of PKCS11 +/// private key object +/// - for EC key, it should also be possible to derive public from private +/// - if that fails, a public key object may also be present on the token +#[derive(Debug, Clone)] +pub struct RemoteKeyPair { + pub cryptoki_config: CryptokiConfig, + pub public_key_raw: Vec, + pub(crate) algorithm: SignatureAlgorithm, +} + +impl rcgen::PublicKeyData for RemoteKeyPair { + fn der_bytes(&self) -> &[u8] { + &self.public_key_raw + } + + fn algorithm(&self) -> &'static rcgen::SignatureAlgorithm { + self.algorithm.into() + } +} + +impl rcgen::SigningKey for RemoteKeyPair { + fn sign(&self, msg: &[u8]) -> Result, rcgen::Error> { + // the error here is not PEM-related, but we need to return a foreign error type, and there + // are no other better variants that could let us return context, so we'll have to use this + // until `rcgen::Error::RemoteKeyError` can take a parameter + trace!(?self.cryptoki_config, msg = %String::from_utf8_lossy(msg), "sign"); + let signer = tedge_p11_server::signing_key(self.cryptoki_config.clone()) + .map_err(|e| rcgen::Error::PemError(e.to_string()))?; + signer + .sign2(msg, self.algorithm.into()) + .map_err(|e| rcgen::Error::PemError(e.to_string())) + } +} + +impl RemoteKeyPair { + fn from_cryptoki_and_existing_cert( + cryptoki_config: CryptokiConfig, + current_cert: &Utf8Path, + ) -> Result { + let cert = PemCertificate::from_pem_file(current_cert)?; + let cert = PemCertificate::extract_certificate(&cert.pem)?; + let public_key_raw = cert.public_key().subject_public_key.data.to_vec(); + + // map public key to signature identifier (some keys support many types of signatures, on + // our side we only do P256/P384/RSA2048 with a single type of signature each) + // for P256/P384, former has only SHA256 and latter only SHA384 signature, so no questions there + // but for RSA, AFAIK we can use SHA256 with all of them. RSA_PSS also isn't supported by + // rcgen, but we're free to use regular RSA_PKCS1_SHA256 + let public_key = cert + .public_key() + .parsed() + .context("Failed to read public key from the certificate")?; + let algorithm = match public_key { + PublicKey::EC(ec) => match ec.key_size() { + 256 => SignatureAlgorithm::EcdsaP256Sha256, + 384 => SignatureAlgorithm::EcdsaP384Sha384, + // P521 (size 528 reported by key_size() is not yet supported by rcgen) + // https://github.com/rustls/rcgen/issues/60 + _ => { + return Err(anyhow::anyhow!("Unsupported public key. Only P256/P384/RSA2048/RSA3072/RSA4096 are supported for certificate renewal").into()); + } + }, + PublicKey::RSA(_) => SignatureAlgorithm::RsaPkcs1Sha256, + _ => return Err(anyhow::anyhow!("Unsupported public key. Only P256/P384/RSA2048/RSA3072/RSA4096 are supported for certificate renewal").into()) + }; + + Ok(RemoteKeyPair { + cryptoki_config, + public_key_raw, + algorithm, + }) + } + + pub fn from_cryptoki( + cryptoki_config: CryptokiConfig, + current_cert: Option<&Utf8Path>, + ) -> Result { + let cryptoki = tedge_p11_server::tedge_p11_service(cryptoki_config.clone())?; + + let pubkey_pem = cryptoki.get_public_key_pem(None); + let pubkey_pem = match pubkey_pem { + Ok(p) => p, + Err(err) => { + let e = format!("{err:#}"); + if e.contains("Failed to parse the received frame") { + // server doesn't understand the request, too old, fallback to the older method of just resigning existing certificate + let Some(current_cert) = current_cert else { + return Err(CertificateError::Other( + anyhow::anyhow!("tedge-p11-server can only renew existing certificates but there's no existing certificate; upgrade tedge-p11-server or generate a self-signed certificate first"))); + }; + return Self::from_cryptoki_and_existing_cert(cryptoki_config, current_cert); + } + return Err(err.into()); + } + }; + + let public_key = pem::parse(&pubkey_pem).unwrap(); + let public_key_raw = public_key.into_contents(); + + let signature_algorithm = cryptoki.choose_scheme(ChooseSchemeRequest { + offered: vec![ + tedge_p11_server::service::SignatureScheme( + rustls::SignatureScheme::ECDSA_NISTP256_SHA256, + ), + tedge_p11_server::service::SignatureScheme( + rustls::SignatureScheme::ECDSA_NISTP384_SHA384, + ), + tedge_p11_server::service::SignatureScheme( + rustls::SignatureScheme::RSA_PKCS1_SHA256, + ), + ], + uri: None, + // pin will be applied anyway by the client + pin: None, + })?; + let signature_algorithm = signature_algorithm + .scheme + .context("No supported scheme found")? + .0; + + let algorithm = match signature_algorithm { + rustls::SignatureScheme::ECDSA_NISTP256_SHA256 => SignatureAlgorithm::EcdsaP256Sha256, + rustls::SignatureScheme::ECDSA_NISTP384_SHA384 => SignatureAlgorithm::EcdsaP384Sha384, + rustls::SignatureScheme::RSA_PKCS1_SHA256 => SignatureAlgorithm::RsaPkcs1Sha256, + _ => return Err(anyhow::anyhow!("Unsupported signature scheme").into()), + }; + + Ok(RemoteKeyPair { + cryptoki_config, + public_key_raw, + algorithm, + }) + } +} diff --git a/crates/common/certificate/src/lib.rs b/crates/common/certificate/src/lib.rs index 9b2cc819273..154777169c6 100644 --- a/crates/common/certificate/src/lib.rs +++ b/crates/common/certificate/src/lib.rs @@ -7,20 +7,21 @@ use sha1::Digest; use sha1::Sha1; use std::path::Path; use std::path::PathBuf; -use tedge_p11_server::service::ChooseSchemeRequest; use tedge_p11_server::CryptokiConfig; use time::Duration; use time::OffsetDateTime; -use tracing::trace; -use x509_parser::public_key::PublicKey; pub use zeroize::Zeroizing; #[cfg(feature = "reqwest")] mod cloud_root_certificate; #[cfg(feature = "reqwest")] pub use cloud_root_certificate::*; +mod cryptoki; +use cryptoki::RemoteKeyPair; + pub mod device_id; pub mod parse_root_certificate; + pub struct PemCertificate { pem: x509_parser::pem::Pem, } @@ -156,151 +157,12 @@ pub enum KeyKind { } impl KeyKind { - fn from_cryptoki_and_existing_cert( - cryptoki_config: CryptokiConfig, - current_cert: &Utf8Path, - ) -> Result { - let cert = PemCertificate::from_pem_file(current_cert)?; - let cert = PemCertificate::extract_certificate(&cert.pem)?; - let public_key_raw = cert.public_key().subject_public_key.data.to_vec(); - - // map public key to signature identifier (some keys support many types of signatures, on - // our side we only do P256/P384/RSA2048 with a single type of signature each) - // for P256/P384, former has only SHA256 and latter only SHA384 signature, so no questions there - // but for RSA, AFAIK we can use SHA256 with all of them. RSA_PSS also isn't supported by - // rcgen, but we're free to use regular RSA_PKCS1_SHA256 - let public_key = cert - .public_key() - .parsed() - .context("Failed to read public key from the certificate")?; - let algorithm = match public_key { - PublicKey::EC(ec) => match ec.key_size() { - 256 => SignatureAlgorithm::EcdsaP256Sha256, - 384 => SignatureAlgorithm::EcdsaP384Sha384, - // P521 (size 528 reported by key_size() is not yet supported by rcgen) - // https://github.com/rustls/rcgen/issues/60 - _ => { - return Err(anyhow::anyhow!("Unsupported public key. Only P256/P384/RSA2048/RSA3072/RSA4096 are supported for certificate renewal").into()); - } - }, - PublicKey::RSA(_) => SignatureAlgorithm::RsaPkcs1Sha256, - _ => return Err(anyhow::anyhow!("Unsupported public key. Only P256/P384/RSA2048/RSA3072/RSA4096 are supported for certificate renewal").into()) - }; - - Ok(Self::ReuseRemote(RemoteKeyPair { - cryptoki_config, - public_key_raw, - algorithm, - })) - } - pub fn from_cryptoki( cryptoki_config: CryptokiConfig, current_cert: Option<&Utf8Path>, ) -> Result { - let cryptoki = tedge_p11_server::tedge_p11_service(cryptoki_config.clone())?; - - let pubkey_pem = cryptoki.get_public_key_pem(None); - let pubkey_pem = match pubkey_pem { - Ok(p) => p, - Err(err) => { - let e = format!("{err:#}"); - if e.contains("Failed to parse the received frame") { - // server doesn't understand the request, too old, fallback to the older method of just resigning existing certificate - let Some(current_cert) = current_cert else { - return Err(CertificateError::Other( - anyhow::anyhow!("tedge-p11-server can only renew existing certificates but there's no existing certificate; upgrade tedge-p11-server or generate a self-signed certificate first"))); - }; - return Self::from_cryptoki_and_existing_cert(cryptoki_config, current_cert); - } - return Err(err.into()); - } - }; - - let public_key = pem::parse(&pubkey_pem).unwrap(); - let public_key_raw = public_key.into_contents(); - - let signature_algorithm = cryptoki.choose_scheme(ChooseSchemeRequest { - offered: vec![ - tedge_p11_server::service::SignatureScheme( - rustls::SignatureScheme::ECDSA_NISTP256_SHA256, - ), - tedge_p11_server::service::SignatureScheme( - rustls::SignatureScheme::ECDSA_NISTP384_SHA384, - ), - tedge_p11_server::service::SignatureScheme( - rustls::SignatureScheme::RSA_PKCS1_SHA256, - ), - ], - uri: None, - // pin will be applied anyway by the client - pin: None, - })?; - let signature_algorithm = signature_algorithm - .scheme - .context("No supported scheme found")? - .0; - - let algorithm = match signature_algorithm { - rustls::SignatureScheme::ECDSA_NISTP256_SHA256 => SignatureAlgorithm::EcdsaP256Sha256, - rustls::SignatureScheme::ECDSA_NISTP384_SHA384 => SignatureAlgorithm::EcdsaP384Sha384, - rustls::SignatureScheme::RSA_PKCS1_SHA256 => SignatureAlgorithm::RsaPkcs1Sha256, - _ => return Err(anyhow::anyhow!("Unsupported signature scheme").into()), - }; - - Ok(Self::ReuseRemote(RemoteKeyPair { - cryptoki_config, - public_key_raw, - algorithm, - })) - } -} - -/// A key pair using a remote private key. -/// -/// To generate a CSR we need: -/// - the public key, because the public key is a part of the certificate (subject public key info) -/// - the private key, to sign the CSR to prove that the public key is ours -/// -/// With private key in the HSM, we can't access its private parts, but we can still use it to sign. -/// For the public key, instead of deriving it from the private key, which needs some additions to -/// our PKCS11 code, we can just reuse the SPKI section from an existing certificate if we have it, -/// i.e. we're renewing and not getting a brand new cert. -/// -/// An alternative, looking at how it's done in gnutls (_pkcs11_privkey_get_pubkey and -/// pkcs11_read_pubkey functions), seems to be: -/// - if the key is RSA, a public key can be trivially derived from the public properties of PKCS11 -/// private key object -/// - for EC key, it should also be possible to derive public from private -/// - if that fails, a public key object may also be present on the token -#[derive(Debug, Clone)] -pub struct RemoteKeyPair { - cryptoki_config: CryptokiConfig, - public_key_raw: Vec, - algorithm: SignatureAlgorithm, -} - -impl rcgen::PublicKeyData for RemoteKeyPair { - fn der_bytes(&self) -> &[u8] { - &self.public_key_raw - } - - fn algorithm(&self) -> &'static rcgen::SignatureAlgorithm { - self.algorithm.into() - } -} - -impl rcgen::SigningKey for RemoteKeyPair { - fn sign(&self, msg: &[u8]) -> Result, rcgen::Error> { - // the error here is not PEM-related, but we need to return a foreign error type, and there - // are no other better variants that could let us return context, so we'll have to use this - // until `rcgen::Error::RemoteKeyError` can take a parameter - trace!(?self.cryptoki_config, msg = %String::from_utf8_lossy(msg), "sign"); - let signer = tedge_p11_server::signing_key(self.cryptoki_config.clone()) - .map_err(|e| rcgen::Error::PemError(e.to_string()))?; - signer - .sign2(msg, self.algorithm.into()) - .map_err(|e| rcgen::Error::PemError(e.to_string())) + let keypair = cryptoki::RemoteKeyPair::from_cryptoki(cryptoki_config, current_cert)?; + Ok(Self::ReuseRemote(keypair)) } } From 8ea27ebefda5296c45d83026d75ded27e31328e6 Mon Sep 17 00:00:00 2001 From: Marcel Guzik Date: Tue, 9 Dec 2025 20:10:40 +0000 Subject: [PATCH 2/2] feat: make tedge cert create use HSM Signed-off-by: Marcel Guzik --- crates/core/tedge/src/cli/certificate/cli.rs | 5 +- .../core/tedge/src/cli/certificate/create.rs | 57 +++++++++++++------ .../tedge/src/cli/certificate/create_csr.rs | 3 +- .../core/tedge/src/cli/certificate/renew.rs | 2 +- .../pkcs11/tedge_cert_create/current.robot | 13 +++++ .../pkcs11/tedge_cert_create/initial.robot | 13 +++++ .../tedge_cert_create.resource | 13 +++++ 7 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 tests/RobotFramework/tests/pkcs11/tedge_cert_create/current.robot create mode 100644 tests/RobotFramework/tests/pkcs11/tedge_cert_create/initial.robot create mode 100644 tests/RobotFramework/tests/pkcs11/tedge_cert_create/tedge_cert_create.resource diff --git a/crates/core/tedge/src/cli/certificate/cli.rs b/crates/core/tedge/src/cli/certificate/cli.rs index ad074e002f9..1efb8a615c7 100644 --- a/crates/core/tedge/src/cli/certificate/cli.rs +++ b/crates/core/tedge/src/cli/certificate/cli.rs @@ -34,7 +34,10 @@ use tracing::debug; #[derive(clap::Subcommand, Debug)] pub enum TEdgeCertCli { - /// Create a self-signed device certificate + /// Create a self-signed device certificate. + /// + /// This command creates the device certificate and private key, and persists them, if they + /// don't already exist. Create { /// The device identifier to be used as the common name for the certificate #[clap(long = "device-id", global = true)] diff --git a/crates/core/tedge/src/cli/certificate/create.rs b/crates/core/tedge/src/cli/certificate/create.rs index 5afa0471aae..c5a13cb5de6 100644 --- a/crates/core/tedge/src/cli/certificate/create.rs +++ b/crates/core/tedge/src/cli/certificate/create.rs @@ -2,6 +2,7 @@ use super::error::CertError; use crate::cli::certificate::show::ShowCertCmd; use crate::command::Command; use crate::log::MaybeFancy; +use anyhow::Context; use camino::Utf8Path; use camino::Utf8PathBuf; use certificate::CsrTemplate; @@ -12,6 +13,7 @@ use std::fs::Permissions; use std::os::unix::fs::PermissionsExt; use std::path::Path; use tedge_config::TEdgeConfig; +use tedge_p11_server::CryptokiConfig; use tokio::fs::File; use tokio::fs::OpenOptions; use tokio::io::AsyncWriteExt; @@ -42,8 +44,10 @@ impl Command for CreateCertCmd { format!("create a test certificate for the device {}.", self.id) } - async fn execute(&self, _: TEdgeConfig) -> Result<(), MaybeFancy> { - self.create_test_certificate(&self.csr_template).await?; + async fn execute(&self, config: TEdgeConfig) -> Result<(), MaybeFancy> { + let cryptoki = config.device.cryptoki_config(None)?; + self.create_test_certificate(&self.csr_template, cryptoki) + .await?; eprintln!("Certificate created successfully"); eprintln!(" => the certificate has to be uploaded to the cloud"); eprintln!(" => before the device can be connected\n"); @@ -53,8 +57,23 @@ impl Command for CreateCertCmd { } impl CreateCertCmd { - pub async fn create_test_certificate(&self, config: &CsrTemplate) -> Result<(), CertError> { - let cert = KeyCertPair::new_selfsigned_certificate(config, &self.id, &KeyKind::New)?; + // test certificate here means it's not yet permanent - establishing the connection using this + // new certificate will be tested during `tedge connect` and if an error happens, we will revert + // to the previous certificate. If not, the certificate will be set as a permanent active + // certificate and will not be tested on further connections. + pub async fn create_test_certificate( + &self, + template: &CsrTemplate, + cryptoki: Option, + ) -> Result<(), CertError> { + let key_kind = if let Some(cryptoki) = cryptoki { + KeyKind::from_cryptoki(cryptoki.clone(), None) + .context("failed to create a remote keykind")? + } else { + KeyKind::New + }; + + let cert = KeyCertPair::new_selfsigned_certificate(template, &self.id, &key_kind)?; let cert_path = &self.cert_path; persist_new_public_key( @@ -66,15 +85,18 @@ impl CreateCertCmd { .await .map_err(|err| err.cert_context(cert_path.clone()))?; - let key_path = &self.key_path; - persist_new_private_key( - key_path, - cert.private_key_pem_string()?, - &self.user, - &self.group, - ) - .await - .map_err(|err| err.key_context(key_path.clone()))?; + // only persist to filesystem if we just created a new key (not using HSM) + if let KeyKind::New = key_kind { + let key_path = &self.key_path; + persist_new_private_key( + key_path, + cert.private_key_pem_string()?, + &self.user, + &self.group, + ) + .await + .map_err(|err| err.key_context(key_path.clone()))?; + } Ok(()) } } @@ -238,7 +260,8 @@ mod tests { }; assert_matches!( - cmd.create_test_certificate(&CsrTemplate::default()).await, + cmd.create_test_certificate(&CsrTemplate::default(), None) + .await, Ok(()) ); assert_eq!(parse_pem_file(&cert_path).tag(), "CERTIFICATE"); @@ -268,7 +291,7 @@ mod tests { }; assert!(cmd - .create_test_certificate(&CsrTemplate::default()) + .create_test_certificate(&CsrTemplate::default(), None) .await .ok() .is_none()); @@ -293,7 +316,7 @@ mod tests { }; let cert_error = cmd - .create_test_certificate(&CsrTemplate::default()) + .create_test_certificate(&CsrTemplate::default(), None) .await .unwrap_err(); assert_matches!(cert_error, CertError::CertificateNotFound { .. }); @@ -315,7 +338,7 @@ mod tests { }; let cert_error = cmd - .create_test_certificate(&CsrTemplate::default()) + .create_test_certificate(&CsrTemplate::default(), None) .await .unwrap_err(); assert_matches!(cert_error, CertError::KeyNotFound { .. }); diff --git a/crates/core/tedge/src/cli/certificate/create_csr.rs b/crates/core/tedge/src/cli/certificate/create_csr.rs index 8b3a70d4d43..ae37e4da269 100644 --- a/crates/core/tedge/src/cli/certificate/create_csr.rs +++ b/crates/core/tedge/src/cli/certificate/create_csr.rs @@ -145,7 +145,8 @@ mod tests { // create private key and public cert with standard command assert_matches!( - cmd.create_test_certificate(&CsrTemplate::default()).await, + cmd.create_test_certificate(&CsrTemplate::default(), None) + .await, Ok(()) ); diff --git a/crates/core/tedge/src/cli/certificate/renew.rs b/crates/core/tedge/src/cli/certificate/renew.rs index 9cf1ccec6f8..048c1108f38 100644 --- a/crates/core/tedge/src/cli/certificate/renew.rs +++ b/crates/core/tedge/src/cli/certificate/renew.rs @@ -89,7 +89,7 @@ mod tests { }; // First create both cert and key - cmd.create_test_certificate(&CsrTemplate::default()) + cmd.create_test_certificate(&CsrTemplate::default(), None) .await .unwrap(); diff --git a/tests/RobotFramework/tests/pkcs11/tedge_cert_create/current.robot b/tests/RobotFramework/tests/pkcs11/tedge_cert_create/current.robot new file mode 100644 index 00000000000..d97b6a6d743 --- /dev/null +++ b/tests/RobotFramework/tests/pkcs11/tedge_cert_create/current.robot @@ -0,0 +1,13 @@ +*** Settings *** +Resource tedge_cert_create.resource + +Suite Setup tedge-p11-server Setup ${TEDGE_P11_SERVER_VERSION} + + +*** Variables *** +${TEDGE_P11_SERVER_VERSION} ${EMPTY} + + +*** Test Cases *** +Tedge cert create should use HSM configuration + Test tedge cert create uses HSM configuration diff --git a/tests/RobotFramework/tests/pkcs11/tedge_cert_create/initial.robot b/tests/RobotFramework/tests/pkcs11/tedge_cert_create/initial.robot new file mode 100644 index 00000000000..d97b6a6d743 --- /dev/null +++ b/tests/RobotFramework/tests/pkcs11/tedge_cert_create/initial.robot @@ -0,0 +1,13 @@ +*** Settings *** +Resource tedge_cert_create.resource + +Suite Setup tedge-p11-server Setup ${TEDGE_P11_SERVER_VERSION} + + +*** Variables *** +${TEDGE_P11_SERVER_VERSION} ${EMPTY} + + +*** Test Cases *** +Tedge cert create should use HSM configuration + Test tedge cert create uses HSM configuration diff --git a/tests/RobotFramework/tests/pkcs11/tedge_cert_create/tedge_cert_create.resource b/tests/RobotFramework/tests/pkcs11/tedge_cert_create/tedge_cert_create.resource new file mode 100644 index 00000000000..5c7b9e64f50 --- /dev/null +++ b/tests/RobotFramework/tests/pkcs11/tedge_cert_create/tedge_cert_create.resource @@ -0,0 +1,13 @@ +*** Settings *** +Resource ../pkcs11_common.resource + + +*** Keywords *** +Test tedge cert create uses HSM configuration + # here assumed that we're configured to use PKCS11 private key + Execute Command tedge cert remove + Execute Command tedge cert create --device-id ${DEVICE_SN} + + Execute Command + ... cmd=C8Y_USER='${C8Y_CONFIG.username}' C8Y_PASSWORD='${C8Y_CONFIG.password}' tedge cert upload c8y + Tedge Reconnect Should Succeed