Repository navigation
fix(crypto): handle primitive field - #2819
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThe PR fixes crypto policy and reporting logic for the flattened metadata schema by reading primitive and parameterSetIdentifier directly from the properties object, and updates the associated tests and fixtures. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="modules/fundamental/src/crypto/service/policy.rs" line_range="110" />
<code_context>
fn props_with_primitive(primitive: &str) -> serde_json::Value {
json!({
- "algorithmProperties": {
- "primitive": primitive
- }
+ "primitive": primitive
</code_context>
<issue_to_address>
**Primitive-field regression passes unnoticed**
When `evaluate_algorithm` does not use the primitive field, `sha1_is_noncompliant` and `keycloak_cbom_algorithms` still pass if `evaluate_algorithm` ignores `primitive`: SHA1 is rejected by its name, while ECDH and EC-secp521r1 return the default Warning. The changed direct-field fixtures therefore do not verify primitive handling.
Add a test whose observable result changes when the direct primitive field is ignored or read from the wrong location.
Also at `modules/fundamental/src/crypto/service/policy.rs:300-302`, `modules/fundamental/src/crypto/service/policy.rs:310`, `modules/fundamental/src/crypto/service/policy.rs:319`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the incoming crypto properties still use the nested algorithmProperties shape, the service will stop reading the primitive and parameter-set identifier and can report an incorrect algorithm policy verdict. This changes crypto-policy evaluation; reverting restores the previous lookups, but any incorrect verdicts already acted on would not be undone.
Blocking findings: modules/fundamental/src/crypto/service/policy.rs:110
Parse the primitive field in AlgorithmProps so evaluate_algorithm actually consumes it. Hash primitives with short digests (≤128 bits) are now classified as NonCompliant, and a dedicated test proves the verdict changes when the primitive field is absent or mislocated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9602a06 to
ed6bee2
Compare
|
Successfully created backport PR for |
Summary by Sourcery
Fix crypto policy handling for primitive fields and enforce minimum hash digest strength.
Bug Fixes:
Tests: