Skip to content

Commit

Permalink
ssh/Serializer: CommitBignum2() strips leading null bytes
Browse files Browse the repository at this point in the history
OpenSSL's EVP_PKEY_derive() function sometimes inserts leading null
bytes: in ossl_ecdh_simple_compute_key(), the difference between
EC_GROUP_get_degree() and BN_num_bytes() is padded with null bytes.

But the SSH2 protocol mandates that leading null bytes must be
stripped.  Without this, the hash used to generate the ECDH reply is
wrong, resulting in OpenSSH error "incorrect signature".

This problem occurs rarely only, which is why I never noticed it
during development.  To reproduce it, the "test_lukko.py" script needs
to be run in a loop for 10 minutes or so.
  • Loading branch information
MaxKellermann committed Feb 2, 2024
1 parent 9fe0ff6 commit 064ebdd
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
2 changes: 1 addition & 1 deletion debian/changelog
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
cm4all-lukko (0.14) unstable; urgency=low

*
* fix bignum padding in ECDH reply ("incorrect signature" error)

--

Expand Down
12 changes: 12 additions & 0 deletions src/ssh/Serializer.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ public:
void CommitBignum2(std::size_t size) {
CommitWriteN(size);

const auto stripped = StripLeadingNullBytes(Last(size));
position -= stripped;
size -= stripped;

if (size > 0 &&
(buffer[position - size] & std::byte{0x80}) != std::byte{})
/* prepend null byte to avoid interpretation as
Expand All @@ -223,6 +227,14 @@ private:
std::span<std::byte> Last(std::size_t n) noexcept {
return std::span{buffer}.first(position).last(n);
}

static std::size_t StripLeadingNullBytes(std::span<std::byte> s) noexcept {
auto i = std::find_if(s.begin(), s.end(), [](std::byte b){ return b != std::byte{}; });
if (i != s.begin())
std::copy(i, s.end(), s.begin());

return std::distance(s.begin(), i);
}
};

} // namespace Mysql

0 comments on commit 064ebdd

Please sign in to comment.