Skip to content

Commit

Permalink
Deserialize plain DI certs as raw DER
Browse files Browse the repository at this point in the history
Because of the deserialize implementation that's automatically
generated, at this moment the expected value for the public_key_store
in the manufacturing server is a CBOR array of the DER certificate.
This commit adds a new type PlainBytes which (de)serializes
transparently, and makes the manufacturing server use it for the public
key store.

NOTE: this means that with this patch, the store format on disk changes.
This store is a ReadOnly (the server will never write to it), but if
anyone would've put a CBOR file in place, that will now fail to open.
Raw DER was always the intention (and documented) format, but it still
is a risk.

Signed-off-by: Patrick Uiterwijk <[email protected]>
Fixes: fdo-rs#477
  • Loading branch information
puiterwijk committed Sep 7, 2023
1 parent 0683461 commit abc9043
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 21 deletions.
2 changes: 1 addition & 1 deletion data-formats/src/cborparser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use thiserror::Error;

use crate::{
constants::HashType,
serializable::{DeserializableMany, MaybeSerializable},
serializable::{private::MaybeSerializable, DeserializableMany},
types::Hash,
Error, Serializable,
};
Expand Down
2 changes: 1 addition & 1 deletion data-formats/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub mod messages;

pub mod cborparser;

mod serializable;
pub mod serializable;
pub use serializable::DeserializableMany;
pub use serializable::Serializable;

Expand Down
2 changes: 1 addition & 1 deletion data-formats/src/ownershipvoucher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
constants::HashType,
errors::Result,
publickey::{PublicKey, X5Chain},
serializable::MaybeSerializable,
serializable::private::MaybeSerializable,
types::{COSESign, Guid, HMac, Hash, RendezvousInfo, UnverifiedValue},
DeserializableMany, Error, ProtocolVersion, Serializable,
};
Expand Down
64 changes: 49 additions & 15 deletions data-formats/src/serializable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,31 @@ pub trait Serializable {
W: std::io::Write;
}

pub trait MaybeSerializable: Serializable {
fn is_nodata_error(err: &Error) -> bool;
/// Some traits that are currently private, as we don't want to commit ourselves to them
pub(crate) mod private {
pub trait MaybeSerializable: super::Serializable {
fn is_nodata_error(err: &super::Error) -> bool;

fn maybe_deserialize_from_reader<R>(reader: R) -> Result<Option<Self>, Error>
where
Self: Sized,
R: std::io::Read,
{
match Self::deserialize_from_reader(reader) {
Ok(value) => Ok(Some(value)),
Err(err) => {
if Self::is_nodata_error(&err) {
Ok(None)
} else {
Err(err)
fn maybe_deserialize_from_reader<R>(reader: R) -> Result<Option<Self>, super::Error>
where
Self: Sized,
R: std::io::Read,
{
match Self::deserialize_from_reader(reader) {
Ok(value) => Ok(Some(value)),
Err(err) => {
if Self::is_nodata_error(&err) {
Ok(None)
} else {
Err(err)
}
}
}
}
}
}

pub trait DeserializableMany: MaybeSerializable {
pub trait DeserializableMany: private::MaybeSerializable {
fn deserialize_many_from_reader<R>(mut reader: R) -> Result<Vec<Self>, Error>
where
Self: Sized,
Expand Down Expand Up @@ -85,3 +88,34 @@ where
ciborium::ser::into_writer(self, &mut writer).map_err(Error::from)
}
}

/// A structure that wraps a Vec<u8>, which (de)serializes its contents without
/// any data format. Can be used when some data needs to be read from a file
/// without expecting a CBOR array.
/// Note that the deserialize methods will read the entire contents of the buffer/reader.
/// Do *not* expect to be able to read any other data after using this on a reader.
#[derive(Debug, Clone)]
pub struct PlainBytes(pub Vec<u8>);

impl Serializable for PlainBytes {
fn deserialize_data(data: &[u8]) -> Result<Self, Error> {
Ok(PlainBytes(Vec::from(data)))
}

fn deserialize_from_reader<R>(mut reader: R) -> Result<Self, Error>
where
Self: Sized,
R: std::io::Read,
{
let mut buffer = Vec::new();
reader.read_to_end(&mut buffer).map_err(Error::from)?;
Ok(PlainBytes(buffer))
}

fn serialize_to_writer<W>(&self, mut writer: W) -> Result<(), Error>
where
W: std::io::Write,
{
writer.write_all(&self.0).map_err(Error::from)
}
}
3 changes: 2 additions & 1 deletion manufacturing-server/src/handlers/di.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ pub(crate) async fn app_start(
Some(store) => store
.load_data(&mfg_info.to_string())
.await
.map_err(Error::from_error::<messages::v11::di::AppStart, _>)?,
.map_err(Error::from_error::<messages::v11::di::AppStart, _>)?
.map(|d| d.0),
},
};
let public_key = match public_key {
Expand Down
6 changes: 4 additions & 2 deletions manufacturing-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use fdo_data_formats::{
constants::{KeyStorageType, MfgStringType, PublicKeyType, RendezvousVariable},
ownershipvoucher::OwnershipVoucher,
publickey::{PublicKey, X5Chain},
serializable::PlainBytes,
types::{Guid, RendezvousInfo},
ProtocolVersion,
};
Expand Down Expand Up @@ -62,8 +63,9 @@ struct ManufacturingServiceUD {
OwnershipVoucherStoreMetadataKey,
>,
>,
public_key_store:
Option<Box<dyn Store<fdo_store::ReadOnlyOpen, String, Vec<u8>, PublicKeyStoreMetadataKey>>>,
public_key_store: Option<
Box<dyn Store<fdo_store::ReadOnlyOpen, String, PlainBytes, PublicKeyStoreMetadataKey>>,
>,

// Certificates
manufacturer_cert: X509,
Expand Down

0 comments on commit abc9043

Please sign in to comment.