Skip to content
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

Add injection and derivation support #8

Closed
wants to merge 6 commits into from
Closed
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
5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,17 @@ delog = "0.1.6"
littlefs2 = "0.4.0"
serde-byte-array = "0.1.2"

# For hmacsha256p256
hmac = { version = "0.12", optional = true }
sha2 = { version = "0.10", default-features = false, optional = true }

[dev-dependencies]
trussed = { version = "0.1.0", default-features = false, features = ["serde-extensions", "virt"] }

[features]
default = []

hmacsha256p256 = ["hmac", "sha2"]
wrap-key-to-file = ["chacha20poly1305"]
chunked = []
encrypted-chunked = ["chunked", "chacha20poly1305/stream"]
Expand Down
241 changes: 241 additions & 0 deletions src/hmacsha256p256/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
// Copyright (C) Nitrokey GmbH
// SPDX-License-Identifier: Apache-2.0 or MIT

use serde::{Deserialize, Serialize};
use trussed::types::Message;
use trussed::{
client::ClientError,
key::{self, Kind},
serde_extensions::{Extension, ExtensionClient, ExtensionImpl, ExtensionResult},
service::{Keystore, ServiceResources},
types::{Bytes, CoreContext, KeyId, Location, Mechanism},
Error,
};

#[derive(Debug, Default)]
pub struct HmacSha256P256Extension;

#[derive(Debug, Deserialize, Serialize)]
#[allow(missing_docs)]
pub enum HmacSha256P256Request {
DeriveFromHash(request::DeriveFromHash),
InjectAnyKey(request::InjectAnyKey),
}

mod request {
use super::*;
use serde::{Deserialize, Serialize};
use trussed::types::{KeyId, Location, Mechanism, Message};
use trussed::Error;

#[derive(Debug, Deserialize, Serialize)]
pub struct DeriveFromHash {
pub mechanism: Mechanism,
pub key: KeyId,
pub location: Location,
pub data: Option<Message>,
}

impl TryFrom<HmacSha256P256Request> for DeriveFromHash {
type Error = Error;
fn try_from(request: HmacSha256P256Request) -> Result<Self, Self::Error> {
match request {
HmacSha256P256Request::DeriveFromHash(request) => Ok(request),
_ => Err(Error::InternalError),
}
}
}

impl From<DeriveFromHash> for HmacSha256P256Request {
fn from(request: DeriveFromHash) -> Self {
Self::DeriveFromHash(request)
}
}

#[derive(Debug, Deserialize, Serialize)]
pub struct InjectAnyKey {
pub location: Location,
pub kind: Kind,
// pub raw_key: SerializedKey,
pub raw_key: Message,
}

impl TryFrom<HmacSha256P256Request> for InjectAnyKey {
type Error = Error;
fn try_from(request: HmacSha256P256Request) -> Result<Self, Self::Error> {
match request {
HmacSha256P256Request::InjectAnyKey(request) => Ok(request),
_ => Err(Error::InternalError),
}
}
}

impl From<InjectAnyKey> for HmacSha256P256Request {
fn from(request: InjectAnyKey) -> Self {
Self::InjectAnyKey(request)
}
}
}

#[derive(Debug, Deserialize, Serialize)]
#[allow(missing_docs)]
pub enum HmacSha256P256Reply {
DeriveFromHash(reply::DeriveFromHash),
InjectAnyKey(reply::InjectAnyKey),
}

mod reply {
use serde::{Deserialize, Serialize};
use trussed::{types::KeyId, Error};

use super::*;

#[derive(Debug, Deserialize, Serialize)]
#[non_exhaustive]
pub struct DeriveFromHash {
pub key: Option<KeyId>,
}

impl TryFrom<HmacSha256P256Reply> for DeriveFromHash {
type Error = Error;
fn try_from(reply: HmacSha256P256Reply) -> Result<Self, Self::Error> {
match reply {
HmacSha256P256Reply::DeriveFromHash(reply) => Ok(reply),
_ => Err(Error::InternalError),
}
}
}

impl From<DeriveFromHash> for HmacSha256P256Reply {
fn from(reply: DeriveFromHash) -> Self {
Self::DeriveFromHash(reply)
}
}

#[derive(Debug, Deserialize, Serialize)]
pub struct InjectAnyKey {
pub key: Option<KeyId>,
}

impl TryFrom<HmacSha256P256Reply> for InjectAnyKey {
type Error = Error;
fn try_from(reply: HmacSha256P256Reply) -> Result<Self, Self::Error> {
match reply {
HmacSha256P256Reply::InjectAnyKey(reply) => Ok(reply),
_ => Err(Error::InternalError),
}
}
}

impl From<InjectAnyKey> for HmacSha256P256Reply {
fn from(reply: InjectAnyKey) -> Self {
Self::InjectAnyKey(reply)
}
}
}

impl Extension for HmacSha256P256Extension {
type Request = HmacSha256P256Request;
type Reply = HmacSha256P256Reply;
}

pub fn derive_key_from_hash(
keystore: &mut impl Keystore,
request: &request::DeriveFromHash,
) -> Result<reply::DeriveFromHash, Error> {
use hmac::{Hmac, Mac};
type HmacSha256P256 = Hmac<sha2::Sha256>;

let key_id = request.key;
let key = keystore.load_key(key::Secrecy::Secret, None, &key_id)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please verify that the key kind is a Symmetric key.

Copy link
Author

Choose a reason for hiding this comment

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

Done, but had to revert. Will check this again later.

let shared_secret = key.material;

let mut mac =
HmacSha256P256::new_from_slice(shared_secret.as_ref()).map_err(|_| Error::InternalError)?;

if let Some(data) = &request.data {
mac.update(data);
}
let derived_key: [u8; 32] = mac
.finalize()
.into_bytes()
.try_into()
.map_err(|_| Error::InternalError)?;
let key_id = keystore.store_key(
request.location,
key::Secrecy::Secret,
key::Kind::P256, // TODO use mechanism/kind from the request
&derived_key,
)?;
Ok(reply::DeriveFromHash { key: Some(key_id) })
Comment on lines +159 to +170
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure that this necessarily creates a key that is valid?

The key generation is a bit more complex than just using random bytes. This is the procedure we currently use in trussed: https://docs.rs/p256-cortex-m4/0.1.0-alpha.6/src/p256_cortex_m4/cortex_m4.rs.html#81-93.

Especially, the private key has to be within the $[1; n-1]$ where $n$ is the prime order of P-256.

Copy link
Author

Choose a reason for hiding this comment

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

That's a correct observation - there are no checks for key validation as of now. I was thinking about adding the same checks like done in the linked implementation, but I am gravitating now more towards using curve25519 instead, and remove the p256 support for derivation, unless proved to be safe to use in this way.
I've just noticed there is no ticket for that - to be done soon.
Connected: trussed-dev/trussed#44 (comment)

Copy link
Collaborator

@sosthene-nitrokey sosthene-nitrokey Jun 29, 2023

Choose a reason for hiding this comment

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

For EdDSA this will work but for X25519 (ECDH) there is the same issue, except the constraints on the scalar are even worse.

Copy link
Collaborator

Choose a reason for hiding this comment

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

One solution could be to simply use the result of the hash to seed a ChaCha8Rng and use it to generate the key. We would need it to commit to having the key generation algorithm be stable so that the same RNG always gives the same key.

}

pub fn inject_any_key(
keystore: &mut impl Keystore,
request: &request::InjectAnyKey,
) -> Result<reply::InjectAnyKey, Error> {
let key_id = keystore.store_key(
request.location,
key::Secrecy::Secret,
request.kind,
&request.raw_key,
)?;

Ok(reply::InjectAnyKey { key: Some(key_id) })
}
Comment on lines +173 to +185
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this syscall bring compared to the existing unsafe_inject_key syscall?

Copy link
Author

@szszszsz szszszsz Jun 29, 2023

Choose a reason for hiding this comment

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

I forgot to check opcard's implementation in this regard. There are no advantages of using this extension over Trussed's currently AFAIK. In the future supporting other kinds like curve25519 could be handy.


impl ExtensionImpl<HmacSha256P256Extension> for super::StagingBackend {
fn extension_request<P: trussed::Platform>(
&mut self,
core_ctx: &mut CoreContext,
_backend_ctx: &mut Self::Context,
request: &HmacSha256P256Request,
resources: &mut ServiceResources<P>,
) -> Result<HmacSha256P256Reply, Error> {
let keystore = &mut resources.keystore(core_ctx)?;
match request {
HmacSha256P256Request::DeriveFromHash(request) => {
derive_key_from_hash(keystore, request).map(Into::into)
}
HmacSha256P256Request::InjectAnyKey(request) => {
inject_any_key(keystore, request).map(Into::into)
}
}
}
}

type HmacSha256P256Result<'a, R, C> = ExtensionResult<'a, HmacSha256P256Extension, R, C>;

pub trait HmacSha256P256Client: ExtensionClient<HmacSha256P256Extension> {
fn derive_from_hash(
&mut self,
mechanism: Mechanism,
key: KeyId,
location: Location,
data: &[u8],
) -> HmacSha256P256Result<'_, reply::DeriveFromHash, Self> {
let data = Bytes::from_slice(data).map_err(|_| ClientError::DataTooLarge)?;
self.extension(request::DeriveFromHash {
mechanism,
key,
location,
data: Some(data),
})
}

fn inject_any_key(
&mut self,
// raw_key: SerializedKey,
raw_key: Message,
location: Location,
kind: Kind,
) -> HmacSha256P256Result<'_, reply::InjectAnyKey, Self> {
self.extension(request::InjectAnyKey {
location,
kind,
raw_key,
})
}
}

impl<C: ExtensionClient<HmacSha256P256Extension>> HmacSha256P256Client for C {}
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ pub mod wrap_key_to_file;
#[cfg(feature = "chunked")]
pub mod streaming;

#[cfg(feature = "hmacsha256p256")]
pub mod hmacsha256p256;

#[derive(Clone, Debug, Default)]
#[non_exhaustive]
pub struct StagingBackend {}
Expand Down
25 changes: 25 additions & 0 deletions src/virt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ use crate::{StagingBackend, StagingContext};
#[cfg(feature = "chunked")]
use crate::streaming::ChunkedExtension;

#[cfg(feature = "hmacsha256p256")]
use crate::hmacsha256p256::HmacSha256P256Extension;

#[derive(Default, Debug)]
pub struct Dispatcher {
backend: StagingBackend,
Expand All @@ -27,6 +30,14 @@ pub enum ExtensionIds {
WrapKeyToFile,
#[cfg(feature = "chunked")]
Chunked,
#[cfg(feature = "hmacsha256p256")]
HmacShaP256,
}

#[cfg(feature = "hmacsha256p256")]
impl ExtensionId<HmacSha256P256Extension> for Dispatcher {
type Id = ExtensionIds;
const ID: ExtensionIds = ExtensionIds::HmacShaP256;
}

#[cfg(feature = "wrap-key-to-file")]
Expand All @@ -48,6 +59,8 @@ impl From<ExtensionIds> for u8 {
ExtensionIds::WrapKeyToFile => 0,
#[cfg(feature = "chunked")]
ExtensionIds::Chunked => 1,
#[cfg(feature = "hmacsha256p256")]
ExtensionIds::HmacShaP256 => 2,
}
}
}
Expand All @@ -60,6 +73,8 @@ impl TryFrom<u8> for ExtensionIds {
0 => Ok(Self::WrapKeyToFile),
#[cfg(feature = "chunked")]
1 => Ok(Self::Chunked),
#[cfg(feature = "hmacsha256p256")]
2 => Ok(Self::HmacShaP256),
_ => Err(Error::FunctionNotSupported),
}
}
Expand Down Expand Up @@ -105,6 +120,16 @@ impl ExtensionDispatch for Dispatcher {
request,
resources,
),
#[cfg(feature = "hmacsha256p256")]
ExtensionIds::HmacShaP256 => <StagingBackend as ExtensionImpl<
HmacSha256P256Extension,
>>::extension_request_serialized(
&mut self.backend,
&mut ctx.core,
&mut ctx.backends,
request,
resources,
),
#[cfg(feature = "chunked")]
ExtensionIds::Chunked => {
<StagingBackend as ExtensionImpl<ChunkedExtension>>::extension_request_serialized(
Expand Down
38 changes: 38 additions & 0 deletions tests/hmacsha256p256.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (C) Nitrokey GmbH
// SPDX-License-Identifier: Apache-2.0 or MIT

#![cfg(all(feature = "virt", feature = "hmacsha256p256"))]

use trussed::client::CryptoClient;
use trussed::key::Kind;
use trussed::syscall;
use trussed::types::{Location::*, Mechanism, SignatureSerialization};

use trussed::types::Location;

use trussed_staging::virt::with_ram_client;

use trussed::client::P256;
use trussed_staging::hmacsha256p256::HmacSha256P256Client;

#[test]
fn hmac_inject_any() {
use trussed::types::Message;
with_ram_client("staging-tests", |mut client| {
let client = &mut client;

let key = syscall!(client.inject_any_key(
Message::from_slice(b"12345678123456781234567812345678").unwrap(),
Volatile,
Kind::P256
))
.key
.unwrap();

let _pk = syscall!(client.derive_p256_public_key(key, Location::Volatile)).key;

let signature =
syscall!(client.sign(Mechanism::P256, key, &[], SignatureSerialization::Raw)).signature;
assert!(signature.len() > 0);
});
}