DMTF / libspdm

BSD 3-Clause "New" or "Revised" License
113 stars 102 forks source link

SET_CERT: ResetRequired handling #2864

Open rw8896 opened 3 weeks ago

rw8896 commented 3 weeks ago

It seems that the following behaviors haven't been implemented in libspdm yet. If that's the case, is there any existing issue or plan to fix it?

"If the device temporarily cannot write to a slot, including in a case when it receives overlapping SET_CERTIFICATE requests from different Requesters, then the device shall respond with anErrorCode=Busy response.

When a reset is required for a pending previous SET_CERTIFICATE request and the device receives a GET_CERTIFICATE request for a pending slot or a GET_DIGESTS request, the device shall respond with an ErrorCode=ResetRequired response."

steven-bellock commented 2 weeks ago

is there any existing issue

Not that I know of.

or plan to fix it?

There can be, if it is needed. I think in a pull request I commented that libspdm needs to check with the Integrator before sending ResetRequired. In particular libspdm_gen_csr has https://github.com/DMTF/libspdm/blob/5149b567a6dc2eecc5dc9c48978a1782edc031ed/include/hal/library/responder/csrlib.h#L23-L26 whereas there's no such check for SET_CERTIFICATE. In particular libspdm blindly sends ResetRequired if the Responder's CERT_INSTALL_RESET_CAP is set : https://github.com/DMTF/libspdm/blob/5149b567a6dc2eecc5dc9c48978a1782edc031ed/library/spdm_responder_lib/libspdm_rsp_set_certificate.c#L302-L309 which is not entirely correct. Ie whether the reset is required may be conditional on other things besides the capability. So maybe libspdm_write_certificate_to_nvm can return whether errors are required to libspdm. For the Busy response to GET_CERTIFICATE we just need a per-slot way for the Integrator to communicate the state of the certificate slot in the spdm_context.