Skip to content

Conversation

kleinesfilmroellchen
Copy link
Collaborator

iffysign is Serenity’s version of minisign, which in turn is largely compatible with OpenBSD’s signify. As is tradition, Ted Unangst wrote something simple and useful that solves a very specific security problem, and we now steal it :^)

More details on the inner workings are available at the above links, but to summarize, we use Ed25519 key pairs to sign the BLAKE2b hashes of files, and can then verify the signatures using the public key. The signatures and keys are tiny and stored in base64, meaning their distribution is very easy across text-only and small-volume transmission mediums. Think QR codes and NFC tags.

To answer some of the why:

  • Package management. The rough idea is to also download signatures for each package .tar.gz that pkg downloads, and check them against a known (widely distributed) public key of the package provider. We can even do the same thing as OpenBSD does and include the future public key with some package (or the git repo or package repository or whatever) and rotate keys. These are all very easy with iffysign
  • Compatibility. Since we’re using a well-known format, creating and verifying the signatures without any SerenityOS tools available is trivial. On something like a Linux server that automates the package signing, minisign is likely available in the package repos. It also goes without saying that minisign and signify have almost ideal design and there is no point in reinventing a good system.
  • There currently isn’t a native Serenity tool for signing and verification tasks, even though we’ve had the Ed25519 implementation for a while. (You might notice that I don’t add a single line of actual crypto in this PR.) We do have a GPG port, but OpenPGP is approximately the worst cryptosystem in widespread use.
  • Fun :^)

The name was specifically chosen to not collide with either minisign or signify, to allow ports of those to coexist. (Both minisign and signify are, by their nature of self-contained unixy file I/O tools, very portable (a specifically cross-platform signify port exists), and we don’t want to prevent useful ports if we can avoid it.)

LibCrypto: Add Minisign signature tooling

This allows us to read and write minisign/signify-compatible keys and signatures, and create new key pairs and signatures. The next commit will make use of this utility in a CLI frontend.

Utilities: Add iffysign

This is a largely minisign/signify-compatible command line tool to create and verify minisign-compatible signatures and keys. It is missing some useful functionality at the moment, namely the ability to recreate public keys from private keys, more output options (quiet, verbose), and some more I/O options for stdin/stdout. It is, however, enough to provide the functionality needed for a signing and verification tool.

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Apr 25, 2025
This allows us to read and write minisign/signify-compatible keys and
signatures, and create new key pairs and signatures. The next commit
will make use of this utility in a CLI frontend.
This is a largely minisign/signify-compatible command line tool to
create and verify minisign-compatible signatures and keys. It is missing
some useful functionality at the moment, namely the ability to recreate
public keys from private keys, more output options (quiet, verbose), and
some more I/O options for stdin/stdout. It is, however, enough to
provide the functionality needed for a signing and verification tool.

The name was specifically chosen to not collide with either minisign or
signify, to allow ports of those to coexist. (Both minisign and signify
are, by their nature of self-contained unixy file I/O tools, very
portable (a specifically cross-platform signify port exists), and we
don’t want to prevent useful ports if we can avoid it.)
Copy link

stale bot commented May 16, 2025

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

@stale stale bot added the stale label May 16, 2025
Copy link

stale bot commented May 24, 2025

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

@stale stale bot closed this May 24, 2025
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label May 24, 2025
@stale stale bot removed the stale label Jun 8, 2025
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 8, 2025
Copy link

stale bot commented Jun 30, 2025

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

@stale stale bot added the stale label Jun 30, 2025
Copy link

stale bot commented Jul 7, 2025

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

@stale stale bot closed this Jul 7, 2025
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Jul 7, 2025
@stale stale bot removed the stale label Oct 10, 2025
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Oct 10, 2025
Copy link
Contributor

@implicitfield implicitfield left a comment

Choose a reason for hiding this comment

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

This is pretty neat, but I guess I'm biased since this uses BLAKE2b :P


auto const secret_key_line = TRY(AK::decode_base64(StringView { base64_secret_key_line }.trim_whitespace()));

ReadonlyBytes reader { secret_key_line };
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd use a FixedMemoryStream here and elsewhere in this file instead of using trim() and slice() on instances of Span (just to error out instead of asserting on invalid input).

Comment on lines +48 to +50
args_parser.add_option(operation, Operation::GenerateKey, "Generate a new key pair", "generate", 'G');
args_parser.add_option(operation, Operation::Sign, "Sign a file", "sign", 'S');
args_parser.add_option(operation, Operation::Verify, "Verify that a file's signature is valid", "verify", 'V');
Copy link
Contributor

Choose a reason for hiding this comment

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

ArgsParser seems kind of broken in this case, since operation always ended up being Operation::Verify when I tested this. I'd just add a fixme and replace the enum with a triplet of booleans to work around this for now (or fix ArgsParser, but I couldn't really figure out why it does this).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For me it worked? Not sure what command line you used

Copy link
Contributor

Choose a reason for hiding this comment

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

So the issue I was having was apparently with a dangling reference in ArgsParser. The accept_value lambda in the Enum specialization of add_option takes a reference to new_value, and since that's just a local copy, the reference becomes invalid after add_option returns.

This fixes it, so feel free to add it (or something similar) to this PR if you want. (I reworked this to explicitly construct an Option to be more consistent with other overloads while at it)

--- a/Userland/Libraries/LibCore/ArgsParser.h
+++ b/Userland/Libraries/LibCore/ArgsParser.h
@@ -91,15 +91,19 @@ public:
     template<Enum T>
     void add_option(T& value, T new_value, char const* help_string, char const* long_name, char short_name = 0, OptionHideMode hide_mode = OptionHideMode::None)
     {
-        add_option({ .argument_mode = Core::ArgsParser::OptionArgumentMode::None,
-            .help_string = help_string,
-            .long_name = long_name,
-            .short_name = short_name,
-            .accept_value = [&](StringView) {
+        Option option {
+            OptionArgumentMode::None,
+            help_string,
+            long_name,
+            short_name,
+            nullptr,
+            [&value, new_value](StringView) -> ErrorOr<bool> {
                 value = new_value;
                 return true;
             },
-            .hide_mode = hide_mode });
+            hide_mode
+        };
+        add_option(move(option));
     }
 
     template<Integral I>

signature_file = ByteString::formatted("{}.minisig", operand_file);

if (secret_key_file.is_empty())
secret_key_file = ByteString::formatted("{}/.config/iffysign/iffysign.sec", Core::StandardPaths::home_directory());
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: the .config/iffysign directory doesn't exist by default, so maybe we should be create it if it's missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

👀 pr-needs-review PR needs review from a maintainer or community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants