Skip to content

Commit

Permalink
Upgrade cryptoki to 0.7. (#1212)
Browse files Browse the repository at this point in the history
This PR upgrades cryptoki to 0.7. It doesn’t take advantage of the changes
yet. This will happen in follow-up PRs.
  • Loading branch information
partim authored Jun 24, 2024
1 parent fff64ce commit 5a86110
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 58 deletions.
48 changes: 24 additions & 24 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ basic-cookies = { version = "0.1", optional = true }
bytes = "1"
chrono = { version = "0.4", features = ["serde"] }
clap = "2.33"
cryptoki = { version = "^0.3", optional = true }
cryptoki-sys = { version = "=0.1.4", optional = true } # pin cryptoki-sys because of compilation issues on various systems
cryptoki = { version = "0.7", optional = true }
fern = { version = "0.6.2", features = ["syslog-6"] }
futures-util = "0.3"
fslock = "0.2.1"
Expand Down Expand Up @@ -56,12 +55,13 @@ rpassword = { version = "7.3.1", optional = true }
rpki = { git = "https://github.com/nLnetLabs/rpki-rs", features = [ "ca", "compat", "rrdp" ] }
rustls-pemfile = "2.1.2"
scrypt = { version = "0.11", optional = true, default-features = false }
secrecy = { version = "0.8", features = ["serde"] }
serde = { version = "1.0", features = ["derive", "rc"] }
serde_json = "1.0"
tokio = { version = "1", features = [ "macros", "rt", "rt-multi-thread", "signal", "time" ] }
tokio-rustls = { version = "0.26", default-features = false, features = [ "ring", "logging", "tls12" ] }
toml = "0.8.14"
unicode-normalization = { version = "^0.1", optional = true }
unicode-normalization = { version = "0.1", optional = true }
url = { version = "2.3.1", features = ["serde"] }
urlparse = { version = "0.7", optional = true }
uuid = { version = "1.1", features = ["v4"] }
Expand Down
4 changes: 3 additions & 1 deletion src/commons/crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,7 @@ pub type CryptoResult<T> = std::result::Result<T, self::error::Error>;

#[cfg(feature = "hsm")]
pub use self::signers::pkcs11::signer::{
Pkcs11ConfigurablePrivateKeyAttributes, Pkcs11ConfigurablePublicKeyAttributes,
Pkcs11ConfigurablePrivateKeyAttributes,
Pkcs11ConfigurablePublicKeyAttributes,
Pkcs11ConfigurableSecrets
};
9 changes: 5 additions & 4 deletions src/commons/crypto/signing/signers/pkcs11/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ use cryptoki::{
context::{CInitializeArgs, Info, Pkcs11},
mechanism::Mechanism,
object::{Attribute, AttributeType, ObjectHandle},
session::{Session, SessionFlags, UserType},
session::{Session, UserType},
slot::{Slot, SlotInfo, TokenInfo},
types::AuthPin,
};
use once_cell::sync::OnceCell;

Expand Down Expand Up @@ -228,8 +229,8 @@ impl Pkcs11Context {
self.logged_cryptoki_call("GetTokenInfo", |cryptoki| cryptoki.get_token_info(slot))
}

pub fn open_session(&self, slot: Slot, flags: SessionFlags) -> Result<Session, Pkcs11Error> {
self.logged_cryptoki_call("OpenSession", |cryptoki| cryptoki.open_session_no_callback(slot, flags))
pub fn open_rw_session(&self, slot: Slot) -> Result<Session, Pkcs11Error> {
self.logged_cryptoki_call("OpenSession", |cryptoki| cryptoki.open_rw_session(slot))
}

pub fn generate_key_pair(
Expand Down Expand Up @@ -262,7 +263,7 @@ impl Pkcs11Context {
&self,
session: Arc<Mutex<Session>>,
user_type: UserType,
pin: Option<&str>,
pin: Option<&AuthPin>,
) -> Result<(), Pkcs11Error> {
self.logged_cryptoki_call("Login", |_| session.lock().unwrap().login(user_type, pin))
}
Expand Down
18 changes: 7 additions & 11 deletions src/commons/crypto/signing/signers/pkcs11/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ use std::sync::{Arc, Mutex};
use cryptoki::error::Error as Pkcs11Error;
use cryptoki::mechanism::Mechanism;
use cryptoki::object::{Attribute, AttributeType, ObjectHandle};
use cryptoki::session::{SessionFlags, UserType};
use cryptoki::{session::Session, slot::Slot};
use cryptoki::session::{Session, UserType};
use cryptoki::slot::Slot;
use cryptoki::types::AuthPin;

use crate::commons::crypto::signers::pkcs11::context::ThreadSafePkcs11Context;

Expand All @@ -17,10 +18,6 @@ pub(super) struct Pkcs11Session {

impl Pkcs11Session {
pub fn new(context: ThreadSafePkcs11Context, slot: Slot) -> Result<Pkcs11Session, Pkcs11Error> {
// Section 11.6 "Session management functions" under "C_OpenSession" says:
// "For legacy reasons, the CKF_SERIAL_SESSION bit must always be set; if a call to C_OpenSession does not
// have this bit set, the call should return unsuccessfully with the error code
// CKR_PARALLEL_NOT_SUPPORTED."
//
// Note that we don't track whether or not the session logs in so that we can later logout because the spec
// we invoke C_CloseSession on drop and the spec for C_CloseSession says:
Expand All @@ -30,10 +27,7 @@ impl Pkcs11Session {
//
// In the spirit of not doing anything we don't have to do, we can keep the code simpler by not calling
// C_Logout because we don't have to.
let mut flags = SessionFlags::new();
flags.set_serial_session(true);
flags.set_rw_session(true);
let session_handle = context.read().unwrap().open_session(slot, flags)?;
let session_handle = context.read().unwrap().open_rw_session(slot)?;
Ok(Pkcs11Session {
context,
session_handle: Arc::new(Mutex::new(session_handle)),
Expand Down Expand Up @@ -67,7 +61,9 @@ impl Pkcs11Session {
.get_attributes(self.session_handle.clone(), pub_handle, pub_template)
}

pub fn login(&self, user_type: UserType, user_pin: Option<&str>) -> Result<(), Pkcs11Error> {
pub fn login(
&self, user_type: UserType, user_pin: Option<&AuthPin>
) -> Result<(), Pkcs11Error> {
self.context
.read()
.unwrap()
Expand Down
42 changes: 31 additions & 11 deletions src/commons/crypto/signing/signers/pkcs11/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use cryptoki::{
object::{Attribute, AttributeType, ObjectClass, ObjectHandle},
session::UserType,
slot::{Slot, SlotInfo, TokenInfo},
types::AuthPin,
};

use rpki::{
Expand All @@ -25,6 +26,7 @@ use rpki::{
SignatureAlgorithm, SigningError,
},
};
use secrecy::ExposeSecret;

use crate::commons::crypto::{
dispatch::signerinfo::SignerMapper,
Expand All @@ -46,7 +48,8 @@ use serde::{de::Visitor, Deserialize};
pub struct Pkcs11SignerConfig {
pub lib_path: String,

pub user_pin: Option<String>,
#[serde(flatten)]
pub secrets: Pkcs11ConfigurableSecrets,

#[serde(deserialize_with = "slot_id_or_label")]
pub slot: SlotIdOrLabel,
Expand All @@ -70,7 +73,21 @@ pub struct Pkcs11SignerConfig {
pub private_key_attributes: Pkcs11ConfigurablePrivateKeyAttributes,
}

impl Eq for Pkcs11SignerConfig {}
#[derive(Clone, Debug, Deserialize)]
pub struct Pkcs11ConfigurableSecrets {
pub user_pin: Option<AuthPin>,
}

impl PartialEq for Pkcs11ConfigurableSecrets {
fn eq(&self, other: &Self) -> bool {
self.user_pin.as_ref().map(|s| s.expose_secret()).eq(
&other.user_pin.as_ref().map(|s| s.expose_secret())
)
}
}

impl Eq for Pkcs11ConfigurableSecrets { }


#[derive(Clone, Debug, Deserialize, PartialEq)]
pub struct Pkcs11ConfigurablePublicKeyAttributes {
Expand Down Expand Up @@ -236,7 +253,7 @@ struct ConnectionSettings {
// the user enters a PIN on a PINpad on the token itself, or on the slot device. Or the user might not even use a
// PIN—authentication could be achieved by some fingerprint-reading device, for example. To log into a token with
// a protected authentication path, the pPin parameter to C_Login should be NULL_PTR."
user_pin: Option<String>,
user_pin: Option<AuthPin>,

login_mode: LoginMode,

Expand All @@ -253,7 +270,7 @@ impl TryFrom<&Pkcs11SignerConfig> for ConnectionSettings {
fn try_from(conf: &Pkcs11SignerConfig) -> Result<Self, Self::Error> {
let lib_path = conf.lib_path.clone();
let slot = conf.slot.clone();
let user_pin = conf.user_pin.clone();
let user_pin = conf.secrets.user_pin.clone();
let login_mode = match conf.login {
true => LoginMode::LoginRequired,
false => LoginMode::LoginNotRequired,
Expand Down Expand Up @@ -361,7 +378,7 @@ impl Pkcs11Signer {
match self.build_key_internal(PublicKeyFormat::Rsa) {
Ok((public_key, _, _, internal_key_id)) => Ok((public_key, internal_key_id)),

Err(err @ InternalConnError::Pkcs11Error(Pkcs11Error::Pkcs11(RvError::TemplateInconsistent))) => {
Err(err @ InternalConnError::Pkcs11Error(Pkcs11Error::Pkcs11(RvError::TemplateInconsistent, _))) => {
// https://github.com/NLnetLabs/krill/issues/1019
let err_msg = format!(
"{} [Note: This error can occur if the signer does not support authenticated \
Expand Down Expand Up @@ -452,7 +469,7 @@ impl Pkcs11Signer {
) -> Result<UsableServerState, ProbeError<SignerError>> {
fn slot_label_eq(ctx: &RwLockReadGuard<Pkcs11Context>, slot: Slot, slot_label: &str) -> bool {
match ctx.get_token_info(slot) {
Ok(info) => String::from_utf8_lossy(&info.label).trim_end() == slot_label,
Ok(info) => info.label().trim_end() == slot_label,
Err(err) => {
warn!(
"Failed to obtain token info for PKCS#11 slot id '{}': {}",
Expand Down Expand Up @@ -490,7 +507,7 @@ impl Pkcs11Signer {
ctx: ThreadSafePkcs11Context,
name: &str,
lib_name: &String,
) -> Result<(Info, Slot, SlotInfo, TokenInfo, Option<String>), ProbeError<SignerError>> {
) -> Result<(Info, Slot, SlotInfo, TokenInfo, Option<AuthPin>), ProbeError<SignerError>> {
let readable_ctx = ctx.read().unwrap();

let cryptoki_info = readable_ctx.get_info().map_err(|err| {
Expand Down Expand Up @@ -591,7 +608,7 @@ impl Pkcs11Signer {
fn login(
session: Pkcs11Session,
login_mode: LoginMode,
user_pin: Option<String>,
user_pin: Option<AuthPin>,
name: &str,
lib_name: &String,
slot: Slot,
Expand All @@ -602,7 +619,7 @@ impl Pkcs11Signer {
Ok(None)
}
LoginMode::LoginRequired => {
session.login(UserType::User, user_pin.as_deref()).map_err(|err| {
session.login(UserType::User, user_pin.as_ref()).map_err(|err| {
error!(
"[{}] Unable to login to PKCS#11 session for library '{}' slot {}: {}",
name, lib_name, slot, err
Expand Down Expand Up @@ -1182,14 +1199,17 @@ fn is_transient_error(err: &Pkcs11Error) -> bool {
| Pkcs11Error::LibraryLoading(_)
| Pkcs11Error::TryFromInt(_)
| Pkcs11Error::TryFromSlice(_)
| Pkcs11Error::ParseInt(_)
| Pkcs11Error::Utf8(_)
| Pkcs11Error::NulError(_)
| Pkcs11Error::InvalidValue
| Pkcs11Error::PinNotSet => {
| Pkcs11Error::PinNotSet
| Pkcs11Error::AlreadyInitialized => {
// The Rust `pkcs11` crate had a serious problem such as the loaded library not exporting a required
// function or that it was asked to initialize an already initialized library.
false
}
Pkcs11Error::Pkcs11(err) => {
Pkcs11Error::Pkcs11(err, _) => {
// Error codes were taken from the `types` module of the Rust `pkcs11` crate.
// See section 11.1 of the PKCS#11 v2.20 specification for an explanation of each value.
// Return true only for errors which might succeed very soon after they failed. Errors which are solvable
Expand Down
14 changes: 10 additions & 4 deletions src/daemon/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,11 +302,17 @@ impl ConfigDefaults {
#[cfg(all(feature = "hsm-tests-pkcs11", not(feature = "hsm-tests-kmip")))]
{
use crate::commons::crypto::{
Pkcs11ConfigurablePrivateKeyAttributes, Pkcs11ConfigurablePublicKeyAttributes, SlotIdOrLabel,
Pkcs11ConfigurablePrivateKeyAttributes,
Pkcs11ConfigurablePublicKeyAttributes,
Pkcs11ConfigurableSecrets,
SlotIdOrLabel,
};

let signer_config = Pkcs11SignerConfig {
lib_path: "/usr/lib/softhsm/libsofthsm2.so".to_string(),
user_pin: Some("1234".to_string()),
secrets: Pkcs11ConfigurableSecrets {
user_pin: Some("1234".to_string().into())
},
slot: SlotIdOrLabel::Label("My token 1".to_string()),
login: true,
retry_seconds: Pkcs11SignerConfig::default_retry_seconds(),
Expand Down Expand Up @@ -1756,7 +1762,7 @@ impl<'de> Deserialize<'de> for AuthType {
// type = "KMIP"
// ...

#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
#[derive(Clone, Debug, Deserialize, PartialEq)]
pub struct SignerConfig {
/// A friendly name for the signer. Used to identify the signer with the `default_signer` and `one_off_signer`
/// settings.
Expand All @@ -1767,7 +1773,7 @@ pub struct SignerConfig {
pub signer_type: SignerType,
}

#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
#[derive(Clone, Debug, Deserialize, PartialEq)]
#[serde(tag = "type")]
pub enum SignerType {
#[serde(alias = "OpenSSL")]
Expand Down

0 comments on commit 5a86110

Please sign in to comment.