Re: [PATCH v2 07/18] spdm: Introduce library to authenticate devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/7/24 22:54, Lukas Wunner wrote:
On Mon, Jul 08, 2024 at 07:57:02PM +1000, Alexey Kardashevskiy wrote:
+	rc = spdm_exchange(spdm_state, req, req_sz, rsp, rsp_sz);

rsp_sz is 36 bytes here. And spdm_exchange() cannot return more than 36
because this is how pci_doe() works...

+	if (rc < 0)
+		return rc;
+
+	length = rc;
+	if (length < sizeof(*rsp) ||
+	    length < sizeof(*rsp) + rsp->param1 * sizeof(*req_alg_struct)) {
+		dev_err(spdm_state->dev, "Truncated algorithms response\n");

... but here you expect more than 36 as realistically rsp->param1 > 0.
How was this tested and what do I miss here?

I assume you tested this patch set against a libspdm responder
and got a "Truncated algorithms response" error.

It is against a device with libspdm in its firmware, likely to be older than 3.1.0.

The short answer is, it's a bug in libspdm and the issue should
go away once you update libspdm to version 3.1.0 or newer.

Easier to hack lib/spdm/req-authenticate.c just to see how far I can get with my device, now it is "Malformed certificate at slot 0 offset 0". It is just a bit inconvenient that CMA is not a module and requires the system reboot after every change.

If you need to stay at an older version, consider cherry-picking
libspdm commits 941f0ae0d24e ("libspdm_rsp_algorithms: fixup spec
conformance") and 065fb17b74c7 ("responder: negotiate algorithms
conformance").

The bug was found and fixed by Wilfred Mallawa when testing the
in-kernel SPDM implementation against libspdm:

https://github.com/l1k/linux/issues/3
https://github.com/DMTF/libspdm/pull/2341
https://github.com/DMTF/libspdm/issues/2344
https://github.com/DMTF/libspdm/pull/2353

Problem is, most SPDM-enabled products right now are based on
libspdm (the DMTF reference implementation) and thus are bug-by-bug
compatible.  However such a software monoculture is dangerous and
having a from-scratch kernel implementation has already proven useful
to identify issues like this which otherwise wouldn't have been noticed.

True and a bit hilarious :)

The in-kernel SPDM implementation currently doesn't send any
ReqAlgStructs and per the spec, the responder isn't supposed to
send any RespAlgStructs which the requester didn't ask for.
Yet libspdm always sent a hardcoded array of RespAlgStructs.

Uff, I see. So it should probably be "Malformed algorithms response" (where param1 is actually checked) than "Truncated algorithms response", a minor detail though. Thanks for the explanation.

So the *reference* implementation wasn't conforming to the spec. :(

Thanks,

Lukas

--
Alexey





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux