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

allow conversion from MessageDigest to Md #2292

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

reaperhulk
Copy link
Contributor

This eases porting from legacy const EVP_MD to dynamic EVP_MD, especially in codebases that need to support boringssl/libressl in addition to OpenSSL 3.

This eases porting from legacy const EVP_MD to dynamic EVP_MD,
especially in codebases that need to support boringssl/libressl in
addition to OpenSSL 3
@reaperhulk reaperhulk marked this pull request as draft September 1, 2024 18:47
mutable MD and needing to free are both ossl 3 constructs though so
we'll leave those gated
openssl/src/md.rs Outdated Show resolved Hide resolved
@reaperhulk
Copy link
Contributor Author

@sfackler Alex and I aren't sure of the motivation behind the design of Md vs MessageDigest so we wanted to get your input here 😄

@sfackler
Copy link
Owner

sfackler commented Sep 5, 2024

The MessageDigest/Hasher API is one of the oldest parts of the crate, and tries to provide a "cleaner" interface which doesn't IIRC actually cover all of the weird edge cases. Md/MdCtx is a newer iteration of the same thing that tries to provide a more direct binding. The same thing is true for the asymmetric encryption APIs, which I think had even more issues.

We can hopefully remove MessageDigest/Hasher in the upcoming breaking release, but in the meantime I think this conversion should be fine.

}
}
} else {
enum Inner {}
Copy link
Owner

Choose a reason for hiding this comment

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

I'm a bit suspicious that this is a safe change. I vaguely remember that the new OpenSSL 3 dynamic lookup APIs were the reason I had to make the new types in the first place.

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