Re: openssh: update ed25519 and squash into a single file
On Sat, Jan 14, 2023 at 04:29:04PM +1100, Damien Miller wrote:
>
>
> On Fri, 13 Jan 2023, Damien Miller wrote:
>
> > Hi,
> >
> > Forewarning: this is a big, noisy diff. Also on Github at
> > https://github.com/djmdjm/openssh-wip/pull/18
> >
> > This updates the ED25519 code to the latest version of SUPERCOP (20221122),
> > but the real motivation for this is to move the ED25519 code to the same
> > approach we use for the Streamlined NTRUPrime code: using a shell-script
> > to extract the bits we want from SUPERCOP and squish them all into a
> > single file.
> >
> > This removes a bunch of exported function names, a bit of unused
> > code and means that all the ED25519 code is in a single file rather
> > than eight.
> >
> > To review this, it's probably best to run the shellscript locally
> > (use sh ed25519.sh /path/to/directory/with/supercop) and inspect the
> > output. Apart from the original ed25519.c (assembled from the keypair.c,
> > sign.c and open.c files in SUPERCOP) there are no substantial changes.
>
> Here's a better way to look at the substantive changes:
>
> 1. Assemble the existing ed25519 code in the same order as how this
>patch arranges things:
>
> cat verify.c fe25519.h fe25519.c sc25519.h sc25519.c \
> ge25519.h ge25519.c ed25519.c | \
> sed -e '/#include "ge25519_base.data"/r ge25519_base.data' \
> -e '/#include.*/d' > ed25519.c.old
>
> 2. Apply the patch
>
> 3. Diff the original and new code (below)
>
> This isn't completely without noise, but it lets you see the substantive
> changes clearly.
>
> -d
works and looks a lot cleaner than before.
ok tobhe@
>
>
>
>
> --- /tmp/ed25519.cSat Jan 14 16:25:09 2023
> +++ ed25519.c Sat Jan 14 16:25:41 2023
> @@ -1,12 +1,30 @@
> -/* $OpenBSD: verify.c,v 1.3 2013/12/09 11:03:45 markus Exp $ */
> +/* $OpenBSD: $ */
>
> /*
> - * Public Domain, Author: Daniel J. Bernstein
> - * Copied from nacl-20110221/crypto_verify/32/ref/verify.c
> + * Public Domain, Authors:
> + * - Daniel J. Bernstein
> + * - Niels Duif
> + * - Tanja Lange
> + * - lead: Peter Schwabe
> + * - Bo-Yin Yang
> */
>
> +#include
>
> -int crypto_verify_32(const unsigned char *x,const unsigned char *y)
> +#include "crypto_api.h"
> +
> +#define int8 crypto_int8
> +#define uint8 crypto_uint8
> +#define int16 crypto_int16
> +#define uint16 crypto_uint16
> +#define int32 crypto_int32
> +#define uint32 crypto_uint32
> +#define int64 crypto_int64
> +#define uint64 crypto_uint64
> +
> +/* from supercop-20221122/crypto_verify/32/ref/verify.c */
> +
> +static int crypto_verify_32(const unsigned char *x,const unsigned char *y)
> {
>unsigned int differentbits = 0;
> #define F(i) differentbits |= x[i] ^ y[i];
> @@ -44,14 +62,7 @@
>F(31)
>return (1 & ((differentbits - 1) >> 8)) - 1;
> }
> -/* $OpenBSD: fe25519.h,v 1.3 2013/12/09 11:03:45 markus Exp $ */
> -
> -/*
> - * Public Domain, Authors: Daniel J. Bernstein, Niels Duif, Tanja Lange,
> - * Peter Schwabe, Bo-Yin Yang.
> - * Copied from supercop-20130419/crypto_sign/ed25519/ref/fe25519.h
> - */
> -
> +/* from supercop-20221122/crypto_sign/ed25519/ref/fe25519.h */
> #ifndef FE25519_H
> #define FE25519_H
>
> @@ -80,52 +91,45 @@
> }
> fe25519;
>
> -void fe25519_freeze(fe25519 *r);
> +static void fe25519_freeze(fe25519 *r);
>
> -void fe25519_unpack(fe25519 *r, const unsigned char x[32]);
> +static void fe25519_unpack(fe25519 *r, const unsigned char x[32]);
>
> -void fe25519_pack(unsigned char r[32], const fe25519 *x);
> +static void fe25519_pack(unsigned char r[32], const fe25519 *x);
>
> -int fe25519_iszero(const fe25519 *x);
> +static int fe25519_iszero(const fe25519 *x);
>
> -int fe25519_iseq_vartime(const fe25519 *x, const fe25519 *y);
> +static int fe25519_iseq_vartime(const fe25519 *x, const fe25519 *y);
>
> -void fe25519_cmov(fe25519 *r, const fe25519 *x, unsigned char b);
> +static void fe25519_cmov(fe25519 *r, const fe25519 *x, unsigned char b);
>
> -void fe25519_setone(fe25519 *r);
> +static void fe25519_setone(fe25519 *r);
>
> -void fe25519_setzero(fe25519 *r);
> +static void fe25519_setzero(fe25519 *r);
>
> -void fe25519_neg(fe25519 *r, const fe25519 *x);
> +static void fe25519_neg(fe25519 *r, const fe25519 *x);
>
> unsigned char fe25519_getparity(const fe25519 *x);
>
> -void fe25519_add(fe25519 *r, const fe25519 *x, const fe25519 *y);
> +static void fe25519_add(fe25519 *r, const fe25519 *x, const fe25519 *y);
>
> -void fe25519_sub(fe25519 *r, const fe25519 *x, const fe25519 *y);
> +static void fe25519_sub(fe25519 *r, const fe25519 *x, const fe25519 *y);
>
> -void fe25519_mul(fe25519 *r, const fe25519 *x, const fe25519 *y);
> +static void fe25519_mul(fe25519 *r, const fe25519 *x, const fe25519 *y);
>
> -void fe25519_square(fe25519 *r, const fe25519 *x);
> +static void fe25519_square(fe25519 *r, const fe25519 *x);
>
> -void fe25519_invert(fe25519 *r, const fe25519 *x);
> +static void fe25519_invert(fe25519 *r, const
Re: openssh: update ed25519 and squash into a single file
> This isn't completely without noise, but it lets you see the substantive > changes clearly. This looks good to me and works fine in my environment. Inlining the weird get_hram() makes things quite a bit clearer. I can't spot anything wrong in this diff. ok tb
Re: openssh: update ed25519 and squash into a single file
On Fri, 13 Jan 2023, Damien Miller wrote:
> Hi,
>
> Forewarning: this is a big, noisy diff. Also on Github at
> https://github.com/djmdjm/openssh-wip/pull/18
>
> This updates the ED25519 code to the latest version of SUPERCOP (20221122),
> but the real motivation for this is to move the ED25519 code to the same
> approach we use for the Streamlined NTRUPrime code: using a shell-script
> to extract the bits we want from SUPERCOP and squish them all into a
> single file.
>
> This removes a bunch of exported function names, a bit of unused
> code and means that all the ED25519 code is in a single file rather
> than eight.
>
> To review this, it's probably best to run the shellscript locally
> (use sh ed25519.sh /path/to/directory/with/supercop) and inspect the
> output. Apart from the original ed25519.c (assembled from the keypair.c,
> sign.c and open.c files in SUPERCOP) there are no substantial changes.
Here's a better way to look at the substantive changes:
1. Assemble the existing ed25519 code in the same order as how this
patch arranges things:
cat verify.c fe25519.h fe25519.c sc25519.h sc25519.c \
ge25519.h ge25519.c ed25519.c | \
sed -e '/#include "ge25519_base.data"/r ge25519_base.data' \
-e '/#include.*/d' > ed25519.c.old
2. Apply the patch
3. Diff the original and new code (below)
This isn't completely without noise, but it lets you see the substantive
changes clearly.
-d
--- /tmp/ed25519.c Sat Jan 14 16:25:09 2023
+++ ed25519.c Sat Jan 14 16:25:41 2023
@@ -1,12 +1,30 @@
-/* $OpenBSD: verify.c,v 1.3 2013/12/09 11:03:45 markus Exp $ */
+/* $OpenBSD: $ */
/*
- * Public Domain, Author: Daniel J. Bernstein
- * Copied from nacl-20110221/crypto_verify/32/ref/verify.c
+ * Public Domain, Authors:
+ * - Daniel J. Bernstein
+ * - Niels Duif
+ * - Tanja Lange
+ * - lead: Peter Schwabe
+ * - Bo-Yin Yang
*/
+#include
-int crypto_verify_32(const unsigned char *x,const unsigned char *y)
+#include "crypto_api.h"
+
+#define int8 crypto_int8
+#define uint8 crypto_uint8
+#define int16 crypto_int16
+#define uint16 crypto_uint16
+#define int32 crypto_int32
+#define uint32 crypto_uint32
+#define int64 crypto_int64
+#define uint64 crypto_uint64
+
+/* from supercop-20221122/crypto_verify/32/ref/verify.c */
+
+static int crypto_verify_32(const unsigned char *x,const unsigned char *y)
{
unsigned int differentbits = 0;
#define F(i) differentbits |= x[i] ^ y[i];
@@ -44,14 +62,7 @@
F(31)
return (1 & ((differentbits - 1) >> 8)) - 1;
}
-/* $OpenBSD: fe25519.h,v 1.3 2013/12/09 11:03:45 markus Exp $ */
-
-/*
- * Public Domain, Authors: Daniel J. Bernstein, Niels Duif, Tanja Lange,
- * Peter Schwabe, Bo-Yin Yang.
- * Copied from supercop-20130419/crypto_sign/ed25519/ref/fe25519.h
- */
-
+/* from supercop-20221122/crypto_sign/ed25519/ref/fe25519.h */
#ifndef FE25519_H
#define FE25519_H
@@ -80,52 +91,45 @@
}
fe25519;
-void fe25519_freeze(fe25519 *r);
+static void fe25519_freeze(fe25519 *r);
-void fe25519_unpack(fe25519 *r, const unsigned char x[32]);
+static void fe25519_unpack(fe25519 *r, const unsigned char x[32]);
-void fe25519_pack(unsigned char r[32], const fe25519 *x);
+static void fe25519_pack(unsigned char r[32], const fe25519 *x);
-int fe25519_iszero(const fe25519 *x);
+static int fe25519_iszero(const fe25519 *x);
-int fe25519_iseq_vartime(const fe25519 *x, const fe25519 *y);
+static int fe25519_iseq_vartime(const fe25519 *x, const fe25519 *y);
-void fe25519_cmov(fe25519 *r, const fe25519 *x, unsigned char b);
+static void fe25519_cmov(fe25519 *r, const fe25519 *x, unsigned char b);
-void fe25519_setone(fe25519 *r);
+static void fe25519_setone(fe25519 *r);
-void fe25519_setzero(fe25519 *r);
+static void fe25519_setzero(fe25519 *r);
-void fe25519_neg(fe25519 *r, const fe25519 *x);
+static void fe25519_neg(fe25519 *r, const fe25519 *x);
unsigned char fe25519_getparity(const fe25519 *x);
-void fe25519_add(fe25519 *r, const fe25519 *x, const fe25519 *y);
+static void fe25519_add(fe25519 *r, const fe25519 *x, const fe25519 *y);
-void fe25519_sub(fe25519 *r, const fe25519 *x, const fe25519 *y);
+static void fe25519_sub(fe25519 *r, const fe25519 *x, const fe25519 *y);
-void fe25519_mul(fe25519 *r, const fe25519 *x, const fe25519 *y);
+static void fe25519_mul(fe25519 *r, const fe25519 *x, const fe25519 *y);
-void fe25519_square(fe25519 *r, const fe25519 *x);
+static void fe25519_square(fe25519 *r, const fe25519 *x);
-void fe25519_invert(fe25519 *r, const fe25519 *x);
+static void fe25519_invert(fe25519 *r, const fe25519 *x);
-void fe25519_pow2523(fe25519 *r, const fe25519 *x);
+static void fe25519_pow2523(fe25519 *r, const fe25519 *x);
#endif
-/* $OpenBSD: fe25519.c,v 1.3 2013/12/09 11:03:45 markus Exp $ */
-
-/*
- * Public Domain, Authors: Daniel J. Bernstein, Niels Duif, Tanja Lange,
- * Peter Schwabe, Bo-Yin Yang.
- * Copied from supercop-20130419/crypto_sign/ed25519/ref/fe25519.c
- */
-
+/* from
