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

Hash algorithms #76

Closed
maths644311798 opened this issue May 30, 2023 · 3 comments
Closed

Hash algorithms #76

maths644311798 opened this issue May 30, 2023 · 3 comments
Assignees

Comments

@maths644311798
Copy link
Contributor

In yacl/crypto/base/hash/hash_interface.h, the following code means a recommendation of some hash algorithms.

enum class HashAlgorithm : int {
UNKNOWN,
// SHA-2 family of algorithms
SHA224 = 1,
SHA256 = 2,
SHA384 = 3,
SHA512 = 4,

SHA_1 = 5,

SM3 = 6,

BLAKE2B = 7,
BLAKE3 = 8
};

My first question is : Why not use SHA3 since SHA1 is not safe?

The second question comes from the following code in hmac.cc:

void Init_HMAC(HashAlgorithm hash_algo, ByteContainerView key,
HMAC_CTX* context) {
int res = 0;
switch (hash_algo) {
case HashAlgorithm::SHA224:
res =
HMAC_Init_ex(context, key.data(), key.size(), EVP_sha224(), nullptr);
break;
case HashAlgorithm::SHA256:
res =
HMAC_Init_ex(context, key.data(), key.size(), EVP_sha256(), nullptr);
break;
case HashAlgorithm::SHA384:
res =
HMAC_Init_ex(context, key.data(), key.size(), EVP_sha384(), nullptr);
break;
case HashAlgorithm::SHA512:
res =
HMAC_Init_ex(context, key.data(), key.size(), EVP_sha512(), nullptr);
break;
case HashAlgorithm::SHA_1:
res = HMAC_Init_ex(context, key.data(), key.size(), EVP_sha1(), nullptr);
break;
case HashAlgorithm::SM3:
res = HMAC_Init_ex(context, key.data(), key.size(), EVP_sm3(), nullptr);
break;
case HashAlgorithm::UNKNOWN:
default:
YACL_THROW("Unsupported hash algo: {}", static_cast(hash_algo));
break;
}

YACL_ENFORCE_EQ(res, 1, "Failed to HMAC_Init_ex.");
}

Why is BLAKE not written here?

@Jamie-Cui
Copy link
Member

Why not use SHA3 since SHA1 is not safe?

Yes, sha3 is supposed to be more secure than sha1 (we'll include a security warning for sha1 or remove sha1 in the future).

Why is BLAKE not written here?

Since there are so many crypto protocols/primitives/tools in the wild, we are not able to include everything in our repo. Therefore, we follow a simple yet efficient logic for adding new algorithms in yacl/crypto/base:

We only include new algorithms when necessary (and the algorithm should be standardised in some way)

Above codes are sufficient for most of yacl's use case as far as I am aware of. However, if in your case blake family is a must, please feel free to submit a PR, will really appreciate that. :P

@maths644311798
Copy link
Contributor Author

maths644311798 commented May 30, 2023

Thanks. The strange point is that BLAKE is in hash_interface.h but not in hmac.cc.

@Jamie-Cui
Copy link
Member

Indeed, the truth is that we only write simple wrapper for those standardised algorithms, as to support other higher-level protocols. If you are using standard PKI more often, I strongly recommend use OpenSSL directly.

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

No branches or pull requests

2 participants