Add SPAKE2 password authenticated key exchange (RFC 9382)

[randombit]

No description provided.

[coveralls]

coverage: 91.074% (+0.001%) from 91.073%
when pulling af3b206 on jack/spake2
into 162a389 on master.

[reneme]

I have a few suggestions re the API and some nits. Most of them, I prototyped in another branch, including an example.cpp.

[reneme]

concat() could make this easier:

Suggested change

	std::vector<uint8_t> ad(a_identity.size() + b_identity.size() + context.size() + 3 * 8);
	BufferStuffer stuffer(ad);
	auto append_with_le64 = [&](std::span<const uint8_t> data) {
	stuffer.append(store_le(static_cast<uint64_t>(data.size())));
	stuffer.append(data);
	};
	append_with_le64(a_identity);
	append_with_le64(b_identity);
	append_with_le64(context);
	return ad;
	auto store_le64 = [](uint64_t s) { return store_le(s); };
	// clang-format off
	return concat<std::vector<uint8_t>>(store_le64(a_identity.size()), a_identity,
	store_le64(b_identity.size()), b_identity,
	store_le64(context.size()), context);
	// clang-format on

[reneme]

Perhaps pull the store_le64 lambda as a free-standing function into the anonymous namespace and reuse it for the hash calculation in process_message().

auto store_le64(uint64_t n) -> std::array<uint8_t, 8> {
   return store_le(n);
}

[reneme]

Suggested change

	EC_AffinePoint peer_pt(m_params.group(), peer_message);
	const EC_AffinePoint peer_pt(m_params.group(), peer_message);

[reneme]

Suggested change

	append_to_hash_with_le64(m_params.a_identity());
	// Calculate TT
	append_to_hash_with_le64(m_params.a_identity());

[reneme]

Sometimes I do like the ternary:

Suggested change

	const EC_AffinePoint& spake2_our_pt(SPAKE2_PeerId whoami) const {
	if(whoami == SPAKE2_PeerId::PeerA) {
	return m_params.first; // M
	} else {
	return m_params.second; // N
	}
	}
	const EC_AffinePoint& spake2_their_pt(SPAKE2_PeerId whoami) const {
	if(whoami == SPAKE2_PeerId::PeerA) {
	return m_params.second; // N
	} else {
	return m_params.first; // M
	}
	}
	const EC_AffinePoint& spake2_our_pt(SPAKE2_PeerId whoami) const {
	return (whoami == SPAKE2_PeerId::PeerA) ? spake2_m() : spake2_n();
	}
	const EC_AffinePoint& spake2_their_pt(SPAKE2_PeerId whoami) const {
	return (whoami == SPAKE2_PeerId::PeerA) ? spake2_n() : spake2_m();
	}

[reneme]

On a second note: there's actually no reason to expose those into the public API. Let's just implement them as free-standing functions in spake2.cpp's anonymous namespace.

[reneme]

Along the lines of m_params.spake2_their_pt(), we could add this to the params class:

      auto spake2_sort_messages(SPAKE2_PeerId whoami,
                                std::span<const uint8_t> ours,
                                std::span<const uint8_t> theirs) const
         -> std::pair<std::span<const uint8_t>, std::span<const uint8_t>> {
         if(whoami == SPAKE2_PeerId::PeerB) {
            std::swap(ours, theirs);
         }
         return {ours, theirs};
      }

... and then reduce this code to:

Suggested change

	// Always pA followed by pB:
	if(m_whoami == SPAKE2_PeerId::PeerA) {
	append_to_hash_with_le64(our_pt);
	append_to_hash_with_le64(peer_message);
	} else {
	append_to_hash_with_le64(peer_message);
	append_to_hash_with_le64(our_pt);
	}
	auto [pA, pB] = m_params.spake2_sort_messages(m_whoami, our_pt, peer_message);
	append_to_hash_with_le64(pA);
	append_to_hash_with_le64(pB);

[reneme]

Nit:

Suggested change

	constexpr uint8_t spake2_m[] = {'S', 'P', 'A', 'K', 'E', '2', ' ', 'M'};
	constexpr uint8_t spake2_n[] = {'S', 'P', 'A', 'K', 'E', '2', ' ', 'N'};
	auto m = EC_AffinePoint::hash_to_curve_ro(group, hash, input, spake2_m);
	auto n = EC_AffinePoint::hash_to_curve_ro(group, hash, input, spake2_n);
	auto as_span = [](std::string_view domsep) {
	return std::span(cast_char_ptr_to_uint8(domsep.data()), domsep.size());
	};
	auto m = EC_AffinePoint::hash_to_curve_ro(group, hash, input, as_span("SPAKE M"));
	auto n = EC_AffinePoint::hash_to_curve_ro(group, hash, input, as_span("SPAKE N"));

[reneme]

Instead of keeping the state visible to the user in a context object, we could make generate_message() return an opaque (non-copyable) state object that process_message() requires to be moved into as a parameter. This introduces a bit of boiler plate, but effectively avoids misuse.

Eg.

      struct Internal;

      struct State {
            State(std::unique_ptr<Internal> i);

            ~State();
            State(const State&) = delete;
            State& operator=(const State&) = delete;
            State(State&&) noexcept;
            State& operator=(State&&) noexcept;

            std::unique_ptr<Internal> internal;  // NOLINT(misc-non-private-member-*)
      };

Internal would be implemented in the *.cpp and essentially contain the content of m_our_message.
In full consequence, this could reduce the API to two free-standing functions and remove the need for the context class entirely. I prototyped this in the branch I mentioned in the PR-submission. The example-code in this branch also uses the free-standing functions for illustration.

[reneme]

I appreciate the UTF-8 emoji, but shouldn't we go for @-warning (sans the -, just to not annoy the GitHub account "warning" with an accidental mention) to let this render properly in Doxygen?

[reneme]

Does it, though? One might argue, that this constructor provides the flexibility to use a different memory-hard hash function than the default (argon2id). Perhaps we should instead add another constructor that takes a std::span<> w_bytes as an already expanded shared secret, that is then just turned into a scalar of the group.

This would be a major footgun, though, because people might just accidentally pass the password (meant for the std::string_view overload) into this proposed constructor and get no pwhash at all.

[reneme]

Perhaps a few negative tests? E.g. when the context string is passed inconsistently.