fix(dht): updates to message padding

[AaronFeickert]

Description

PR 4362 mitigates a metadata leak whereby encrypted messages are the same length as plaintext messages due to the use of a stream cipher. This work adds more complete length checks, such that padding can fail. It also more efficiently handles the edge case where no padding is needed.

Motivation and Context

To avoid directly leaking the length of plaintext messages after stream cipher encryption, PR 4362 pads such messages to a multiple of a fixed base length after first prepending the original message length using a fixed encoding.

However, the following cases do not appear to be handled by the padding and unpadding code:

• The plaintext message length exceeds the fixed encoding length
• The ciphertext message is not long enough for extraction of the fixed encoding length
• The ciphertext message is not a multiple of the base length

Further, in the case where the message length (after length prepending) is exactly a multiple of the base length, an entire base length of padding is unnecessarily applied.

This work addresses these issues. The padding process now checks that the plaintext message does not exceed the limit enforced by the length encoding; as a result, it can now return an error that propagates to the encryption function caller. The padding algorithm has been simplified and now handles the multiple-of-the-base-length edge case by correctly applying no padding. The unpadding process now checks that it can safely extract the message length, and checks that the ciphertext message is a multiple of the base length.

How Has This Been Tested?

No test has been added for the case where the message length exceeds the limit allowed by the encoding, as this would imply very high memory usage (or swapping) exceeding 4 GB. Existing tests pass. A new test exercises the other failure modes.

[AaronFeickert]

Manually tested the case where the plaintext message is too large for the length encoding.

[jorgeantonio21]

Very nice ! LGTM :)

PS: the failing tests seem to be unrelated with the current code refactoring, and probably are due to some previous PR merge to development.

[AaronFeickert]

Is it worth it (or even possible) to add the plaintext-is-too-long test in a way that won't horribly clog up the CI runners?

[sdbondi]

Thanks looks good. Some fairly minor comments.