Merge pull request #317 from Superhepper/pcr_select_bugs

Fixes some pcr issues.

Author: ionut-arm

tss-esapi/src/abstraction/pcr.rs

@@ -53,7 +53,8 @@ pub use data::PcrData;
 ///             PcrSlot::Slot20,
 ///             PcrSlot::Slot21,
 ///     ])
-///     .build();
+///     .build()
+///     .expect("Failed to build PcrSelectionList");
 /// let _pcr_data = tss_esapi::abstraction::pcr::read_all(&mut context, pcr_selection_list)
 ///     .expect("pcr::read_all failed");
 /// ```

tss-esapi/src/abstraction/pcr/bank.rs

@@ -22,7 +22,7 @@ impl PcrBank {
     /// # Details
     /// The order of pcr slots are assumed to match the order of the Digests.
     ///
-    /// # Error
+    /// # Errors
     /// - If number of pcr slots does not match the number of pcr digests
     ///   InconsistentParams error is returned.
     ///
@@ -88,7 +88,7 @@ impl PcrBank {
 
     /// Inserts [Digest] value associated with a [PcrSlot] into the bank.
     ///
-    /// # Error
+    /// # Errors
     /// Returns an error if a [Digest] is already associated with the
     /// provided [PcrSlot].
     pub fn insert_digest(&mut self, pcr_slot: PcrSlot, digest: Digest) -> Result<()> {
@@ -99,7 +99,7 @@ impl PcrBank {
 
     /// Attempts to extend the [PcrBank] with `other`.
     ///
-    /// # Error
+    /// # Errors
     /// Returns an error if the a value in `other` already
     /// exists.
     pub fn try_extend(&mut self, other: PcrBank) -> Result<()> {

tss-esapi/src/attributes/locality.rs

@@ -38,7 +38,7 @@ impl LocalityAttributes {
 
     /// Returns the LocalityAttributes as a number.
     ///
-    /// # Error
+    /// # Errors
     /// If the attributes are not extended en InvalidParams error
     /// is returned.
     pub fn as_extended(&self) -> Result<u8> {

tss-esapi/src/context/handle_manager.rs

@@ -50,7 +50,7 @@ impl HandleManager {
 
     /// Sets the handle as flushed which removes it from the manager.
     ///
-    /// # Error
+    /// # Errors
     /// If the handle was not set to be flushed then this will cause an
     /// error but the handle will still be removed from the handler.
     pub fn set_as_flushed(&mut self, handle: ObjectHandle) -> Result<()> {
@@ -75,7 +75,7 @@ impl HandleManager {
 
     /// Sets the handles as closed which removes it from the handler.
     ///
-    /// # Error
+    /// # Errors
     /// If the handle was set to be flushed then this will cause an
     /// error but the handle will still be removed from the handler.
     pub fn set_as_closed(&mut self, handle: ObjectHandle) -> Result<()> {

tss-esapi/src/context/tpm_commands/integrity_collection_pcr.rs

@@ -146,7 +146,8 @@ impl Context {
     /// // that is going to be read.
     /// let pcr_selection_list = PcrSelectionListBuilder::new()
     ///     .with_selection(HashingAlgorithm::Sha256, &[PcrSlot::Slot0, PcrSlot::Slot1])
-    ///     .build();
+    ///     .build()
+    ///     .expect("Failed to build PcrSelectionList");
     ///
     /// let (update_counter, read_pcr_list, digest_list) = context.pcr_read(pcr_selection_list)
     ///     .expect("Call to pcr_read failed");

tss-esapi/src/structures/lists/pcr_selection.rs

@@ -10,7 +10,7 @@ use std::convert::TryFrom;
 
 /// A struct representing a pcr selection list. This
 /// corresponds to the TSS TPML_PCR_SELECTION.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Default)]
 pub struct PcrSelectionList {
     items: Vec<PcrSelection>,
 }
@@ -56,7 +56,7 @@ impl PcrSelectionList {
     ///
     /// This returns an empty list if None is passed
     pub fn list_from_option(pcr_list: Option<PcrSelectionList>) -> PcrSelectionList {
-        pcr_list.unwrap_or_else(|| PcrSelectionListBuilder::new().build())
+        pcr_list.unwrap_or_default()
     }
 
     /// Private methods for removing pcr selections that are empty.
@@ -89,7 +89,7 @@ impl PcrSelectionList {
 }
 
 impl From<PcrSelectionList> for TPML_PCR_SELECTION {
-    fn from(pcr_selections: PcrSelectionList) -> TPML_PCR_SELECTION {
+    fn from(pcr_selections: PcrSelectionList) -> Self {
         let mut tss_pcr_selection_list: TPML_PCR_SELECTION = Default::default();
         for pcr_selection in pcr_selections.items {
             tss_pcr_selection_list.pcrSelections[tss_pcr_selection_list.count as usize] =
@@ -178,17 +178,20 @@ impl PcrSelectionListBuilder {
     /// provided.
     ///
     /// If no size of select have been provided then it will
-    /// be defaulted to 3. This may not be the correct size for
+    /// be defaulted to to the most suitable with regard to TPM2_PCR_SELECT_MAX.
+    /// This may not be the correct size for
     /// the current platform. The correct values can be obtained
     /// by querying the tpm for its capabilities.
-    pub fn build(self) -> PcrSelectionList {
+    pub fn build(self) -> Result<PcrSelectionList> {
         let size_of_select = self.size_of_select.unwrap_or_default();
-        PcrSelectionList {
-            items: self
-                .items
-                .iter()
-                .map(|(k, v)| PcrSelection::new(*k, size_of_select, v.as_slice()))
-                .collect(),
-        }
+        self.items
+            .iter()
+            .try_fold(Vec::<PcrSelection>::new(), |mut acc, (&k, v)| {
+                PcrSelection::create(k, size_of_select, v.as_slice()).map(|pcr_select| {
+                    acc.push(pcr_select);
+                    acc
+                })
+            })
+            .map(|items| PcrSelectionList { items })
     }
 }

tss-esapi/src/structures/mod.rs

@@ -130,6 +130,8 @@ pub use self::command_code_attributes_list::CommandCodeAttributesList;
 pub mod command_code_attributes_list {
     pub use super::lists::command_code_attributes::*;
 }
+
+pub(crate) use pcr::slot_collection::PcrSlotCollection;
 /////////////////////////////////////////////////////////
 /// The parameters section
 /////////////////////////////////////////////////////////

tss-esapi/src/structures/pcr/mod.rs

@@ -4,3 +4,4 @@ pub mod select;
 pub mod select_size;
 pub mod selection;
 pub mod slot;
+pub mod slot_collection;

tss-esapi/src/structures/pcr/select.rs

@@ -1,14 +1,11 @@
 // Copyright 2020 Contributors to the Parsec project.
 // SPDX-License-Identifier: Apache-2.0
 use crate::{
-    structures::{PcrSelectSize, PcrSlot},
-    tss2_esys::{TPM2_PCR_SELECT_MAX, TPMS_PCR_SELECT},
-    Error, Result, WrapperErrorKind,
+    structures::{PcrSelectSize, PcrSlot, PcrSlotCollection},
+    tss2_esys::TPMS_PCR_SELECT,
+    Error, Result,
 };
 
-use enumflags2::BitFlags;
-use log::error;
-
 use std::convert::TryFrom;
 /// This module contains necessary representations
 /// of the items belonging to the TPMS_PCR_SELECT
@@ -21,17 +18,15 @@ use std::convert::TryFrom;
 /// not adhering to a platform-specific specification.
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct PcrSelect {
-    size_of_select: PcrSelectSize,
-    selected_pcrs: BitFlags<PcrSlot>,
+    pcr_slot_collection: PcrSlotCollection,
 }
 
 impl PcrSelect {
     /// Creates a new PcrSelect
-    pub fn new(size_of_select: PcrSelectSize, pcr_slots: &[PcrSlot]) -> Self {
-        PcrSelect {
-            size_of_select,
-            selected_pcrs: pcr_slots.iter().copied().collect(),
-        }
+    pub fn create(pcr_select_size: PcrSelectSize, pcr_slots: &[PcrSlot]) -> Result<Self> {
+        PcrSlotCollection::create(pcr_select_size, pcr_slots).map(|pcr_slot_collection| PcrSelect {
+            pcr_slot_collection,
+        })
     }
 
     /// Returns the size of the select.
@@ -41,46 +36,32 @@ impl PcrSelect {
     /// octets that are needed to hold the bit field
     /// that indicate what slots that are selected.
     pub fn size_of_select(&self) -> PcrSelectSize {
-        self.size_of_select
+        self.pcr_slot_collection.size_of_select()
     }
 
     /// Returns the selected PCRs in the select.
     pub fn selected_pcrs(&self) -> Vec<PcrSlot> {
-        self.selected_pcrs.iter().collect()
+        self.pcr_slot_collection.collection()
     }
 }
 
 impl TryFrom<TPMS_PCR_SELECT> for PcrSelect {
     type Error = Error;
     fn try_from(tss_pcr_select: TPMS_PCR_SELECT) -> Result<Self> {
-        // Parse the sizeofSelect into a SelectSize.
-        let size_of_select = PcrSelectSize::try_from(tss_pcr_select.sizeofSelect)?;
-
-        // Select only the octets indicated by sizeofSelect
-        let mut selected_octets = [0u8; TPM2_PCR_SELECT_MAX as usize];
-        let number_of_selected_octets: usize = size_of_select.into();
-        selected_octets[..number_of_selected_octets]
-            .copy_from_slice(&tss_pcr_select.pcrSelect[..number_of_selected_octets]);
-
-        // Parse selected pcrs into BitFlags
-        let selected_pcrs = BitFlags::<PcrSlot>::try_from(u32::from_le_bytes(selected_octets))
-            .map_err(|e| {
-                error!("Error parsing pcrSelect to a BitFlags<PcrSlot>: {}.", e);
-                Error::local_error(WrapperErrorKind::UnsupportedParam)
-            })?;
-
-        Ok(PcrSelect {
-            size_of_select,
-            selected_pcrs,
-        })
+        PcrSlotCollection::try_from((tss_pcr_select.sizeofSelect, tss_pcr_select.pcrSelect)).map(
+            |pcr_slot_collection| PcrSelect {
+                pcr_slot_collection,
+            },
+        )
     }
 }
 
 impl From<PcrSelect> for TPMS_PCR_SELECT {
     fn from(pcr_select: PcrSelect) -> Self {
+        let (size_of_select, pcr_select) = pcr_select.pcr_slot_collection.into();
         TPMS_PCR_SELECT {
-            sizeofSelect: pcr_select.size_of_select.into(),
-            pcrSelect: pcr_select.selected_pcrs.bits().to_le_bytes(),
+            sizeofSelect: size_of_select,
+            pcrSelect: pcr_select,
         }
     }
 }

tss-esapi/src/structures/pcr/select_size.rs

@@ -1,6 +1,6 @@
 // Copyright 2022 Contributors to the Parsec project.
 // SPDX-License-Identifier: Apache-2.0
-use crate::{Error, Result, WrapperErrorKind};
+use crate::{tss2_esys::TPM2_PCR_SELECT_MAX, Error, Result, WrapperErrorKind};
 
 use num_derive::{FromPrimitive, ToPrimitive};
 use num_traits::{FromPrimitive, ToPrimitive};
@@ -12,47 +12,105 @@ use std::convert::TryFrom;
 #[derive(FromPrimitive, ToPrimitive, Debug, Copy, Clone, PartialEq, Eq)]
 #[repr(u8)]
 pub enum PcrSelectSize {
-    OneByte = 1,
-    TwoBytes = 2,
-    ThreeBytes = 3,
-    FourBytes = 4,
+    OneOctet = 1,
+    TwoOctets = 2,
+    ThreeOctets = 3,
+    FourOctets = 4,
 }
 
-/// The default for PcrSelectSize is three bytes.
+impl PcrSelectSize {
+    /// Returns the PcrSelectSize value as u8
+    pub fn as_u8(&self) -> u8 {
+        // The value is well defined so unwrap will
+        // never cause panic.
+        self.to_u8().unwrap()
+    }
+
+    /// Returns the PcrSelectSize value as u32
+    pub fn as_u32(&self) -> u32 {
+        // The value is well defined so unwrap will
+        // never cause panic.
+        self.to_u32().unwrap()
+    }
+
+    /// Returns the PcrSelectSize value as usize
+    pub fn as_usize(&self) -> usize {
+        // The value is well defined so unwrap will
+        // never cause panic.
+        self.to_usize().unwrap()
+    }
+
+    /// Parses the u8 value as PcrSelectSize
+    pub fn try_parse_u8(value: u8) -> Result<Self> {
+        PcrSelectSize::from_u8(value).ok_or_else(|| {
+            error!(
+                "Error converting sizeofSelect to a SelectSize: Invalid value {}",
+                value
+            );
+            Error::local_error(WrapperErrorKind::InvalidParam)
+        })
+    }
+
+    /// Parses the u32 value as PcrSelectSize
+    pub fn try_parse_u32(value: u32) -> Result<Self> {
+        PcrSelectSize::from_u32(value).ok_or_else(|| {
+            error!(
+                "Error converting sizeofSelect to a SelectSize: Invalid value {}",
+                value
+            );
+            Error::local_error(WrapperErrorKind::InvalidParam)
+        })
+    }
+
+    /// Parses the usize value as PcrSelectSize
+    pub fn try_parse_usize(value: usize) -> Result<Self> {
+        PcrSelectSize::from_usize(value).ok_or_else(|| {
+            error!(
+                "Error converting sizeofSelect to a SelectSize: Invalid value {}",
+                value
+            );
+            Error::local_error(WrapperErrorKind::InvalidParam)
+        })
+    }
+}
+
+/// The default for PcrSelectSize is three octets.
 /// A value for the sizeofSelect that works
 /// on most platforms.
 impl Default for PcrSelectSize {
     fn default() -> PcrSelectSize {
-        PcrSelectSize::ThreeBytes
+        match TPM2_PCR_SELECT_MAX {
+            1 => PcrSelectSize::OneOctet,
+            2 => PcrSelectSize::TwoOctets,
+            _ => PcrSelectSize::ThreeOctets,
+        }
     }
 }
 
 impl TryFrom<u8> for PcrSelectSize {
     type Error = Error;
 
     fn try_from(value: u8) -> Result<Self> {
-        PcrSelectSize::from_u8(value).ok_or_else(|| {
+        if u32::from(value) > TPM2_PCR_SELECT_MAX {
             error!(
-                "Error converting sizeofSelect to a SelectSize: Invalid value {}",
-                value
+                "Found size of select value(= {}) that is larger then TPM2_PCR_SELECT_MAX(={}",
+                value, TPM2_PCR_SELECT_MAX
             );
-            Error::local_error(WrapperErrorKind::InvalidParam)
-        })
-    }
-}
+            return Err(Error::local_error(WrapperErrorKind::InvalidParam));
+        }
 
-impl From<PcrSelectSize> for u8 {
-    fn from(pcr_select_size: PcrSelectSize) -> Self {
-        // The value is well defined so unwrap will
-        // never cause panic.
-        pcr_select_size.to_u8().unwrap()
+        PcrSelectSize::try_parse_u8(value)
     }
 }
 
-impl From<PcrSelectSize> for usize {
-    fn from(pcr_select_size: PcrSelectSize) -> Self {
-        // The value is well defined so unwrap will
-        // never cause panic.
-        pcr_select_size.to_usize().unwrap()
+impl TryFrom<PcrSelectSize> for u8 {
+    type Error = Error;
+
+    fn try_from(pcr_select_size: PcrSelectSize) -> Result<Self> {
+        if pcr_select_size.as_u32() > TPM2_PCR_SELECT_MAX {
+            error!("The number of octets specified by PcrSelectSize value us greater then TPM2_PCR_SELECT_MAX");
+            return Err(Error::local_error(WrapperErrorKind::InvalidParam));
+        }
+        Ok(pcr_select_size.as_u8())
     }
 }