Niels Möller <[email protected]> writes:
> And int32_divmod_uint14 looked unused.
My mistake, it's not unused. It is used (via int32_mod_uint14) by
F3_freeze and Fq_freeze, which appear to use signed representation, |x|
<= 1 and |x| <= (q-1)/2 respectively.
> For sorting, it may need a minor reorg to get rid of the unneeded
> variant.
See patch below. I think that makes the code simpler, but it might be
best to leave as is for now.
>> My take was that it would be nice to add sntrup761 to Nettle ASAP to
>> stabilize API and establish support for the algorithm -- we can optimize
>> or improve the implementation later on (there are many optimized
>> implementations around for different architectures out there).
>
> Makes sense, if it's clear what api and abi should look like (but, e.g.,
> use of union nettle_block16 does affect the abi, I think).
Having a closer look at the header file defining the api. I see no abi
subtleties here, only naming nits.
* sntrup761_keypair: should be sntrup761_generate_keypair, for consistency.
* sntrup761_enc, sntrup761_dec: Maybe abbreviate less, is
_encapsulate and _decapsulate too much? Or is _enc and _dec really
established in the area?
* SNTRUP761_PUBLICKEY_SIZE: I think it would be more consistent with
an underscore, _PUBLIC_KEY_SIZE.
* SNTRUP761_SECRETKEY_SIZE: I prefer SNTRUP761_PRIVATE_KEY_SIZE is
more consistent (maybe "private key" is not modern, but it's the
terminology used for all other asymmetric algorithms in nettle).
* SNTRUP761_CIPHERTEXT_SIZE: Probably right, even though I'm a bit
confused by the "ciphertext" terminology when there's no
corresponding plaintext.
* SNTRUP761_SIZE: This needs a more specific name, maybe _SECRET_SIZE,
_SHARED_SECRET_SIZE, _SESSION_KEY_SIZE, _OUTPUT_SIZE, ...?
In your docs, I noticed a copy-paste error in the docs for
SNTRUP761_PUBLICKEY_SIZE.
Things I think are desirable to do before merging an initial version:
Agree on naming. Rename the single-lower-case-letter macros in the .c
file to macro-like names. Add valgrind-based tests of side-channel silence.
(I'd need to read both spec and implementation closer to have more
opinions on the implementation).
Regards,
/Niels
diff --git a/sntrup761.c b/sntrup761.c
index dc1ca015..fb7fd761 100644
--- a/sntrup761.c
+++ b/sntrup761.c
@@ -55,24 +55,20 @@ crypto_hash_sha512 (unsigned char *out, const unsigned char
*in, int inlen)
sha512_digest (&ctx, SHA512_DIGEST_SIZE, out);
}
-/* from supercop-20201130/crypto_sort/int32/portable4/int32_minmax.inc */
-#define int32_MINMAX(a,b) \
+#define uint32_MINMAX(a,b) \
do { \
- int64_t ab = (int64_t)b ^ (int64_t)a; \
- int64_t c = (int64_t)b - (int64_t)a; \
- c ^= ab & (c ^ b); \
- c >>= 31; \
- c &= ab; \
- a ^= c; \
- b ^= c; \
+ uint64_t d = (uint64_t)b - (uint64_t)a; \
+ uint32_t masked_d = (d >> 32) & d; \
+ a += masked_d; \
+ b -= masked_d; \
} while(0)
-/* from supercop-20201130/crypto_sort/int32/portable4/sort.c */
+/* Based on supercop-20201130/crypto_sort/int32/portable4/sort.c, but
+ using uint32_t rather than int32_t. */
static void
-crypto_sort_int32 (void *array, long long n)
+crypto_sort_uint32 (uint32_t *x, long long n)
{
long long top, p, q, r, i, j;
- int32_t *x = array;
if (n < 2)
return;
@@ -86,11 +82,11 @@ crypto_sort_int32 (void *array, long long n)
while (i + 2 * p <= n)
{
for (j = i; j < i + p; ++j)
- int32_MINMAX (x[j], x[j + p]);
+ uint32_MINMAX (x[j], x[j + p]);
i += 2 * p;
}
for (j = i; j < n - p; ++j)
- int32_MINMAX (x[j], x[j + p]);
+ uint32_MINMAX (x[j], x[j + p]);
i = 0;
j = 0;
@@ -101,9 +97,9 @@ crypto_sort_int32 (void *array, long long n)
{
if (j == n - q)
goto done;
- int32_t a = x[j + p];
+ uint32_t a = x[j + p];
for (r = q; r > p; r >>= 1)
- int32_MINMAX (a, x[j + r]);
+ uint32_MINMAX (a, x[j + r]);
x[j + p] = a;
++j;
if (j == i + p)
@@ -116,9 +112,9 @@ crypto_sort_int32 (void *array, long long n)
{
for (j = i; j < i + p; ++j)
{
- int32_t a = x[j + p];
+ uint32_t a = x[j + p];
for (r = q; r > p; r >>= 1)
- int32_MINMAX (a, x[j + r]);
+ uint32_MINMAX (a, x[j + r]);
x[j + p] = a;
}
i += 2 * p;
@@ -127,9 +123,9 @@ crypto_sort_int32 (void *array, long long n)
j = i;
while (j < n - q)
{
- int32_t a = x[j + p];
+ uint32_t a = x[j + p];
for (r = q; r > p; r >>= 1)
- int32_MINMAX (a, x[j + r]);
+ uint32_MINMAX (a, x[j + r]);
x[j + p] = a;
++j;
}
@@ -139,23 +135,6 @@ crypto_sort_int32 (void *array, long long n)
}
}
-/* from supercop-20201130/crypto_sort/uint32/useint32/sort.c */
-
-/* can save time by vectorizing xor loops */
-/* can save time by integrating xor loops with int32_sort */
-
-static void
-crypto_sort_uint32 (void *array, long long n)
-{
- uint32_t *x = array;
- long long j;
- for (j = 0; j < n; ++j)
- x[j] ^= 0x80000000;
- crypto_sort_int32 (array, n);
- for (j = 0; j < n; ++j)
- x[j] ^= 0x80000000;
-}
-
/* from supercop-20201130/crypto_kem/sntrup761/ref/uint32.c */
/*
--
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
