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.

Sign up for GitHub

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

Jump to bottom

SessionID creation is backwards #1424

Closed
steven-bellock opened this issue Nov 23, 2022 · 4 comments · Fixed by #1450
Closed

SessionID creation is backwards #1424

steven-bellock opened this issue Nov 23, 2022 · 4 comments · Fixed by #1450
Assignees
@Wenxing-hou
Labels
bug Something isn't working stable For upcoming stable release
Milestone
Q4 2022

Comments

@steven-bellock
Copy link
Contributor

The specification says

The final session ID (SessionID) = Concatenate (ReqSessionID, RspSessionID).

where presumably ReqSessionID occupies the least-significant bytes and RspSessionID occupies the most-significant bytes. However in libspdm it is the opposite :

libspdm/library/spdm_requester_lib/libspdm_req_key_exchange.c

Line 530 in 8a5f88b

*session_id = (req_session_id << 16) | rsp_session_id;

@steven-bellock steven-bellock added the bug Something isn't working label Nov 23, 2022
@steven-bellock steven-bellock added this to the Q4 2022 milestone Nov 23, 2022
@steven-bellock
Copy link
Contributor Author

Additionally the specification doesn't formally define Concatenate(), which it should. I'll file an issue against the specification.

@steven-bellock
Copy link
Contributor Author

Issue has been filed at https://github.com/DMTF/SPDM-WG/issues/2281

@steven-bellock steven-bellock mentioned this issue Nov 28, 2022
Closed
@jyao1 jyao1 added the stable For upcoming stable release label Nov 28, 2022
@jyao1
Copy link
Member

jyao1 commented Nov 28, 2022

This will break the compatibility. We need highlight in release notes.

@steven-bellock
Copy link
Contributor Author

(req_session_id << 16) | rsp_session_id is in enough places (four) that it should be its own function.

uint32_t libspdm_generate_session_id (uint16_t req_session_id, uint16_t rsp_session_id)

Wenxing-hou added a commit to Wenxing-hou/libspdm that referenced this issue Dec 1, 2022
@Wenxing-hou
Fix session_id generate
eb72748 
Fix the issue: DMTF#1424
Make session_id generate right.

Signed-off-by: Wenxing Hou <[email protected]>
@Wenxing-hou Wenxing-hou mentioned this issue Dec 1, 2022
Merged
steven-bellock pushed a commit that referenced this issue Dec 1, 2022
@Wenxing-hou @steven-bellock
Fix session_id generate
fc93e8b 
Fix the issue: #1424
Make session_id generate right.

Signed-off-by: Wenxing Hou <[email protected]>
@jyao1 jyao1 mentioned this issue Dec 11, 2022
Closed
@xiangfeima xiangfeima mentioned this issue Jan 2, 2023
Closed
@longlongyang longlongyang mentioned this issue Mar 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Wenxing-hou Wenxing-hou

Labels
bug Something isn't working stable For upcoming stable release
Projects
None yet
Milestone
Q4 2022
Development

Successfully merging a pull request may close this issue.

Fix session_id generate Wenxing-hou/libspdm
3 participants
@steven-bellock @jyao1 @Wenxing-hou