Enable different endians for signature verification on SPDM 1.0/1.1. … #2360
Conversation
…Version 6 Signed-off-by: Kong, Richard <richard.kong@intel.com>
include/library/spdm_crypt_lib.h
Outdated
| @@ -414,16 +439,26 @@ void libspdm_asym_free(uint32_t base_asym_algo, void *context); | |||
| * @param message_size Size of the message in bytes. | |||
| * @param signature Pointer to asymmetric signature to be verified. | |||
| * @param sig_size Size of signature in bytes. | |||
| * @param endian Endian to be tried. If both endians is selected, | |||
There was a problem hiding this comment.
both endians is -> both endians are
There was a problem hiding this comment.
Think I fixed it now.
include/library/spdm_crypt_lib.h
Outdated
| void* context, | ||
| const uint8_t* message, size_t message_size, | ||
| const uint8_t* signature, size_t sig_size, | ||
| uint32_t *endian); |
There was a problem hiding this comment.
I liked how this parameter was called flags, since other information could be passed through. If it's just for endianness then change to uint8_t.
There was a problem hiding this comment.
I liked flags too, but it seemed strange to me to have an in/out parameter as flags. I can change it back to flags. It's a little more work to isolate the endian value and modify only it as the out parameter too.
There was a problem hiding this comment.
Ah ok. Since it's an output then using it just for endianness is fine. But it can be a uint8_t.
| @@ -688,6 +688,18 @@ libspdm_return_t libspdm_set_data(void *spdm_context, libspdm_data_type_t data_t | |||
| context->max_spdm_session_sequence_number = LIBSPDM_MAX_SPDM_SESSION_SEQUENCE_NUMBER; | |||
| } | |||
| break; | |||
| case LIBSPDM_DATA_SPDM_VERSION_10_11_VERIFY_SIGNATURE_ENDIAN: | |||
| if (data_size != sizeof(uint32_t)) { | |||
There was a problem hiding this comment.
I'm not understanding why this should be int. Should it not follow the flags (or endian) parameter in spdm_crypt_lib.h? (It's not an enum).
There was a problem hiding this comment.
Ah ok. There's a header issue then. For getting and setting the context LIBSPDM_SPDM_10_11_VERIFY_SIGNATURE_ENDIAN_* should be #defined in spdm_common_lib.h.
There was a problem hiding this comment.
Still not understanding. This data member of the context will be used as the "endian" in/out parameter to the verify function described by the previous comment where you were requesting it change from uint32_t* to uin8_t*. It will have one of these values:
/* The Endian values apply only if the spdm version is 1.0/1.1.
* The default verification mode is big endian only. */
#define LIBSPDM_SPDM_10_11_VERIFY_SIGNATURE_ENDIAN_BIG_ONLY (0x0)
#define LIBSPDM_SPDM_10_11_VERIFY_SIGNATURE_ENDIAN_LITTLE_ONLY (0x1)
#define LIBSPDM_SPDM_10_11_VERIFY_SIGNATURE_ENDIAN_BIG_OR_LITTLE (0x2)
And the parameter type to the verify function will be uint8_t* as you requested in the previous comment.
So shouldn't think member in the context also be of type uint8_t?
There was a problem hiding this comment.
So shouldn't think member in the context also be of type uint8_t?
Yes, but now the issue is that those #defines are currently in spdm_crypt_lib.h. They should be in spdm_common_lib.h.
| dst[i] = dst[dst_size - i - 1]; | ||
| dst[dst_size - i - 1] = byte; | ||
| } | ||
| } |
There was a problem hiding this comment.
}
else {
->
} else {
Lots of other instances as well. I'll see if I can configure uncrustify to catch that.
There was a problem hiding this comment.
@steven-bellock , I will file a new issue to catch format issue in uncrustify.
richkong88
force-pushed
the
rkong_endian_6
branch
from
c1a551b
to
9a93dd6
Compare
There was a problem hiding this comment.
Approved once https://github.com/DMTF/libspdm/pull/2360/files#r1328149511 is resolved.
Signed-off-by: Kong, Richard <richard.kong@intel.com>
richkong88
force-pushed
the
rkong_endian_6
branch
from
9a93dd6
to
1a95738
Compare
|
Fix #2151 |
…Version 6. Incorporating all requests so far.