Merge pull-request #594

Author: Turnalek

src/Cargo.lock

@@ -23,6 +23,7 @@ tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
 borsh = { workspace = true }
 nix = { workspace = true }
 rustls = { workspace = true }
+serde_json = { workspace = true }
 webpki-roots = { workspace = true }
 tokio-rustls = { workspace = true }
 

src/integration/Cargo.toml

@@ -7,7 +7,6 @@ use std::{
 	process::{Command, Stdio},
 };
 
-use borsh::de::BorshDeserialize;
 use integration::{LOCAL_HOST, PCR3_PRE_IMAGE_PATH, QOS_DIST_DIR};
 use qos_core::protocol::{
 	services::{
@@ -104,14 +103,13 @@ async fn main() {
 		.success());
 
 	// Check the manifest written to file
-	let manifest =
-		Manifest::try_from_slice(&fs::read(&cli_manifest_path).unwrap())
-			.unwrap();
+	let manifest: Manifest =
+		serde_json::from_slice(&fs::read(&cli_manifest_path).unwrap()).unwrap();
 
-	let genesis_output = {
+	let genesis_output: GenesisOutput = {
 		let contents =
 			fs::read("./mock/boot-e2e/genesis-dir/genesis_output").unwrap();
-		GenesisOutput::try_from_slice(&contents).unwrap()
+		serde_json::from_slice(&contents).unwrap()
 	};
 	// For simplicity sake, we use the same keys for the share set and manifest
 	// set.
@@ -222,9 +220,8 @@ async fn main() {
 		assert!(child.wait().unwrap().success());
 
 		// Read in the generated approval to check it was created correctly
-		let approval =
-			Approval::try_from_slice(&fs::read(approval_path).unwrap())
-				.unwrap();
+		let approval: Approval =
+			serde_json::from_slice(&fs::read(approval_path).unwrap()).unwrap();
 		let personal_pair = P256Pair::from_hex_file(format!(
 			"{}/{}.secret",
 			personal_dir(alias),

src/integration/examples/boot_enclave.rs

@@ -103,9 +103,8 @@ async fn standard_boot_e2e() {
 		.success());
 
 	// Check the manifest written to file
-	let manifest =
-		Manifest::try_from_slice(&fs::read(&cli_manifest_path).unwrap())
-			.unwrap();
+	let manifest: Manifest =
+		serde_json::from_slice(&fs::read(&cli_manifest_path).unwrap()).unwrap();
 
 	let genesis_output = {
 		let contents =
@@ -238,9 +237,8 @@ async fn standard_boot_e2e() {
 		assert!(child.wait().unwrap().success());
 
 		// Read in the generated approval to check it was created correctly
-		let approval =
-			Approval::try_from_slice(&fs::read(approval_path).unwrap())
-				.unwrap();
+		let approval: Approval =
+			serde_json::from_slice(&fs::read(approval_path).unwrap()).unwrap();
 		let personal_pair = P256Pair::from_hex_file(format!(
 			"{}/{}.secret",
 			personal_dir(alias),

src/integration/tests/boot.rs

@@ -113,9 +113,8 @@ pub enum Error {
 	},
 	/// Failed to decode some hex
 	CouldNotDecodeHex(qos_hex::HexError),
-	/// Failed to deserialize something from borsh.
-	#[allow(clippy::enum_variant_names)]
-	BorshError,
+	/// Failed to deserialize something from borsh or json
+	Deserialize,
 	FailedToReadDrKey(qos_p256::P256Error),
 	QosAttest(String),
 	/// Pivot file
@@ -141,9 +140,15 @@ pub enum Error {
 	SecretDoesNotMatch,
 }
 
+impl From<serde_json::Error> for Error {
+	fn from(_: serde_json::Error) -> Self {
+		Self::Deserialize
+	}
+}
+
 impl From<borsh::io::Error> for Error {
 	fn from(_: borsh::io::Error) -> Self {
-		Self::BorshError
+		Self::Deserialize
 	}
 }
 
@@ -759,7 +764,7 @@ pub(crate) fn generate_manifest<P: AsRef<Path>>(
 
 	write_with_msg(
 		manifest_path.as_ref(),
-		&borsh::to_vec(&manifest).unwrap(),
+		&serde_json::to_vec(&manifest).expect("failed to serialize manifest"),
 		"Manifest",
 	);
 
@@ -862,7 +867,7 @@ pub(crate) fn approve_manifest<P: AsRef<Path>>(
 	));
 	write_with_msg(
 		&approval_path,
-		&borsh::to_vec(&approval).expect("Failed to serialize approval"),
+		&serde_json::to_vec(&approval).expect("Failed to serialize approval"),
 		"Manifest Approval",
 	);
 
@@ -1010,7 +1015,7 @@ pub(crate) fn generate_manifest_envelope<P: AsRef<Path>>(
 	);
 	write_with_msg(
 		&path,
-		&borsh::to_vec(&manifest_envelope)
+		&serde_json::to_vec(&manifest_envelope)
 			.expect("Failed to serialize manifest envelope"),
 		"Manifest Envelope",
 	);
@@ -1076,7 +1081,7 @@ pub(crate) fn export_key<P: AsRef<Path>>(
 
 	write_with_msg(
 		encrypted_quorum_key_path.as_ref(),
-		&borsh::to_vec(&encrypted_quorum_key).expect("valid borsh. qed."),
+		&serde_json::to_vec(&encrypted_quorum_key).expect("valid borsh. qed."),
 		"Encrypted Quorum Key",
 	);
 
@@ -1087,10 +1092,10 @@ pub(crate) fn inject_key<P: AsRef<Path>>(
 	uri: &str,
 	encrypted_quorum_key_path: P,
 ) -> Result<(), Error> {
-	let encrypted_quorum_key = {
+	let encrypted_quorum_key: EncryptedQuorumKey = {
 		let bytes = std::fs::read(encrypted_quorum_key_path)
 			.map_err(|_| Error::FailedToReadEncryptedQuorumKey)?;
-		EncryptedQuorumKey::try_from_slice(&bytes)
+		serde_json::from_slice(&bytes)
 			.map_err(|_| Error::InvalidEncryptedQuorumKey)?
 	};
 
@@ -1201,8 +1206,8 @@ pub(crate) fn get_attestation_doc<P: AsRef<Path>>(
 	);
 	write_with_msg(
 		manifest_envelope_path.as_ref(),
-		&borsh::to_vec(&manifest_envelope)
-			.expect("manifest enevelope is valid borsh"),
+		&serde_json::to_vec(&manifest_envelope)
+			.expect("manifest enevelope is valid json"),
 		"Manifest envelope",
 	);
 }
@@ -1313,7 +1318,7 @@ pub(crate) fn proxy_re_encrypt_share<P: AsRef<Path>>(
 		eph_pub.encrypt(plaintext_share).expect("Envelope encryption error")
 	};
 
-	let approval = borsh::to_vec(&Approval {
+	let approval = serde_json::to_vec(&Approval {
 		signature: pair
 			.sign(&manifest_envelope.manifest.qos_hash())
 			.expect("Failed to sign"),
@@ -1573,26 +1578,28 @@ pub(crate) fn display<P: AsRef<Path>>(
 	file_path: P,
 	json: bool,
 ) -> Result<(), Error> {
-	let bytes =
-		fs::read(file_path).map_err(|e| Error::ReadShare(e.to_string()))?;
 	match *display_type {
 		DisplayType::Manifest => {
-			let decoded = Manifest::try_from_slice_compat(&bytes)?;
+			let decoded = read_manifest(file_path)?;
+
 			if json {
 				println!("{}", serde_json::to_string(&decoded).unwrap());
 			} else {
 				println!("{decoded:#?}");
 			}
 		}
 		DisplayType::ManifestEnvelope => {
-			let decoded = ManifestEnvelope::try_from_slice(&bytes)?;
+			let decoded = read_manifest_envelope(file_path)?;
+
 			if json {
 				println!("{}", serde_json::to_string(&decoded).unwrap());
 			} else {
 				println!("{decoded:#?}");
 			}
 		}
 		DisplayType::GenesisOutput => {
+			let bytes = fs::read(file_path)
+				.map_err(|e| Error::ReadShare(e.to_string()))?;
 			let decoded = GenesisOutput::try_from_slice(&bytes)?;
 			println!("{decoded:#?}");
 		}
@@ -1952,7 +1959,7 @@ fn find_approvals<P: AsRef<Path>>(
 				return None;
 			};
 
-			let approval = Approval::try_from_slice(
+			let approval: Approval = serde_json::from_slice(
 				&fs::read(path).expect("Failed to read in approval"),
 			)
 			.expect("Failed to deserialize approval");
@@ -1981,9 +1988,16 @@ fn find_approvals<P: AsRef<Path>>(
 }
 
 fn read_manifest<P: AsRef<Path>>(file: P) -> Result<Manifest, Error> {
-	let buf = fs::read(file).map_err(Error::FailedToReadManifestFile)?;
-	Manifest::try_from_slice(&buf)
-		.map_err(|_| Error::FileDidNotHaveValidManifest)
+	let bytes = fs::read(file).map_err(Error::FailedToReadManifestFile)?;
+
+	// try getting Manifest from json
+	let result = serde_json::from_slice::<Manifest>(&bytes);
+	if result.is_err() {
+		// if not try the old borsh format
+		Manifest::try_from_slice_compat(&bytes).map_err(Error::from)
+	} else {
+		result.map_err(Error::from)
+	}
 }
 
 fn read_attestation_doc<P: AsRef<Path>>(
@@ -2003,10 +2017,16 @@ fn read_attestation_doc<P: AsRef<Path>>(
 fn read_manifest_envelope<P: AsRef<Path>>(
 	file: P,
 ) -> Result<ManifestEnvelope, Error> {
-	let buf =
-		fs::read(file).map_err(Error::FailedToReadManifestEnvelopeFile)?;
-	ManifestEnvelope::try_from_slice(&buf)
-		.map_err(|_| Error::FileDidNotHaveValidManifestEnvelope)
+	let bytes = fs::read(file).map_err(Error::FailedToReadManifestFile)?;
+
+	// try getting Manifest from json
+	let result = serde_json::from_slice::<ManifestEnvelope>(&bytes);
+	if result.is_err() {
+		// if not try the old borsh format
+		ManifestEnvelope::try_from_slice_compat(&bytes).map_err(Error::from)
+	} else {
+		result.map_err(Error::from)
+	}
 }
 
 fn read_attestation_approval<P: AsRef<Path>>(
@@ -2015,7 +2035,7 @@ fn read_attestation_approval<P: AsRef<Path>>(
 	let manifest_envelope =
 		fs::read(path).map_err(Error::FailedToReadAttestationApproval)?;
 
-	Approval::try_from_slice(&manifest_envelope)
+	serde_json::from_slice(&manifest_envelope)
 		.map_err(|_| Error::FileDidNotHaveValidAttestationApproval)
 }
 

src/qos_client/src/cli/services.rs

@@ -22,6 +22,7 @@ borsh = { workspace = true }
 aws-nitro-enclaves-nsm-api = { workspace = true }
 
 serde_bytes = { workspace = true }
+serde_json = { workspace = true }
 serde = { workspace = true }
 
 futures = { workspace = true }

src/qos_core/Cargo.toml

@@ -1,8 +1,11 @@
 //! Logic for accessing read only QOS state.
 
-use std::{fs, os::unix::fs::PermissionsExt, path::Path};
+use std::{
+	fs,
+	os::unix::fs::PermissionsExt,
+	path::{Path, PathBuf},
+};
 
-use borsh::BorshDeserialize;
 use qos_p256::P256Pair;
 
 use crate::protocol::{services::boot::ManifestEnvelope, ProtocolError};
@@ -179,8 +182,9 @@ impl Handles {
 	) -> Result<ManifestEnvelope, ProtocolError> {
 		let contents = fs::read(&self.manifest)
 			.map_err(|_| ProtocolError::FailedToGetManifestEnvelope)?;
-		let manifest = ManifestEnvelope::try_from_slice(&contents)
+		let manifest = serde_json::from_slice(&contents)
 			.map_err(|_| ProtocolError::FailedToGetManifestEnvelope)?;
+
 		Ok(manifest)
 	}
 
@@ -195,7 +199,8 @@ impl Handles {
 	) -> Result<(), ProtocolError> {
 		Self::write_as_read_only(
 			&self.manifest,
-			&borsh::to_vec(manifest_envelope)?,
+			&serde_json::to_vec(manifest_envelope)
+				.map_err(|_| ProtocolError::FailedToPutManifestEnvelope)?,
 			ProtocolError::FailedToPutManifestEnvelope,
 		)
 	}
@@ -219,8 +224,12 @@ impl Handles {
 			&self.manifest,
 			std::fs::Permissions::from_mode(0o666),
 		)?;
-		fs::write(&self.manifest, borsh::to_vec(&manifest_envelope)?)
-			.map_err(|_| ProtocolError::FailedToPutManifestEnvelope)?;
+		fs::write(
+			&self.manifest,
+			serde_json::to_vec(&manifest_envelope)
+				.map_err(|_| ProtocolError::FailedToPutManifestEnvelope)?,
+		)
+		.map_err(|_| ProtocolError::FailedToPutManifestEnvelope)?;
 
 		// Set the permissions back to read only
 		fs::set_permissions(
@@ -272,7 +281,7 @@ impl Handles {
 		Path::new(&self.pivot).exists()
 	}
 
-	/// Helper function for ready only writes.
+	/// Helper function for ready only writes that also ensures full write atomicity by renaming at the end.
 	fn write_as_read_only<P: AsRef<Path>>(
 		path: P,
 		buf: &[u8],
@@ -288,7 +297,13 @@ impl Handles {
 			}
 		}
 
-		fs::write(&path, buf).map_err(|_| err.clone())?;
+		let tmp_path = PathBuf::from(path.as_ref()).with_extension("tmp");
+
+		fs::write(&tmp_path, buf).map_err(|_| err.clone())?;
+
+		// atomically move to destination once fully written to prevent partial reads
+		fs::rename(&tmp_path, &path)?;
+
 		fs::set_permissions(&path, fs::Permissions::from_mode(0o444))
 			.map_err(|_| err)?;