Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] lib: libspdm_rsp_capabilities: fixup spec conformance #2329

Closed
wants to merge 1 commit into from

Conversation

twilfredo
Copy link
Contributor

Overview [RFC]

When chunking is not supported, MaxSPDMmsgSize shall equal DataTransferSize in a GET_CAPABILITIES request. However, this is only specified in the SPDM 1.3 spec, but libspdm enforces it for >= 1.2 spec.

This change

A requester implementing 1.2 >= version < 1.3 may not conform to this as it is not specified in the 1.2 or 1.2.1 specification for a GET_CAPABILITIES request. So only enforce this if the version used is 1.3.

Context

I was testing the ongoing effort for the Linux Kernel SPDM requester implementation. When it's sending a get_capabilities request using 1.2, without chunking, the kernel implementation does not set MaxSPDMmsgSize=DataTransferSize. So such a request would be responded to with an SPDM_ERROR (0x7F) by a libspdm based responder.

Log:

SpdmSendResponse[0]: msg SPDM_VERSION(0x4), size (0xc): 
0000: 10 04 00 00 00 03 00 10 00 11 00 12 
Platform port Transmit command: 00 00 00 01 
Platform port Transmit transport_type: 00 00 00 02 
Platform port Transmit size: 00 00 00 14 
Platform port Transmit buffer:
    01 00 01 00 05 00 00 00 10 04 00 00 00 03 00 10 00 11 00 12 
Platform port Receive command: 00 00 00 01 
Platform port Receive transport_type: 00 00 00 02 
Platform port Receive size: 00 00 00 1c 
Platform port Receive buffer:
    01 00 01 00 07 00 00 00 12 e1 00 00 00 14 00 00 06 00 00 00 f8 ff 0f 00 ff ff ff ff 
SpdmReceiveRequest[.] ...
SpdmReceiveRequest[0] msg SPDM_GET_CAPABILITIES(0xe1), size (0x14): 
0000: 12 e1 00 00 00 14 00 00 06 00 00 00 f8 ff 0f 00 ff ff ff ff 
SpdmSendResponse[0] ...
SpdmSendResponse[0]: msg SPDM_ERROR(0x7f), size (0x4): 
0000: 12 7f 01 00 
Platform port Transmit command: 00 00 00 01 
Platform port Transmit transport_type: 00 00 00 02 
Platform port Transmit size: 00 00 00 0c 
Platform port Transmit buffer:
    01 00 01 00 03 00 00 00 12 7f 01 00 

TODO

Is there a more appropriate solution to this?

When chunking is not supported, MaxSPDMmsgSize shall equal
DataTransferSize in a GET_CAPABILITIES request. However,
this is only specified in the SPDM 1.3 spec.

A requester implementing 1.2 >= version < 1.3 may not conform to this as
it is not specified in the 1.2 or 1.2.1 specification for a
GET_CAPABILITIES request. So only enfore this if the version used is
1.3.

Signed-off-by: Wilfred Mallawa <[email protected]>
@steven-bellock
Copy link
Contributor

If an endpoint does not support chunking, and it sets MaxSPDMmsgSize > DataTransferSize, then what is it conveying to the peer endpoint? Presumably if chunking is not supported then, from the peer's perspective, there is only one buffer whose size is MaxSPDMmsgSize == DataTransferSize.

@twilfredo
Copy link
Contributor Author

twilfredo commented Aug 29, 2023

If an endpoint does not support chunking, and it sets MaxSPDMmsgSize > DataTransferSize, then what is it conveying to the peer endpoint? Presumably if chunking is not supported then, from the peer's perspective, there is only one buffer whose size is MaxSPDMmsgSize == DataTransferSize.

This makes sense and although I agree, does it need to be enforced with an error if the spec doesn't explicitly state as such? It's possible that a non-libspdm responder doesn't follow this given that it's not a part of the spec right?

@steven-bellock
Copy link
Contributor

We could have a flag, LIBSPDM_DISABLE_STRICT_CHECKS, that disables checks that are inferred from, but not explicitly stated in the specification. However I would consider endpoints that violate those checks to be buggy.

Note that (CHUNK_CAP == 0) -> (MaxSPDMmsgSize == DataTransferSize) as well as clarifications around ResponseTooLarge will be in the 1.2.2 specification.

@twilfredo
Copy link
Contributor Author

We could have a flag, LIBSPDM_DISABLE_STRICT_CHECKS, that disables checks that are inferred from, but not explicitly stated in the specification. However I would consider endpoints that violate those checks to be buggy.

Note that (CHUNK_CAP == 0) -> (MaxSPDMmsgSize == DataTransferSize) as well as clarifications around ResponseTooLarge will be in the 1.2.2 specification.

Great, I think this particular case we can wait for the 1.2.2 spec and just patch the kernel implementation.

As far as LIBSPDM_DISABLE_STRICT_CHECKS, sounds like a good idea also. Perhaps worth adding if we run into more cases like this... not sure if there already are many other places where things are inferred...are there?

@steven-bellock
Copy link
Contributor

not sure if there already are many other places where things are inferred...are there?

Some of the capabilities checks are inferred from the specification. But again, if an endpoint violates them then it's buggy / misconfigured.

@twilfredo
Copy link
Contributor Author

not sure if there already are many other places where things are inferred...are there?

Some of the capabilities checks are inferred from the specification. But again, if an endpoint violates them then it's buggy / misconfigured.

I see, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants