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.

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

Adds local signing for metadata #451

Conversation

enyinna1234
Copy link
Contributor

@enyinna1234 enyinna1234 commented Nov 13, 2023

This enables the use of a locally stored metadata to create a signature. It also saves the created signature locally.

Closes #423

Description

Fixes #(issue)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)

Additional requirements

  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Code of Conduct

By submitting this PR, you agree to follow our Code of Conduct.

  • I agree to follow this project's Code of Conduct

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.07%. Comparing base (47d0a69) to head (522e4f1).
Report is 41 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #451      +/-   ##
==========================================
+ Coverage   98.90%   99.07%   +0.17%     
==========================================
  Files          20       26       +6     
  Lines        1367     1844     +477     
==========================================
+ Hits         1352     1827     +475     
- Misses         15       17       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@enyinna1234 enyinna1234 force-pushed the feature/local-md-sign branch from bbd972a to dc139e2 Compare November 15, 2023 07:58

assert test_result.exit_code == 0, test_result.output
assert "Metadata Signed! 🔑" in test_result.output
assert "SIGNING KEYS" in test_result.output
Copy link
Member

Choose a reason for hiding this comment

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

We've discussed this name in the table summary SIGNING KEYS is a bit confusing. Do you have a suggestion here @enyinna1234?

@kairoaraujo
Copy link
Member

@enyinna1234 it will require a rebase

This enables the use of a locally stored metadata to create a
signature. It also saves the created signature locally.

Closes repository-service-tuf#423
@enyinna1234 enyinna1234 force-pushed the feature/local-md-sign branch from dc139e2 to cb5872d Compare November 16, 2023 15:54
kairoaraujo
kairoaraujo previously approved these changes Nov 17, 2023
@kairoaraujo
Copy link
Member

@enyinna1234 I spotted a required doc update as part of this PR.

sign (``sign``)
...............
.. Note:: It's required to have access to the private key used for signing.
.. warning:: Do not share the private key.
.. code:: shell
❯ rstuf admin metadata sign
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Metadata Signing ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
Metadata signing allows sending signature of pending Repository Service for TUF
(RSTUF) role metadata.
It retrieves the pending metadata from the RSTUF API. Select the metadata role
pending signature and the private key to load.
After loading the key it will sign the role metadata and send the request to the
RSTUF API with the signature.
API URL address: https://api.rstuf.example.com
Choose a metadata to sign [root]: root
Signing root version 1
Choose a private key to load [Jimi Hendrix]: Jimi Hendrix
Sending signature
Metadata signature status: ACCEPTED (09659992156445238f60bd5f96a43479)
Metadata Signature status: STARTED
Metadata Signature status: SUCCESS
Metadata Signed! 🔑

@kairoaraujo kairoaraujo self-requested a review November 18, 2023 09:53
@kairoaraujo kairoaraujo dismissed their stale review November 18, 2023 09:54

Waiting the docs update.

Comment on lines +1223 to +1234
with open("tests/files/das-root.json", "r") as file:
das_root = file.read()
fake_offline_md = {"metadata": json.loads(das_root)}
json.load = pretend.call_recorder(lambda *a: fake_offline_md)
json.dump = pretend.call_recorder(lambda *a: "fake_write")
test_result = client.invoke(
metadata.sign,
["tests/files/das-root.json"],
input="\n".join(input_step),
obj=test_context,
catch_exceptions=False,
)
Copy link
Member

Choose a reason for hiding this comment

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

Make it more readable, separate function calls of a function you are testing from mocking.

Suggested change
with open("tests/files/das-root.json", "r") as file:
das_root = file.read()
fake_offline_md = {"metadata": json.loads(das_root)}
json.load = pretend.call_recorder(lambda *a: fake_offline_md)
json.dump = pretend.call_recorder(lambda *a: "fake_write")
test_result = client.invoke(
metadata.sign,
["tests/files/das-root.json"],
input="\n".join(input_step),
obj=test_context,
catch_exceptions=False,
)
with open("tests/files/das-root.json", "r") as file:
das_root = file.read()
fake_offline_md = {"metadata": json.loads(das_root)}
json.load = pretend.call_recorder(lambda *a: fake_offline_md)
json.dump = pretend.call_recorder(lambda *a: "fake_write")
test_result = client.invoke(
metadata.sign,
["tests/files/das-root.json"],
input="\n".join(input_step),
obj=test_context,
catch_exceptions=False,
)

Comment on lines +1226 to +1227
json.load = pretend.call_recorder(lambda *a: fake_offline_md)
json.dump = pretend.call_recorder(lambda *a: "fake_write")
Copy link
Member

Choose a reason for hiding this comment

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

When you add a pretend.call_recorder you should also check the calls afterwards.
I made the same comment to Ivana before in this comment: #447 (comment)

@MVrachev
Copy link
Member

We need to close this pr as we will towards a new implementation of admin2 signing commands.
This means local signing will need to be implemented in a new way for rstuf admin2 metadata sign command.

@MVrachev MVrachev closed this Apr 24, 2024
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.

Task: Add option to sign a metadata without using API (local metadata)
3 participants