Merge pull request #377 from Superhepper/session-attributes-clear-bits

Adds validation of session attributes.

Author: Superhepper

tss-esapi/src/attributes/session.rs

@@ -1,10 +1,11 @@
-use crate::tss2_esys::TPMA_SESSION;
+use crate::{tss2_esys::TPMA_SESSION, Error, Result, WrapperErrorKind};
 use bitfield::bitfield;
+use std::convert::TryFrom;
 
 // SESSION ATTRIBUTES
 
 bitfield! {
-    /// Bitfield representing the session attributes.
+    /// Struct representing the session attributes.
     #[derive(Copy, Clone, Eq, PartialEq)]
     pub struct SessionAttributes(TPMA_SESSION);
     impl Debug;
@@ -15,7 +16,7 @@ bitfield! {
     pub audit_exclusive, _: 1;
     _, set_audit_reset: 2;
     pub audit_reset, _: 2;
-    // Reserved 3,4 (Shall be clear)
+    reserved, _: 4, 3; // Reserved 3,4 (Shall be clear)
     _, set_decrypt: 5;
     pub decrypt, _: 5;
     _, set_encrypt: 6;
@@ -29,32 +30,47 @@ impl SessionAttributes {
     pub const fn builder() -> SessionAttributesBuilder {
         SessionAttributesBuilder::new()
     }
+
+    /// Validates the session attributes
+    pub fn validate(&self) -> Result<()> {
+        if self.reserved() != 0 {
+            return Err(Error::local_error(WrapperErrorKind::InvalidParam));
+        }
+        Ok(())
+    }
 }
 
-impl From<TPMA_SESSION> for SessionAttributes {
-    fn from(tss_session_attributes: TPMA_SESSION) -> SessionAttributes {
-        SessionAttributes(tss_session_attributes)
+impl TryFrom<TPMA_SESSION> for SessionAttributes {
+    type Error = Error;
+
+    fn try_from(tss_session_attributes: TPMA_SESSION) -> Result<SessionAttributes> {
+        let result = SessionAttributes(tss_session_attributes);
+        result.validate()?;
+        Ok(result)
     }
 }
 
-impl From<SessionAttributes> for TPMA_SESSION {
-    fn from(session_attributes: SessionAttributes) -> TPMA_SESSION {
-        session_attributes.0
+impl TryFrom<SessionAttributes> for TPMA_SESSION {
+    type Error = Error;
+
+    fn try_from(session_attributes: SessionAttributes) -> Result<TPMA_SESSION> {
+        session_attributes.validate()?;
+        Ok(session_attributes.0)
     }
 }
 
 // SESSION ATTRIBUTES MASK
 
 bitfield! {
-    /// Bitfield representing the session attributes mask.
+    /// Struct representing the session attributes mask.
     #[derive(Copy, Clone, Eq, PartialEq)]
     pub struct SessionAttributesMask(TPMA_SESSION);
     impl Debug;
 
     _, use_continue_session: 0;
     _, use_audit_exclusive: 1;
     _, use_audit_reset: 2;
-    // Reserved 3,4 (Shall be clear)
+    reserved, _: 4, 3; // Reserved 3,4 (Shall be clear)
     _, use_decrypt: 5;
     _, use_encrypt: 6;
     _, use_audit: 7;
@@ -65,17 +81,32 @@ impl SessionAttributesMask {
     pub const fn builder() -> SessionAttributesBuilder {
         SessionAttributesBuilder::new()
     }
+
+    /// Validates the session attributes
+    pub fn validate(&self) -> Result<()> {
+        if self.reserved() != 0 {
+            return Err(Error::local_error(WrapperErrorKind::InvalidParam));
+        }
+        Ok(())
+    }
 }
 
-impl From<TPMA_SESSION> for SessionAttributesMask {
-    fn from(tss_session_attributes: TPMA_SESSION) -> SessionAttributesMask {
-        SessionAttributesMask(tss_session_attributes)
+impl TryFrom<TPMA_SESSION> for SessionAttributesMask {
+    type Error = Error;
+
+    fn try_from(tpma_session: TPMA_SESSION) -> Result<SessionAttributesMask> {
+        let result = SessionAttributesMask(tpma_session);
+        result.validate()?;
+        Ok(result)
     }
 }
 
-impl From<SessionAttributesMask> for TPMA_SESSION {
-    fn from(session_attributes_mask: SessionAttributesMask) -> TPMA_SESSION {
-        session_attributes_mask.0
+impl TryFrom<SessionAttributesMask> for TPMA_SESSION {
+    type Error = Error;
+
+    fn try_from(session_attributes_mask: SessionAttributesMask) -> Result<TPMA_SESSION> {
+        session_attributes_mask.validate()?;
+        Ok(session_attributes_mask.0)
     }
 }
 

tss-esapi/src/context/session_administration.rs

@@ -8,6 +8,7 @@ use crate::{
     Context, Result, ReturnCode,
 };
 use log::error;
+use std::convert::TryInto;
 
 impl Context {
     /// Set the given attributes on a given session.
@@ -22,8 +23,8 @@ impl Context {
                 Esys_TRSess_SetAttributes(
                     self.mut_context(),
                     SessionHandle::from(session).into(),
-                    attributes.into(),
-                    mask.into(),
+                    attributes.try_into()?,
+                    mask.try_into()?,
                 )
             },
             |ret| {

tss-esapi/tests/integration_tests/attributes_tests/session_attributes_tests.rs

@@ -1,39 +1,79 @@
 // Copyright 2022 Contributors to the Parsec project.
 // SPDX-License-Identifier: Apache-2.0
+use std::convert::{TryFrom, TryInto};
 use tss_esapi::{
     attributes::{SessionAttributes, SessionAttributesBuilder, SessionAttributesMask},
     tss2_esys::TPMA_SESSION,
+    Error, WrapperErrorKind,
 };
 
 macro_rules! test_valid_conversion {
     ($tpm_value:expr, $method:ident) => {
-        let tpma_session_attribute: TPMA_SESSION = $tpm_value;
-        let session_attributes = SessionAttributes::from(tpma_session_attribute);
+        let tpma_session: TPMA_SESSION = $tpm_value;
+        let session_attributes = SessionAttributes::try_from(tpma_session).expect("Failed to convert TPMA_SESSION into SessionAttributes");
         assert_eq!(
             true,
             session_attributes.$method(),
-            "SessionAttributes converted from TPMA_SESSION = {} did not produce the expected result with regard to {}.", tpma_session_attribute, stringify!($method),
+            "SessionAttributes converted from TPMA_SESSION = {} did not produce the expected result with regard to {}.", tpma_session, stringify!($method),
         );
         assert_eq!(
-            tpma_session_attribute,
-            session_attributes.into(),
+            tpma_session,
+            session_attributes.try_into().expect("Failed to convert SessionAttributes into TPMA_SESSION_ATTRIBUTE."),
             "Converting session attributes with {} set did not convert into the expected TPMA_SESSION value", std::stringify!($method),
         );
     };
 }
 
 macro_rules! test_valid_mask_conversion {
     ($tpm_value:expr, $attribute:tt) => {
-        let tpma_session_attribute: TPMA_SESSION = $tpm_value;
-        let session_attributes_mask = SessionAttributesMask::from(tpma_session_attribute);
+        let tpma_session: TPMA_SESSION = $tpm_value;
+        let session_attributes_mask = SessionAttributesMask::try_from(tpma_session).expect("Failed to convert TPMA_SESSION into SessionAttributesMask");
         assert_eq!(
-            tpma_session_attribute,
-            session_attributes_mask.into(),
+            tpma_session,
+            session_attributes_mask.try_into().expect("Failed to convert SessionAttributesMask into TPMA_SESSION"),
             "Converting session attributes mask with {} set did not convert into the expected TPMA_SESSION value", $attribute,
         );
     };
 }
 
+macro_rules! test_conversion_with_reserved_bits_set {
+    ($tpm_value:expr) => {
+        let tpma_session: TPMA_SESSION = $tpm_value;
+        assert_eq!(
+            Err(Error::WrapperError(WrapperErrorKind::InvalidParam)),
+            SessionAttributes::try_from(tpma_session),
+            "Converting TPMA_SESSION into SessionAttributes with reserved bits sets did not produce the correct error"
+        );
+    };
+}
+
+macro_rules! test_mask_conversion_with_reserved_bits_set {
+    ($tpm_value:expr) => {
+        let tpma_session: TPMA_SESSION = $tpm_value;
+        assert_eq!(
+            Err(Error::WrapperError(WrapperErrorKind::InvalidParam)),
+            SessionAttributesMask::try_from(tpma_session),
+            "Converting TPMA_SESSION into SessionAttributesMask with reserved bits sets did not produce the correct error"
+        );
+    };
+}
+
+#[test]
+fn test_validate_() {
+    let valid = SessionAttributes(0b11100111u8);
+    let invalid = SessionAttributes(0b00011000u8);
+    assert_eq!(
+        Ok(()),
+        valid.validate(),
+        "Valid SessionAttributes value generated an unexpected error when calling 'validate'"
+    );
+    assert_eq!(
+        Err(Error::WrapperError(WrapperErrorKind::InvalidParam)),
+        invalid.validate(),
+        "Invalid SessionAttributes value did not generate the expected error."
+    );
+}
+
 #[test]
 fn test_valid_session_attributes_conversions() {
     test_valid_conversion!(
@@ -67,12 +107,14 @@ fn test_valid_session_attributes_conversions() {
     );
 }
 
-#[ignore]
 #[test]
 fn test_invalid_session_attributes_conversions() {
-    // bit 3 and 4 are reserved and shall be cleared.
-    // No error for this is implemented.
-    // See https://github.com/parallaxsecond/rust-tss-esapi/issues/330
+    test_conversion_with_reserved_bits_set!(1u8
+        .checked_shl(3)
+        .expect("Failed to create value with resrved bit 3 set"));
+    test_conversion_with_reserved_bits_set!(1u8
+        .checked_shl(4)
+        .expect("Failed to create value with resrved bit 4 set"));
 }
 
 #[test]
@@ -109,12 +151,14 @@ fn test_valid_session_attributes_mask_conversions() {
     );
 }
 
-#[ignore]
 #[test]
 fn test_invalid_session_attributes_mask_conversions() {
-    // bit 3 and 4 are reserved and shall be cleared.
-    // No error for this is implemented.
-    // See https://github.com/parallaxsecond/rust-tss-esapi/issues/330
+    test_mask_conversion_with_reserved_bits_set!(1u8
+        .checked_shl(3)
+        .expect("Failed to create value with resrved bit 3 set"));
+    test_mask_conversion_with_reserved_bits_set!(1u8
+        .checked_shl(4)
+        .expect("Failed to create value with resrved bit 4 set"));
 }
 
 #[test]
@@ -128,35 +172,53 @@ fn test_session_attributes_builder_constructing() {
 #[test]
 fn test_builder_from_session_attributes() {
     let (attributes, mask) = SessionAttributes::builder().build();
-    assert_eq!(SessionAttributes::from(0), attributes, "Building session attributes without anything set using SessionAttributes::builder() did not produce expected result");
-    assert_eq!(SessionAttributesMask::from(0), mask, "Building sesssion attributes mask without anything set using SessionAttributes::builder() did not produce expected result")
+    assert_eq!(SessionAttributes::try_from(0).expect("Failed to convert 0 into SessionAttributes"), attributes, "Building session attributes without anything set using SessionAttributes::builder() did not produce expected result");
+    assert_eq!(SessionAttributesMask::try_from(0).expect("Failed to convert 0 into SessionAttributesMask"), mask, "Building session attributes mask without anything set using SessionAttributes::builder() did not produce expected result")
 }
 
 #[test]
 fn test_builder_from_session_attributes_mask() {
     let (attributes, mask) = SessionAttributesMask::builder().build();
-    assert_eq!(SessionAttributes::from(0), attributes, "Building session attributes without anything set using SessionAttributesMask::builder() did not produce expected result");
-    assert_eq!(SessionAttributesMask::from(0), mask, "Building sesssion attributes mask without anything set using SessionAttributesMask::builder() did not produce expected result")
+    assert_eq!(SessionAttributes::try_from(0).expect("Failed to convert 0 into SessionAttributes"), attributes, "Building session attributes without anything set using SessionAttributesMask::builder() did not produce expected result");
+    assert_eq!(SessionAttributesMask::try_from(0).expect("Failed to convert 0 into SessionAttributesMask"), mask, "Building session attributes mask without anything set using SessionAttributesMask::builder() did not produce expected result")
 }
 
 #[test]
 fn test_builder_from_session_attributes_builder_default() {
     let (attributes, mask) = SessionAttributesBuilder::default().build();
-    assert_eq!(SessionAttributes::from(0), attributes, "Building session attributes without anything set using SessionAttributesBuilder::default() did not produce expected result");
-    assert_eq!(SessionAttributesMask::from(0), mask, "Building sesssion attributes mask without anything set using SessionAttributesBuilder::default() did not produce expected result")
+    assert_eq!(SessionAttributes::try_from(0).expect("Failed to convert 0 into SessionAttributes"), attributes, "Building session attributes without anything set using SessionAttributesBuilder::default() did not produce expected result");
+    assert_eq!(SessionAttributesMask::try_from(0).expect("Failed to convert 0 into SessionAttributesMask"), mask, "Building session attributes mask without anything set using SessionAttributesBuilder::default() did not produce expected result")
 }
 
 #[test]
 fn test_builder_from_session_attributes_builder_new() {
     let (attributes, mask) = SessionAttributesBuilder::new().build();
-    assert_eq!(SessionAttributes::from(0), attributes, "Building session attributes without anything set using SessionAttributesBuilder::new() did not produce expected result");
-    assert_eq!(SessionAttributesMask::from(0), mask, "Building sesssion attributes mask without anything set using SessionAttributesBuilder::new() did not produce expected result")
+    assert_eq!(SessionAttributes::try_from(0).expect("Failed to convert 0 into SessionAttributes"), attributes, "Building session attributes without anything set using SessionAttributesBuilder::new() did not produce expected result");
+    assert_eq!(SessionAttributesMask::try_from(0).expect("Failed to convert 0 into SessionAttributesMask"), mask, "Building session attributes mask without anything set using SessionAttributesBuilder::new() did not produce expected result")
+}
+
+#[test]
+fn test_mask_validate_() {
+    let valid = SessionAttributesMask(0b11100111u8);
+    let invalid = SessionAttributesMask(0b00011000u8);
+    assert_eq!(
+        Ok(()),
+        valid.validate(),
+        "Valid SessionAttributesMask value generated an unexpected error when calling 'validate'"
+    );
+    assert_eq!(
+        Err(Error::WrapperError(WrapperErrorKind::InvalidParam)),
+        invalid.validate(),
+        "Invalid SessionAttributesMask value did not generate the expected error."
+    );
 }
 
 #[test]
 fn test_session_attributes_builder() {
-    let expected_session_attributes = SessionAttributes::from(0b11100111u8);
-    let expected_session_attributes_mask = SessionAttributesMask::from(0b11100111u8);
+    let expected_session_attributes = SessionAttributes::try_from(0b11100111u8)
+        .expect("Failed to convert 0b11100111u8 into SessionAttributes");
+    let expected_session_attributes_mask = SessionAttributesMask::try_from(0b11100111u8)
+        .expect("Failed to convert 0b11100111u8 into SessionAttributesMask");
 
     let (actual_session_attributes, actual_session_attributes_mask) =
         SessionAttributesBuilder::new()