Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add new output format: "signify" #16

Closed
wants to merge 2 commits into from

Conversation

KAction
Copy link

@KAction KAction commented Mar 18, 2023

This is BSD tool used to sign release, with simpler interface and without
support of old key types. It does not do encryption.

This is how signify can be used:

$ signify -G -p new.pub -s new.key
$ signify -S -s new.key -m message.txt # creates message.txt.sig
$ signify -V -p new.pub -m message.txt # checks signature

Now first step can be replaced with

$ passphrase2pgp -f signify > new.combined

First two lines in generated file are public key, next two -- secret key. Note
that first line in both starts with "untrusted comment: ". signify(1) insists
for it to be the case.

@skeeto
Copy link
Owner

skeeto commented Mar 18, 2023 via email

@KAction
Copy link
Author

KAction commented Mar 18, 2023

Thanks, Dmitry! I'm interested in accepting this, and I've pushed some
additional changes reflecting how I plan to do so. They make no actual
changes to the output.

Looks fine. I don't have much to say about style of go code.

However, I don't like the keynum derivation. If two people happen to pick
the same User ID, they'll have the same keynum, which defeats the purpose
of the keynum as a unique key identifier. Instead it should be derived
from the public key, itself derived from the ID and passphrase. If two
people pick the same passphrase, then the game is already lost.

Makes sense. I am sure I can do this.

With zero KDF rounds, the salt is unused, right? It could simply be
zeroes. If this output format supported --protect (-e) — which is
currently not the case — then the salt matters, but it should also be
randomly generated, not derived from the ID or even the key. Protection
encryption shouldn't be deterministic.

This format supports passphrase that protects key. I was not exactly correct in my commit message.
Actually, signify -G asks you for passphrase unless you also pass -n. So I guess I can also respect --protect
part of config. You suggest that I do 64 rounds, like with ssh, or it should be configurable?

So for keynum I'm thinking hash the public key, just a straight SHA-512
truncated to 8 bytes. SHA-224 which is merely SHA-512 truncated (well,
plus a different initial state), and truncating twice instead of once
seems arbitrary.

Wasn't aware of this relation. Sure, sounds easy.

Dmitry Bogatov added 2 commits March 18, 2023 18:19
This is BSD tool used to sign release, with simpler interface and without
support of old key types. It does not do encryption.

This is how signify can be used:

  $ signify -G -n -p new.pub -s new.key
  $ signify -S -s new.key -m message.txt # creates message.txt.sig
  $ signify -V -p new.pub -m message.txt # checks signature

Now first step can be replaced with

  $ passphrase2pgp -f signify > new.combined

First two lines in generated file are public key, next two -- secret key. Note
that first line in both starts with "untrusted comment: ". signify(1) insists
for it to be the case.

Co-Authored: Christopher Wellons <wellons@nullprogram.com>
@skeeto
Copy link
Owner

skeeto commented Mar 18, 2023 via email

@skeeto
Copy link
Owner

skeeto commented Mar 19, 2023 via email

@KAction
Copy link
Author

KAction commented Mar 19, 2023

Looks good to me. Took some time to understand that it works because arrays are zero-initialized.

@skeeto
Copy link
Owner

skeeto commented Mar 20, 2023

Merged as ed7f62f and cf2aee7. Thanks! I also tagged v1.3.0 with the new feature, so it's now "live."

@skeeto skeeto closed this Mar 20, 2023
@KAction KAction deleted the contrib/0/signify/out branch March 25, 2023 20:12
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants