Skip to content

Add list keys #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cargo.lock

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

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ name = "parsec"
path = "src/bin/main.rs"

[dependencies]
parsec-interface = "0.20.2"
# Set to the newest interface version before releasing
parsec-interface = { git = "https://github.com/parallaxsecond/parsec-interface-rs.git", rev = "4986c0d6b4610237c85eb799fc469d1658c1a2f0" }
rand = { version = "0.7.3", features = ["small_rng"], optional = true }
base64 = "0.12.3"
uuid = "0.8.1"
Expand Down
2 changes: 1 addition & 1 deletion e2e_tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ publish = false

[dependencies]
serde = { version = "1.0.115", features = ["derive"] }
parsec-client = { version = "0.10.0", features = ["testing"] }
parsec-client = { git = "https://github.com/parallaxsecond/parsec-client-rust", rev = "dbd449a9e736f9f972e5489fb2d67949b93484dc", features = ["testing"] }
log = "0.4.11"
rand = "0.7.3"

Expand Down
6 changes: 6 additions & 0 deletions e2e_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use log::error;
use parsec_client::auth::AuthenticationData;
use parsec_client::core::basic_client::BasicClient;
use parsec_client::core::interface::operations::list_authenticators::AuthenticatorInfo;
use parsec_client::core::interface::operations::list_keys::KeyInfo;
use parsec_client::core::interface::operations::list_providers::ProviderInfo;
use parsec_client::core::interface::operations::psa_algorithm::{
Aead, AeadWithDefaultLengthTag, Algorithm, AsymmetricEncryption, AsymmetricSignature, Hash,
Expand Down Expand Up @@ -904,6 +905,11 @@ impl TestClient {
.map_err(convert_error)
}

/// Lists the keys created.
pub fn list_keys(&mut self) -> Result<Vec<KeyInfo>> {
self.basic_client.list_keys().map_err(convert_error)
}

/// Executes a ping operation.
pub fn ping(&mut self) -> Result<(u8, u8)> {
self.basic_client.ping().map_err(convert_error)
Expand Down
34 changes: 34 additions & 0 deletions e2e_tests/tests/all_providers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ fn list_opcodes() {
let _ = core_provider_opcodes.insert(Opcode::ListProviders);
let _ = core_provider_opcodes.insert(Opcode::ListAuthenticators);
let _ = core_provider_opcodes.insert(Opcode::ListOpcodes);
let _ = core_provider_opcodes.insert(Opcode::ListKeys);

assert_eq!(
client
Expand Down Expand Up @@ -113,3 +114,36 @@ fn sign_verify_with_provider_discovery() -> Result<()> {
let key_name = String::from("sign_verify_with_provider_discovery");
client.generate_rsa_sign_key(key_name)
}

#[test]
fn list_keys() {
let mut client = TestClient::new();
client.set_auth("list_keys test".to_string());

let keys = client.list_keys().expect("list_keys failed");

assert!(keys.is_empty());

let key1 = String::from("list_keys1");
let key2 = String::from("list_keys2");
let key3 = String::from("list_keys3");

client.set_provider(ProviderID::MbedCrypto);
client.generate_rsa_sign_key(key1.clone()).unwrap();
client.set_provider(ProviderID::Pkcs11);
client.generate_rsa_sign_key(key2.clone()).unwrap();
client.set_provider(ProviderID::Tpm);
client.generate_rsa_sign_key(key3.clone()).unwrap();

let key_names: Vec<String> = client
.list_keys()
.expect("list_keys failed")
.into_iter()
.map(|k| k.name)
.collect();

assert_eq!(key_names.len(), 3);
assert!(key_names.contains(&key1));
assert!(key_names.contains(&key2));
assert!(key_names.contains(&key3));
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl Authenticate for UnixPeerCredentialsAuthenticator {
version_maj: 0,
version_min: 1,
version_rev: 0,
id: AuthType::PeerCredentials,
id: AuthType::UnixPeerCredentials,
})
}

Expand Down
17 changes: 16 additions & 1 deletion src/back/backend_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use crate::authenticators::ApplicationName;
use crate::providers::Provide;
use derivative::Derivative;
use log::trace;
use log::{error, trace};
use parsec_interface::operations::Convert;
use parsec_interface::operations::{NativeOperation, NativeResult};
use parsec_interface::requests::{
Expand Down Expand Up @@ -53,12 +53,19 @@ impl BackEndHandler {
/// the request.
///
/// # Errors
/// - if the provider ID can not perform the type of operation, returns
/// `ResponseStatus::PsaErrorNotSupported`
/// - if the provider ID does not match, returns `ResponseStatus::WrongProviderID`
/// - if the content type does not match, returns `ResponseStatus::ContentTypeNotSupported`
/// - if the accept type does not match, returns `ResponseStatus::AcceptTypeNotSupported`
pub fn is_capable(&self, request: &Request) -> Result<()> {
let header = &request.header;

if (self.provider_id == ProviderID::Core) != header.opcode.is_core() {
error!("The request's operation is not compatible with the provider targeted.");
return Err(ResponseStatus::PsaErrorNotSupported);
}

if header.provider != self.provider_id {
Err(ResponseStatus::WrongProviderID)
} else if header.content_type != self.content_type {
Expand Down Expand Up @@ -207,6 +214,14 @@ impl BackEndHandler {
trace!("list_authenticators egress");
self.result_to_response(NativeResult::ListAuthenticators(result), header)
}
NativeOperation::ListKeys(op_list_keys) => {
let app_name =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check I was talking in the other comment could be here but could also be in the is_capable method where we would check if the request's opcode is a Core or a Crypto operation and based on that check if the provider can do it or not.

unwrap_or_else_return!(app_name.ok_or(ResponseStatus::NotAuthenticated));
let result =
unwrap_or_else_return!(self.provider.list_keys(app_name, op_list_keys));
trace!("list_keys egress");
self.result_to_response(NativeResult::ListKeys(result), header)
}
NativeOperation::PsaHashCompute(op_hash_compute) => {
let _app_name =
unwrap_or_else_return!(app_name.ok_or(ResponseStatus::NotAuthenticated));
Expand Down
37 changes: 37 additions & 0 deletions src/key_info_managers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,40 @@ pub trait ManageKeyInfo {
/// Returns an error as a String if there was a problem accessing the Key Info Manager.
fn exists(&self, key_triple: &KeyTriple) -> Result<bool, String>;
}

/// Returns a Vec of the KeyInfo objects corresponding to the given application name and
/// provider ID.
///
/// # Errors
///
/// Returns an error as a String if there was a problem accessing the Key Info Manager.
pub fn list_keys(
manager: &dyn ManageKeyInfo,
app_name: &ApplicationName,
provider_id: ProviderID,
) -> Result<Vec<parsec_interface::operations::list_keys::KeyInfo>, String> {
use parsec_interface::operations::list_keys::KeyInfo;

let mut keys: Vec<KeyInfo> = Vec::new();
let key_triples = manager.get_all(provider_id)?;

for key_triple in key_triples {
if key_triple.app_name() != app_name {
continue;
}

let key_info = manager.get(key_triple)?;
let key_info = match key_info {
Some(key_info) => key_info,
_ => continue,
};

keys.push(KeyInfo {
provider_id: ProviderID::MbedCrypto,
name: key_triple.key_name().to_string(),
attributes: key_info.attributes,
});
}

Ok(keys)
}
Comment on lines +155 to +184
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can actually include this in the ManageKeyInfo trait, because you only rely on methods on manager that are exposed through that trait. You'd just change manager.get<all> with self.get<all>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why I did not include it is to prevent overwriting this method. I think the result is the same?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to prevent overwriting this method

To prevent overwriting what?

I think the result is the same?

It is, in the end, I just tend to prefer methods to functions when the function is clearly acting on a specific type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I might be wrong but if a method is in a trait then implementation can either not implement this method and use the default implementation, or overwrite it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, true, but we have control over that. And it might be that that's an advantage, because for a database, for example, we might be able to build a query to get only the entries that match those requirements instead of filtering ourselves.

26 changes: 23 additions & 3 deletions src/providers/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
//! aiding clients in discovering the capabilities offered by their underlying
//! platform.
use super::Provide;
use crate::authenticators::ApplicationName;
use derivative::Derivative;
use log::trace;
use parsec_interface::operations::{list_authenticators, list_opcodes, list_providers, ping};
use parsec_interface::operations::{
list_authenticators::AuthenticatorInfo, list_providers::ProviderInfo,
list_authenticators, list_keys, list_opcodes, list_providers, ping,
};
use parsec_interface::operations::{
list_authenticators::AuthenticatorInfo, list_keys::KeyInfo, list_providers::ProviderInfo,
};
use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
use std::collections::{HashMap, HashSet};
Expand All @@ -20,11 +23,12 @@ use std::sync::Arc;
use uuid::Uuid;
use version::{version, Version};

const SUPPORTED_OPCODES: [Opcode; 4] = [
const SUPPORTED_OPCODES: [Opcode; 5] = [
Opcode::ListProviders,
Opcode::ListOpcodes,
Opcode::Ping,
Opcode::ListAuthenticators,
Opcode::ListKeys,
];

/// Service information provider
Expand Down Expand Up @@ -73,6 +77,22 @@ impl Provide for Provider {
})
}

fn list_keys(
&self,
app_name: ApplicationName,
_op: list_keys::Operation,
) -> Result<list_keys::Result> {
trace!("list_keys ingress");

let mut keys: Vec<KeyInfo> = Vec::new();
for provider in &self.prov_list {
let mut result = provider.list_keys(app_name.clone(), _op)?;
keys.append(&mut result.keys);
}

Ok(list_keys::Result { keys })
}

fn ping(&self, _op: ping::Operation) -> Result<ping::Result> {
trace!("ping ingress");
let result = ping::Result {
Expand Down
24 changes: 22 additions & 2 deletions src/providers/mbed_crypto/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
//! This provider is a software based implementation of PSA Crypto, Mbed Crypto.
use super::Provide;
use crate::authenticators::ApplicationName;
use crate::key_info_managers::{KeyTriple, ManageKeyInfo};
use crate::key_info_managers::{self, KeyTriple, ManageKeyInfo};
use derivative::Derivative;
use log::{error, trace};
use parsec_interface::operations::list_providers::ProviderInfo;
use parsec_interface::operations::{list_keys, list_providers::ProviderInfo};
use parsec_interface::operations::{
psa_aead_decrypt, psa_aead_encrypt, psa_asymmetric_decrypt, psa_asymmetric_encrypt,
psa_destroy_key, psa_export_key, psa_export_public_key, psa_generate_key, psa_generate_random,
Expand All @@ -19,6 +19,7 @@ use parsec_interface::requests::{Opcode, ProviderID, ResponseStatus, Result};
use psa_crypto::types::{key, status};
use std::collections::HashSet;
use std::io::{Error, ErrorKind};
use std::ops::Deref;
use std::sync::{
atomic::{AtomicU32, Ordering::Relaxed},
Arc, Mutex, RwLock,
Expand Down Expand Up @@ -161,6 +162,25 @@ impl Provide for Provider {
}, SUPPORTED_OPCODES.iter().copied().collect()))
}

fn list_keys(
&self,
app_name: ApplicationName,
_op: list_keys::Operation,
) -> Result<list_keys::Result> {
let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
Ok(list_keys::Result {
keys: key_info_managers::list_keys(
store_handle.deref(),
&app_name,
ProviderID::MbedCrypto,
)
.map_err(|e| {
format_error!("Error occurred when fetching key information", e);
ResponseStatus::KeyInfoManagerError
})?,
})
}

fn psa_generate_key(
&self,
app_name: ApplicationName,
Expand Down
16 changes: 13 additions & 3 deletions src/providers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ impl ProviderConfig {

use crate::authenticators::ApplicationName;
use parsec_interface::operations::{
list_authenticators, list_opcodes, list_providers, ping, psa_aead_decrypt, psa_aead_encrypt,
psa_asymmetric_decrypt, psa_asymmetric_encrypt, psa_destroy_key, psa_export_key,
psa_export_public_key, psa_generate_key, psa_generate_random, psa_hash_compare,
list_authenticators, list_keys, list_opcodes, list_providers, ping, psa_aead_decrypt,
psa_aead_encrypt, psa_asymmetric_decrypt, psa_asymmetric_encrypt, psa_destroy_key,
psa_export_key, psa_export_public_key, psa_generate_key, psa_generate_random, psa_hash_compare,
psa_hash_compute, psa_import_key, psa_raw_key_agreement, psa_sign_hash, psa_verify_hash,
};
use parsec_interface::requests::{ResponseStatus, Result};
Expand Down Expand Up @@ -132,6 +132,16 @@ pub trait Provide {
Err(ResponseStatus::PsaErrorNotSupported)
}

/// Lists all keys belonging to the application.
fn list_keys(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nicer to only have the list_keys method and:

  • in all the crypto providers make it the same as what find_keys currently does
  • in the code provider call list_keys on all provider

Although we need to be sure somewhere that ListKeys can not be called directly on the crypto providers. Probably somewhere higher on the stack.

&self,
_app_name: ApplicationName,
_op: list_keys::Operation,
) -> Result<list_keys::Result> {
trace!("list_keys ingress");
Err(ResponseStatus::PsaErrorNotSupported)
}

/// Execute a Ping operation to get the wire protocol version major and minor information.
///
/// # Errors
Expand Down
20 changes: 18 additions & 2 deletions src/providers/pkcs11/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
//! through the Parsec interface.
use super::Provide;
use crate::authenticators::ApplicationName;
use crate::key_info_managers::{KeyInfo, KeyTriple, ManageKeyInfo};
use crate::key_info_managers::{self, KeyInfo, KeyTriple, ManageKeyInfo};
use derivative::Derivative;
use log::{error, info, trace, warn};
use parsec_interface::operations::list_providers::ProviderInfo;
use parsec_interface::operations::{list_keys, list_providers::ProviderInfo};
use parsec_interface::operations::{
psa_asymmetric_decrypt, psa_asymmetric_encrypt, psa_destroy_key, psa_export_public_key,
psa_generate_key, psa_import_key, psa_sign_hash, psa_verify_hash,
Expand All @@ -20,6 +20,7 @@ use pkcs11::types::{CKF_OS_LOCKING_OK, CK_C_INITIALIZE_ARGS, CK_SLOT_ID};
use pkcs11::Ctx;
use std::collections::HashSet;
use std::io::{Error, ErrorKind};
use std::ops::Deref;
use std::str::FromStr;
use std::sync::{Arc, Mutex, RwLock};
use utils::{KeyPairType, ReadWriteSession, Session};
Expand Down Expand Up @@ -212,6 +213,21 @@ impl Provide for Provider {
))
}

fn list_keys(
&self,
app_name: ApplicationName,
_op: list_keys::Operation,
) -> Result<list_keys::Result> {
let store_handle = self.key_info_store.read().expect("Key store lock poisoned");
Ok(list_keys::Result {
keys: key_info_managers::list_keys(store_handle.deref(), &app_name, ProviderID::Pkcs11)
.map_err(|e| {
format_error!("Error occurred when fetching key information", e);
ResponseStatus::KeyInfoManagerError
})?,
})
}

fn psa_generate_key(
&self,
app_name: ApplicationName,
Expand Down
Loading