Copilot commented on code in PR #209:
URL: 
https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/209#discussion_r2197139357


##########
optee-utee/src/object/object_info.rs:
##########
@@ -0,0 +1,78 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use optee_utee_sys as raw;
+
+/// Represent the characteristics of an object.
+/// This info can be returned by [GenericObject](crate::GenericObject) function
+/// [info](crate::GenericObject::info)
+pub struct ObjectInfo {
+    pub(crate) raw: raw::TEE_ObjectInfo,
+}
+
+// Since raw struct is not implemented Copy attribute yet, every item in raw
+// struct needs a function to extract.
+impl ObjectInfo {
+    /// Return an [ObjectInfo](crate::ObjectInfo) struct based on the raw
+    /// structure `TEE_ObjectInfo`.
+    /// The raw structure contains following fields:
+    ///
+    /// 1) `objectType`: The parameter represents one of the
+    ///    [TransientObjectType](crate::TransientObjectType).
+    /// 2) `objectSize`: The current size in bits of the object as determined
+    ///    by its attributes.
+    ///    This will always be less than or equal to maxObjectSize. Set to 0 
for
+    ///    uninitialized and data only objects.
+    /// 3) `maxObjectSize`: The maximum objectSize which this object can
+    ///    represent.
+    ///     * For a [PersistentObject](crate::PersistentObject), set to
+    ///       `objectSize`.
+    ///     * For a [TransientObject](crate::TransientObject), set to the
+    ///       parameter `maxObjectSize` passed to
+    ///       [allocate](crate::TransientObject::allocate).
+    /// 4) `objectUsage`: A bit vector of UsageFlag.
+    /// 5) `dataSize`:
+    ///     * For a [PersistentObject](crate::PersistentObject), set to the
+    ///       current size of the data associated with the object.
+    ///     * For a [TransientObject](crate::TransientObject), always set to 0.
+    /// 6) `dataPosition`:
+    ///     * For a [PersistentObject](crate::PersistentObject), set to the
+    ///       current position in the data for this handle.
+    ///       Data positions for different handles on the same object may
+    ///       differ.
+    ///     * For a [TransientObject](crate::TransientObject), set to 0.
+    /// 7) `handleFlags`: A bit vector containing one or more
+    ///    [HandleFlag](crate::HandleFlag) or [DataFlag](crate::DataFlag).
+    pub fn from_raw(raw: raw::TEE_ObjectInfo) -> Self {
+        Self { raw }
+    }
+
+    /// Return the `dataSize` field of the raw structure `TEE_ObjectInfo`.
+    pub fn data_size(&self) -> usize {

Review Comment:
   Only `data_size`, `object_size`, and `object_type` are exposed, but other 
fields in `TEE_ObjectInfo` (e.g., `maxObjectSize`, `objectUsage`, 
`dataPosition`, `handleFlags`) lack accessors. Consider adding getters for 
those fields to complete the object info API.



##########
optee-utee/src/object/object_handle.rs:
##########
@@ -0,0 +1,185 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use alloc::boxed::Box;
+use optee_utee_sys as raw;
+
+use super::GenericObject;
+
+/// An opaque handle on an object.
+#[derive(Debug)]
+pub struct ObjectHandle(Box<raw::TEE_ObjectHandle>);
+
+impl ObjectHandle {
+    /// A null handle for later usage
+    ///
+    /// Example:
+    /// ``` rust,no_run
+    /// # use optee_utee::ObjectHandle;
+    /// let handle = ObjectHandle::new_null();
+    /// assert!(handle.is_null());
+    /// ```
+    pub fn new_null() -> Self {
+        Self(Box::new(core::ptr::null_mut()))
+    }
+
+    pub fn from_raw(raw: raw::TEE_ObjectHandle) -> ObjectHandle {
+        Self(Box::new(raw))
+    }
+
+    pub fn handle(&self) -> raw::TEE_ObjectHandle {
+        *self.0
+    }
+
+    pub fn handle_mut(&mut self) -> &mut raw::TEE_ObjectHandle {
+        &mut self.0
+    }
+
+    pub fn is_null(&self) -> bool {
+        self.0.is_null() || (*self.0).is_null()

Review Comment:
   [nitpick] The null check is duplicated (`self.0.is_null()` already checks 
the inner handle). Simplify to a single check on the dereferenced inner pointer.
   ```suggestion
           self.0.is_null()
   ```



##########
optee-utee/src/object/object_define.rs:
##########
@@ -0,0 +1,128 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use bitflags::bitflags;
+use optee_utee_sys as raw;
+
+/// Indicate the possible start offset when moving a data position in the data
+/// stream associated with a [PersistentObject](crate::PersistentObject).
+pub enum Whence {
+    /// The data position is set to offset bytes from the beginning of the 
data stream.
+    DataSeekSet,
+    /// The data position is set to its current position plus offset.
+    DataSeekCur,
+    /// The data position is set to the size of the object data plus offset.
+    DataSeekEnd,
+}
+
+impl Into<raw::TEE_Whence> for Whence {
+    fn into(self) -> raw::TEE_Whence {
+        match self {

Review Comment:
   [nitpick] Implementing `Into` is less idiomatic than providing `From<Whence> 
for raw::TEE_Whence`. Consider adding `impl From<Whence> for raw::TEE_Whence` 
and relying on the blanket `Into` implementation.
   ```suggestion
   impl From<Whence> for raw::TEE_Whence {
       fn from(value: Whence) -> raw::TEE_Whence {
           match value {
   ```



##########
optee-utee/src/object/transient_object.rs:
##########
@@ -0,0 +1,453 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use alloc::vec::Vec;
+
+use optee_utee_sys as raw;
+
+use super::{Attribute, GenericObject, ObjectHandle};
+use crate::{Error, Result};
+
+/// Define types of [TransientObject](crate::TransientObject) with
+/// predefined maximum sizes.
+#[repr(u32)]
+pub enum TransientObjectType {
+    /// 128, 192, or 256 bits
+    Aes = 0xA0000010,
+    /// Always 64 bits including the parity bits. This gives an effective key
+    /// size of 56 bits
+    Des = 0xA0000011,
+    /// 128 or 192 bits including the parity bits. This gives effective key
+    /// sizes of 112 or 168 bits
+    Des3 = 0xA0000013,
+    /// Between 64 and 512 bits, multiple of 8 bits
+    HmacMd5 = 0xA0000001,
+    /// Between 80 and 512 bits, multiple of 8 bits
+    HmacSha1 = 0xA0000002,
+    /// Between 112 and 512 bits, multiple of 8 bits
+    HmacSha224 = 0xA0000003,
+    /// Between 192 and 1024 bits, multiple of 8 bits
+    HmacSha256 = 0xA0000004,
+    /// Between 256 and 1024 bits, multiple of 8 bits
+    HmacSha384 = 0xA0000005,
+    /// Between 256 and 1024 bits, multiple of 8 bits
+    HmacSha512 = 0xA0000006,
+    /// The number of bits in the modulus. 256, 512, 768, 1024, 1536 and
+    /// 2048-bit keys SHALL be supported.
+    /// Support for other key sizes including bigger key sizes is
+    /// implementation-dependent. Minimum key size is 256 bits
+    RsaPublicKey = 0xA0000030,
+    /// Same as [RsaPublicKey](crate::TransientObjectType::RsaPublicKey) key
+    /// size.
+    RsaKeypair = 0xA1000030,
+    /// Depends on Algorithm:
+    /// 1) [DsaSha1](crate::AlgorithmId::DsaSha1):
+    /// Between 512 and 1024 bits, multiple of 64 bits
+    /// 2) [DsaSha224](crate::AlgorithmId::DsaSha224): 2048 bits
+    /// 3) [DsaSha256](crate::AlgorithmId::DsaSha256): 2048 or 3072 bits
+    DsaPublicKey = 0xA0000031,
+    /// Same as [DsaPublicKey](crate::TransientObjectType::DsaPublicKey) key
+    /// size.
+    DsaKeypair = 0xA1000031,
+    /// From 256 to 2048 bits, multiple of 8 bits.
+    DhKeypair = 0xA1000032,
+    /// Between 160 and 521 bits. Conditional: Available only if at least
+    /// one of the ECC the curves defined in Table 6-14 with "generic"
+    /// equal to "Y" is supported.
+    EcdsaPublicKey = 0xA0000041,
+    /// Between 160 and 521 bits. Conditional: Available only if at least
+    /// one of the ECC curves defined in Table 6-14 with "generic" equal to
+    /// "Y" is supported. SHALL be same value as for ECDSA public key size.
+    EcdsaKeypair = 0xA1000041,
+    /// Between 160 and 521 bits. Conditional: Available only if at least
+    /// one of the ECC curves defined in Table 6-14 with "generic" equal to
+    /// "Y" is supported.
+    EcdhPublicKey = 0xA0000042,
+    /// Between 160 and 521 bits. Conditional: Available only if at least
+    /// one of the ECC curves defined in Table 6-14 with "generic" equal to
+    /// "Y" is supported. SHALL be same value as for ECDH public key size
+    EcdhKeypair = 0xA1000042,
+    /// 256 bits. Conditional: Available only if TEE_ECC_CURVE_25519
+    /// defined in Table 6-14 is supported.
+    Ed25519PublicKey = 0xA0000043,
+    /// 256 bits. Conditional: Available only if TEE_ECC_CURVE_25519
+    /// defined in Table 6-14 is supported.
+    Ed25519Keypair = 0xA1000043,
+    /// 256 bits. Conditional: Available only if TEE_ECC_CURVE_25519
+    /// defined in Table 6-14 is supported.
+    X25519PublicKey = 0xA0000044,
+    /// 256 bits. Conditional: Available only if TEE_ECC_CURVE_25519
+    /// defined in Table 6-14 is supported.
+    X25519Keypair = 0xA1000044,
+    /// Multiple of 8 bits, up to 4096 bits. This type is intended for secret
+    /// data that has been derived from a key derivation scheme.
+    GenericSecret = 0xA0000000,
+    /// Object is corrupted.
+    CorruptedObject = 0xA00000BE,
+    /// 0 – All data is in the associated data stream.
+    Data = 0xA00000BF,
+}
+
+/// An object containing attributes but no data stream, which is reclaimed
+/// when closed or when the TA instance is destroyed.
+/// Transient objects are used to hold a cryptographic object (key or 
key-pair).
+///
+/// Contrast [PersistentObject](crate::PersistentObject).
+#[derive(Debug)]
+pub struct TransientObject(ObjectHandle);
+
+impl TransientObject {
+    /// Create an object with a null handle which points to nothing.
+    pub fn null_object() -> Self {
+        Self(ObjectHandle::new_null())
+    }
+
+    /// Check if current object is created with null handle.
+    ///
+    /// # See Also
+    ///
+    /// - [Self::null_object](Self::null_object).
+    pub fn is_null_object(&self) -> bool {
+        self.0.is_null()
+    }
+
+    /// Allocate an uninitialized object, i.e. a container for attributes.
+    ///
+    /// As allocated, the object is uninitialized.
+    /// It can be initialized by subsequently importing the object material,
+    /// generating an object, deriving an object, or loading an object from the
+    /// Trusted Storage.
+    ///
+    /// # Parameters
+    ///
+    /// 1) `object_type`: Type of uninitialized object container to be created
+    ///    as defined in [TransientObjectType](crate::TransientObjectType).
+    /// 2) `max_object_size`: Key Size of the object. Valid values depend on 
the
+    ///    object type and are defined in
+    ///    [TransientObjectType](crate::TransientObjectType).
+    ///
+    /// # Example
+    ///
+    /// ``` rust,no_run
+    /// # use optee_utee::{TransientObject, TransientObjectType};
+    /// # fn main() -> optee_utee::Result<()> {
+    /// match TransientObject::allocate(TransientObjectType::Aes, 128) {
+    ///     Ok(object) =>
+    ///     {
+    ///         // ...
+    ///         Ok(())
+    ///     }
+    ///     Err(e) => Err(e),
+    /// }
+    /// # }
+    /// ```
+    ///
+    /// # Errors
+    ///
+    /// 1) `OutOfMemory`: If not enough resources are available to allocate the
+    ///    object handle.
+    /// 2) `NotSupported`: If the key size is not supported or the object type
+    ///    is not supported.
+    ///
+    /// # Panics
+    ///
+    /// 1) If the Implementation detects any error associated with this 
function
+    ///    which is not explicitly associated with a defined return code for
+    ///    this function.
+    pub fn allocate(object_type: TransientObjectType, max_object_size: usize) 
-> Result<Self> {
+        let mut handle = ObjectHandle::new_null();
+        match unsafe {
+            raw::TEE_AllocateTransientObject(
+                object_type as u32,
+                max_object_size as u32,
+                handle.handle_mut(),
+            )
+        } {
+            raw::TEE_SUCCESS => Ok(Self(handle)),
+            code => Err(Error::from_raw_error(code)),
+        }
+    }
+
+    /// Reset the object to its initial state after allocation.
+    /// If the object is currently initialized, the function clears the object
+    /// of all its material.
+    /// The object is then uninitialized again.
+    pub fn reset(&mut self) {
+        unsafe {
+            raw::TEE_ResetTransientObject(self.handle());
+        }
+    }
+
+    /// Populate an uninitialized object container with object attributes 
passed
+    /// by the TA in the `attrs` parameter.
+    /// When this function is called, the object SHALL be uninitialized.
+    /// If the object is initialized, the caller SHALL first clear it using the
+    /// function reset.
+    /// Note that if the object type is a key-pair, then this function sets 
both
+    /// the private and public attributes of the keypair.
+    ///
+    /// # Parameters
+    ///
+    /// 1) `attrs`: Array of object attributes.
+    ///
+    /// # Example
+    ///
+    /// ``` rust,no_run
+    /// # use optee_utee::{
+    /// #     TransientObject,
+    /// #     TransientObjectType,
+    /// #     AttributeMemref,
+    /// #     AttributeId,
+    /// # };
+    /// # fn main() -> optee_utee::Result<()> {
+    /// match TransientObject::allocate(TransientObjectType::Aes, 128) {
+    ///     Ok(mut object) =>
+    ///     {
+    ///         let attrs = 
[AttributeMemref::from_ref(AttributeId::SecretValue, &[0u8;1]).into()];
+    ///         object.populate(&attrs);
+    ///         Ok(())
+    ///     }
+    ///     Err(e) => Err(e),
+    /// }
+    /// # }
+    /// ```
+    ///
+    /// # Errors
+    ///
+    /// 1) `BadParameters`: If an incorrect or inconsistent attribute value is
+    ///    detected. In this case, the content of the object SHALL remain
+    ///    uninitialized.
+    ///
+    /// # Panics
+    ///
+    /// 1) If object is not a valid opened object that is transient and
+    ///    uninitialized.
+    /// 2) If some mandatory attribute is missing.
+    /// 3) If an attribute which is not defined for the object’s type is
+    ///    present in attrs.
+    /// 4) If an attribute value is too big to fit within the maximum object
+    ///    size specified when the object was created.
+    /// 5) If the Implementation detects any other error associated with this
+    ///    function which is not explicitly associated with a defined return
+    ///    code for this function.
+    pub fn populate(&mut self, attrs: &[Attribute]) -> Result<()> {
+        let p: Vec<raw::TEE_Attribute> = attrs.iter().map(|p| 
p.raw()).collect();
+        match unsafe {
+            raw::TEE_PopulateTransientObject(self.0.handle(), p.as_ptr() as _, 
attrs.len() as u32)
+        } {
+            raw::TEE_SUCCESS => Ok(()),
+            code => return Err(Error::from_raw_error(code)),
+        }
+    }
+
+    /// Populates an uninitialized object handle with the attributes of another
+    /// object handle;
+    /// that is, it populates the attributes of this handle with the attributes
+    /// of src_handle.
+    /// It is most useful in the following situations:
+    /// 1) To extract the public key attributes from a key-pair object.
+    /// 2) To copy the attributes from a
+    ///    [PersistentObject](crate::PersistentObject) into a
+    ///    [TransientObject](crate::TransientObject).
+    ///
+    /// # Parameters
+    ///
+    /// 1) `src_object`: Can be either a
+    ///    [TransientObject](crate::TransientObject) or
+    ///    [PersistentObject](crate::PersistentObject).
+    ///
+    /// # Example
+    ///
+    /// ``` rust,no_run
+    /// # use optee_utee::{TransientObject, TransientObjectType};
+    /// # fn main() -> optee_utee::Result<()> {
+    /// match TransientObject::allocate(TransientObjectType::Aes, 128) {
+    ///     Ok(mut object1) =>
+    ///     {
+    ///         match TransientObject::allocate(TransientObjectType::Aes, 256) 
{
+    ///             Ok(object2) => {
+    ///                 object1.copy_attribute_from(&object2);
+    ///                 Ok(())
+    ///             }
+    ///             Err(e) => Err(e),
+    ///         }
+    ///     }
+    ///     Err(e) => Err(e),
+    /// }
+    /// # }
+    /// ```
+    ///
+    /// # Errors
+    ///
+    /// 1) `CorruptObject`: If the persistent object is corrupt. The object
+    ///    handle SHALL behave based on the
+    ///    `gpd.ta.doesNotCloseHandleOnCorruptObject` property.
+    /// 2) `StorageNotAvailable`: If the persistent object is stored in a
+    ///    storage area which is currently inaccessible.
+    ///
+    /// # Panics
+    ///
+    /// 1) If src_object is not initialized.
+    /// 2) If self is initialized.
+    /// 3) If the type and size of src_object and self are not compatible.
+    /// 4) If the Implementation detects any other error associated with this
+    ///    function which is not explicitly associated with a defined return
+    ///    code for this function.
+    pub fn copy_attribute_from<T: GenericObject>(&mut self, src_object: &T) -> 
Result<()> {
+        match unsafe { raw::TEE_CopyObjectAttributes1(self.handle(), 
src_object.handle()) } {
+            raw::TEE_SUCCESS => Ok(()),
+            code => Err(Error::from_raw_error(code)),
+        }
+    }
+
+    /// Generates a random key or a key-pair and populates a transient key
+    /// object with the generated key material.
+    ///
+    /// # Parameters
+    ///
+    /// 1) `key_size`: the size of the desired key. It SHALL be less than or
+    ///    equal to the maximum key size specified when the transient object
+    ///    was created.
+    ///
+    /// # Example
+    ///
+    /// ``` rust,no_run
+    /// # use optee_utee::{TransientObject, TransientObjectType};
+    /// # fn main() -> optee_utee::Result<()> {
+    /// match TransientObject::allocate(TransientObjectType::Aes, 128) {
+    ///     Ok(object) =>
+    ///     {
+    ///         object.generate_key(128, &[])?;
+    ///         Ok(())
+    ///     }
+    ///     Err(e) => Err(e),
+    /// }
+    /// # }
+    /// ```
+    ///
+    /// # Errors
+    ///
+    /// 1) `BadParameters`: If an incorrect or inconsistent attribute value is 
detected. In this
+    ///    case, the content of the object SHALL remain uninitialized.
+    ///
+    /// # Panics
+    ///
+    /// 1) If object is not a valid opened object.
+    /// 2) If some mandatory attribute is missing.
+    /// 3) If an attribute which is not defined for the object’s type is 
present
+    ///    in attrs.
+    /// 4) If an attribute value is too big to fit within the maximum object
+    ///    size specified when the object was created.
+    /// 5) If the Implementation detects any other error associated with this
+    ///    function which is not explicitly associated with a defined return
+    ///    code for this function.
+    pub fn generate_key(&self, key_size: usize, params: &[Attribute]) -> 
Result<()> {
+        let p: Vec<raw::TEE_Attribute> = params.iter().map(|p| 
p.raw()).collect();
+        unsafe {
+            match raw::TEE_GenerateKey(
+                self.handle(),
+                key_size as u32,
+                p.as_slice().as_ptr() as _,

Review Comment:
   [nitpick] Using `p.as_slice().as_ptr()` is redundant; `p.as_ptr()` is 
sufficient and clearer when obtaining a raw pointer to the Vec's contents.
   ```suggestion
                   p.as_ptr() as _,
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org
For additional commands, e-mail: dev-h...@teaclave.apache.org

Reply via email to