This repository has been archived by the owner on Dec 11, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 37
(MINOR) allow clients to sign with a process outside of c2patool #169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dyro
force-pushed
the
dyross/remote-signer
branch
from
March 14, 2024 17:37
d7bd699
to
f4cecc6
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files@@ Coverage Diff @@
## main #169 +/- ##
===========================================
- Coverage 76.10% 46.31% -29.80%
===========================================
Files 3 7 +4
Lines 406 719 +313
===========================================
+ Hits 309 333 +24
- Misses 97 386 +289 ☔ View full report in Codecov by Sentry. |
dyro
force-pushed
the
dyross/remote-signer
branch
from
March 14, 2024 21:22
c8db773
to
f0d498e
Compare
jrglasgow
reviewed
Mar 15, 2024
gpeacock
reviewed
Mar 15, 2024
gpeacock
reviewed
Mar 15, 2024
gpeacock
reviewed
Mar 15, 2024
gpeacock
reviewed
Mar 15, 2024
gpeacock
reviewed
Mar 15, 2024
dyro
force-pushed
the
dyross/remote-signer
branch
from
March 18, 2024 17:59
4340f8a
to
b5dcc38
Compare
dkozma
approved these changes
Mar 18, 2024
dyro
changed the title
allow clients to sign with a process outside of c2patool
(minor) allow clients to sign with a process outside of c2patool
Mar 20, 2024
dyro
changed the title
(minor) allow clients to sign with a process outside of c2patool
(Minor) allow clients to sign with a process outside of c2patool
Mar 20, 2024
dyro
changed the title
(Minor) allow clients to sign with a process outside of c2patool
(MINOR) allow clients to sign with a process outside of c2patool
Mar 20, 2024
scouten-adobe
pushed a commit
to contentauth/c2pa-rs
that referenced
this pull request
Dec 10, 2024
…ntauth/c2patool#169) * allow clients to sign with a process outside of c2patool * builds bins for coverage tests * removes pub key * removes extra println * includes stderr; shows stderr from child process is included in error * adds better docs * improves docs; renames signer-process to signer-path. * adds docs and default value for reserve_size * replaces TODO with proper docs * removes extraneous else --------- Co-authored-by: Dylan Ross <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes in this pull request
This PR introduces a way for clients to sign claim bytes using an external process. There are two open questions I have:
signer-process-success
usesopenssl
, so I had to add that as a dependency toc2patool
. I tried adding these asexamples
, but cargo examples aren't compiled before runningcargo test
. Those tests would fail unless we rancargo build --examples; cargo test
.--reserve-size
argument dynamically. This will change if a user includes a custom TSA URL. In claims-signer we hard-coded a value, but I'm worried about doing that here. Any suggestions? I'm currently requiring the user to supply the value, is that okay? We could also use the cert size for the reserve size if there's no TSA url in the sign config and only require the reserve size argument if the user specified a tsa url.I'll write docs after we resolve the reserve-size question!
Checklist
TO DO
items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.