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

luks-list: show tang key thumbprints #375

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rpigott
Copy link

@rpigott rpigott commented Jun 8, 2022

Implements #351, adding the thp to the list output. The displayed thp can be used to fetch the specific stored key from the tang server at GET /adv/<thp>.

@rpigott
Copy link
Author

rpigott commented Jun 8, 2022

Sadly this disturbs the pin cfg in clevis luks regen.

@rpigott rpigott force-pushed the luks-list-thp branch 2 times, most recently from 0a77b32 to b3567f9 Compare June 9, 2022 18:52
@rpigott
Copy link
Author

rpigott commented Jun 9, 2022

Ok should work now. I changed it to print the thumbprint only with a "-t" argument.

@sarroutbi
Copy link
Collaborator

Hello. Thanks for your change. Changes LGTM. Could you please complete your commit to:

  • Include format changes to man page? File: ./src/luks/clevis-luks-list.1.adoc
  • Include a unit test to verify this new parameter works as expected?

Thanks

pin=$(printf '{"url":"%s"}' "${url}")
else
adv="$(jose fmt -j- -g adv -o- <<< "${content}")"
thp="$(jose jwk thp -i- <<< "${adv}" | tail -n1)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not correct, as you will get thumbprints for both the exchange and signing keys. You should do something like tang-show-keys [1] does, with the jose jwk use -u verify, before getting the thumbprints.

It may be good to also specify the default hash algorithm for the thumbprints.

[1] https://github.com/latchset/tang/blob/master/src/tang-show-keys#L34

@sarroutbi
Copy link
Collaborator

Hello @rpigott. May I help you uploading proposed changes?

@rpigott
Copy link
Author

rpigott commented May 9, 2023

Hi, thanks for the feedback but I actually have no use for this feature anymore, so I don't really care about the patch. If you or anyone else still want this feature feel free to use or modify the patch as you see fit.

@sarroutbi
Copy link
Collaborator

@sergio-correia : do you think this is useful? Do you want me to recover it from here?

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.

3 participants