Re: Add RSA-OAEP encryption/decryption to Nettle

[Niels Möller]

Niels Möller  writes:

>> This is similar to this issue:
>> https://gitlab.com/gnutls/gnutls/-/issues/1306
>> where we passed NULL to sha*_update in the GnuTLS code, though it turned
>> to be a non-issue.
>
> I don't remember seeing that issue. I think it should be allowed to call
> sha*_update with 0, NULL (when size is null, there's no reason to ever
> attempt to dereference that pointer). I'll see if I can fix that.

Below patch seems to fix this issue, but not entirely sure that's the
way I want to do it. I think I'd rather not touch the MD_* macros
defined in macros.h, and do improved macros in md-internal.h instead.
Since, for historic reasons, the macros.h file is public.

To get this thoroughly fixed, one would need tests where every nettle
function, that accepts a potentially empty buffer, is called with 0,
NULL, and make sure ubsan is happy with that.

Regards,
/Niels

diff --git a/macros.h b/macros.h
index 990d32ee..e67a403f 100644
--- a/macros.h
+++ b/macros.h
@@ -180,6 +180,8 @@ do {\
length and data. */
 #define MD_UPDATE(ctx, length, data, f, incr)  \
   do { \
+if (length == 0)   \
+  goto __md_done;  \
 if ((ctx)->index)  \
   {
\
/* Try to fill partial block */ \
diff --git a/sha256.c b/sha256.c
index 0c9c21a0..907271bc 100644
--- a/sha256.c
+++ b/sha256.c
@@ -105,6 +105,9 @@ sha256_update(struct sha256_ctx *ctx,
  size_t length, const uint8_t *data)
 {
   size_t blocks;
+  if (length == 0)
+return;
+
   if (ctx->index > 0)
 {
   /* Try to fill partial block */


-- 
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se
To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se

Re: Add RSA-OAEP encryption/decryption to Nettle

[Niels Möller]

Daiki Ueno  writes:

> Niels Möller  writes:

>> One failure is the new side-channel test failing with mini-gmp. Which is
>> expected, the test should just be skipped in mini-gmp builds (similar to
>> several other sc tests).
>
> Yes, I'm attaching the patch for this.

I've committed and pushed that part of patch.

>> The other is a complaint from ubsan. I guess it's related to the label
>> == NULL case. I don't know what's the proper place for a fix, maybe it's
>> not in the new code. I think the Nettle APIs should generally allow size
>> == 0, ptr == NULL more or less everywhere, even where libc functions we
>> use formally require ptr != NULL.
>
> This is similar to this issue:
> https://gitlab.com/gnutls/gnutls/-/issues/1306
> where we passed NULL to sha*_update in the GnuTLS code, though it turned
> to be a non-issue.

I don't remember seeing that issue. I think it should be allowed to call
sha*_update with 0, NULL (when size is null, there's no reason to ever
attempt to dereference that pointer). I'll see if I can fix that.

Regards,
/Niels

-- 
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.
___
nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se
To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se