Skip to content

Accept consistently dash-separated args in ccf_cose_sign1 #7008

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

Conversation

eddyashton
Copy link
Member

Whenever using ccf_cose_sign1 by hand, we previously had to parse errors like:

$ ccf_cose_sign1 --content ./trigger_ledger_chunk_proposal.json --signing-cert member0_cert.pem --ccf-gov-msg-type proposal --ccf-gov-msg-created-at $(date -Is) --signing-key member0_privk.pem 
usage: ccf_cose_sign1 [-h] --content CONTENT --signing-cert SIGNING_CERT --ccf-gov-msg-type
                      {proposal,ack,state_digest,recovery_share,encrypted_recovery_share,ballot,withdrawal}
                      [--ccf-gov-msg-proposal_id CCF_GOV_MSG_PROPOSAL_ID] --ccf-gov-msg-created_at
                      CCF_GOV_MSG_CREATED_AT --signing-key SIGNING_KEY
ccf_cose_sign1: error: the following arguments are required: --ccf-gov-msg-created_at

To eventually realise that --ccf-gov-msg-created-at != --ccf-gov-msg-created_at, because the tool expects the final separator to be an underscore, not a dash. All because the eventual protected header is ccf.gov.msg.created_at. I think this is unhelpful, and there's no reason to expose this detail at the CLI level.

This change accepts both formats for this arg, so we lose no backwards compatibility but going forwards the tool is more likely to work first-time.

@eddyashton eddyashton requested a review from a team as a code owner May 14, 2025 10:30
@achamayou
Copy link
Member

Longer term I think we could and should switch to the SCITT format, with a CWT_Claims, and use iss instead of member id and iat instead of created_at, and sub instead of proposal_id.

@achamayou achamayou enabled auto-merge May 16, 2025 15:39
@achamayou achamayou added this pull request to the merge queue May 16, 2025
Merged via the queue into microsoft:main with commit da429c5 May 16, 2025
15 checks passed
@achamayou achamayou deleted the python_cose_sign1_tool_arg_dashes-arein_SANE branch May 16, 2025 17:11
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