Add Sigsum support to tkey-verification #32
Merged
Conversation
This file contains hidden or 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
Takes a sigsum submit request from each file in the `submissions-dir` directory and submits it to the log. Each submission file generates a corresponding verification file. The verification files are written to the `verifications-dir` directory.
- Let the type live in its own package - Add some parsing functions on the type.
Move: - appbins - vendorpubkeys.go -> vendorkey to their own packages. Rewrite package verification to use the above, to be more picky in parsing to native types and support both types of verifications.
- Parsing test - Proof verification with digest only
Describe the new system in both a better overview and more detail. Include open questions.
- Change "provider" back to "vendor". - Change control the identity to check identity.
- Add a FromEmbedded() function to get the embedded vendor keys. - Add a FromString() function to use for testing our from dynamic configuration.
Make it easier to follow and understand what it does. Move challenge/response to the tkey package.
Always the default when vendor is Tillitis with product ID == Castor.
- Describe why we want to use a Sigsum log. - Add at least something about how we plan to use a Sigsum monitor. - Fix details about what we sign. - Fix details in structure. - Remove something that looked like HTML but was really a metasyntactic variable.
* Use the new helper methods on Verification (Which has moved to the new verification package * Add and use Verification.toJSON/.ToFile * Move the Submission type to it's own package and change it to match the structure of the new Verification and VerificatioJson types
…file Instead of outputting a file with an ed25519 signature we output a file with a Sigsum submit request
Assume that TKeys with the default UDI 0001020304050607 have the latest firmware (TK1-24.03).
Use the pubkey of a TKey built using the default UDS while developing.
Changing VerifyProofDigest() to return the used submit key
Use a simple [ed25519.Size]byte for public keys
Refactor submission functions to be functions with a receiver type be able to do this easier.
- Use SigsumConf instead of VendorPubKeys - App binaries have moved to internal/appbins/bins - Update show-pubkey example command - Use SSH format instead of hex format
- signer built with TKEY_SIGNER_APP_NO_TOUCH=yes from https://github.com/tillitis/tkey-device-signer/ - Using tag tkey-verification1 - Set as active for remote-sign.
Describe the configuration files to both serve-signer and remote-sign.
- Mention the Sigsum configuration under Maintenance. - Describe how to add a new device app, both for remote-sign and, as a new key for serve-signer.
- Remove a lot of old vendor key stuff and mistakes like mentioning tkey-verification when it should have been tkey-verify. - Provide different usage instructions for end user and vendor. - Update provisioning instructions. - Simplify the test setup description. - Move the TKey verification process design to its own file. Link to it from README. - Integrate some of the implementation notes under maintenance and remove the old doc/implementation-notes.md.
End user, even advanced end users, might not need to know exactly what assets are built in to tkey-verify. If an advanced user really want to know it should be possibly to find with --version and then checking in the code.
We introduce a .String() function on the Log type to help us do this.
Removing the last step and keeping step 8 since the monitor need to know about the produced device before it is logged.
Member
Author
|
A known issue is that when running We could probably automate handling this, but this might be outside of this scope. If so, remember to create an issue about it after merge. Automate the handling, a suggestion:
|
Otherwise the verification file will be overwritten by the submission file.
dehanj
approved these changes
Oct 1, 2025
Member
There was a problem hiding this comment.
I have done my review, including:
Getting the entire flow up and running:
-
tkey-verification remote-sign speaking to serve-sign
-
Submitting to the Sigsum log with tkey-sigsum-submit.
-
Verify with tkey-verify.
-
Gone through the documentation and left a few comments
-
I have made some commits, mostly typos and some fixes/clean up. Nothing major. Please look them through before any merging.
I have not gone through every piece of code.
I have a few questions, but they are more tied to Sigsum than tkey-verification so will ask them in another forum.
| "Set serial port device `PATH`. If this is not passed, auto-detection will be attempted.") | ||
| pflag.IntVarP(&dev.Speed, "speed", "s", tkeyclient.SerialSpeed, | ||
| "Set serial port `SPEED` in bits per second.") | ||
| pflag.BoolVar(&verbose, "verbose", false, |
Member
There was a problem hiding this comment.
This isn't new for this version and might be out of scope for this review. But I would suggest there would be 2 levels for verbose. One to follow the verification process, more or less get the steps printed to be able to follow, and a second for debugging.
If you use verbose now you get all the packages send between client and host printed.
- tkey-verify does the verification. - Remove text about the checksum at the monitor. It is already covered by point 8. - Avoid unnecessary explanation of what the verification file contains.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Main goal: Add Sigsum support to tkey-verification. See docs/design.md for our thoughts on how we do this.
key pair. Vendor key support is retained for verification.
keys.
https://github.com/tillitis/tkey-device-signer without touch. This
version supports both the older Bellatrix and the upcoming Castor
TKeys.
Fixes #31
I think a review should focus on:
Getting the entire flow up and running:
Understand the design (see doc/design.md).
Compare the code with the design.
There are just too numerous refactoring things that I think a review should focus on the above, and not necessarily all the details of the implementation.
Type of change
Submission checklist
Internal documentation is updated, not yet the website introduction, but that can wait until a release.