From abc9043a699a39d0e5548ac58be6d5c820d86a5a Mon Sep 17 00:00:00 2001 From: Patrick Uiterwijk Date: Thu, 7 Sep 2023 14:14:01 +0000 Subject: [PATCH] Deserialize plain DI certs as raw DER 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 Fixes: #477 --- data-formats/src/cborparser.rs | 2 +- data-formats/src/lib.rs | 2 +- data-formats/src/ownershipvoucher.rs | 2 +- data-formats/src/serializable.rs | 64 +++++++++++++++++++------ manufacturing-server/src/handlers/di.rs | 3 +- manufacturing-server/src/main.rs | 6 ++- 6 files changed, 58 insertions(+), 21 deletions(-) diff --git a/data-formats/src/cborparser.rs b/data-formats/src/cborparser.rs index 409fd6ce8..a2dba851f 100644 --- a/data-formats/src/cborparser.rs +++ b/data-formats/src/cborparser.rs @@ -5,7 +5,7 @@ use thiserror::Error; use crate::{ constants::HashType, - serializable::{DeserializableMany, MaybeSerializable}, + serializable::{private::MaybeSerializable, DeserializableMany}, types::Hash, Error, Serializable, }; diff --git a/data-formats/src/lib.rs b/data-formats/src/lib.rs index c11495fcf..99d116409 100644 --- a/data-formats/src/lib.rs +++ b/data-formats/src/lib.rs @@ -19,7 +19,7 @@ pub mod messages; pub mod cborparser; -mod serializable; +pub mod serializable; pub use serializable::DeserializableMany; pub use serializable::Serializable; diff --git a/data-formats/src/ownershipvoucher.rs b/data-formats/src/ownershipvoucher.rs index 409126076..19935ed4d 100644 --- a/data-formats/src/ownershipvoucher.rs +++ b/data-formats/src/ownershipvoucher.rs @@ -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, }; diff --git a/data-formats/src/serializable.rs b/data-formats/src/serializable.rs index 89ad9fbfd..1522653f0 100644 --- a/data-formats/src/serializable.rs +++ b/data-formats/src/serializable.rs @@ -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(reader: R) -> Result, 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(reader: R) -> Result, 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(mut reader: R) -> Result, Error> where Self: Sized, @@ -85,3 +88,34 @@ where ciborium::ser::into_writer(self, &mut writer).map_err(Error::from) } } + +/// A structure that wraps a Vec, 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); + +impl Serializable for PlainBytes { + fn deserialize_data(data: &[u8]) -> Result { + Ok(PlainBytes(Vec::from(data))) + } + + fn deserialize_from_reader(mut reader: R) -> Result + 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(&self, mut writer: W) -> Result<(), Error> + where + W: std::io::Write, + { + writer.write_all(&self.0).map_err(Error::from) + } +} diff --git a/manufacturing-server/src/handlers/di.rs b/manufacturing-server/src/handlers/di.rs index 9dec13167..2ddfb432c 100644 --- a/manufacturing-server/src/handlers/di.rs +++ b/manufacturing-server/src/handlers/di.rs @@ -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::)?, + .map_err(Error::from_error::)? + .map(|d| d.0), }, }; let public_key = match public_key { diff --git a/manufacturing-server/src/main.rs b/manufacturing-server/src/main.rs index c99bd1eac..86b4f3893 100644 --- a/manufacturing-server/src/main.rs +++ b/manufacturing-server/src/main.rs @@ -17,6 +17,7 @@ use fdo_data_formats::{ constants::{KeyStorageType, MfgStringType, PublicKeyType, RendezvousVariable}, ownershipvoucher::OwnershipVoucher, publickey::{PublicKey, X5Chain}, + serializable::PlainBytes, types::{Guid, RendezvousInfo}, ProtocolVersion, }; @@ -62,8 +63,9 @@ struct ManufacturingServiceUD { OwnershipVoucherStoreMetadataKey, >, >, - public_key_store: - Option, PublicKeyStoreMetadataKey>>>, + public_key_store: Option< + Box>, + >, // Certificates manufacturer_cert: X509,