Re: Relax blocking requirement of gcm_update?

2024-03-09 Thread Niels Möller
Justus Winter  writes:

> What happens if that restriction is violated?  As the function cannot
> signal an error, does it lead to silent corruption of the data stream?
> Or does it assert that restriction?

It triggers an assert. Likewise, if you call gcm_update after
gcm_encrypt that also triggers an assert. While I think (without
checking the code closely) an invalid mix of gcm_encrypt and gcm_decrypt
will just result in a garbage digest.

So if you want the bindings to report errors in some friendler way on
misuse, you'd need to keep track of the state of the context and check
that each call is appropriate for the current state.
 
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


Re: Relax blocking requirement of gcm_update?

2024-03-09 Thread Justus Winter
Hi Niels :)

Niels Möller  writes:

> While looking at extended tests of the aead update function (for the
> associated data), I stumbled on a restriction of gcm_update that is
> different from most (all?) other update functions in Nettle. According
> to the docs,
>
>  -- Function: void gcm_update (struct gcm_ctx *CTX, const struct gcm_key
>   *KEY, size_t LENGTH, const uint8_t *DATA)
>  Provides associated data to be authenticated.  If used, must be
>  called before ‘gcm_encrypt’ or ‘gcm_decrypt’.  All but the last
>  call for each message _must_ use a length that is a multiple of the
>  block size.

What happens if that restriction is violated?  As the function cannot
signal an error, does it lead to silent corruption of the data stream?
Or does it assert that restriction?

> Would it be worthwhile to drop the restriction of the last sentence, and
> allow all calls to gcm_update to use any size? This requirement may be
> particularly surprising when using nettle_aead; then gcm has different
> requirements for the update function than all other aead algorithms.

In the Rust bindings, we don't check for this, and we also didn't make
the function fallible (likely because at the time we bound GCM we were
not aware of the limitation), so we're not able to communicate any
failures to the user.  As such, dropping the requirement would be most
welcome for us, because then we wouldn't need to change our interface.

Best,
Justus


signature.asc
Description: PGP signature
___
nettle-bugs mailing list -- nettle-bugs@lists.lysator.liu.se
To unsubscribe send an email to nettle-bugs-le...@lists.lysator.liu.se


Relax blocking requirement of gcm_update?

2024-03-09 Thread Niels Möller
While looking at extended tests of the aead update function (for the
associated data), I stumbled on a restriction of gcm_update that is
different from most (all?) other update functions in Nettle. According
to the docs,

 -- Function: void gcm_update (struct gcm_ctx *CTX, const struct gcm_key
  *KEY, size_t LENGTH, const uint8_t *DATA)
 Provides associated data to be authenticated.  If used, must be
 called before ‘gcm_encrypt’ or ‘gcm_decrypt’.  All but the last
 call for each message _must_ use a length that is a multiple of the
 block size.

Would it be worthwhile to drop the restriction of the last sentence, and
allow all calls to gcm_update to use any size? This requirement may be
particularly surprising when using nettle_aead; then gcm has different
requirements for the update function than all other aead algorithms.

I think that might be doable without any ABI break, by the following
hack: reuse the ctr field of struct gcm_context as a block buffer, while
processing the associated data. The ctr field is clearly needed also for
encrypt/decrypt, but we could move initialization for that purpose from
gcm_set_iv to the first call to encrypt/decrypt.

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


Re: additional API for SHAKE streaming read

2024-03-09 Thread Daiki Ueno
Hello Niels,

Niels Möller  writes:

> Daiki Ueno  writes:
>
>> When I'm trying to implement ML-KEM (Kyber), I realized that the current
>> API for SHAKE (sha3_256_shake) is a bit too limited: while ML-KEM uses
>> SHAKE128 as a source of pseudorandom samples[1], the the current API
>> requires the total number of bytes are determined prior to the call, and
>> after the call the hash context is reset.
>
> I vaguely recall discussing that when shake256 was added, and we
> concluded it was good enough as a start, and could be extended later.
>
> I think it would be nice if one could support the streaming case with
> the existing struct sha3_256_ctx, and little extra wrapping. Question is
> what the interface should be. I see a few variants:
>
> 1.
>   void /* Essentially the same as _sha3_pad_shake */
>   sha3_256_shake_start (struct sha3_256_ctx *ctx);
>
>   void /* Unbuffered, length must be a multiple of SHA3_256_BLOCK_SIZE */
>   sha3_256_shake_output (struct sha3_256_ctx *ctx
>  size_t length, uint8_t *dst);
>
>   void /* Last call, length can be arbitrary, context reinitialized */
>   sha3_256_shake_end (struct sha3_256_ctx *ctx
>   size_t length, uint8_t *dst);
>
> Requiring all calls but the last to be full blocks is consistent with
> nettle's funtions for block ciphers. But since we anyway have a buffer
> available (to support arbitrary sizes for streaming the input), we could
> perhaps just as well reuse that buffer.
>
> 2.
>   void /* Essentially the same as _sha3_pad_shake */
>   sha3_256_shake_start (struct sha3_256_ctx *ctx);
>
>   void /* Arbitrary length, no need to signal end of data */
>   sha3_256_shake_output (struct sha3_256_ctx *ctx
>  size_t length, uint8_t *dst);
>
>   void /* Explicit init call needed to start a new input message */
>   sha3_256_init (struct sha3_256_ctx *ctx);
>
> In this case, sha3_256_shake_output would use ctx->index and ctx->buffer
> for partial blocks.
>
> With some hacking (say, using the unused high bit of ctx->index to
> signal that shake is in output mode), then we could have just
>
> 3.
>   void /* Arbitrary length, no need to signal start or end of output */
>   sha3_256_shake_output (struct sha3_256_ctx *ctx
>  size_t length, uint8_t *dst);
>
>   void /* Explicit init call needed to start a new input message */
>   sha3_256_init (struct sha3_256_ctx *ctx);
>
> As always, naming is also a crucial question. Is _shake_output a good
> name? Or _shake_read, or _shake_generate? From the terminology in the
> spec (https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.202.pdf), I think
> "_shake_output" is reasonable.
>
> When deciding on naming and conventions, we should strive to define
> somthing that can be reused for later hash functions with variable
> output size (called extendable-output functions, "XOF", in the spec).
>
> So what do you think makes most sense?

Thank you.  The option (3) sounds like a great idea as it only need one
more function to be added for streaming.  I tried to implement it as the
attached patch.

Regards,
-- 
Daiki Ueno
>From fa02672ce0ee7956a444381c88693c9b03c2bd15 Mon Sep 17 00:00:00 2001
From: Daiki Ueno 
Date: Sun, 10 Mar 2024 09:43:04 +0900
Subject: [PATCH] sha3: Extend SHAKE256 API with incremental output

This adds an alternative function sha3_256_shake_output in the
SHAKE256 support, which enables to read output multiple times in an
incremental manner.

Signed-off-by: Daiki Ueno 
---
 sha3.h|  8 ++
 shake256.c| 52 +++
 testsuite/shake256-test.c | 58 +++
 3 files changed, 118 insertions(+)

diff --git a/sha3.h b/sha3.h
index 9220829d..4b7e186c 100644
--- a/sha3.h
+++ b/sha3.h
@@ -49,6 +49,7 @@ extern "C" {
 #define sha3_256_update nettle_sha3_256_update
 #define sha3_256_digest nettle_sha3_256_digest
 #define sha3_256_shake nettle_sha3_256_shake
+#define sha3_256_shake_output nettle_sha3_256_shake_output
 #define sha3_384_init nettle_sha3_384_init
 #define sha3_384_update nettle_sha3_384_update
 #define sha3_384_digest nettle_sha3_384_digest
@@ -143,6 +144,13 @@ sha3_256_shake(struct sha3_256_ctx *ctx,
 	   size_t length,
 	   uint8_t *digest);
 
+/* Unlike sha3_256_shake, this function can be called multiple times
+   to retrieve output from shake256 in an incremental manner */
+void
+sha3_256_shake_output(struct sha3_256_ctx *ctx,
+		  size_t length,
+		  uint8_t *digest);
+
 struct sha3_384_ctx
 {
   struct sha3_state state;
diff --git a/shake256.c b/shake256.c
index f5c77a43..1356218b 100644
--- a/shake256.c
+++ b/shake256.c
@@ -36,6 +36,7 @@
 # include "config.h"
 #endif
 
+#include 
 #include 
 #include 
 
@@ -44,6 +45,8 @@
 
 #include "nettle-write.h"
 
+#define MIN(x,y) ((x)<(y)?(x):(y))
+
 void
 sha3_256_shake (struct sha3_256_ctx *ctx,
 		size_t length,
@@ -61,3 +64,52 @@ sha3_256_shake (struct 

Re: Add RSA-OAEP encryption/decryption to Nettle

2024-03-09 Thread Niels Möller
Niels Möller  writes:

> Below patch seems to fix this issue, but not entirely sure that's the
> way I want to do it.

I've pushed a fix, along the same lines, see
https://git.lysator.liu.se/nettle/nettle/-/commit/99e62003c3916fdef04a2d3327281f8f498b609e

I believe that should fix all hash update functions (and with proper test
coverage). 

There are probably a few more functions where 0, NULL should be allowed,
but currently result in ubsan issues: Corresponding aead update
functions, functions accepting optional nonces, empty messages for rsa
encryption functions, maybe some of the cipher modes.

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