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

Store private seed as the secret for newly generated keys #2472

Merged
merged 5 commits into from Nov 23, 2023

Conversation

zorgiepoo
Copy link
Member

@zorgiepoo zorgiepoo commented Nov 21, 2023

Today the private/public keypair that orlp/ed25519 generates and that we store in the Keychain is "incompatible" with other cryptography libraries/tools. In particular the private key this library generates (which includes a variation of the hashed seed) cannot be used with other libraries for signing (without modification).

For newly generated keys, this change instead stores the base64-encoded seed as the private key we store in Keychain and we use for exporting/importing keys. We still keep compatibility with older generated keys whose format more resembles a base64-encoding of hash(seed) + public_key.

Base64 encoding/decoding was kept around to allowing copying the secret from Keychain Access. I also did not want to use a different format when exporting/importing keys for consistency.

The private seed was picked because it's the minimal/defacto info needed to be able to construct the private/public keypair using any library or tool even if they may prefer a different internal representation. For instance, orlp/ed25519 internally prefers hash(seed) + public_key, CryptoKit prefers seed, golang & libsodium prefer seed + public_key, openssl prefers an OID + seed for DER files, etc. This representation we now store does not give any preference to any particular tool or internal representation. I am also not worried about performance of not using a precomputed public key or hashed seed.

As part of this change, I am also disabling command line flags that allow passing the private key as a command line argument. This option was already deprecated and should not be supported for new keys.

Fixes #2470

An example for exporting to DER file (assuming we have a newly generated key):

generate_keys -x exported_key
cat exported_key | base64 --decode > exported_key_binary
echo -n -e '\x30\x2e\x02\x01\x00\x30\x05\x06\x03\x2b\x65\x70\x04\x22\x04\x20' > oid_header
cat oid_header exported_key_binary > exported_key.der

For other libraries/tools you just need to pass exported_key_binary as the seed to construct the key-pair.

Importing from a DER file is easier:

tail -c 32 key.der | base64 > key
generate_keys -f key

Past related discussions:
#2467
#2160
#2298

Misc Checklist

  • My change requires a documentation update on Sparkle's website repository
  • My change requires changes to generate_appcast, generate_keys, or sign_update

Testing

I tested and verified my change by using one or multiple of these methods:

  • Sparkle Test App
  • Unit Tests
  • My own app
  • Other (please specify)

Things to test:

  • Importing new generated key from openssl tools work
  • Exporting new generated key to openssl tools work
  • generate_keys can generate new key and export/import the new key
  • generate_keys can export/import old keys
  • sign_update can sign/verify new keys and has same results as other tools (e.g openssl)
  • sign_update can sign/verify old keys (try on an existing update)
  • sign_update -f does not work for newly generated keys but still works for older generated keys
  • generate_appcast works still with generating signatures with old keys
  • generate_appcast works with generating signatures with new keys
  • generate_appcast -s works for older generated keys
  • generate_appcast -s does not work for newly generated keys
  • Test a live app with newly generated keys
  • Test compatibility when trying to use old tools (tested; these crash when passing private key, reading 32-bytes out of buffer bounds, which is good enough and better than leaking private data)

macOS version tested: 14.1.1 (23B81)

@zorgiepoo zorgiepoo added this to the 2.6 milestone Nov 21, 2023
@zorgiepoo
Copy link
Member Author

cc @jozefizso

@zorgiepoo zorgiepoo merged commit 1a0b023 into 2.x Nov 23, 2023
2 checks passed
@zorgiepoo zorgiepoo deleted the private-signing-compatibility branch November 23, 2023 14:23
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.

Improve signing key compatibility with other cryptography libraries
1 participant