Skip to content

Commit

Permalink
Add tests for operations edge cases
Browse files Browse the repository at this point in the history 
Adds many tests to make sure that the operations API is respected. Fixes
in the service the parts that are not compliant.
Also fixes parallaxsecond#139

Signed-off-by: Hugues de Valon <[email protected]>
  • Loading branch information
@hug-dev
hug-dev committed Apr 16, 2020
1 parent b595ff7 commit 2968fde
Show file tree
Hide file tree
Showing 13 changed files with 752 additions and 90 deletions.
80 changes: 78 additions & 2 deletions Cargo.lock
View file

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

6 changes: 5 additions & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ derivative = "1.0.3"
version = "3.0.0"

[dev-dependencies]
parsec-client-test = { git = "https://github.com/parallaxsecond/parsec-client-test", tag = "0.2.2" }
parsec-client-test = { git = "https://github.com/parallaxsecond/parsec-client-test", tag = "0.3.0" }
num_cpus = "1.10.1"
picky-asn1-der = "0.2.2"
picky-asn1 = "0.2.1"
serde = { version = "1.0", features = ["derive"] }
sha2 = "0.8.1"

[build-dependencies]
bindgen = "0.50.0"
Expand Down
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ This project uses the following third party crates:
* flexi_logger (MIT and Apache-2.0)
* lazy_static (MIT and Apache-2.0)
* version (MIT and Apache-2.0)
* sha2 (MIT and Apache-2.0)

This project uses the following third party libraries:
* [**Mbed Crypto**](https://github.com/ARMmbed/mbed-crypto) (Apache-2.0)
22 changes: 20 additions & 2 deletions src/providers/mbed_provider/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,16 @@ impl Provide for MbedProvider {
&mut local_ids_handle,
)?;

let key_attrs = utils::convert_key_attributes(&key_attributes, key_id)?;
let key_attrs = utils::convert_key_attributes(&key_attributes, key_id).or_else(|e| {
remove_key_id(
&key_triple,
key_id,
&mut *store_handle,
&mut local_ids_handle,
)?;
error!("Failed converting key attributes.");
Err(e)
})?;

let _guard = self
.key_handle_mutex
Expand Down Expand Up @@ -331,7 +340,16 @@ impl Provide for MbedProvider {
&mut local_ids_handle,
)?;

let key_attrs = utils::convert_key_attributes(&key_attributes, key_id)?;
let key_attrs = utils::convert_key_attributes(&key_attributes, key_id).or_else(|e| {
remove_key_id(
&key_triple,
key_id,
&mut *store_handle,
&mut local_ids_handle,
)?;
error!("Failed converting key attributes.");
Err(e)
})?;

let _guard = self
.key_handle_mutex
Expand Down
4 changes: 2 additions & 2 deletions src/providers/mbed_provider/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@ pub fn convert_key_usage(operation: &psa_key_attributes::UsageFlags) -> psa_key_
usage |= PSA_KEY_USAGE_EXPORT;
}

if operation.sign_message || operation.sign_hash {
if operation.sign_message && operation.sign_hash {
usage |= PSA_KEY_USAGE_SIGN;
}

if operation.verify_message || operation.verify_hash {
if operation.verify_message && operation.verify_hash {
usage |= PSA_KEY_USAGE_VERIFY;
}

Expand Down
53 changes: 38 additions & 15 deletions src/providers/pkcs11_provider/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,11 @@ impl Provide for Pkcs11Provider {

let modulus_object = &public_key.modulus.as_unsigned_bytes_be();
let exponent_object = &public_key.public_exponent.as_unsigned_bytes_be();
let key_bits = key_attributes.key_bits;
if key_bits != 0 && modulus_object.len() * 8 != key_bits as usize {
error!("If the key_bits field is non-zero (value is {}) it must be equal to the size of the key in data.", key_attributes.key_bits);
return Err(ResponseStatus::PsaErrorInvalidArgument);
}

template.push(
CK_ATTRIBUTE::new(pkcs11::types::CKA_CLASS)
Expand Down Expand Up @@ -711,9 +716,7 @@ impl Provide for Pkcs11Provider {
let key_name = op.key_name;
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, key_name);
let store_handle = self.key_id_store.read().expect("Key store lock poisoned");
let (key_id, key_attributes) = get_key_id(&key_triple, &*store_handle)?;

key_attributes.can_export()?;
let (key_id, _key_attributes) = get_key_id(&key_triple, &*store_handle)?;

let session = Session::new(self, ReadWriteSession::ReadOnly)?;
info!(
Expand Down Expand Up @@ -869,8 +872,8 @@ impl Provide for Pkcs11Provider {
info!("Pkcs11 Provider - Asym Sign");

let key_name = op.key_name;
let alg = op.alg;
let mut hash = op.hash;
let alg = op.alg;
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, key_name);
let store_handle = self.key_id_store.read().expect("Key store lock poisoned");
let (key_id, key_attributes) = get_key_id(&key_triple, &*store_handle)?;
Expand All @@ -890,17 +893,27 @@ impl Provide for Pkcs11Provider {
}
}

if alg
!= (AsymmetricSignature::RsaPkcs1v15Sign {
hash_alg: Hash::Sha256,
})
{
error!(
"The PKCS 11 provider currently only supports signature algorithm to be RSA PKCS#1 v1.5 and the text hashed with SHA-256.");
return Err(ResponseStatus::PsaErrorNotSupported);
}

if hash.len() != 32 {
error!("The SHA-256 hash must be 32 bytes long.");
return Err(ResponseStatus::PsaErrorInvalidArgument);
}

let mech = CK_MECHANISM {
mechanism: pkcs11::types::CKM_RSA_PKCS,
pParameter: std::ptr::null_mut(),
ulParameterLen: 0,
};

if hash.len() != 32 {
error!("The PKCS11 provider currently only supports 256 bits long digests.");
return Err(ResponseStatus::PsaErrorNotSupported);
}

let session = Session::new(self, ReadWriteSession::ReadWrite)?;
info!("Asymmetric sign in session {}", session.session_handle());

Expand Down Expand Up @@ -946,9 +959,9 @@ impl Provide for Pkcs11Provider {
info!("Pkcs11 Provider - Asym Verify");

let key_name = op.key_name;
let alg = op.alg;
let mut hash = op.hash;
let signature = op.signature;
let alg = op.alg;
let key_triple = KeyTriple::new(app_name, ProviderID::Pkcs11, key_name);
let store_handle = self.key_id_store.read().expect("Key store lock poisoned");
let (key_id, key_attributes) = get_key_id(&key_triple, &*store_handle)?;
Expand All @@ -968,18 +981,28 @@ impl Provide for Pkcs11Provider {
}
}

if alg
!= (AsymmetricSignature::RsaPkcs1v15Sign {
hash_alg: Hash::Sha256,
})
{
error!(
"The PKCS 11 provider currently only supports signature algorithm to be RSA PKCS#1 v1.5 and the text hashed with SHA-256.");
return Err(ResponseStatus::PsaErrorNotSupported);
}

if hash.len() != 32 {
error!("The SHA-256 hash must be 32 bytes long.");
return Err(ResponseStatus::PsaErrorInvalidArgument);
}

let mech = CK_MECHANISM {
// Verify without hashing.
mechanism: pkcs11::types::CKM_RSA_PKCS,
pParameter: std::ptr::null_mut(),
ulParameterLen: 0,
};

if hash.len() != 32 {
error!("The PKCS11 provider currently only supports 256 bits long digests.");
return Err(ResponseStatus::PsaErrorNotSupported);
}

let session = Session::new(self, ReadWriteSession::ReadWrite)?;
info!("Asymmetric verify in session {}", session.session_handle());

Expand Down
42 changes: 32 additions & 10 deletions src/providers/tpm_provider/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,14 @@ impl Provide for TpmProvider {
return Err(ResponseStatus::PsaErrorNotSupported);
}
let key_data = public_key.modulus.as_unsigned_bytes_be();

let len = key_data.len();

let key_bits = attributes.key_bits;
if key_bits != 0 && len * 8 != key_bits as usize {
error!("If the key_bits field is non-zero (value is {}) it must be equal to the size of the key in data.", attributes.key_bits);
return Err(ResponseStatus::PsaErrorInvalidArgument);
}

if len != 128 && len != 256 {
error!(
"The TPM provider only supports 1024 and 2048 bits RSA public keys ({} bits given).",
Expand Down Expand Up @@ -295,9 +301,7 @@ impl Provide for TpmProvider {
.lock()
.expect("ESAPI Context lock poisoned");

let (password_context, key_attributes) = get_password_context(&*store_handle, key_triple)?;

key_attributes.can_export()?;
let (password_context, _key_attributes) = get_password_context(&*store_handle, key_triple)?;

let pub_key_data = esapi_context
.read_public_key(password_context.context)
Expand Down Expand Up @@ -361,12 +365,21 @@ impl Provide for TpmProvider {
.lock()
.expect("ESAPI Context lock poisoned");

let len = hash.len();
if len > 64 {
error!("The buffer given to sign is too big. Its length is {} and maximum authorised in the TPM provider is 64.", len);
if alg
!= (AsymmetricSignature::RsaPkcs1v15Sign {
hash_alg: Hash::Sha256,
})
{
error!(
"The TPM provider currently only supports signature algorithm to be RSA PKCS#1 v1.5 and the text hashed with SHA-256.");
return Err(ResponseStatus::PsaErrorNotSupported);
}

if hash.len() != 32 {
error!("The SHA-256 hash must be 32 bytes long.");
return Err(ResponseStatus::PsaErrorInvalidArgument);
}

let (password_context, key_attributes) = get_password_context(&*store_handle, key_triple)?;

key_attributes.can_sign_hash()?;
Expand Down Expand Up @@ -417,12 +430,21 @@ impl Provide for TpmProvider {
.lock()
.expect("ESAPI Context lock poisoned");

let len = hash.len();
if len > 64 {
error!("The buffer given to sign is too big. Its length is {} and maximum authorised is 64 in the TPM provider.", len);
if alg
!= (AsymmetricSignature::RsaPkcs1v15Sign {
hash_alg: Hash::Sha256,
})
{
error!(
"The TPM provider currently only supports signature algorithm to be RSA PKCS#1 v1.5 and the text hashed with SHA-256.");
return Err(ResponseStatus::PsaErrorNotSupported);
}

if hash.len() != 32 {
error!("The SHA-256 hash must be 32 bytes long.");
return Err(ResponseStatus::PsaErrorInvalidArgument);
}

let signature = Signature {
scheme: AsymSchemeUnion::RSASSA(TPM2_ALG_SHA256),
signature,
Expand Down
Loading

0 comments on commit 2968fde

Please sign in to comment.