fix: use runtime OpenSSL algorithm detection to handle missing SM3 support#1259
fix: use runtime OpenSSL algorithm detection to handle missing SM3 support#1259ansasaki wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughMessageDigest construction now uses fallible conversion from HashAlgorithm, with unsupported digests returning errors. TPM filtering, IMA digest construction, and emulator/context call sites were updated to propagate those failures. ChangesFallible digest conversion
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ansasaki
force-pushed
the
fix-sm3-openssl-no-sm3
branch
from
2e0c8cf to
fe96e1b
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@keylime/src/algorithms.rs`:
- Around line 116-135: The SM3 fallback in TryFrom<HashAlgorithm> for
MessageDigest is gated on a cfg that is only checked, not emitted, so the
unsupported path never becomes active in this crate. Add a crate-local
build-time cfg emission so #[cfg(osslconf = "OPENSSL_NO_SM3")] can actually
fire, and keep the HashAlgorithm::Sm3_256 match arm in algorithms.rs split
between the supported and unsupported OpenSSL cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cbe8ad81-157e-4b18-bc1b-7750daac9567
📒 Files selected for processing (12)
keylime-ima-emulator/Cargo.tomlkeylime-ima-emulator/src/main.rskeylime-push-model-agent/Cargo.tomlkeylime-push-model-agent/src/struct_filler.rskeylime/Cargo.tomlkeylime/src/algorithms.rskeylime/src/context_info.rskeylime/src/ima/entry.rskeylime/src/structures/capabilities_negotiation.rskeylime/src/tpm.rsrpm/centos/rust-keylime-metadata.patchrpm/fedora/rust-keylime-metadata.patch
sergio-correia
left a comment
sergio-correia
left a comment
There was a problem hiding this comment.
A few issues to address, mostly around completeness of the SM3 gating.
Not in this diff, but related: get_supported_hash_algorithms() in keylime/src/tpm.rs:1768-1822 returns SM3 from the TPM unfiltered. The PCR banks in available_subjects are correctly filtered (the new cfg guard in struct_filler.rs), and PcrBanks::to_map() also drops SM3. But the hash_algorithms field at struct_filler.rs:249 calls get_supported_hash_algorithms() with no SM3 filtering. If a verifier selects SM3 from the advertised list, the agent fails at hash_alg_to_message_digest().
The simplest fix would be to filter SM3 from the returned list in get_supported_hash_algorithms():
// After building usable_algs, before returning:
#[cfg(osslconf = "OPENSSL_NO_SM3")]
usable_algs.retain(|alg| !matches!(alg, KeylimeInternalHashAlgorithm::Sm3_256));
ansasaki
force-pushed
the
fix-sm3-openssl-no-sm3
branch
from
fe96e1b to
607647b
Compare
ansasaki
changed the title
ansasaki
force-pushed
the
fix-sm3-openssl-no-sm3
branch
from
607647b to
865e1ca
Compare
sergio-correia
left a comment
sergio-correia
left a comment
There was a problem hiding this comment.
A few things to address, mostly around diagnostic logging and fail-fast behavior.
Not in this diff, but related: ContextInfo::new() in keylime/src/context_info.rs:160 accepts the configured tpm_hash_alg and immediately proceeds to create EK/AK objects in the TPM without checking is_supported(). OpenSSL support is only validated much later when get_ak_local_identifier_str() or quote operations invoke MessageDigest::try_from().
If an operator configures tpm_hash_alg = "sm3_256" on a system where OpenSSL lacks SM3, the agent starts, creates EK/AK handles, registers with the verifier, then fails at the first attestation attempt. Would it make sense to add an early is_supported() check before the TPM operations?
let tpm_hash_alg = config.tpm_hash_alg;
if !tpm_hash_alg.is_supported() {
return Err(ContextInfoError::InvalidAlgorithm(
AlgorithmError::UnsupportedHashingAlgorithm(format!(
"{tpm_hash_alg} (not supported by this OpenSSL build)"
))
));
}
…ime symbols Fedora's OpenSSL is now built with OPENSSL_NO_SM3, which removes MessageDigest::sm3() from the openssl crate, causing build failures. Replace all MessageDigest::sha*() and MessageDigest::sm3() associated function calls with MessageDigest::from_name() runtime lookups via EVP_get_digestbyname. This makes the code resilient to any algorithm being removed from OpenSSL without requiring code changes. Changes: - algorithms.rs: change From<HashAlgorithm> for MessageDigest to TryFrom using from_name() for all algorithms; add is_supported() method and openssl_name() mapping to HashAlgorithm - tpm.rs: hash_alg_to_message_digest delegates to TryFrom instead of duplicating the conversion; get_supported_hash_algorithms filters out algorithms unavailable in the OpenSSL build with a warning log for each dropped algorithm - entry.rs: Digest::start() and Digest::ff() return Result, using MessageDigest::try_from() for digest size; Digest::new() does the same for validation - context_info.rs: early is_supported() check in ContextInfo::new() to fail fast if the configured hash algorithm is not available in the OpenSSL build - keylime-ima-emulator/main.rs: update call sites from infallible .into() to TryFrom Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
ansasaki
force-pushed
the
fix-sm3-openssl-no-sm3
branch
from
865e1ca to
e9fa193
Compare
2 participants
Type of Change
Related Issues
See also: Fedora COPR build failure —
openssl::hash::MessageDigest::sm3()not found when building against Fedora's OpenSSL (compiled withOPENSSL_NO_SM3).Change Description
Concise Summary
Replace all
MessageDigest::sha*()/MessageDigest::sm3()associated function calls withMessageDigest::from_name()runtime lookups, making the code resilient to any hash algorithm being removed from OpenSSL without requiring code changes.Technical Details
Fedora's OpenSSL is now built with
OPENSSL_NO_SM3, which removesMessageDigest::sm3()from theopensslcrate at compile time. This caused two build errors inalgorithms.rsandtpm.rs.Rather than using compile-time
cfggates (which don't propagate transitively from theopensslcrate), the fix usesMessageDigest::from_name()— a runtime lookup viaEVP_get_digestbyname— for all hash algorithms. This is future-proof: if OpenSSL drops another algorithm, the code handles it gracefully without any changes.Changes:
algorithms.rs:From<HashAlgorithm> for MessageDigestbecomesTryFrom, usingfrom_name()for all algorithms. Addsdigest_size()(compile-time constant sizes),openssl_name(), andis_supported()methods toHashAlgorithm.tpm.rs:hash_alg_to_message_digestnow delegates to theTryFromimpl instead of duplicating the conversion logic.get_supported_hash_algorithmsfilters out algorithms unavailable in the current OpenSSL build, preventing the agent from advertising capabilities it cannot fulfill.entry.rs:Digest::start()andDigest::ff()usedigest_size()instead of going through OpenSSL — they only need byte lengths, not digest objects.Digest::new()also usesdigest_size()for validation. This removes the OpenSSL dependency fromentry.rsentirely.context_info.rs,keylime-ima-emulator/main.rs: Call sites updated from infallible.into()toTryFrom/try_from.What is NOT changed
Cargo.tomlchanges (no new dependencies, no lint configs)cfggates orbuild.rsfilesHashAlgorithm::Sm3_256enum variant remains (TPM protocol compatibility)PcrBanks.sm3_256field remains (wire format / deserialization compatibility)capabilities_negotiation.rsandstruct_filler.rsare unchanged — they are pure data structures that don't use OpenSSLAlternatives Considered
cfg(osslconf = "OPENSSL_NO_SM3"): The openssl crate'sosslconfcfg doesn't propagate to downstream crates, requiringbuild.rs+openssl-sysdependency additions — more complex for no benefit.Documentation Updates Required
Verification Process
OPENSSL_NO_SM3):cargo build— compiles cleanlycargo build— compiles cleanlybash tests/run.sh— all tests passMessageDigest::sm3()references remain:grep -r "MessageDigest::sm3" --include="*.rs" | grep -v target/Checklist
Additional Context
Summary by CodeRabbit
start/ffnow returnResultto reflect potential failures.