Only implement encoding.BinaryMarshaler for hashers provided by built-in providers #161
Conversation
Co-authored-by: Davis Goodin <dagood@users.noreply.github.com>
| @@ -232,6 +235,9 @@ func (h *evpHash) sum(out []byte) { | |||
| // The EVP_MD_CTX memory layout has changed in OpenSSL 3 | |||
| // and the property holding the internal structure is no longer md_data but algctx. | |||
| func (h *evpHash) hashState() unsafe.Pointer { | |||
| if !h.marshallable { | |||
| panic("openssl: hash state is not marshallable") | |||
There was a problem hiding this comment.
Would this ever come up from user code or OpenSSL oddity, or would there have to be a typo+missed test in golang-fips/openssl?
Wondering if it might make more sense to return this as a second value from newEvpHash rather than store it in the struct (at the cost of not having this runtime check).
There was a problem hiding this comment.
There would have to be a typo+missed test, aka it is a programmer error, not something that should happen at runtime.
I initially returned that property as second value from newEvpHash, but that made all NewSHAX a bit less cleaner, and having the runtime check is a nice bonus. Also, with the current implementation, NewSHAX functions are inlinable, which can help escape analysis take better decisions.
We can only marshal/unmarshal the internal state of hashers provided by built-in providers, which are the only ones from which we know the memory layout.
This PR does some struct embedding tricks so the hashers returned by
NewMD5,NewSHA1,NewSHA224,NewSHA256,NewSHA384, andNewSHA512, only implement theencoding.BinaryMarshalerandencoding.BinaryUnmarshalerinterface if the hashers are provided by the fips or the default provider.Fixes #156.