Re: [PATCH v3 16/21] lib: rspdm: Support SPDM negotiate_algorithms

[Aksh Garg <a-garg7@ti.com>]

On 01/09/26 06:33, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair@alistair23.me>
> 
> Support the NEGOTIATE_ALGORITHMS SPDM command.
> 
> Signed-off-by: Alistair Francis <alistair@alistair23.me>
> ---
>   lib/rspdm/consts.rs    |  56 +++++++++-
>   lib/rspdm/lib.rs       |   9 +-
>   lib/rspdm/state.rs     | 243 ++++++++++++++++++++++++++++++++++++++++-
>   lib/rspdm/validator.rs | 110 ++++++++++++++++++-
>   4 files changed, 412 insertions(+), 6 deletions(-)

[...]

> +
> +    pub(crate) fn negotiate_algs(&mut self) -> Result<(), Error> {
> +        let mut request = NegotiateAlgsReq::default();
> +        request.version = self.version;
> +
> +        if self.version >= SPDM_VER_12 && (self.rsp_caps & SPDM_KEY_EX_CAP) == SPDM_KEY_EX_CAP {
> +            request.other_params_support = SPDM_OPAQUE_DATA_FMT_GENERAL;
> +        }
> +
> +        let req_sz = core::mem::size_of::<NegotiateAlgsReq>();
> +        let rsp_sz = core::mem::size_of::<NegotiateAlgsRsp>();
> +
> +        request.length = (req_sz as u16).to_le();
> +
> +        // SAFETY: `request` is repr(C) and packed, so we can convert it to a slice
> +        let request_buf = unsafe { from_raw_parts_mut(&mut request as *mut _ as *mut u8, req_sz) };
> +
> +        let mut response_vec: KVec<u8> = KVec::from_elem(0u8, rsp_sz, GFP_KERNEL)?;
> +
> +        let rc = self.spdm_exchange(request_buf, response_vec.as_mut_slice())? as usize;
> +
> +        // The transport must report a length within the buffer we provided.
> +        if rc > response_vec.len() {
> +            pr_err!("Overflowed capabilities response\n");
> +            return Err(EIO);
> +        }
> +        response_vec.truncate(rc);
> +
> +        let response: &NegotiateAlgsRsp = Untrusted::new(response_vec.as_slice()).validate()?;
> +
> +        self.base_asym_alg = u32::from_le(response.base_asym_sel);
> +        self.base_hash_alg = u32::from_le(response.base_hash_sel);
> +        self.meas_hash_alg = u32::from_le(response.measurement_hash_algo);
> +
> +        if self.base_asym_alg & SPDM_ASYM_ALGOS == 0 || self.base_hash_alg & SPDM_HASH_ALGOS == 0 {
> +            pr_err!("No common supported algorithms\n");
> +            return Err(EPROTO);
> +        }
> +
> +        // /* Responder shall select exactly 1 alg (SPDM 1.0.0 table 14) */
> +        if self.base_asym_alg.count_ones() != 1
> +            || self.base_hash_alg.count_ones() != 1
> +            || self.meas_hash_alg.count_ones() != 1

Hi Alistair,

As per the SPDM spec v1.4.0 Table 25 (ALGORITHMS response message
format), description of the field 'MeasurementHashAlgo' states "If the
Responder supports measurements (MEAS_CAP=01b or MEAS_CAP=10b in its
CAPABILITIES response) and if MeasurementSpecificationSel is non-zero,
then exactly one bit in this bit field shall be set. Otherwise, the
Responder shall set this field to 0".

Currently, the code unconditionally checks for exactly one bit. If a
responder does not support measurements, it will legitimately set this
field to 0, which causes the current check to incorrectly fail.

A change somewhat similar to the diff shown below might help here:


              return Err(EPROTO);
          }

+        let meas_hash_valid = if self.rsp_caps & SPDM_MEAS_CAP != 0
+            && response.measurement_specification_sel != 0
+        {
+            self.meas_hash_alg.count_ones() == 1
+        } else {
+            self.meas_hash_alg == 0
+        };
+
          // /* Responder shall select exactly 1 alg (SPDM 1.0.0 table 
14) */
          if self.base_asym_alg.count_ones() != 1
              || self.base_hash_alg.count_ones() != 1
-            || self.meas_hash_alg.count_ones() != 1
+            || !meas_hash_valid
              || response.ext_asym_sel_count != 0
              || response.ext_hash_sel_count != 0
              || response.param1 > request.param1

Regards,
Aksh Garg

> +            || response.ext_asym_sel_count != 0
> +            || response.ext_hash_sel_count != 0
> +            || response.param1 > request.param1
> +            || response.other_params_sel != request.other_params_support
> +        {
> +            pr_err!("Malformed algorithms response\n");
> +            return Err(EPROTO);
> +        }
> +
> +        self.update_response_algs()?;
> +
> +        Ok(())
> +    }