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

question on RSA/ECDSA signature endianness in SPDM 1.0/1.1 #2151

Closed
jyao1 opened this issue Jun 19, 2023 · 7 comments
Closed

question on RSA/ECDSA signature endianness in SPDM 1.0/1.1 #2151

jyao1 opened this issue Jun 19, 2023 · 7 comments
Assignees
Milestone

Comments

@jyao1
Copy link
Member

jyao1 commented Jun 19, 2023

Current signature endianness is clarified in SPDM 1.2.1.

But SPDM 1.0/1.1 does not include the clarification.

SPDM issue: https://github.com/DMTF/SPDM-WG/issues/2929

@steven-bellock
Copy link
Contributor

Ideally there should be no byte-swapping, as that was the intent starting with 1.0.

@jyao1 jyao1 self-assigned this Jun 21, 2023
@xiaoyuruan
Copy link

According to the SPDM WG, SPDM 1.0/1.1 allows big or little endian, chosen by implementation. As such, I propose that:

  1. Provide compile-time option for libspdm Requester's signature generation, if negotiated VERSION == 1.0 or 1.1: choose one from: {big endian; little endian}.
  2. Provide compile-time option for libspdm Responder's signature verification, if negotiated VERSION == 1.0 or 1.1: choose one from: {big endian; little endian; flexible - try both and accept signature if one passes}.

Understood this will not intercept libspdm3.0.0, but later.

Please assign Rich Kong to it. @richkong88

@jyao1 jyao1 assigned richkong88 and unassigned jyao1 Jul 1, 2023
@jyao1
Copy link
Member Author

jyao1 commented Jul 1, 2023

I think we need define a set of MACRO to control the different behavior:

LIBSPDM_SPDM_10_RSA_LITTLE_ENDIAN
LIBSPDM_SPDM_10_ECDSA_LITTLE_ENDIAN
LIBSPDM_SPDM_10_RSA_DUAL_VERIFICATION
LIBSPDM_SPDM_10_ECDSA_DUAL_VERIFICATION
LIBSPDM_SPDM_11_RSA_LITTLE_ENDIAN
LIBSPDM_SPDM_11_ECDSA_LITTLE_ENDIAN
LIBSPDM_SPDM_11_RSA_DUAL_VERIFICATION
LIBSPDM_SPDM_11_ECDSA_DUAL_VERIFICATION

@richkong88
Copy link
Contributor

I think we need define a set of MACRO to control the different behavior:

LIBSPDM_SPDM_10_RSA_LITTLE_ENDIAN
LIBSPDM_SPDM_10_ECDSA_LITTLE_ENDIAN
LIBSPDM_SPDM_10_RSA_DUAL_VERIFICATION
LIBSPDM_SPDM_10_ECDSA_DUAL_VERIFICATION
LIBSPDM_SPDM_11_RSA_LITTLE_ENDIAN
LIBSPDM_SPDM_11_ECDSA_LITTLE_ENDIAN
LIBSPDM_SPDM_11_RSA_DUAL_VERIFICATION
LIBSPDM_SPDM_11_ECDSA_DUAL_VERIFICATION

@jyao1 These would be defines in spdm_lib_config.h right?
Something like:

#ifndef LIBSPDM_SPDM_10_RSA_LITTLE_ENDIAN
#define LIBSPDM_SPDM_10_RSA_LITTLE_ENDIAN 1
#endif
...

@steven-bellock
Copy link
Contributor

Requesters may want this to be runtime configurable. If signature verification fails with one endianness it may try with another endianness.

@richkong88
Copy link
Contributor

Is a good place to put the logic changes in spdm_crypt_lib\libspdm_crypt_asm.c in the functions:

libspdm_asym_sign
libspdm_asym_sign_hash
libspdm_asym_verify
libspdm_asym_verify_hash

@jyao1
Copy link
Member Author

jyao1 commented Sep 7, 2023

@richkong88 , here is discussion result between @steven-bellock and me.

  1. We use different way for signing and verification. The reason is: Signing is NOT controlled by libspdm, but in a customer spdm_device_secret_lib. While Verification is controlled by libspdm.

  2. For signing, it is OK to use MACRO. We can put macro to spdm_device_secret_lib.

  3. For verification, we use runtime switch. If there is any API change is needed, we need add the new API, and keep old API as is for compatibility. To avoid code duplication, the old API can call new API as default behavior.

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 a pull request may close this issue.

4 participants