Enforce non-zero CertModel

[steven-bellock]

Fix #2581.

Signed-off-by: Steven Bellock [email protected]

[steven-bellock]

I'll add the unit tests for CSR in a subsequent push.

[Wenxing-hou]

Hi. It is better to use:
req_cert_model >= SPDM_GET_CSR_REQUEST_ATTRIBUTES_MAX_CSR_CERT_MODEL

[steven-bellock]

It can't be 0 either. I'll make a helper function in common_lib and then this logic can be consolidated.

[steven-bellock]

Didn't make a helper function for now.

[steven-bellock]

I'll add the unit tests for CSR in a subsequent push.

The unit test has been added.

[jyao1]

@steven-bellock , have you validated spdm-emu?

I notice spdm-emu failed on set_cert with latest libspdm. Would you please take a look?

libspdm_set_certificate - 80010001
do_certificate_provising_via_spdm - 80010001
do_session_via_spdm - 80010001

[steven-bellock]

have you validated spdm-emu?

I have not. I'll have a look at it.

[steven-bellock]

@jyao1 the spdm-emu issue is due to the Requester using the illegal combination

• MULTI_KEY_CONN_RSP is true.
• Erase is false.
• CertModel is 0.

In particular if MULTI_KEY_CONN_RSP is true then the Integrator needs to use the libspdm_set_certificate_ex function.

[steven-bellock]

As an aside, after re-reading the specification and looking at this pull request, it might be too aggressive with the key_pair_id check. For SET_CERTIFICATE it currently has

        if (spdm_context->connection_info.multi_key_conn_rsp) {
            if (key_pair_id == 0) {
                return LIBSPDM_STATUS_INVALID_PARAMETER;
            }

But maybe this should only be checked if Erase is false? I'll make a follow-up issue and pull request.

[jyao1]

Yes, please submit PR to fix. We need this patch for Q2 release.

[steven-bellock]

I have a fix for spdm-emu but #2746 needs to be fixed on the libspdm side first.