From 7b0489b2d2116d67ed54e575ba7ba98449ef5eba Mon Sep 17 00:00:00 2001 From: Nick Santana Date: Wed, 15 Mar 2023 14:40:58 -0700 Subject: [PATCH 1/2] Add `Display` implementation for new type wrappers Previously the new type implementations didn't implement the `Display` trait. Now the `Display` trait is implemented on any new type that uses one of `impl_newtype` or `impl_newtype_for_bytestruct`. Renamed the internal macro `new_type_accessors_impls` to `impl_newtype` for consistency. --- core/types/Cargo.toml | 5 +- core/types/src/attestation_key.rs | 12 ++-- core/types/src/attributes.rs | 8 +-- core/types/src/config_id.rs | 4 +- core/types/src/key_request.rs | 6 +- core/types/src/macros.rs | 99 ++++++++++++++++++++++++++++++- core/types/src/quote.rs | 12 ++-- core/types/src/report.rs | 16 ++--- core/types/src/svn.rs | 6 +- core/types/src/target_info.rs | 5 +- dcap/types/src/quoting_enclave.rs | 4 +- 11 files changed, 135 insertions(+), 42 deletions(-) diff --git a/core/types/Cargo.toml b/core/types/Cargo.toml index 029cfa79..ae40c492 100644 --- a/core/types/Cargo.toml +++ b/core/types/Cargo.toml @@ -13,9 +13,7 @@ rust-version = "1.62.1" [features] default = [] -serde = [ - "dep:serde" -] +serde = ["dep:serde"] alloc = [] [dependencies] @@ -34,4 +32,5 @@ getrandom = { version = "0.2", default-features = false, features = ["custom"] } [dev-dependencies] rand = "0.8.5" +textwrap = "0.16.0" yare = "1.0.1" diff --git a/core/types/src/attestation_key.rs b/core/types/src/attestation_key.rs index 8f1510b3..f7b4bea4 100644 --- a/core/types/src/attestation_key.rs +++ b/core/types/src/attestation_key.rs @@ -2,7 +2,7 @@ //! Attestation Key types use crate::{ - new_type_accessors_impls, + impl_newtype, report::{ExtendedProductId, FamilyId}, ConfigId, FfiError, }; @@ -49,7 +49,7 @@ impl TryFrom for Algorithm { #[repr(transparent)] pub struct Version(u16); -new_type_accessors_impls! { +impl_newtype! { Version, u16; } @@ -57,7 +57,7 @@ new_type_accessors_impls! { #[repr(transparent)] pub struct Id(u16); -new_type_accessors_impls! { +impl_newtype! { Id, u16; } @@ -145,7 +145,7 @@ impl AttestationKeyId { } } -new_type_accessors_impls! { +impl_newtype! { AttestationKeyId, sgx_ql_att_key_id_t; } @@ -154,7 +154,7 @@ new_type_accessors_impls! { #[repr(transparent)] pub struct ServiceProviderId([u8; 16]); -new_type_accessors_impls! { +impl_newtype! { ServiceProviderId, [u8; 16]; } @@ -180,7 +180,7 @@ impl ExtendedAttestationKeyId { } } -new_type_accessors_impls! { +impl_newtype! { ExtendedAttestationKeyId, sgx_att_key_id_ext_t; } diff --git a/core/types/src/attributes.rs b/core/types/src/attributes.rs index 33a0b7b7..8c612c51 100644 --- a/core/types/src/attributes.rs +++ b/core/types/src/attributes.rs @@ -2,14 +2,14 @@ //! SGX Attributes types -use crate::new_type_accessors_impls; +use crate::impl_newtype; use mc_sgx_core_sys_types::{sgx_attributes_t, sgx_misc_attribute_t, sgx_misc_select_t}; /// Attributes of the enclave #[repr(transparent)] #[derive(Default, Debug, Clone, Hash, PartialEq, Eq, Copy)] pub struct Attributes(sgx_attributes_t); -new_type_accessors_impls! { +impl_newtype! { Attributes, sgx_attributes_t; } @@ -40,7 +40,7 @@ impl Attributes { #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Default)] pub struct MiscellaneousSelect(sgx_misc_select_t); -new_type_accessors_impls! { +impl_newtype! { MiscellaneousSelect, sgx_misc_select_t; } @@ -49,7 +49,7 @@ new_type_accessors_impls! { #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct MiscellaneousAttribute(sgx_misc_attribute_t); -new_type_accessors_impls! { +impl_newtype! { MiscellaneousAttribute, sgx_misc_attribute_t; } diff --git a/core/types/src/config_id.rs b/core/types/src/config_id.rs index 336678b1..3f83ca21 100644 --- a/core/types/src/config_id.rs +++ b/core/types/src/config_id.rs @@ -1,7 +1,7 @@ // Copyright (c) 2022-2023 The MobileCoin Foundation //! SGX Config ID -use crate::new_type_accessors_impls; +use crate::impl_newtype; use mc_sgx_core_sys_types::{sgx_config_id_t, SGX_CONFIGID_SIZE}; /// Config ID @@ -9,7 +9,7 @@ use mc_sgx_core_sys_types::{sgx_config_id_t, SGX_CONFIGID_SIZE}; #[repr(transparent)] pub struct ConfigId(sgx_config_id_t); -new_type_accessors_impls! { +impl_newtype! { ConfigId, sgx_config_id_t; } diff --git a/core/types/src/key_request.rs b/core/types/src/key_request.rs index f8e03525..69b82d7b 100644 --- a/core/types/src/key_request.rs +++ b/core/types/src/key_request.rs @@ -2,7 +2,7 @@ //! SGX key request rust types use crate::{ - impl_newtype_for_bytestruct, new_type_accessors_impls, Attributes, ConfigSvn, CpuSvn, IsvSvn, + impl_newtype, impl_newtype_for_bytestruct, Attributes, ConfigSvn, CpuSvn, IsvSvn, MiscellaneousSelect, }; use bitflags::bitflags; @@ -73,7 +73,7 @@ bitflags! { #[derive(Debug, Clone, Hash, PartialEq, Eq)] #[repr(transparent)] pub struct KeyRequest(sgx_key_request_t); -new_type_accessors_impls! { +impl_newtype! { KeyRequest, sgx_key_request_t; } @@ -189,7 +189,7 @@ impl KeyRequestBuilder { #[derive(Default, Debug, Clone, Hash, PartialEq, Eq)] pub struct Key128bit(sgx_key_128bit_t); -new_type_accessors_impls! { +impl_newtype! { Key128bit, sgx_key_128bit_t; } diff --git a/core/types/src/macros.rs b/core/types/src/macros.rs index 52ff20f9..ef5b6bc7 100644 --- a/core/types/src/macros.rs +++ b/core/types/src/macros.rs @@ -7,7 +7,7 @@ pub(crate) use alloc::vec::Vec; /// an SgxWrapperType that don't depend on the contents of the inner /// type. #[macro_export] -macro_rules! new_type_accessors_impls { +macro_rules! newtype_accessors_impls { ($($wrapper:ident, $inner:ty;)*) => {$( impl AsMut<$inner> for $wrapper { fn as_mut(&mut self) -> &mut $inner { @@ -42,6 +42,23 @@ macro_rules! new_type_accessors_impls { )*} } +/// Newtype wrapper for a primitive or struct type +#[macro_export] +macro_rules! impl_newtype { + ($($wrapper:ident, $inner:ty;)*) => {$( + $crate::newtype_accessors_impls! { + $wrapper, $inner; + } + + impl core::fmt::Display for$wrapper { + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + core::fmt::Debug::fmt(&self.0, f) + } + } + + )*} +} + /// This macro provides common byte-handling operations when the type being /// wrapped is a struct containing a single fixed-size array of bytes. /// @@ -50,7 +67,7 @@ macro_rules! new_type_accessors_impls { macro_rules! impl_newtype_for_bytestruct { ($($wrapper:ident, $inner:ident, $size:ident, $fieldname:ident;)*) => {$( - $crate::new_type_accessors_impls! { + $crate::newtype_accessors_impls! { $wrapper, $inner; } @@ -100,12 +117,27 @@ macro_rules! impl_newtype_for_bytestruct { } } + impl core::fmt::Display for $wrapper { + fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { + write!(f, "0x")?; + let inner: &[u8] = self.as_ref(); + for byte in inner { + write!(f, "{:02X}", byte)?; + } + Ok(()) + } + } + )*} } #[cfg(test)] mod test { + extern crate std; + use crate::FfiError; + use std::format; + use std::string::ToString; use yare::parameterized; const FIELD_SIZE: usize = 24; @@ -178,4 +210,67 @@ mod test { let outer = Outer::default(); assert_eq!(outer.0.field, [0u8; Outer::SIZE]); } + + #[test] + fn newtype_byte_array_display() { + let outer = Outer::from([ + 0xABu8, 0x00, 0xcd, 0x12, 0xfe, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0x0a, + 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, 0x10, 0x11, 0x12, 0x13, + ]); + assert_eq!( + outer.to_string(), + "0xAB00CD12FE0102030405060708090A0B0C0D0E0F10111213" + ); + } + + #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] + struct StructInner { + field: u32, + } + + #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] + #[repr(transparent)] + struct StructOuter(StructInner); + impl_newtype! { + StructOuter, StructInner; + } + + #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] + #[repr(transparent)] + struct PrimitiveOuter(u32); + impl_newtype! { + PrimitiveOuter, u32; + } + + #[test] + fn newtype_for_struct() { + let inner = StructInner { field: 30 }; + let outer: StructOuter = inner.into(); + assert_eq!(outer.0, inner); + } + + #[test] + fn display_newtype_for_struct() { + let inner = StructInner { field: 20 }; + let outer: StructOuter = inner.into(); + assert_eq!(outer.to_string(), "StructInner { field: 20 }"); + } + + #[test] + fn display_newtype_for_struct_alternate() { + let inner = StructInner { field: 20 }; + let outer: StructOuter = inner.into(); + let expected = r#" + StructInner { + field: 20, + }"#; + assert_eq!(format!("\n{outer:#}"), textwrap::dedent(expected)); + } + + #[test] + fn display_newtype_for_primitive() { + let inner = 42; + let outer: PrimitiveOuter = inner.into(); + assert_eq!(outer.to_string(), "42"); + } } diff --git a/core/types/src/quote.rs b/core/types/src/quote.rs index 8e30b7f9..b7134f2f 100644 --- a/core/types/src/quote.rs +++ b/core/types/src/quote.rs @@ -2,8 +2,8 @@ //! Quote types use crate::{ - attestation_key::QuoteSignatureKind, impl_newtype_for_bytestruct, new_type_accessors_impls, - report::Report, FfiError, IsvSvn, ReportBody, TargetInfo, + attestation_key::QuoteSignatureKind, impl_newtype, impl_newtype_for_bytestruct, report::Report, + FfiError, IsvSvn, ReportBody, TargetInfo, }; use mc_sgx_core_sys_types::{ sgx_basename_t, sgx_epid_group_id_t, sgx_platform_info_t, sgx_qe_report_info_t, @@ -32,7 +32,7 @@ impl QuotingEnclaveReportInfo { } } -new_type_accessors_impls! { +impl_newtype! { QuotingEnclaveReportInfo, sgx_qe_report_info_t; } @@ -50,7 +50,7 @@ impl_newtype_for_bytestruct! { #[repr(transparent)] pub struct UpdateInfoBit(sgx_update_info_bit_t); -new_type_accessors_impls! { +impl_newtype! { UpdateInfoBit, sgx_update_info_bit_t; } @@ -76,7 +76,7 @@ impl UpdateInfoBit { #[repr(transparent)] pub struct EpidGroupId(sgx_epid_group_id_t); -new_type_accessors_impls! { +impl_newtype! { EpidGroupId, sgx_epid_group_id_t; } @@ -114,7 +114,7 @@ pub struct RawQuote<'a> { #[repr(transparent)] pub struct Version(u16); -new_type_accessors_impls! { +impl_newtype! { Version, u16; } diff --git a/core/types/src/report.rs b/core/types/src/report.rs index 0f85b62f..e905b336 100644 --- a/core/types/src/report.rs +++ b/core/types/src/report.rs @@ -2,8 +2,8 @@ //! SGX Report use crate::{ - config_id::ConfigId, impl_newtype_for_bytestruct, key_request::KeyId, new_type_accessors_impls, - Attributes, ConfigSvn, CpuSvn, FfiError, IsvSvn, MiscellaneousSelect, MrEnclave, MrSigner, + config_id::ConfigId, impl_newtype, impl_newtype_for_bytestruct, key_request::KeyId, Attributes, + ConfigSvn, CpuSvn, FfiError, IsvSvn, MiscellaneousSelect, MrEnclave, MrSigner, }; use core::ops::BitAnd; use mc_sgx_core_sys_types::{ @@ -21,7 +21,7 @@ use nom::number::complete::{le_u16, le_u32, le_u64}; #[repr(transparent)] pub struct Mac(sgx_mac_t); -new_type_accessors_impls! { +impl_newtype! { Mac, sgx_mac_t; } @@ -63,7 +63,7 @@ impl BitAnd for ReportData { #[repr(transparent)] pub struct FamilyId(sgx_isvfamily_id_t); -new_type_accessors_impls! { +impl_newtype! { FamilyId, sgx_isvfamily_id_t; } @@ -72,7 +72,7 @@ new_type_accessors_impls! { #[repr(transparent)] pub struct ExtendedProductId(sgx_isvext_prod_id_t); -new_type_accessors_impls! { +impl_newtype! { ExtendedProductId, sgx_isvext_prod_id_t; } @@ -81,7 +81,7 @@ new_type_accessors_impls! { #[repr(transparent)] pub struct IsvProductId(sgx_prod_id_t); -new_type_accessors_impls! { +impl_newtype! { IsvProductId, sgx_prod_id_t; } @@ -152,7 +152,7 @@ impl ReportBody { } } -new_type_accessors_impls! { +impl_newtype! { ReportBody, sgx_report_body_t; } @@ -242,7 +242,7 @@ impl Report { } } -new_type_accessors_impls! { +impl_newtype! { Report, sgx_report_t; } diff --git a/core/types/src/svn.rs b/core/types/src/svn.rs index 839b2410..6fa61869 100644 --- a/core/types/src/svn.rs +++ b/core/types/src/svn.rs @@ -1,7 +1,7 @@ // Copyright (c) 2022-2023 The MobileCoin Foundation //! SGX core SVN (Security Version Numbers) -use crate::{impl_newtype_for_bytestruct, new_type_accessors_impls}; +use crate::{impl_newtype, impl_newtype_for_bytestruct}; use mc_sgx_core_sys_types::{sgx_config_svn_t, sgx_cpu_svn_t, sgx_isv_svn_t, SGX_CPUSVN_SIZE}; /// Config security version number (SVN) @@ -9,7 +9,7 @@ use mc_sgx_core_sys_types::{sgx_config_svn_t, sgx_cpu_svn_t, sgx_isv_svn_t, SGX_ #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Default)] pub struct ConfigSvn(sgx_config_svn_t); -new_type_accessors_impls! { +impl_newtype! { ConfigSvn, sgx_config_svn_t; } @@ -18,7 +18,7 @@ new_type_accessors_impls! { #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Default)] pub struct IsvSvn(sgx_isv_svn_t); -new_type_accessors_impls! { +impl_newtype! { IsvSvn, sgx_isv_svn_t; } diff --git a/core/types/src/target_info.rs b/core/types/src/target_info.rs index 02a643db..2e4fe20e 100644 --- a/core/types/src/target_info.rs +++ b/core/types/src/target_info.rs @@ -2,8 +2,7 @@ //! SGX TargetInfo use crate::{ - config_id::ConfigId, new_type_accessors_impls, Attributes, ConfigSvn, MiscellaneousSelect, - MrEnclave, + config_id::ConfigId, impl_newtype, Attributes, ConfigSvn, MiscellaneousSelect, MrEnclave, }; use mc_sgx_core_sys_types::sgx_target_info_t; @@ -39,7 +38,7 @@ impl TargetInfo { } } -new_type_accessors_impls! { +impl_newtype! { TargetInfo, sgx_target_info_t; } diff --git a/dcap/types/src/quoting_enclave.rs b/dcap/types/src/quoting_enclave.rs index 577b6cf1..97301c34 100644 --- a/dcap/types/src/quoting_enclave.rs +++ b/dcap/types/src/quoting_enclave.rs @@ -2,7 +2,7 @@ //! Types specific to the quoting enclave -use mc_sgx_core_types::{new_type_accessors_impls, QuoteNonce, Report, TargetInfo}; +use mc_sgx_core_types::{impl_newtype, QuoteNonce, Report, TargetInfo}; use mc_sgx_dcap_sys_types::sgx_ql_qe_report_info_t; /// Report info for the Quoting Enclave @@ -10,7 +10,7 @@ use mc_sgx_dcap_sys_types::sgx_ql_qe_report_info_t; #[derive(Default, Debug, Clone, Hash, PartialEq, Eq)] pub struct ReportInfo(sgx_ql_qe_report_info_t); -new_type_accessors_impls! { +impl_newtype! { ReportInfo, sgx_ql_qe_report_info_t; } From 6a993f4708647b8b6ff0ab54782997fd74051e46 Mon Sep 17 00:00:00 2001 From: Nick Santana Date: Mon, 20 Mar 2023 08:08:19 -0700 Subject: [PATCH 2/2] Use absolute type paths in macros Previously some `core::...` usages in macros lacked the leading `::` to indicate absolute crate paths. Now the usages are of the form `::core::...`. Made the hex output of wrapped C byte structs use an underscore every 2 bytes. --- core/types/src/macros.rs | 41 ++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/core/types/src/macros.rs b/core/types/src/macros.rs index ef5b6bc7..1391e038 100644 --- a/core/types/src/macros.rs +++ b/core/types/src/macros.rs @@ -50,9 +50,9 @@ macro_rules! impl_newtype { $wrapper, $inner; } - impl core::fmt::Display for$wrapper { - fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { - core::fmt::Debug::fmt(&self.0, f) + impl ::core::fmt::Display for $wrapper { + fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { + ::core::fmt::Debug::fmt(&self.0, f) } } @@ -91,7 +91,7 @@ macro_rules! impl_newtype_for_bytestruct { impl<'bytes> TryFrom<&'bytes [u8]> for $wrapper { type Error = $crate::FfiError; - fn try_from(src: &[u8]) -> core::result::Result { + fn try_from(src: &[u8]) -> ::core::result::Result { if src.len() < $size { return Err($crate::FfiError::InvalidInputLength); } @@ -106,7 +106,7 @@ macro_rules! impl_newtype_for_bytestruct { impl TryFrom<$crate::macros::Vec> for $wrapper { type Error = $crate::FfiError; - fn try_from(src: $crate::macros::Vec) -> core::result::Result { + fn try_from(src: $crate::macros::Vec) -> ::core::result::Result { Self::try_from(src.as_slice()) } } @@ -117,17 +117,28 @@ macro_rules! impl_newtype_for_bytestruct { } } - impl core::fmt::Display for $wrapper { - fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result { - write!(f, "0x")?; + impl ::core::fmt::UpperHex for $wrapper { + fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { let inner: &[u8] = self.as_ref(); - for byte in inner { - write!(f, "{:02X}", byte)?; + let prefix = if f.alternate() { "0x" } else { "" }; + let separators = ::core::iter::once(prefix).chain(::core::iter::repeat("_")); + let segments = separators.zip(inner.chunks(2)); + for (separator, chunk) in segments { + write!(f, "{separator}")?; + for byte in chunk { + write!(f, "{:02X}", byte)?; + } } Ok(()) } } + impl ::core::fmt::Display for $wrapper { + fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { + write!(f, "{:#X}", self) + } + } + )*} } @@ -142,12 +153,12 @@ mod test { const FIELD_SIZE: usize = 24; - #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] + #[derive(Default, Debug, Clone, Copy, PartialEq)] struct Inner { field: [u8; FIELD_SIZE], } - #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] + #[derive(Default, Debug, Clone, PartialEq)] #[repr(transparent)] struct Outer(Inner); @@ -219,23 +230,21 @@ mod test { ]); assert_eq!( outer.to_string(), - "0xAB00CD12FE0102030405060708090A0B0C0D0E0F10111213" + "0xAB00_CD12_FE01_0203_0405_0607_0809_0A0B_0C0D_0E0F_1011_1213" ); } - #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] + #[derive(Debug, Clone, Copy, PartialEq)] struct StructInner { field: u32, } - #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] #[repr(transparent)] struct StructOuter(StructInner); impl_newtype! { StructOuter, StructInner; } - #[derive(Default, Debug, Clone, Copy, Eq, PartialEq)] #[repr(transparent)] struct PrimitiveOuter(u32); impl_newtype! {