Skip to content

feat(ffi): add direct COSE callback signer - #2848

Open
s35-lee wants to merge 2 commits into
contentauth:mainfrom
s35-lee:feat/direct-cose-callback-signer
Open

s35-lee wants to merge 2 commits into
contentauth:mainfrom
s35-lee:feat/direct-cose-callback-signer

Conversation

@s35-lee

@s35-lee s35-lee commented Oct 11, 2026 •

Copy link
Copy Markdown

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.

  • Adds CallbackSigner::new_direct_cose with sync and async support.
  • Exposes c2pa_signer_create_direct_cose through the C FFI.
  • Requires callback output to exactly match reserve_size.
  • Rejects invalid reservation sizes and callback failures.
  • Documents callback lifetime, concurrency, timeout, and verification boundaries.
  • Preserves existing raw-signature callback behavior.

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

  • This PR represents a single feature, fix, or change.
  • All applicable changes have been documented.
  • Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

@gpeacock gpeacock left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_size of cap + 1 is rejected; exactly the cap is accepted.

@s35-lee

s35-lee commented Oct 14, 2026

Copy link
Copy Markdown
Author

@gpeacock
Thanks for taking a look!

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.
@s35-lee
s35-lee force-pushed the feat/direct-cose-callback-signer branch from 8310be4 to 5e3d87b Compare October 14, 2026 15:32
@s35-lee

s35-lee commented Oct 14, 2026

Copy link
Copy Markdown
Author

I resolved the merge conflicts and force-pushed the updated branch.
Sorry for any inconvenience this may cause during review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants