Add more check for supported algorithm

[Wenxing-hou]

Signed-off-by: Wenxing Hou [email protected]

[steven-bellock]

Some (most) of these algorithms are conditional on the endpoint's capabilities. For example if an endpoint does not support asymmetric key exchange then LIBSPDM_FIPS_DHE_ALGO can be 0 / false.

[Wenxing-hou]

Thanks. I have deleted the LIBSPDM_FIPS_DHE_ALGO check.

[jyao1]

If we follow the capability, we should remove LIBSPDM_FIPS_AEAD_ALGO as well, since it is not required in authentication only use case.

We should remove LIBSPDM_FIPS_SIGNATURE_ALGO as well, since it is not required in PSK use case.

Then we only need LIBSPDM_FIPS_HASH_ALGO.

However, that at least one bit set according to capability should be enforced by the logic in https://github.com/DMTF/libspdm/blob/main/include/internal/libspdm_macro_check.h.
I dont think it is related to FIPS.

[jyao1]

Here is my suggestion:

1. We should improve the algo check with capability in https://github.com/DMTF/libspdm/blob/main/include/internal/libspdm_macro_check.h. E.g LIBSPDM_HASH_ALGO_SUPPORT is missing, LIBSPDM_AEAD_ALGO_SUPPORT is missing.

2. We dont need LIBSPDM_FIPS_xxx_ALGO, because at least one bit rule is already applied.

This should be a generic algo check, not specific FIPS algo check.

[Wenxing-hou]

Thanks. I have added the LIBSPDM_HASH_ALGO_SUPPORT and LIBSPDM_AEAD_ALGO_SUPPORT.
But for LIBSPDM_AEAD_ALGO_SUPPORT, I don't know how to add the check.

[jyao1]

It is valid use case that if SPDM only supports unsigned measurement with raw bit stream.

[jyao1]

It has been a while since we discuss this. I recommend:

1. For AEAD:

#if (LIBSPDM_ENABLE_CAPABILITY_KEY_EX_CAP) && !LIBSPDM_AEAD_ALGO_SUPPORT
    #error If KEY_EX_CAP is enabled then at least one AEAD algorithm must also be enabled.
#endif

#if (LIBSPDM_ENABLE_CAPABILITY_PSK_EX_CAP) && !LIBSPDM_AEAD_ALGO_SUPPORT
    #error If PSK_EX_CAP is enabled then at least one AEAD algorithm must also be enabled.
#endif

2. For HASH:

#if (LIBSPDM_ENABLE_CAPABILITY_CERT_CAP) && !LIBSPDM_HASH_ALGO_SUPPORT
    #error If CERT_CAP is enabled then at least one HASH algorithm must also be enabled.
#endif

#if (LIBSPDM_ENABLE_CAPABILITY_CHAL_CAP) && !LIBSPDM_HASH_ALGO_SUPPORT
    #error If CHAL_CAP is enabled then at least one HASH algorithm must also be enabled.
#endif

#if (LIBSPDM_ENABLE_CAPABILITY_KEY_EX_CAP) && !LIBSPDM_HASH_ALGO_SUPPORT
    #error If KEY_EX_CAP is enabled then at least one HASH algorithm must also be enabled.
#endif

#if (LIBSPDM_ENABLE_CAPABILITY_PSK_EX_CAP) && !LIBSPDM_HASH_ALGO_SUPPORT
    #error If PSK_EX_CAP is enabled then at least one HASH algorithm must also be enabled.
#endif

NOTE:

1. If fail, we know something is wrong.
2. If pass, we still not know if everything is correct, because the information is not enough.

[jyao1]

For FIPS specific check as addition, I think we can do following:

/* 1. Collect info before FIPS */
#if LIBSPDM_FIPS_MODE
#define LIBSPDM_ASYM_ALGO_SUPPORT \
    ((LIBSPDM_RSA_SSA_SUPPORT) || (LIBSPDM_RSA_PSS_SUPPORT) || (LIBSPDM_ECDSA_SUPPORT) || \
     (LIBSPDM_SM2_DSA_SUPPORT) || (LIBSPDM_EDDSA_ED25519_SUPPORT) || (LIBSPDM_EDDSA_ED448_SUPPORT))

#define LIBSPDM_DHE_ALGO_SUPPORT \
    ((LIBSPDM_FFDHE_SUPPORT) || (LIBSPDM_ECDHE_SUPPORT) || (LIBSPDM_SM2_KEY_EXCHANGE_SUPPORT))

#define LIBSPDM_AEAD_ALGO_SUPPORT ...

#define LIBSPDM_HASH_ALGO_SUPPORT ...
#endif /*LIBSPDM_FIPS_MODE*/

/* 2. Enforce FIPS algo */
#if LIBSPDM_FIPS_MODE
#undef LIBSPDM_SM2_DSA_SUPPORT
#define LIBSPDM_SM2_DSA_SUPPORT 0
......
#endif /*LIBSPDM_FIPS_MODE*/

/* 3. Collect info and check after FIPS enforcement */
#if LIBSPDM_FIPS_MODE
#define LIBSPDM_FIPS_ASYM_ALGO_SUPPORT \
    ((LIBSPDM_RSA_SSA_SUPPORT) || (LIBSPDM_RSA_PSS_SUPPORT) || (LIBSPDM_ECDSA_SUPPORT))

#define LIBSPDM_FIPS_DHE_ALGO_SUPPORT \
    ((LIBSPDM_FFDHE_SUPPORT) || (LIBSPDM_ECDHE_SUPPORT))

#define LIBSPDM_FIPS_AEAD_ALGO_SUPPORT ...

#define LIBSPDM_FIPS_HASH_ALGO_SUPPORT ...
#endif /*LIBSPDM_FIPS_MODE*/

/* 4. Check FIPS algo after enforcement*/
#if LIBSPDM_FIPS_MODE
#if (LIBSPDM_ASYM_ALGO_SUPPORT) && !LIBSPDM_FIPS_ASYM_ALGO_SUPPORT
    #error ASYM algo is cleared after FIPS enforcement.
#endif

#if (LIBSPDM_DHE_ALGO_SUPPORT) && !LIBSPDM_FIPS_DHE_ALGO_SUPPORT
    #error DHE algo is cleared after FIPS enforcement.
#endif

#if (LIBSPDM_AEAD_ALGO_SUPPORT) && !LIBSPDM_FIPS_AEAD_ALGO_SUPPORT
    #error AEAD algo is cleared after FIPS enforcement.
#endif

#if (LIBSPDM_HASH_ALGO_SUPPORT) && !LIBSPDM_FIPS_HASH_ALGO_SUPPORT
    #error HASH algo is cleared after FIPS enforcement.
#endif

#endif /*LIBSPDM_FIPS_MODE*/

#endif

[Wenxing-hou]

Thanks. I have added the check for generic algo support and FIPS approved algo.
But I think the DHE_algo need to be AEAD_algo. It is a typo error.