Support SM3 and SM4 #271
Support SM3 and SM4 #271
Conversation
Signed-off-by: JerryDevis <seclab@huawei.com>
Signed-off-by: JerryDevis <seclab@huawei.com>
Signed-off-by: JerryDevis <seclab@huawei.com>
Pull Request Test Coverage Report for Build 2312
💛 - Coveralls |
|
We do need to add a CI machine that supports SM3/SM4, like ubuntu 20.04 as you mentioned. |
| { | ||
| uint32_t len = SM3_256_DIGEST_SIZE; | ||
| int ret = EVP_DigestFinal_ex(*c, md, &len); | ||
| if (ret != SM3_SUCCESS || len != SM3_256_DIGEST_SIZE) { |
There was a problem hiding this comment.
empty line after variables declaration.
There was a problem hiding this comment.
Please just use '1' for OpenSSL function success checking, no SM3_SUCCESS.
| NV_HEADER hdr; | ||
| SM3_CTX *sm3_ctx = NULL; | ||
|
|
||
| (*data) = EVP_MD_CTX_new(); |
There was a problem hiding this comment.
I think we shouldn't create a context here but fill the data just like we fill it with sha1 etc.
There was a problem hiding this comment.
The risk with sm3 is that its data structure has always been opaque it seems. That leaves OpenSSL the opportunity to shuffle things around. They may not do that but the risk is there. In contrast to that we have the SHAT_CTX, SHA256_CTX, etc. that are available via sha.h, so there's less of a risk with them on that level.
There was a problem hiding this comment.
In the OpenSSL 3.0 migration guide , users are strongly encouraged to update their code to use the high level APIs instead. Low level APIs have been deprecated in OpenSSL 3.0, and will most likely be removed in a future versions.
OpenSSL intentionally avoids the user to perceive the internal structure of XXX_CTX, we can only use its pointer. So we don't need to care about its structure.
In the long term, we need to switch to the "high level" APIs (such as the EVP APIs).
There was a problem hiding this comment.
I know... but it's not going to be that simple. See my comment over there: #215 (comment)
src/tpm2/crypto/openssl/Helpers.c
Outdated
| } | ||
| EVP_MD_CTX_destroy(*c); | ||
| *c = NULL; | ||
| return ret; |
| int sm3_update(SM3_TPM_CTX *c, const void *data, size_t len); | ||
| int sm3_final(unsigned char *md, SM3_TPM_CTX *c); | ||
| #endif | ||
| #endif |
There was a problem hiding this comment.
Large parts of this file are already in TpmToOsslHash.h and shouldn't be duplicated. Please try to use that file.
| int sm3_final(unsigned char *md, SM3_CTX *c); | ||
| # endif // OpenSSL < 1.2 | ||
| #include "Sm3Helper_fp.h" | ||
| #endif | ||
| #endif // ALG_SM3_256 |
There was a problem hiding this comment.
Why do you remove it from here? I'd rather not move this here to keep the differences to the reference code at a minimum.
| #if ALG_SM3_256 | ||
| # error "The version of OpenSSL used by this code does not support SM3" | ||
| #endif | ||
| #define tpmHashStateSM3_256_t SM3_TPM_CTX | ||
| /* The defines below are only needed when compiling CryptHash.c or CryptSmac.c. This isolation |
There was a problem hiding this comment.
Also this should stay here.
| #if ALG_SM3_256 | ||
| int sm3_init(SM3_TPM_CTX *c) | ||
| { | ||
| *c = EVP_MD_CTX_new(); |
There was a problem hiding this comment.
I don't think this is correct. It's probably better to initialize SM3_TPM_CTX ourselves here.
|
I don't think that SM3 will be an easy fit with the code considering the level at which OpenSSL makes it accessible via the EVP API versus the lower level API in use by the SHA functions. My suggestion is to split SM3 and SM4 into two independent PRs because SM4 seems to work like the recently enabled Camellia does but has that other issue that not all distros are enabling it, which in turn breaks the single-TpmProfile.h-for-all-distros . |
Signed-off-by: JerryDevis <seclab@huawei.com>
|
The support of SM3 also bothered me for a long time, and it was hard to find an elegant way to implement it. |
The only answer I have is to have our own sm3 implementation ... not that I like the answer but I don't know of any other way to do this. But this is also the only way to support the SHA 1&2 families in the long-term if OpenSSL was to ever get rid of the context structures but then for SHA 3 the answer is the same. |
we add a compiling macro(i.e. --with-openssl-smX) to control SM3 and SM4, default not support.