Skip to content

Added macro calls for sign output size and export key buffer size #31

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
Jun 23, 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
4 changes: 3 additions & 1 deletion ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ fi
#############
# Run tests #
#############
RUST_BACKTRACE=1 cargo test
RUST_BACKTRACE=1 cargo test -- --test-threads=1

# Remove mbedtls directory if it exists
rm -rf psa-crypto/mbedtls
################################
# Check feature configurations #
################################
Expand Down
18 changes: 18 additions & 0 deletions psa-crypto-sys/src/c/shim.c
Original file line number Diff line number Diff line change
Expand Up @@ -236,3 +236,21 @@ shim_PSA_KEY_TYPE_DH_PUBLIC_KEY(psa_dh_group_t group)
{
return PSA_KEY_TYPE_DH_PUBLIC_KEY(group);
}

psa_key_type_t
shim_PSA_KEY_TYPE_PUBLIC_KEY_OF_KEY_PAIR(psa_key_type_t key_type)
{
return PSA_KEY_TYPE_PUBLIC_KEY_OF_KEY_PAIR(key_type);
}

size_t
shim_PSA_SIGN_OUTPUT_SIZE(psa_key_type_t key_type, size_t key_bits, psa_algorithm_t alg)
{
return PSA_SIGN_OUTPUT_SIZE(key_type, key_bits, alg);
}

size_t
shim_PSA_KEY_EXPORT_MAX_SIZE(psa_key_type_t key_type, size_t key_bits)
{
return PSA_KEY_EXPORT_MAX_SIZE(key_type, key_bits);
}
3 changes: 3 additions & 0 deletions psa-crypto-sys/src/c/shim.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,6 @@ psa_key_type_t shim_PSA_KEY_TYPE_ECC_KEY_PAIR(psa_ecc_curve_t curve);
psa_key_type_t shim_PSA_KEY_TYPE_ECC_PUBLIC_KEY(psa_ecc_curve_t curve);
psa_key_type_t shim_PSA_KEY_TYPE_DH_KEY_PAIR(psa_dh_group_t group);
psa_key_type_t shim_PSA_KEY_TYPE_DH_PUBLIC_KEY(psa_dh_group_t group);
psa_key_type_t shim_PSA_KEY_TYPE_PUBLIC_KEY_OF_KEY_PAIR(psa_key_type_t key_type);
size_t shim_PSA_SIGN_OUTPUT_SIZE(psa_key_type_t key_type, size_t key_bits, psa_algorithm_t alg);
size_t shim_PSA_KEY_EXPORT_MAX_SIZE(psa_key_type_t key_type, size_t key_bits);
16 changes: 16 additions & 0 deletions psa-crypto-sys/src/shim_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,19 @@ pub fn PSA_KEY_TYPE_DH_KEY_PAIR(group: psa_dh_group_t) -> psa_key_type_t {
pub fn PSA_KEY_TYPE_DH_PUBLIC_KEY(group: psa_dh_group_t) -> psa_key_type_t {
unsafe { psa_crypto_binding::shim_PSA_KEY_TYPE_DH_PUBLIC_KEY(group) }
}

pub unsafe fn PSA_KEY_TYPE_PUBLIC_KEY_OF_KEY_PAIR(key_type: psa_key_type_t) -> psa_key_type_t {
psa_crypto_binding::shim_PSA_KEY_TYPE_PUBLIC_KEY_OF_KEY_PAIR(key_type)
}

pub unsafe fn PSA_SIGN_OUTPUT_SIZE(
key_type: psa_key_type_t,
key_bits: usize,
alg: psa_algorithm_t,
) -> usize {
psa_crypto_binding::shim_PSA_SIGN_OUTPUT_SIZE(key_type, key_bits, alg)
}

pub unsafe fn PSA_EXPORT_KEY_OUTPUT_SIZE(key_type: psa_key_type_t, key_bits: usize) -> usize {
psa_crypto_binding::shim_PSA_KEY_EXPORT_MAX_SIZE(key_type, key_bits)
}
14 changes: 9 additions & 5 deletions psa-crypto/src/operations/asym_signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,15 @@ use crate::types::status::{Result, Status};
/// # 0x94, 0x8E, 0x92, 0x50, 0x35, 0xC2, 0x8C, 0x5C, 0x3C, 0xCA, 0xFE, 0x18, 0xE8, 0x81, 0x37, 0x78,
/// # ];
/// psa_crypto::init().unwrap();
/// let mut signature = vec![0; 256];
/// let my_key = generate(attributes, None).unwrap();
/// let size = sign_hash(my_key,
/// AsymmetricSignature::RsaPkcs1v15Sign {
/// let alg = AsymmetricSignature::RsaPkcs1v15Sign {
/// hash_alg: Hash::Sha256.into(),
/// },
/// };
/// let buffer_size = attributes.sign_output_size(alg).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the update of the example 💯

/// let mut signature = vec![0; buffer_size];
///
/// let size = sign_hash(my_key,
/// alg,
/// &HASH,
/// &mut signature).unwrap();
/// signature.resize(size, 0);
Expand Down Expand Up @@ -112,10 +115,11 @@ pub fn sign_hash(
/// # 0x94, 0x8E, 0x92, 0x50, 0x35, 0xC2, 0x8C, 0x5C, 0x3C, 0xCA, 0xFE, 0x18, 0xE8, 0x81, 0x37, 0x78,
/// # ];
/// psa_crypto::init().unwrap();
/// let mut signature = vec![0; 256];
/// let alg = AsymmetricSignature::RsaPkcs1v15Sign {
/// hash_alg: Hash::Sha256.into(),
/// };
/// let buffer_size = attributes.sign_output_size(alg).unwrap();
/// let mut signature = vec![0; buffer_size];
/// let my_key = generate(attributes, None).unwrap();
/// let size = sign_hash(my_key,
/// alg,
Expand Down
3 changes: 2 additions & 1 deletion psa-crypto/src/operations/key_management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ pub fn import(attributes: Attributes, id: Option<u32>, data: &[u8]) -> Result<Id
/// # },
/// # };
/// psa_crypto::init().unwrap();
/// let mut data = vec![0; 256];
/// let buffer_size = attributes.export_public_key_output_size().unwrap();
/// let mut data = vec![0; buffer_size];
/// let my_key = key_management::generate(attributes, None).unwrap();
/// let size = key_management::export_public(my_key, &mut data).unwrap();
/// data.resize(size, 0);
Expand Down
67 changes: 67 additions & 0 deletions psa-crypto/src/types/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#![allow(deprecated)]
#[cfg(feature = "with-mbed-crypto")]
use crate::initialized;
#[cfg(feature = "with-mbed-crypto")]
use crate::types::algorithm::AsymmetricSignature;
use crate::types::algorithm::{Algorithm, Cipher};
#[cfg(feature = "with-mbed-crypto")]
use crate::types::status::Status;
Expand Down Expand Up @@ -305,6 +307,50 @@ impl Attributes {
get_attributes_res?;
Ok(attributes?)
}

/// Sufficient size for a buffer to export the key, if supported
#[cfg(feature = "with-mbed-crypto")]
pub fn export_key_output_size(self) -> Result<usize> {
Attributes::export_key_output_size_base(self.key_type, self.bits)
}

/// Sufficient size for a buffer to export the public key, if supported
#[cfg(feature = "with-mbed-crypto")]
pub fn export_public_key_output_size(self) -> Result<usize> {
match self.key_type {
Copy link
Member

Choose a reason for hiding this comment

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

The specification says that this macro allows for key_type:

key_type A public key or key pair key type.

So I think you can add DH both for key pair and public key as a valid type as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is DH the same as DSA? I've been looking at this to try and make sure its not possible to pass it something invalid.

Copy link
Member

Choose a reason for hiding this comment

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

Is DH the same as DSA?

No, Diffie-Hellman is a type of key exchange, Digital Signature Algorithm is (as its name says) an asymmetric signature algorithm.

For DH public keys, the spec dictates:

For Diffie-Hellman key exchange public keys, with key types for which PSA_KEY_TYPE_IS_DH_PUBLIC_KEY is true, the format is the representation of the public key y = g^x mod p as a big-endian byte string. The length of the byte string is the length of the base prime p in bytes.

Which brings me to another realisation - that method there, export_key_output_size is improperly named, as it refers only to public keys, not to keys in general.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see, you're using the spec-provided macros, I think that's alright (although, will those disappear in the future?). But we do have to make sure we document whether this method refers to the full key or just the public part

Type::RsaKeyPair
| Type::RsaPublicKey
| Type::EccKeyPair { .. }
| Type::EccPublicKey { .. }
| Type::DhKeyPair { .. }
| Type::DhPublicKey { .. } => {
let pub_type = self.key_type.key_type_public_key_of_key_pair()?;
Attributes::export_key_output_size_base(pub_type, self.bits)
Comment on lines +327 to +328
Copy link
Member

Choose a reason for hiding this comment

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

That's clean!

}
_ => Err(Error::InvalidArgument),
}
}

/// Sufficient size for a buffer to export the given key type, if supported
#[cfg(feature = "with-mbed-crypto")]
fn export_key_output_size_base(key_type: Type, bits: usize) -> Result<usize> {
let size =
unsafe { psa_crypto_sys::PSA_EXPORT_KEY_OUTPUT_SIZE(key_type.try_into()?, bits) };
if size > 0 {
Ok(size)
} else {
Err(Error::NotSupported)
}
Comment on lines +337 to +343
Copy link
Member

Choose a reason for hiding this comment

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

Same as what Hugues said, this works fine, no need to change, just for reference:

match unsafe { 
    psa_crypto_sys::PSA_EXPORT_KEY_OUTPUT_SIZE(key_type.try_into()?, bits) 
  } {
    0 => Err(Error::NotSupported),
    val => Ok(val),
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, I was sure there would be a more succinct way of writing that bit, didn't think of doing it like your example!

}

/// Sufficient buffer size for a signature using the given key, if the key is supported
#[cfg(feature = "with-mbed-crypto")]
pub fn sign_output_size(self, alg: AsymmetricSignature) -> Result<usize> {
self.compatible_with_alg(Algorithm::AsymmetricSignature(alg))?;
Copy link
Member

Choose a reason for hiding this comment

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

This is perfectly fine like this (no need to change), just showing for reference that you can also do:

self.compatible_with_alg(alg.into())?;

Ok(unsafe {
psa_crypto_sys::PSA_SIGN_OUTPUT_SIZE(self.key_type.try_into()?, self.bits, alg.into())
})
}
}

/// The lifetime of a key indicates where it is stored and which application and system actions
Expand Down Expand Up @@ -407,6 +453,27 @@ impl Type {
_ => false,
}
}

/// If key is public or key pair, returns the corresponding public key type.
#[cfg(feature = "with-mbed-crypto")]
pub fn key_type_public_key_of_key_pair(self) -> Result<Type> {
match self {
Type::RsaKeyPair
| Type::RsaPublicKey
| Type::EccKeyPair { .. }
| Type::EccPublicKey { .. }
| Type::DhKeyPair { .. }
| Type::DhPublicKey { .. } => {
Ok(
unsafe {
psa_crypto_sys::PSA_KEY_TYPE_PUBLIC_KEY_OF_KEY_PAIR(self.try_into()?)
}
.try_into()?,
)
}
_ => Err(Error::InvalidArgument),
}
}
}

/// Enumeration of elliptic curve families supported. They are needed to create an ECC key.
Expand Down