Skip to content

Add FIPS selftest and get_result_selftest#1647

Merged
jyao1 merged 3 commits into
DMTF:mainfrom
Wenxing-hou:Add_FIPS_selftest
Jan 31, 2023
Merged

Add FIPS selftest and get_result_selftest#1647
jyao1 merged 3 commits into
DMTF:mainfrom
Wenxing-hou:Add_FIPS_selftest

Conversation

@Wenxing-hou

@Wenxing-hou Wenxing-hou commented Jan 17, 2023

Copy link
Copy Markdown
Contributor

reference: #1260

Commit 1: Add HMAC self_test;
Commit 2: Add AES_GCM self_test;
Commit 3: Add RSA_SSA self_test;

When the LIBSPDM_FIPS_MODE is open, the test result:
image

When the LIBSPDM_FIPS_MODE is closed, the test result:
image

Signed-off-by: Wenxing Hou wenxing.hou@intel.com

@Wenxing-hou Wenxing-hou marked this pull request as draft January 17, 2023 09:04
Comment thread CMakeLists.txt Outdated
Comment thread include/internal/libspdm_fips_lib.h Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated
@jyao1

jyao1 commented Jan 18, 2023

Copy link
Copy Markdown
Member

for selftest location, please refer to #1406 (comment)

@Wenxing-hou Wenxing-hou force-pushed the Add_FIPS_selftest branch 7 times, most recently from ac78a4f to 9422b9d Compare January 29, 2023 09:11
@jyao1

jyao1 commented Jan 30, 2023

Copy link
Copy Markdown
Member

Please add all sym, deterministic asym algo at first.
Then probabilistic asym algo with hardcoded random.

Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_aes_gcm.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_aes_gcm.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_rsa_ssa.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_rsa_ssa.c Outdated
@jyao1

jyao1 commented Jan 30, 2023

Copy link
Copy Markdown
Member

This is enough as the first patch.

I recommend, we review the design at first, then merge, then continue with rest key exchange or probabilistic algo.

@jyao1 jyao1 marked this pull request as ready for review January 30, 2023 02:18
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_aes_gcm.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_aes_gcm.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_rsa_ssa.c Outdated
Comment thread include/internal/libspdm_fips_lib.h Outdated
@Wenxing-hou Wenxing-hou force-pushed the Add_FIPS_selftest branch 3 times, most recently from 951f9d4 to cc613fa Compare January 30, 2023 09:12

@steven-bellock steven-bellock left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jyao1 Is this level of granularity ok? For example, if SHA-256 passes but SHA-384 fails, would you like to know that information? Or is any failure deemed fatal?

Comment thread include/internal/libspdm_fips_lib.h Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Only list the algorithms that will be covered. For example SPDM doesn't support SHA1.

Copy link
Copy Markdown
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 made the comments more clear.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can get rid of the explicit array sizes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. I deleted all explicit array sizes.

Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated
Comment thread library/spdm_crypt_lib/fips/libspdm_selftest_hmac.c Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For C89 compatibility we don't allow procedural statements until all variables have been declared for a scope. So I would make this bool result = true;.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I will remember it.

reference: DMTF#1260

Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
Reference: DMTF#1260

Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
Reference: DMTF#1260

Signed-off-by: Wenxing Hou <wenxing.hou@intel.com>
@jyao1 jyao1 merged commit 15ba29b into DMTF:main Jan 31, 2023
@Wenxing-hou Wenxing-hou deleted the Add_FIPS_selftest branch March 23, 2023 07:34
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.

3 participants