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

Add Set_cert key_pair_id parameter to sync spdm 1.3 #2613

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

Wenxing-hou
Copy link
Contributor

Fix: #2292

@Wenxing-hou Wenxing-hou force-pushed the update_setcert_keypairid branch from 3fc0e5e to 94dd5d2 Compare March 7, 2024 06:21
@jyao1
Copy link
Member

jyao1 commented Mar 7, 2024

Is this a full patch?

@Wenxing-hou Wenxing-hou force-pushed the update_setcert_keypairid branch 2 times, most recently from c50e48c to b231ae9 Compare March 11, 2024 01:37
@Wenxing-hou
Copy link
Contributor Author

Is this a full patch?

Thanks. I have updated the patch to make it completely.

/*write KeyPairID.*/
spdm_context->local_context.local_key_pair_id[slot_id] = key_pair_id;
} else {
if (spdm_request->header.param2 != 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check should only in 1.3+.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have deleted the check in 1.2.

@Wenxing-hou Wenxing-hou force-pushed the update_setcert_keypairid branch from b231ae9 to cf6a10f Compare March 11, 2024 02:06
key_pair_id = spdm_request->header.param2;
/*write KeyPairID.*/
spdm_context->local_context.local_key_pair_id[slot_id] = key_pair_id;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to check version >= 1.3 && multi_key_conn_rsp == false.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have added the check.

(spdm_context->connection_info.multi_key_conn_rsp)) {
key_pair_id = spdm_request->header.param2;
/*write KeyPairID.*/
spdm_context->local_context.local_key_pair_id[slot_id] = key_pair_id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add comment: key_pair_id / slot_id check will be done in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have added the comment.

@Wenxing-hou Wenxing-hou force-pushed the update_setcert_keypairid branch 2 times, most recently from ab3220d to 99b4331 Compare March 12, 2024 07:11
@jyao1 jyao1 requested a review from steven-bellock March 13, 2024 02:00
key_pair_id = spdm_request->header.param2;
if (spdm_context->connection_info.multi_key_conn_rsp) {
/*write KeyPairID. And key_pair_id/slot_id check will be done in the future.*/
spdm_context->local_context.local_key_pair_id[slot_id] = key_pair_id;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libspdm currently does not update the local context for SET_CERTIFICATE. That's left to the Integrator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. It should just check if key_pair_id is 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have updated the code.

@Wenxing-hou Wenxing-hou force-pushed the update_setcert_keypairid branch 2 times, most recently from 9be5608 to f95ec3a Compare March 14, 2024 13:23
Ref the issue: DMTF#2292

Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@Wenxing-hou Wenxing-hou force-pushed the update_setcert_keypairid branch from f95ec3a to f4d7fe1 Compare March 15, 2024 01:17
@jyao1 jyao1 requested a review from steven-bellock March 15, 2024 01:53
@jyao1
Copy link
Member

jyao1 commented Mar 20, 2024

Have you validated spdm-emu?

@jyao1 jyao1 merged commit c9088e0 into DMTF:main Mar 21, 2024
97 checks passed
@Wenxing-hou Wenxing-hou deleted the update_setcert_keypairid branch June 25, 2024 03:10
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.

[SPDM 1.3] SET_CERT: param1 and param2
3 participants