Repository navigation
Conversation
There was a problem hiding this comment.
Thanks for this. This looks like a solid implementation. I wasn't aware that anyone actually wanted to create cose in the signer. I'd like to know more about why that is required.
The exact-length check, the absence of set_len, and the documented callback contract are all what we want. Two changes before merge:
1. Make callback nullable at the boundary.
callback: SignerCallback is a non-nullable function pointer type, so a C caller passing NULL is undefined behavior before any of our code runs. Take Option<SignerCallback> and return NULL with a NullParameter error:
callback: Option<SignerCallback>,
...
let callback = some_or_return_null!(callback, CimplError::null_parameter("callback"));cbindgen emits the same C function-pointer type for Option<fn>, so the header does not change. The existing c2pa_signer_create and other callback-taking entry points have the same problem; we are fixing those separately, so please apply it here so the new function does not add another one.
2. Cap reserve_size.
reserve_size is bounded only by isize::MAX. It sets the size of the signature box reserved in every signed asset, and each sign call allocates a zeroed buffer of that size. A very large value fails with an allocation error at best. Please reject values above a named constant, proposed MAX_SIGNATURE_RESERVE_SIZE = 1 MiB (the SDK default is 10,000 bytes plus the certificate chain), with InvalidBufferSize, matching the existing zero check.
Tests to add:
- NULL callback returns NULL and the error mentions
callback. reserve_sizeof cap + 1 is rejected; exactly the cap is accepted.
|
@gpeacock Our system supports C2PA and is split into several components. The signing component is responsible for producing the complete COSE_Sign1 object, including the certificate chain and timestamp. The component using the SDK handles claim construction and manifest embedding. The existing callback API expects a raw signature, so it does not support that separation of responsibilities. Since we use c2pa-python, this PR first exposes the existing Rust direct COSE path through the C FFI. And c2pa-python repo will follow. |
Add CallbackSigner::new_direct_cose and c2pa_signer_create_direct_cose for callbacks that return complete tagged COSE_Sign1 structures.
8310be4 to
5e3d87b
Compare
|
I resolved the merge conflicts and force-pushed the updated branch. |
Changes in this pull request
Adds a callback signer for external signing services that receive serialized C2PA claim bytes and return a complete tagged COSE_Sign1 structure.
The callback is responsible for COSE construction, certificate chains, timestamps, and padding. The existing direct-COSE path bypasses post-signing verification by the SDK.
Python binding PR: contentauth/c2pa-python#339
Checklist
TO DOitems (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.