Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove need for using OpenSSL to generate certs by using pre-generated certs #41

Merged
merged 6 commits into from
Jun 13, 2022
Merged

Remove need for using OpenSSL to generate certs by using pre-generated certs #41

merged 6 commits into from
Jun 13, 2022

Conversation

mauricefisher64
Copy link
Collaborator

Changes in this pull request

User can use the c2pa_rs without the need generate certs via opensll

Checklist

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

@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2022 •
edited
Loading

Codecov Report

Merging #41 (8dcabc2) into main (718e0c7) will decrease coverage by 1.09%.
The diff coverage is 86.53%.

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
- Coverage   72.66%   71.57%   -1.10%     
==========================================
  Files          62       62              
  Lines       12547    12604      +57     
==========================================
- Hits         9117     9021      -96     
- Misses       3430     3583     +153
Impacted Files Coverage Δ
sdk/src/lib.rs 100.00% <ø> (ø)
sdk/src/openssl/temp_signer.rs 25.00% <71.42%> (-55.43%) ⬇️
make_test_images/src/make_test_images.rs 73.48% <100.00%> (+0.76%) ⬆️
sdk/src/asset_handlers/c2pa_io.rs 80.00% <100.00%> (+0.45%) ⬆️
sdk/src/cose_validator.rs 58.47% <100.00%> (+0.72%) ⬆️
sdk/src/manifest.rs 87.57% <100.00%> (+0.04%) ⬆️
sdk/src/openssl/ec_signer.rs 87.50% <100.00%> (-4.61%) ⬇️
sdk/src/openssl/ec_validator.rs 96.47% <100.00%> (-0.48%) ⬇️
sdk/src/openssl/ed_signer.rs 71.60% <100.00%> (ø)
sdk/src/openssl/ed_validator.rs 100.00% <100.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 718e0c7...8dcabc2. Read the comment docs.

gpeacock
gpeacock approved these changes Jun 13, 2022
View reviewed changes
Copy link
Collaborator

@gpeacock gpeacock left a comment

Choose a reason for hiding this comment

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

A couple of suggestions included

sdk/src/manifest.rs Show resolved Hide resolved
sdk/src/openssl/ec_validator.rs Show resolved Hide resolved
sdk/src/openssl/temp_signer.rs Show resolved Hide resolved
@scouten-adobe scouten-adobe changed the title Remove need for using openssl to generate certs by using pre generated certs Remove need for using OpenSSL to generate certs by using pre-generated certs Jun 13, 2022
scouten-adobe
scouten-adobe requested changes Jun 13, 2022
View reviewed changes
Copy link
Collaborator

@scouten-adobe scouten-adobe left a comment

Choose a reason for hiding this comment

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

Stand by … I'm reviewing this PR, too.

scouten-adobe
scouten-adobe requested changes Jun 13, 2022
View reviewed changes
Copy link
Collaborator

@scouten-adobe scouten-adobe left a comment

Choose a reason for hiding this comment

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

A couple minor tweaks; otherwise 👍.

sdk/src/openssl/temp_signer.rs Show resolved Hide resolved
sdk/tests/fixtures/certs/ed25519.pem Show resolved Hide resolved
@mauricefisher64 mauricefisher64 merged commit a42186f into main Jun 13, 2022
@mauricefisher64 mauricefisher64 deleted the cert_removal branch June 13, 2022 21:33
mauricefisher64 added a commit that referenced this pull request Jun 14, 2022
@mauricefisher64
Merge branch 'main' of https://github.com/contentauth/c2pa-rs into bmff
27c7f9e 
* 'main' of https://github.com/contentauth/c2pa-rs:
  Apply fix from c2patool publish workflow (#40)
  Remove need for using OpenSSL to generate certs by using pre-generated certs (#41)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scouten-adobe scouten-adobe scouten-adobe approved these changes

@gpeacock gpeacock gpeacock approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mauricefisher64 @codecov-commenter @gpeacock @scouten-adobe