CORE-7 Add MD5 Support in FIPS mode #17181
Conversation
Signed-off-by: Michael Boquard <michael@redpanda.com>
|
new failures in https://buildkite.com/redpanda/redpanda/builds/46436#018e5700-6a2a-4590-8a1f-89d7ac6f3e49: new failures in https://buildkite.com/redpanda/redpanda/builds/46459#018e5871-c4e9-4854-acf6-2bcad0b92aa3: new failures in https://buildkite.com/redpanda/redpanda/builds/46459#018e5871-c4e2-40de-a0d3-4d55e3dfcc5c: |
src/v/crypto/crypto.cc
Outdated
| return uses_default_provider(type); | ||
| } | ||
| // Validates that the message digest is using the correct provider when in FIPS | ||
| // mode. This function will assert if: |
There was a problem hiding this comment.
nit: only because I misunderstood it the first time
| // mode. This function will assert if: | |
| // mode. This function will assert if all of: |
or something
src/v/crypto/crypto.cc
Outdated
| static const absl::flat_hash_set<digest_type> non_fips_digest_types{ | ||
| digest_type::MD5}; | ||
|
|
||
| return non_fips_digest_types.contains(type); |
There was a problem hiding this comment.
Any reason to use a static container here rather than like a switch/case? I'm increasingly hip to the argument that any "what is this enum" type operations should fail when the enum list changes.
There was a problem hiding this comment.
yeah that makes sense I can do that :)
src/v/crypto/crypto.cc
Outdated
| bool digest_type_set = false; | ||
| digest_type type; | ||
| struct callback_func_data { | ||
| bool& digest_type_set; | ||
| digest_type& type; | ||
| }; |
There was a problem hiding this comment.
question: what is the purpose of the extra indirection? it looks like data and its constituents should have the same lifetime?
There was a problem hiding this comment.
good point!
|
|
||
| callback_func_data data{ | ||
| .digest_type_set = std::ref(digest_type_set), .type = std::ref(type)}; | ||
| const auto fn = [](const char* name, void* vdata) { |
There was a problem hiding this comment.
double whammy: type erased callback interface that ALSO returns an arbitrary char*? 🤩
Redpanda uses MD5 for checksum operations in the S3 client. To continue supporting them, the crypto library and OpenSSL library context service has been changed to always load the default provider. Additional checks have been added to ensure that when in FIPS mode, non-MD5 message digests use the FIPS provider when they are fetched, otherwise the application will crash. Signed-off-by: Michael Boquard <michael@redpanda.com>
Signed-off-by: Michael Boquard <michael@redpanda.com>
michael-redpanda
force-pushed
the
CORE-7-Add-MD5-Support
branch
from
51ad5c8
to
49017c8
Compare
|
Force push
|
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46459#018e5871-c4e5-4cbf-a6db-f61d227ffbbe |
| } | ||
| }; | ||
|
|
||
| EVP_MD_names_do_all(md, fn, &data); |
There was a problem hiding this comment.
where can i learn more about lambda's being able to be passed like this i didn't realize they'd have the same calling conventions for C too?
There was a problem hiding this comment.
Appears to be an implicit conversion: https://en.cppreference.com/w/cpp/language/lambda (section ClosureType::operator ret(*)(params)()). Pretty cool 🙂
Adds support for MD5 message digests in FIPS mode.
Backports Required
Release Notes