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

config: Port openssl sha224 to use EVP functions #244

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

kkuehlz
Copy link
Contributor

@kkuehlz kkuehlz commented Feb 10, 2020

As suggested by SHA224(3ssl), use the EVP_DigestInit functions. Calling
the hash functions directly is deprecated for applications. Further,
actually check the return values from the OpenSSL functions and throw
appropriate runtime errors.

@kkuehlz kkuehlz requested a review from GreaterFire February 10, 2020 01:20
@claassistantio
Copy link

claassistantio commented Feb 10, 2020

CLA assistant check
All committers have signed the CLA.

As suggested by SHA224(3ssl), use the EVP_DigestInit functions. Calling
the hash functions directly is deprecated for applications. Further,
actually check the return values from the OpenSSL functions and throw
appropriate runtime errors.
@GreaterFire GreaterFire changed the base branch from master to dev February 10, 2020 07:11
@GreaterFire
Copy link
Member

LGTM. Thanks!

@GreaterFire GreaterFire merged commit 0649be2 into trojan-gfw:dev Feb 10, 2020
throw runtime_error("could not output hash");
}

for (unsigned int i = 0; i < digest_len; ++i) {
sprintf(mdString + (i << 1), "%02x", (unsigned int)digest[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you can see from the diff, I didn’t change this line. But I’d be curious in seeing how you intend to exploit this with a single byte ...

@GF-Huang
Copy link

Why not sha256?

@kkuehlz
Copy link
Contributor Author

kkuehlz commented Feb 10, 2021 via email

@GF-Huang
Copy link

I means why choose sha224 to define the protocol rather than the popular sha256?

I have no experience in cryptography, but I'm just curious.

@GreaterFire
Copy link
Member

@GF-Huang It's shorter

@GF-Huang
Copy link

If it's for shorter, why not just use the 28 bytes output from SHA224, rather than convert to hex string that make it 56 bytes?

@GreaterFire
Copy link
Member

@GF-Huang That was one of my design failures :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants