Mailing List Archive

On Mon, 25 Jan 2021 at 08:00, Tim Rice <tim@multitalents.net> wrote:
> I've been building OpenSSH on UnixWare 7 using the native compiler
> for a long time. I pulled the latest source yesterday and got this error.
[...]
> UX:acomp: ERROR: "/opt/src/networking/openssh/openssh/sntrup761.c", line 298: integral constant expression expected
> In the source we see lines like this
> uint16 R2[(len+1)/2];
[...]
> UnixWare's USL compiler doesn't know what to do when "len" is not known
> at compile time.

OpenSSH has historically been ANSI C / C89, and we've largely avoided
relying on anything outside of that. (There are some exceptions such
as the XMSS post-quantum key exchange, but that's experimental and not
enabled by default).

Variable length arrays like this are not part of C89. They seem to be
supported as an extension by more or less every GCC even in c89 mode
(I tested 3.0.4), so this will only be an issue for non-gcc compilers.

> I know how to "fix" this but as this is 2021 and UnixWare 7D2M1
> has a GCC 7.3.0 available, I'm not sure it is worth the effort.
> It may be time to drop support for old crufty compilers.
>
> The question in my mind is, are UnixWare (and Openserver 6) the only
> platforms that will be tripped up on this?

My guess is that this won't be the only one. Fixing it has a
complicating factor for sntrup761.c in particular as it's generated by
a script from the original code from SuperCop, so any non-trivial
edits will be a significant maintenance headache.

> But what about IRIX, HP-UX, SGI, Tru64?
> I'm guessing they updated their compilers long ago but I do not know.

The (very old) HP ANSI C compiler on my (very old) C-class workstation
accepts it. Can't speak to any of the others.

I like the fact that now we have --without-openssl and --without-zlib,
you can build a functional OpenSSH (albeit with a limited set of
supported options) using only a C89 compiler and a half-decent make.
I would like to continue supporting that, as long as doing so does not
compromise supporting modern platforms.

To that end, I think we should disable sntrup761 if the compiler
doesn't support variable length arrays. We can do that by providing
some no-op KEX functions that just return an error (similar to what we
do in kexecdh.c) and a couple of ifdefs so I don't think it'll be too
much of a headache.

--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

On Mon, Jan 25, 2021 at 03:08:41PM +1100, Darren Tucker wrote:
[...]
> To that end, I think we should disable sntrup761 if the compiler
> doesn't support variable length arrays. We can do that by providing
> some no-op KEX functions that just return an error (similar to what we
> do in kexecdh.c) and a couple of ifdefs so I don't think it'll be too
> much of a headache.

Like this:

diff --git a/configure.ac b/configure.ac
index 35d1aca9..0cd1025f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -297,6 +297,16 @@ typedef void foo(const char *, ...) __attribute__((format(printf, 1, 2)));]],
[compiler does not accept __attribute__ on prototype args]) ]
)

+AC_MSG_CHECKING([if compiler supports variable length arrays])
+AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM([[#include <stdlib.h>]],
+ [[ int i; for (i=0; i<3; i++){int a[i]; a[i-1]=0;} exit(0); ]])],
+ [ AC_MSG_RESULT([yes])
+ AC_DEFINE(VARIABLE_LENGTH_ARRAYS, [1],
+ [compiler supports variable length arrays]) ],
+ [ AC_MSG_RESULT([no]) ]
+)
+
if test "x$no_attrib_nonnull" != "x1" ; then
AC_DEFINE([HAVE_ATTRIBUTE__NONNULL__], [1], [Have attribute nonnull])
fi
diff --git a/defines.h b/defines.h
index 79dcb507..d6a1d014 100644
--- a/defines.h
+++ b/defines.h
@@ -894,4 +894,11 @@ struct winsize {
# define USE_SYSTEM_GLOB
#endif

+/*
+ * sntrup761 uses variable length arrays, only enable if the compiler
+ * supports them.
+ */
+#ifdef VARIABLE_LENGTH_ARRAYS
+# define USE_SNTRUP761X25519 1
+#endif
#endif /* _DEFINES_H */
diff --git a/kex.c b/kex.c
index f08143a5..3269b2c3 100644
--- a/kex.c
+++ b/kex.c
@@ -110,8 +110,10 @@ static const struct kexalg kexalgs[] = {
#if defined(HAVE_EVP_SHA256) || !defined(WITH_OPENSSL)
{ KEX_CURVE25519_SHA256, KEX_C25519_SHA256, 0, SSH_DIGEST_SHA256 },
{ KEX_CURVE25519_SHA256_OLD, KEX_C25519_SHA256, 0, SSH_DIGEST_SHA256 },
+#ifdef USE_SNTRUP761X25519
{ KEX_SNTRUP761X25519_SHA512, KEX_KEM_SNTRUP761X25519_SHA512, 0,
SSH_DIGEST_SHA512 },
+#endif
#endif /* HAVE_EVP_SHA256 || !WITH_OPENSSL */
{ NULL, 0, -1, -1},
};
diff --git a/kexsntrup761x25519.c b/kexsntrup761x25519.c
index 3d5c6bdf..e3007fa2 100644
--- a/kexsntrup761x25519.c
+++ b/kexsntrup761x25519.c
@@ -25,6 +25,8 @@

#include "includes.h"

+#ifdef USE_SNTRUP761X25519
+
#include <sys/types.h>

#include <stdio.h>
@@ -217,3 +219,33 @@ kex_kem_sntrup761x25519_dec(struct kex *kex,
sshbuf_free(buf);
return r;
}
+
+#else
+
+#include "ssherr.h"
+
+struct kex;
+struct sshbuf;
+struct sshkey;
+
+int
+kex_kem_sntrup761x25519_keypair(struct kex *kex)
+{
+ return SSH_ERR_SIGN_ALG_UNSUPPORTED;
+}
+
+int
+kex_kem_sntrup761x25519_enc(struct kex *kex,
+ const struct sshbuf *client_blob, struct sshbuf **server_blobp,
+ struct sshbuf **shared_secretp)
+{
+ return SSH_ERR_SIGN_ALG_UNSUPPORTED;
+}
+
+int
+kex_kem_sntrup761x25519_dec(struct kex *kex,
+ const struct sshbuf *server_blob, struct sshbuf **shared_secretp)
+{
+ return SSH_ERR_SIGN_ALG_UNSUPPORTED;
+}
+#endif /* USE_SNTRUP761X25519 */
diff --git a/sntrup761.c b/sntrup761.c
index 01f1bc34..c63e600f 100644
--- a/sntrup761.c
+++ b/sntrup761.c
@@ -10,6 +10,8 @@

#include "includes.h"

+#ifdef USE_SNTRUP761X25519
+
#include <string.h>
#include "crypto_api.h"

@@ -1268,4 +1270,4 @@ int crypto_kem_sntrup761_dec(unsigned char *k,const unsigned char *c,const unsig
Decap(k,c,sk);
return 0;
}
-
+#endif /* USE_SNTRUP761X25519 */

--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

On Mon, 25 Jan 2021, Darren Tucker wrote:

> On Mon, Jan 25, 2021 at 03:08:41PM +1100, Darren Tucker wrote:
> [...]
> > To that end, I think we should disable sntrup761 if the compiler
> > doesn't support variable length arrays. We can do that by providing
> > some no-op KEX functions that just return an error (similar to what we
> > do in kexecdh.c) and a couple of ifdefs so I don't think it'll be too
> > much of a headache.
>
> Like this:
[snip]

Works as expected.
Thanks.

--
Tim Rice Multitalents
tim@multitalents.net


_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev