Repository navigation
Conversation
…on the owned tree The libxmlsec1 keys, KeyInfo, keys managers, templates, xmltree helpers and the digest, HMAC and signature transforms run over the owned document tree through its handles; no C library is used. - Core: key values, incremental digests and HMAC, private key import into a key inventory. The owned tree keeps an unlinked run of siblings, parses a fragment in the namespace context of an element or the document node under the policy, replaces a node with such a run while handing over its IDs, and copies a subtree from another tree with its ID registrations and the bindings it uses. A fragment or copy linked away from its context declares the bindings it uses. An ID that two attached elements carry names neither, and a value a carrier still holds cannot be claimed again. The serializer writes a carriage return in text as 
 and namespace declarations before attributes, as libxml2 does. - Tree layer: xsFreeNodeList, xsParseInNodeContext, xsDocCopyNode, document::subtree, parsing under an explicit policy. The library policy is published there as one verification and signing pair, which republishing replaces as a whole, one caller at a time, with the edits run outside the snapshot lock. - Every libxmlsec1 operation reads one policy revision: a KeyInfo read pins it in its context and hands it, with the read's resource budget and the URI classes admitted for each key reference, to the transform chains its references run; a run of a transform context pins its own. Key reads enforce the verification policy's key sources and key-retrieval URI classes and depth; digest, HMAC and signature transforms admit their method and key under the operation's policy before taking input. A verification with a caller-supplied key and no expected targets has one admission, VerificationPolicy::check_supplied_key_verification, which signature-value verification and the transforms share. - xmltree.h, templates.h, keys.h, keysdata.h (binary value, symmetric and RSA/DSA/EC/DEREncodedKeyValue klasses), keyinfo.h (KeyName, KeyValue, RetrievalMethod, KeyInfoReference), keysmngr.h (manager, store protocol, simple store), digest/HMAC/signature klasses in the built-in crypto table, xmlSecCryptoAppKeyLoadMemory. MD5 and RIPEMD-160 klasses come with the legacy-algorithms feature. Tree edits report the failed libxml2-shaped call and return an error instead of leaving a half-written node; setters and adopt calls keep objects the caller passes back to them alive. - Functions that read or write a file themselves are not offered (xmlSecKeyReadBinaryFile, xmlSecSimpleKeysStoreLoad, xmlSecSimpleKeysStoreLoad_ex, xmlSecSimpleKeysStoreSave); the headers say so and point to the memory entry points. Templates are built in the document they belong to. A KeyValue writer asked for the private key fails rather than writing only the public part. - OpenSSL-compatible key import resizes a DSA generator to the modulus precision, wipes the private exponent and budgets against retained inventory material; a decrypted key of the wrong usage stays a candidate miss. Standalone signature-value verification checks the HMAC output minimum before any key hook, charges the supplied key and passes the certificate session to the provider. The CLI preflights signing keys against the remaining resource budget, and resource, policy and protected-container refusals stop lax key search. - Every C program under crates/xml-sec-xmlsec1/tests/c prints the same as the libxmlsec1 oracle, a 1.3.13 pre-release snapshot. Part of #451
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 15 minutes. View limit details
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. |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc7c03b52e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for pos in 0..xmlSecPtrListGetSize(list) { | ||
| let key = xmlSecPtrListGetItem(list, pos).cast::<XmlSecKey>(); | ||
| if !key.is_null() && xmlSecKeyMatch(key, name, &raw mut (*ctx).key_req) == 1 { | ||
| return xmlSecKeyDuplicate(key); |
There was a problem hiding this comment.
When a simple store contains more keys than ResourcePolicy::max_key_candidates, a document-controlled KeyName can make this loop inspect the entire store without charging or checking that allowance. Multiple unresolved names repeat the unbounded search, and the later signature-value verifier only charges its supplied key, so it cannot account for this earlier work. Unlike the core's find_named_entry, this lookup bypasses the operation's configured candidate limit. Charge each inspected entry against a budget shared by the pinned KeyInfo operation and propagate budget exhaustion rather than treating it as an ordinary missing key.
AGENTS.md reference: AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
| pub struct IncrementalHmac { | ||
| state: State, | ||
| } |
There was a problem hiding this comment.
This introduces a new general-purpose incremental HMAC mechanism inside xml-sec: its state initialization, update and finalization operate on arbitrary bytes and contain no XML-specific behavior. That mechanism can serve the other signature consumers, so implementing it here violates the root AGENTS.md Shared Primitive Ownership requirement and creates another locally maintained crypto-dispatch implementation alongside the existing HMAC paths. Coordinate and link a signature-primitives dependency for the mechanism, keeping only the XMLDSig algorithm mapping and policy/transform integration here.
AGENTS.md reference: AGENTS.md:L9-L13
Useful? React with 👍 / 👎.
| private: None, | ||
| }, | ||
| ); | ||
| if xmlSecKeySetValue(key, data) < 0 { |
There was a problem hiding this comment.
When a KeyInfo contains multiple key candidates and the first asymmetric value does not meet ctx.key_req (for example, an RSA value before the EC value requested by an ECDSA transform), this reader nevertheless installs that first value. xmlSecKeyInfoNodeRead then continues because the key does not match, but the next asymmetric reader fails with "key already has a value"; a subsequent KeyName also skips manager lookup because the value is non-null. Consequently, an unusable first candidate prevents selection of a later matching key. Check the newly constructed value against the context's requirements before installing it, and leave the destination empty when the candidate is inapplicable; apply the same handling to DEREncodedKeyValue.
Useful? React with 👍 / 👎.
Summary
Changes
unlink_children), parses a fragment in the namespace context of an element or the document node under the policy (parse_in_context), replaces a node with such a run while handing over its IDs (replace_with_run), and copies a subtree from another tree with its ID registrations and the bindings it uses (copy_from). A fragment or copy linked away from its context declares the bindings it uses. An ID that two attached elements carry names neither, and a value a carrier still holds cannot be claimed again. The serializer writes a carriage return in text as
and namespace declarations before attributes, as libxml2 does.xsFreeNodeList,xsParseInNodeContext,xsDocCopyNode,document::subtree, parsing under an explicit policy. The library policy is published there as one verification and signing pair, replaced as a whole by one republishing caller at a time.VerificationPolicy::check_supplied_key_verification, shared by signature-value verification and the transforms.xmlSecCryptoAppKeyLoadMemory. MD5 and RIPEMD-160 klasses come with thelegacy-algorithmsfeature.xmlSecKeyReadBinaryFile,xmlSecSimpleKeysStoreLoad,xmlSecSimpleKeysStoreLoad_ex,xmlSecSimpleKeysStoreSave; the headers say so and point to the memory entry points. Templates are built in the document they belong to.Testing
Every C program under
crates/xml-sec-xmlsec1/tests/cprints the same as the libxmlsec1 oracle, a 1.3.13 pre-release snapshot (capi --xmlsec1); tests (with the xmlsec1 oracle and SoftHSM), clippy with all features, formatting, the header check and the clean C API check pass locally on macOS; CI runs the full matrix.Related
Part of #451