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

Make algorithms more robust to unsupported hashes #185

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Make algorithms more robust to unsupported hashes #185

merged 5 commits into from
Sep 23, 2024

Conversation

qmuntal
Copy link
Collaborator

@qmuntal qmuntal commented Sep 13, 2024

While testing different provider configurations I found out that we are still panicking instead of returning an error in several places where the desired hash function is not supported by the OpenSSL provider.

This PR fixes all cases where we are calling a user-provided func() hash.Hash function that can panic, e.g. when calling openssl.NewHMAC(openssl.NewSHA224, nil) with a provider that doesn't support SHA224 (aka SymCrypt).

Our azurelinux CI job hasn't triggered this situation because it is configured to use SymCrypt by default and fall back to the built-in default provider for algorithms that SymCrypt doesn't support. I triggered the error by making the built-in default provider unavailable. Anyway, I've added some tests so that we cover this situation with any provider configuration.

Copy link
Collaborator

@dagood dagood left a comment

Choose a reason for hiding this comment

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

I triggered the error by making the built-in default provider unavailable.

Is this a configuration that we can/should add to CI?

evp.go Outdated Show resolved Hide resolved
evp.go Show resolved Hide resolved
hkdf_test.go Outdated Show resolved Hide resolved
@qmuntal qmuntal requested a review from dagood September 16, 2024 07:24
@qmuntal
Copy link
Collaborator Author

qmuntal commented Sep 20, 2024

@derekparker could someone from your team review this PR? Thanks!

Copy link
Collaborator

@ueno ueno left a comment

Choose a reason for hiding this comment

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

LGTM

hkdf_test.go Show resolved Hide resolved
@qmuntal qmuntal merged commit 731002f into v2 Sep 23, 2024
26 checks passed
@qmuntal qmuntal deleted the hashall branch September 23, 2024 08:24
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