Add Session Pro master key derivation #61
Conversation
src/ed25519.cpp
Outdated
| std::pair<std::array<unsigned char, 32>, std::array<unsigned char, 64>> | ||
| ed25519_pro_key_pair_for_ed25519_seed(std::span<const unsigned char> ed25519_seed) { | ||
| if (ed25519_seed.size() != 32) | ||
| throw std::invalid_argument{"Invalid ed25519_seed: expected 32 bytes"}; |
There was a problem hiding this comment.
This could also allow a 64-byte libsodium "secret key" as input (which is simply the seed+pubkey concatenated together, and so the first 32 bytes of that are still the seed).
There was a problem hiding this comment.
Done, added support for 32/64 byte keys
tests/test_ed25519.cpp
Outdated
| auto kp1 = session::ed25519::ed25519_pro_key_pair_for_ed25519_seed(to_span(ed_seed1)); | ||
| auto kp2 = session::ed25519::ed25519_pro_key_pair_for_ed25519_seed(to_span(ed_seed2)); | ||
| CHECK_THROWS(session::ed25519::ed25519_pro_key_pair_for_ed25519_seed(to_span(ed_seed_invalid))); |
There was a problem hiding this comment.
To explicit to_spans should not be needed here: spans are implicitly convertible from vectors or arrays, so that span arguments are a sort of generic "accept anything span-like", somewhat similar to string_view.
(With the _hex_u change suggestion above, these will simply be spans to begin with, but even without that this should work).
tests/test_ed25519.cpp
Outdated
| auto ed_seed1 = "e5481635020d6f7b327e94e6d63e33a431fccabc4d2775845c43a8486a9f2884"_hexbytes; | ||
| auto ed_seed2 = "743d646706b6b04b97b752036dd6cf5f2adc4b339fcfdfb4b496f0764bb93a84"_hexbytes; | ||
| auto ed_seed_invalid = "010203040506070809"_hexbytes; |
There was a problem hiding this comment.
I believe we added _hexbytes before we had the constexpr versions in oxen-encoding:
| auto ed_seed1 = "e5481635020d6f7b327e94e6d63e33a431fccabc4d2775845c43a8486a9f2884"_hexbytes; | |
| auto ed_seed2 = "743d646706b6b04b97b752036dd6cf5f2adc4b339fcfdfb4b496f0764bb93a84"_hexbytes; | |
| auto ed_seed_invalid = "010203040506070809"_hexbytes; | |
| constexpr auto ed_seed1 = "e5481635020d6f7b327e94e6d63e33a431fccabc4d2775845c43a8486a9f2884"_hex_u; | |
| constexpr auto ed_seed2 = "743d646706b6b04b97b752036dd6cf5f2adc4b339fcfdfb4b496f0764bb93a84"_hex_u; | |
| constexpr auto ed_seed_invalid = "010203040506070809"_hex_u; |
(which might also need a using namespace oxenc::literals; added). _hexbytes was the old implementation that constructs a vector of values at runtime; _hex_u from oxenc yields a constexpr span of unsigned char, with compile-time validity checking. (There's also _hex for constexpr string_view, and _hex_b for constexpr span of std::byte).
tests/test_ed25519.cpp
Outdated
| auto ed_seed_invalid = "010203040506070809"_hexbytes; | ||
|
|
||
| auto kp1 = session::ed25519::ed25519_pro_key_pair_for_ed25519_seed(to_span(ed_seed1)); | ||
| auto kp2 = session::ed25519::ed25519_pro_key_pair_for_ed25519_seed(to_span(ed_seed2)); |
There was a problem hiding this comment.
Perhaps: auto [pk1, sk1] = session::ed25519::ed25519_pro_key_pair... to avoid all the .first/.second's later on.
tests/test_ed25519.cpp
Outdated
| CHECK(kp1.first.size() == 32); | ||
| CHECK(kp1.second.size() == 64); | ||
| CHECK(kp1.first != kp2.first); | ||
| CHECK(kp1.second != kp2.second); |
There was a problem hiding this comment.
Might also be worth checking that the last 32 bytes of the secret key value equal the public key value.
There was a problem hiding this comment.
Just a little bit below this we check the full 64 bytes of the SK which checks the PK matches.
src/ed25519.cpp
Outdated
| @@ -86,6 +89,29 @@ bool verify( | |||
| crypto_sign_ed25519_verify_detached(sig.data(), msg.data(), msg.size(), pubkey.data())); | |||
| } | |||
|
|
|||
| std::pair<std::array<unsigned char, 32>, std::array<unsigned char, 64>> | |||
| ed25519_pro_key_pair_for_ed25519_seed(std::span<const unsigned char> ed25519_seed) { | |||
There was a problem hiding this comment.
I can very easily see us using the same key derivation for other seeds in the future, so perhaps a tiny rearrangment like this:
std::pair<std::array<unsigned char, 32>, std::array<unsigned char, 64>> derived_ed25519_keypair(seed, std::string_view derivation_key) {
// exactly as the current implementation, but using `derivation_key` instead of `BLAKE2B_KEY`
}
auto ed25519_pro_key_pair_for_ed25519_seed(std::span<const unsigned char> ed25519_seed) {
return derived_ed25519_keypair(seed, "SessionProRandom");
}makes that dead simple (and obvious) to a future dev wanting to do something similar.
include/session/ed25519.hpp
Outdated
| /// Outputs: | ||
| /// - The Master Session Pro ed25519 key | ||
| std::pair<std::array<unsigned char, 32>, std::array<unsigned char, 64>> | ||
| ed25519_pro_key_pair_for_ed25519_seed(std::span<const unsigned char> ed25519_seed); |
There was a problem hiding this comment.
I'm tempted to only return the second element here (the 64-byte value), because the first value will always just be the last 32 bytes of the second value and is easily obtainable by just taking a subspan. (libsodium's secret key is simply seed+pubkey, and is done because that saves the necessity to recompute the pubkey from seed -- which is one of the more expensive operations in signature generation -- without requiring you to actually pass two arguments everywhere).
This simplifies the API and minimises the API surface with code interacting with it since the public key is already bundled with the secret. - Add helper function to derive keys from the original seed - Use _hex_u literals to simplify test construction code
|
And some additional minor changes, updated the rest of the ed25519 tests to use constexpr compile time hex and reused cleared_uc32 and the 64 bit variant from the sodium array header. I've also used the cleared_uc32 on the seed we derived for the pro key. |
Derive the master session pro key from a given ed25519 seed as per: