| Message ID | X7FAtjmU7/+Ib+b/@Renats-MacBook-Air.local |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] Fix SHA-1 algorithm for data bigger than 512 megabytes. | expand |
On 11/15/20 3:52 PM, Renat Nurgaliyev wrote: > In modern systems, size_t is 64 bits. There is a 32 bit overflow check > in sha1_update(), which will not work correctly, because compiler will > do an automatic cast to 64 bits, since size_t type variable is in the > expression. We do want however to lose data, since this is the whole > idea of this overflow check. > > Because of this, computation of SHA-1 checksum will always be incorrect > for any data, that is bigger than 512 megabytes, which in bits is the > boundary of 32 bits integer. > > In practice it means that any OVSDB transaction, bigger or equal to 512 > megabytes, is considered corrupt and ovsdb-server will refuse to work > with the database file. This is especially critical for OVN southbound > database, since it tends to grow rapidly. > > Signed-off-by: Renat Nurgaliyev <impleman@gmail.com> > --- > v2: > replace size_t with uint32_t where necessary instead of explicit cast > --- Thanks! Applied to master and backported all the way down to 2.5. I also crafted a unit test for this issue. Patch available here: https://patchwork.ozlabs.org/project/openvswitch/patch/20201116190822.1199992-1-i.maximets@ovn.org/ Best regards, Ilya Maximets.
diff --git a/lib/sha1.c b/lib/sha1.c index 4f48ef210..87360d9cd 100644 --- a/lib/sha1.c +++ b/lib/sha1.c @@ -197,7 +197,7 @@ sha1_init(struct sha1_ctx *sha_info) * inputLen: The length of the input buffer. */ void -sha1_update(struct sha1_ctx *ctx, const void *buffer_, size_t count) +sha1_update(struct sha1_ctx *ctx, const void *buffer_, uint32_t count) { const uint8_t *buffer = buffer_; unsigned int i; @@ -274,7 +274,7 @@ sha1_final(struct sha1_ctx *ctx, uint8_t digest[SHA1_DIGEST_SIZE]) /* Computes the hash of 'n' bytes in 'data' into 'digest'. */ void -sha1_bytes(const void *data, size_t n, uint8_t digest[SHA1_DIGEST_SIZE]) +sha1_bytes(const void *data, uint32_t n, uint8_t digest[SHA1_DIGEST_SIZE]) { struct sha1_ctx ctx; diff --git a/lib/sha1.h b/lib/sha1.h index eda265dfc..a6e5a8cc0 100644 --- a/lib/sha1.h +++ b/lib/sha1.h @@ -45,9 +45,9 @@ struct sha1_ctx { }; void sha1_init(struct sha1_ctx *); -void sha1_update(struct sha1_ctx *, const void *, size_t); +void sha1_update(struct sha1_ctx *, const void *, uint32_t); void sha1_final(struct sha1_ctx *, uint8_t digest[SHA1_DIGEST_SIZE]); -void sha1_bytes(const void *, size_t, uint8_t digest[SHA1_DIGEST_SIZE]); +void sha1_bytes(const void *, uint32_t, uint8_t digest[SHA1_DIGEST_SIZE]); #define SHA1_FMT \ "%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x" \
In modern systems, size_t is 64 bits. There is a 32 bit overflow check in sha1_update(), which will not work correctly, because compiler will do an automatic cast to 64 bits, since size_t type variable is in the expression. We do want however to lose data, since this is the whole idea of this overflow check. Because of this, computation of SHA-1 checksum will always be incorrect for any data, that is bigger than 512 megabytes, which in bits is the boundary of 32 bits integer. In practice it means that any OVSDB transaction, bigger or equal to 512 megabytes, is considered corrupt and ovsdb-server will refuse to work with the database file. This is especially critical for OVN southbound database, since it tends to grow rapidly. Signed-off-by: Renat Nurgaliyev <impleman@gmail.com> --- v2: replace size_t with uint32_t where necessary instead of explicit cast --- lib/sha1.c | 4 ++-- lib/sha1.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)