[patch 1/2] use chacha20 from openssl (1.1.0+) when possible

Damien Miller djm at mindrot.org
Thu Jan 23 13:37:26 AEDT 2020


On Thu, 23 Jan 2020, Damien Miller wrote:

> On Thu, 16 Jan 2020, Jakub Jelen wrote:
> 
> > > Thanks for this - it seems to work okay with OpenSSL when patched to
> > > -current, but when I adapt it for OpenBSD/LibreSSL the encryption is
> > > broken and the connection fails right after KEX.
> 
> I've worked with the LibreSSL developers and they should land a fix
> for EVP_chacha20 soon. Hopefully this will happen soon enough to get
> this committed before the looming OpenSSH release.

Here's the working patches BTW. I've separated the libcrypto chacha/poly
implementation to its own file for clarity.

-d
-------------- next part --------------
From 741027d3663db1f93f1ccd06d4e8ca96446fdc6b Mon Sep 17 00:00:00 2001
From: Damien Miller <djm at mindrot.org>
Date: Wed, 22 Jan 2020 18:51:50 +1100
Subject: [PATCH 1/2] refactor chachapoly AEAD to use an opaque struct

---
 cipher-chachapoly.c | 23 ++++++++++++++++++-----
 cipher-chachapoly.h |  9 ++++-----
 cipher.c            | 16 +++++++++-------
 3 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/cipher-chachapoly.c b/cipher-chachapoly.c
index 0899c5ad5..a638c3416 100644
--- a/cipher-chachapoly.c
+++ b/cipher-chachapoly.c
@@ -28,15 +28,28 @@
 #include "ssherr.h"
 #include "cipher-chachapoly.h"
 
-int
-chachapoly_init(struct chachapoly_ctx *ctx,
-    const u_char *key, u_int keylen)
+struct chachapoly_ctx {
+	struct chacha_ctx main_ctx, header_ctx;
+};
+
+struct chachapoly_ctx *
+chachapoly_new(const u_char *key, u_int keylen)
 {
+	struct chachapoly_ctx *ctx;
+
 	if (keylen != (32 + 32)) /* 2 x 256 bit keys */
-		return SSH_ERR_INVALID_ARGUMENT;
+		return NULL;
+	if ((ctx = calloc(1, sizeof(*ctx))) == NULL)
+		return NULL;
 	chacha_keysetup(&ctx->main_ctx, key, 256);
 	chacha_keysetup(&ctx->header_ctx, key + 32, 256);
-	return 0;
+	return ctx;
+}
+
+void
+chachapoly_free(struct chachapoly_ctx *cpctx)
+{
+	freezero(cpctx, sizeof(*cpctx));
 }
 
 /*
diff --git a/cipher-chachapoly.h b/cipher-chachapoly.h
index b7072be7d..21636bee6 100644
--- a/cipher-chachapoly.h
+++ b/cipher-chachapoly.h
@@ -24,13 +24,12 @@
 
 #define CHACHA_KEYLEN	32 /* Only 256 bit keys used here */
 
-struct chachapoly_ctx {
-	struct chacha_ctx main_ctx, header_ctx;
-};
+struct chachapoly_ctx;
 
-int	chachapoly_init(struct chachapoly_ctx *cpctx,
-    const u_char *key, u_int keylen)
+struct chachapoly_ctx *chachapoly_new(const u_char *key, u_int keylen)
     __attribute__((__bounded__(__buffer__, 2, 3)));
+void chachapoly_free(struct chachapoly_ctx *cpctx);
+
 int	chachapoly_crypt(struct chachapoly_ctx *cpctx, u_int seqnr,
     u_char *dest, const u_char *src, u_int len, u_int aadlen, u_int authlen,
     int do_encrypt);
diff --git a/cipher.c b/cipher.c
index 25f98ba8e..daec5d955 100644
--- a/cipher.c
+++ b/cipher.c
@@ -59,7 +59,7 @@ struct sshcipher_ctx {
 	int	plaintext;
 	int	encrypt;
 	EVP_CIPHER_CTX *evp;
-	struct chachapoly_ctx cp_ctx; /* XXX union with evp? */
+	struct chachapoly_ctx *cp_ctx;
 	struct aesctr_ctx ac_ctx; /* XXX union with evp? */
 	const struct sshcipher *cipher;
 };
@@ -262,7 +262,8 @@ cipher_init(struct sshcipher_ctx **ccp, const struct sshcipher *cipher,
 
 	cc->cipher = cipher;
 	if ((cc->cipher->flags & CFLAG_CHACHAPOLY) != 0) {
-		ret = chachapoly_init(&cc->cp_ctx, key, keylen);
+		cc->cp_ctx = chachapoly_new(key, keylen);
+		ret = cc->cp_ctx != NULL ? 0 : SSH_ERR_INVALID_ARGUMENT;
 		goto out;
 	}
 	if ((cc->cipher->flags & CFLAG_NONE) != 0) {
@@ -339,7 +340,7 @@ cipher_crypt(struct sshcipher_ctx *cc, u_int seqnr, u_char *dest,
    const u_char *src, u_int len, u_int aadlen, u_int authlen)
 {
 	if ((cc->cipher->flags & CFLAG_CHACHAPOLY) != 0) {
-		return chachapoly_crypt(&cc->cp_ctx, seqnr, dest, src,
+		return chachapoly_crypt(cc->cp_ctx, seqnr, dest, src,
 		    len, aadlen, authlen, cc->encrypt);
 	}
 	if ((cc->cipher->flags & CFLAG_NONE) != 0) {
@@ -402,7 +403,7 @@ cipher_get_length(struct sshcipher_ctx *cc, u_int *plenp, u_int seqnr,
     const u_char *cp, u_int len)
 {
 	if ((cc->cipher->flags & CFLAG_CHACHAPOLY) != 0)
-		return chachapoly_get_length(&cc->cp_ctx, plenp, seqnr,
+		return chachapoly_get_length(cc->cp_ctx, plenp, seqnr,
 		    cp, len);
 	if (len < 4)
 		return SSH_ERR_MESSAGE_INCOMPLETE;
@@ -415,9 +416,10 @@ cipher_free(struct sshcipher_ctx *cc)
 {
 	if (cc == NULL)
 		return;
-	if ((cc->cipher->flags & CFLAG_CHACHAPOLY) != 0)
-		explicit_bzero(&cc->cp_ctx, sizeof(cc->cp_ctx));
-	else if ((cc->cipher->flags & CFLAG_AESCTR) != 0)
+	if ((cc->cipher->flags & CFLAG_CHACHAPOLY) != 0) {
+		chachapoly_free(cc->cp_ctx);
+		cc->cp_ctx = NULL;
+	} else if ((cc->cipher->flags & CFLAG_AESCTR) != 0)
 		explicit_bzero(&cc->ac_ctx, sizeof(cc->ac_ctx));
 #ifdef WITH_OPENSSL
 	EVP_CIPHER_CTX_free(cc->evp);
-- 
2.25.0.341.g760bfbb309-goog

-------------- next part --------------
From 2b48f39cc8753c996f0df01e480b56dbc1b7d224 Mon Sep 17 00:00:00 2001
From: Damien Miller <djm at mindrot.org>
Date: Wed, 22 Jan 2020 19:51:33 +1100
Subject: [PATCH 2/2] chacha20-poly1305 AEAD using libcrypto EVP_chacha20

Based on patch from Yuriy M. Kaminskiy
---
 Makefile.in                     |   2 +-
 cipher-chachapoly-libcrypto.c   | 163 ++++++++++++++++++++++++++++++++
 cipher-chachapoly.c             |   3 +
 openbsd-compat/openssl-compat.h |   7 ++
 4 files changed, 174 insertions(+), 1 deletion(-)
 create mode 100644 cipher-chachapoly-libcrypto.c

diff --git a/Makefile.in b/Makefile.in
index 963f99fb5..d6fcfe2c3 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -103,7 +103,7 @@ LIBSSH_OBJS=${LIBOPENSSH_OBJS} \
 	ssh-ed25519-sk.o ssh-rsa.o dh.o \
 	msg.o progressmeter.o dns.o entropy.o gss-genr.o umac.o umac128.o \
 	ssh-pkcs11.o smult_curve25519_ref.o \
-	poly1305.o chacha.o cipher-chachapoly.o \
+	poly1305.o chacha.o cipher-chachapoly.o cipher-chachapoly-libcrypto.o \
 	ssh-ed25519.o digest-openssl.o digest-libc.o \
 	hmac.o sc25519.o ge25519.o fe25519.o ed25519.o verify.o hash.o \
 	kex.o kexdh.o kexgex.o kexecdh.o kexc25519.o \
diff --git a/cipher-chachapoly-libcrypto.c b/cipher-chachapoly-libcrypto.c
new file mode 100644
index 000000000..ad2be45bf
--- /dev/null
+++ b/cipher-chachapoly-libcrypto.c
@@ -0,0 +1,163 @@
+/*
+ * Copyright (c) 2013 Damien Miller <djm at mindrot.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/* $OpenBSD: cipher-chachapoly.c,v 1.8 2016/08/03 05:41:57 djm Exp $ */
+
+#include "includes.h"
+
+#if defined(WITH_OPENSSL) && !defined(INCOMPAT_EVP_CHACHA)
+
+#include <sys/types.h>
+#include <stdarg.h> /* needed for log.h */
+#include <string.h>
+#include <stdio.h>  /* needed for misc.h */
+
+#include <openssl/evp.h>
+
+#include "log.h"
+#include "sshbuf.h"
+#include "ssherr.h"
+#include "cipher-chachapoly.h"
+
+struct chachapoly_ctx {
+	EVP_CIPHER_CTX *main_evp, *header_evp;
+};
+
+struct chachapoly_ctx *
+chachapoly_new(const u_char *key, u_int keylen)
+{
+	struct chachapoly_ctx *ctx;
+
+	if (keylen != (32 + 32)) /* 2 x 256 bit keys */
+		return NULL;
+	if ((ctx = calloc(1, sizeof(*ctx))) == NULL)
+		return NULL;
+	if ((ctx->main_evp = EVP_CIPHER_CTX_new()) == NULL ||
+	    (ctx->header_evp = EVP_CIPHER_CTX_new()) == NULL)
+		goto fail;
+	if (!EVP_CipherInit(ctx->main_evp, EVP_chacha20(), key, NULL, 1))
+		goto fail;
+	if (!EVP_CipherInit(ctx->header_evp, EVP_chacha20(), key + 32, NULL, 1))
+		goto fail;
+	if (EVP_CIPHER_CTX_iv_length(ctx->header_evp) != 16)
+		goto fail;
+	return ctx;
+ fail:
+	chachapoly_free(ctx);
+	return NULL;
+}
+
+void
+chachapoly_free(struct chachapoly_ctx *cpctx)
+{
+	if (cpctx == NULL)
+		return;
+	EVP_CIPHER_CTX_free(cpctx->main_evp);
+	EVP_CIPHER_CTX_free(cpctx->header_evp);
+	freezero(cpctx, sizeof(*cpctx));
+}
+
+/*
+ * chachapoly_crypt() operates as following:
+ * En/decrypt with header key 'aadlen' bytes from 'src', storing result
+ * to 'dest'. The ciphertext here is treated as additional authenticated
+ * data for MAC calculation.
+ * En/decrypt 'len' bytes at offset 'aadlen' from 'src' to 'dest'. Use
+ * POLY1305_TAGLEN bytes at offset 'len'+'aadlen' as the authentication
+ * tag. This tag is written on encryption and verified on decryption.
+ */
+int
+chachapoly_crypt(struct chachapoly_ctx *ctx, u_int seqnr, u_char *dest,
+    const u_char *src, u_int len, u_int aadlen, u_int authlen, int do_encrypt)
+{
+	u_char seqbuf[16]; /* layout: u64 counter || u64 seqno */
+	int r = SSH_ERR_INTERNAL_ERROR;
+	u_char expected_tag[POLY1305_TAGLEN], poly_key[POLY1305_KEYLEN];
+
+	/*
+	 * Run ChaCha20 once to generate the Poly1305 key. The IV is the
+	 * packet sequence number.
+	 */
+	memset(seqbuf, 0, sizeof(seqbuf));
+	POKE_U64(seqbuf + 8, seqnr);
+	memset(poly_key, 0, sizeof(poly_key));
+	if (!EVP_CipherInit(ctx->main_evp, NULL, NULL, seqbuf, 1) ||
+	    EVP_Cipher(ctx->main_evp, poly_key,
+	    poly_key, sizeof(poly_key)) < 0) {
+		r = SSH_ERR_LIBCRYPTO_ERROR;
+		goto out;
+	}
+
+	/* If decrypting, check tag before anything else */
+	if (!do_encrypt) {
+		const u_char *tag = src + aadlen + len;
+
+		poly1305_auth(expected_tag, src, aadlen + len, poly_key);
+		if (timingsafe_bcmp(expected_tag, tag, POLY1305_TAGLEN) != 0) {
+			r = SSH_ERR_MAC_INVALID;
+			goto out;
+		}
+	}
+
+	/* Crypt additional data */
+	if (aadlen) {
+		if (!EVP_CipherInit(ctx->header_evp, NULL, NULL, seqbuf, 1) ||
+		    EVP_Cipher(ctx->header_evp, dest, src, aadlen) < 0) {
+			r = SSH_ERR_LIBCRYPTO_ERROR;
+			goto out;
+		}
+	}
+
+	/* Set Chacha's block counter to 1 */
+	seqbuf[0] = 1;
+	if (!EVP_CipherInit(ctx->main_evp, NULL, NULL, seqbuf, 1) ||
+	    EVP_Cipher(ctx->main_evp, dest + aadlen, src + aadlen, len) < 0) {
+		r = SSH_ERR_LIBCRYPTO_ERROR;
+		goto out;
+	}
+
+	/* If encrypting, calculate and append tag */
+	if (do_encrypt) {
+		poly1305_auth(dest + aadlen + len, dest, aadlen + len,
+		    poly_key);
+	}
+	r = 0;
+ out:
+	explicit_bzero(expected_tag, sizeof(expected_tag));
+	explicit_bzero(seqbuf, sizeof(seqbuf));
+	explicit_bzero(poly_key, sizeof(poly_key));
+	return r;
+}
+
+/* Decrypt and extract the encrypted packet length */
+int
+chachapoly_get_length(struct chachapoly_ctx *ctx,
+    u_int *plenp, u_int seqnr, const u_char *cp, u_int len)
+{
+	u_char buf[4], seqbuf[16];
+
+	if (len < 4)
+		return SSH_ERR_MESSAGE_INCOMPLETE;
+	memset(seqbuf, 0, sizeof(seqbuf));
+	POKE_U64(seqbuf + 8, seqnr);
+	if (!EVP_CipherInit(ctx->header_evp, NULL, NULL, seqbuf, 0))
+		return SSH_ERR_LIBCRYPTO_ERROR;
+	if (EVP_Cipher(ctx->header_evp, buf, (u_char *)cp, sizeof(buf)) < 0)
+		return SSH_ERR_LIBCRYPTO_ERROR;
+	*plenp = PEEK_U32(buf);
+	return 0;
+}
+#endif /* defined(WITH_OPENSSL) && !defined(INCOMPAT_EVP_CHACHA) */
diff --git a/cipher-chachapoly.c b/cipher-chachapoly.c
index a638c3416..af5295524 100644
--- a/cipher-chachapoly.c
+++ b/cipher-chachapoly.c
@@ -18,6 +18,8 @@
 
 #include "includes.h"
 
+#if !defined(WITH_OPENSSL) || defined(INCOMPAT_EVP_CHACHA)
+
 #include <sys/types.h>
 #include <stdarg.h> /* needed for log.h */
 #include <string.h>
@@ -130,3 +132,4 @@ chachapoly_get_length(struct chachapoly_ctx *ctx,
 	*plenp = PEEK_U32(buf);
 	return 0;
 }
+#endif /* !defined(WITH_OPENSSL) || defined(INCOMPAT_EVP_CHACHA) */
diff --git a/openbsd-compat/openssl-compat.h b/openbsd-compat/openssl-compat.h
index abdcb8763..98a974763 100644
--- a/openbsd-compat/openssl-compat.h
+++ b/openbsd-compat/openssl-compat.h
@@ -55,6 +55,13 @@ void ssh_libcrypto_init(void);
 # define LIBCRYPTO_EVP_INL_TYPE size_t
 #endif
 
+/* LibreSSL < 3.1.0 has an incompatible EVP_chacha20() */
+#ifdef LIBRESSL_VERSION_NUMBER
+# if LIBRESSL_VERSION_NUMBER < 0x3010000fL
+#  define INCOMPAT_EVP_CHACHA
+# endif
+#endif
+
 #ifndef OPENSSL_RSA_MAX_MODULUS_BITS
 # define OPENSSL_RSA_MAX_MODULUS_BITS	16384
 #endif
-- 
2.25.0.341.g760bfbb309-goog



More information about the openssh-unix-dev mailing list