Export Signify methods, support BLAKE2s - #38
Conversation
55951e6 to
e1e3779
Compare
d26286c to
2dd940a
Compare
mchack-work
changed the title
c251ce0 to
36039e8
Compare
agren
left a comment
agren
left a comment
There was a problem hiding this comment.
I've tested this and the signing functionality seems intact. I have a few comments on added functionality.
| func (p *PubKey) ToBuffer(comment string) ([]byte, error) { | ||
| signifyKey := signifyPubKey{ | ||
| Alg: [2]uint8{'E', 'd'}, | ||
| KeyNum: [8]uint8{0x1, 0x7, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, |
There was a problem hiding this comment.
Do we want to hard code KeyNum here? Since we're exporting this for use elsewhere we should avoid to force it on the user if we can. So instead of setting KeyNum here we could set in cmd/tkey-sign.
There was a problem hiding this comment.
This probably means we need to represent it in either the PubKey and Signature types in the signify package, too. What do you think?
AFAIU the KeyNum is basically just used as an extra check that a specific key pair corresponds to a certain signature.
I had a look at how the OpenBSD signify command generates it and handles it. It's in signify.c:generate():
crypto_sign_ed25519_keypair(pubkey.pubkey, enckey.seckey);
arc4random_buf(keynum, sizeof(keynum));which means it's just some random numbers generated when the key pair is created.
There was a problem hiding this comment.
I have addresses this by adding support for KeyNum in the way both signify and minisign supports them.
See commit messages.
| return fmt.Errorf("%w", err) | ||
| } | ||
|
|
||
| if pubKey.Alg != [2]byte{'E', 'd'} || pubKey.KeyNum != [8]byte{1, 7} { |
There was a problem hiding this comment.
This check seems new. AFAIK previous versions of tkey-sign doesn't expect KeyNum to be a specific value. Do we need to check it?
There was a problem hiding this comment.
In the old tkey-sign the KeyNum was at least hardcoded to 1,7 when generating the files.
It's probably better to handle it some other way and make it visible in the types, as per the former comment.
There was a problem hiding this comment.
Done. If the user has just a slice from, say, signer in TKey KeyNum will be randomized from NewPubKey(). If they want to set it themselves they have to instantiate a PubKey variable themselves. Documented in the function.
- Make public key and signatures available externally. - Add methods to import and export these types to and from buffers and files. - Add tests for parsing and generating. - Move writeRetry() to util.go - Signify uses "Ed", we use "Eb", indicating to us that our message should be hashed with BLAKE2s.
- Clean up the main function and hide the complexities of all the commands in their own functions. Also reduces the cyclomatic complexity. - Hide some args, like device path, device speed, USS filename and request USS from user, in structs.
- Introduce new function getMessage() that produces the message to be signed or verified. - Use message produced by getMessage() in Sign() and Verified() and dependent helper functions.
Choose hash algorithm to use with -a/--alg. Options are "ed" (default, SHA-512) and "b2s" (BLAKE2s).
36039e8 to
5d40c41
Compare
- The Digest takes Alg for algorithm. We change the getMessage() to getDigest() and set the alg type when calling. We pass a Digest around to all consumers, so we know the algorithm type all the way to export. - Compare digest algo from command line with algo from sig file.
5d40c41 to
880988f
Compare
|
Made a proof of concept to export the pubkey in binary format. |
- Don't repeat the manual page. Instead refer to it. - Describe how tkey-sign differs in signing from signify.
In signify the KeyNum is used to associate a signature with a keypair. Do the same thing here: - When we get a public key from the Tkey, assign a random KeyNum and store in the public key file. - When we do a signature, get the same KeyNum from the public key file we force the user to provide and store in the signature file. - When verifying signatures, first compare the KeyNum in the public key file against what's in the signature file and bail early if they're different. Change the type of signify.PubKey to a struct including a KeyNum both to that and to Signature. We change the FromBuffer() and ToBuffer() methods accordingly.
0664d05 to
71d6451
Compare
3 participants
Description
Make Signify types like public key and signatures available externally, for instance for use in
sign-tool,tkey-mgt,tkeyimageand the like. And, of course, external project who would like to read and write to these this kind of files.Write methods to handle import and export more gracefully.
Add tests for signify methods.
Change the
tkey-signcommand accordingly.Add support for choice of hash algoritm (
-aflag)Add support for BLAKE2s algorithm.
Fixes #36
See also:
Type of change
Submission checklist