Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. OpenSSH>
  3. Dev
[PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0
mkl at pengutronix
Dec 3, 2020, 6:49 AM
Post #1 of 8 (948 views)
Permalink
From: Thomas Dwyer III <tomiii@tomiii.com>

During OpenSSL 3.0 development since OpenSSL commits:

| 718b133a5328 Implement AES CBC ciphers in the default provider
| 819a7ae9fc77 Implement AES CTR ciphers in the default provider

the dhgex tests (make t-exec LTESTS="dhgex") are failing.

The issue is that openssh needs the "current" IV state (which the
now-deprecated EVP_CIPHER_CTX_iv() used to return), but it's calling the wrong
openssl function to obtain it. See openssl PR #12233 for additional discussion.

The latest changes in OpenSSL 3.0 in combination with this patch fixes the
non-GCM ciphers. All but the chacha20-poly1305 test are not working again:

| dhgex bits 3072 diffie-hellman-group-exchange-sha1 3des-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 3des-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha1 aes128-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 aes128-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha1 aes128-ctr
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 aes128-ctr
| dhgex bits 3072 diffie-hellman-group-exchange-sha1 aes128-gcm@openssh.com
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 aes128-gcm@openssh.com
| dhgex bits 7680 diffie-hellman-group-exchange-sha1 aes192-cbc
| dhgex bits 7680 diffie-hellman-group-exchange-sha256 aes192-cbc
| dhgex bits 7680 diffie-hellman-group-exchange-sha1 aes192-ctr
| dhgex bits 7680 diffie-hellman-group-exchange-sha256 aes192-ctr
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 aes256-cbc
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 aes256-cbc
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 aes256-ctr
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 aes256-ctr
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 aes256-gcm@openssh.com
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 aes256-gcm@openssh.com
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 rijndael-cbc@lysator.liu.se
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 rijndael-cbc@lysator.liu.se
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 chacha20-poly1305@openssh.com
| ssh failed ()
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 chacha20-poly1305@openssh.com
| ssh failed ()

Link: https://www.spinics.net/lists/openssh-unix-dev/msg06860.html
Link: https://github.com/openssl/openssl/pull/12233
---
cipher.c | 2 +-
configure.ac | 5 +++++
openbsd-compat/libressl-api-compat.c | 8 ++++++++
openbsd-compat/openssl-compat.h | 5 +++++
4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/cipher.c b/cipher.c
index 8195199b32a2..5fac23d938e1 100644
--- a/cipher.c
+++ b/cipher.c
@@ -498,7 +498,7 @@ cipher_get_keyiv(struct sshcipher_ctx *cc, u_char *iv, size_t len)
if (!EVP_CIPHER_CTX_ctrl(cc->evp, EVP_CTRL_GCM_IV_GEN,
len, iv))
return SSH_ERR_LIBCRYPTO_ERROR;
- } else if (!EVP_CIPHER_CTX_get_iv(cc->evp, iv, len))
+ } else if (!EVP_CIPHER_CTX_get_iv_state(cc->evp, iv, len))
return SSH_ERR_LIBCRYPTO_ERROR;
#endif
return 0;
diff --git a/configure.ac b/configure.ac
index 35d1aca9fc90..2b14c8de1ad3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2875,6 +2875,11 @@ if test "x$openssl" = "xyes" ; then
EVP_chacha20 \
])

+ # LibreSSL/OpenSSL 3.x API
+ AC_CHECK_FUNCS([ \
+ EVP_CIPHER_CTX_get_iv_state \
+ ])
+
if test "x$openssl_engine" = "xyes" ; then
AC_MSG_CHECKING([for OpenSSL ENGINE support])
AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
diff --git a/openbsd-compat/libressl-api-compat.c b/openbsd-compat/libressl-api-compat.c
index ae00ff593b7e..a04938fa8260 100644
--- a/openbsd-compat/libressl-api-compat.c
+++ b/openbsd-compat/libressl-api-compat.c
@@ -363,6 +363,14 @@ EVP_CIPHER_CTX_get_iv(const EVP_CIPHER_CTX *ctx, unsigned char *iv, size_t len)
}
#endif /* HAVE_EVP_CIPHER_CTX_GET_IV */

+#ifndef HAVE_EVP_CIPHER_CTX_GET_IV_STATE
+int
+EVP_CIPHER_CTX_get_iv_state(const EVP_CIPHER_CTX *ctx, unsigned char *iv, size_t len)
+{
+ return EVP_CIPHER_CTX_get_iv(ctx, iv, len);
+}
+#endif /* HAVE_EVP_CIPHER_CTX_GET_IV_STATE */
+
#ifndef HAVE_EVP_CIPHER_CTX_SET_IV
int
EVP_CIPHER_CTX_set_iv(EVP_CIPHER_CTX *ctx, const unsigned char *iv, size_t len)
diff --git a/openbsd-compat/openssl-compat.h b/openbsd-compat/openssl-compat.h
index 388ae8aa0077..c7ff5f0a1f0f 100644
--- a/openbsd-compat/openssl-compat.h
+++ b/openbsd-compat/openssl-compat.h
@@ -117,6 +117,11 @@ int EVP_CIPHER_CTX_get_iv(const EVP_CIPHER_CTX *ctx,
unsigned char *iv, size_t len);
#endif /* HAVE_EVP_CIPHER_CTX_GET_IV */

+#ifndef HAVE_EVP_CIPHER_CTX_GET_IV_STATE
+int EVP_CIPHER_CTX_get_iv_state(const EVP_CIPHER_CTX *ctx,
+ unsigned char *iv, size_t len);
+#endif /* HAVE_EVP_CIPHER_CTX_GET_IV_STATE */
+
#ifndef HAVE_EVP_CIPHER_CTX_SET_IV
int EVP_CIPHER_CTX_set_iv(EVP_CIPHER_CTX *ctx,
const unsigned char *iv, size_t len);
--
2.20.1

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0 [ In reply to ]
dtucker at dtucker
Dec 3, 2020, 6:36 PM
Post #2 of 8 (948 views)
Permalink
On Fri, 4 Dec 2020 at 01:59, Marc Kleine-Budde <mkl@pengutronix.de> wrote:
[...]

Thanks for the investigation.

> The issue is that openssh needs the "current" IV state (which the
> now-deprecated EVP_CIPHER_CTX_iv() used to return), but it's calling the
wrong
> openssl function to obtain it.

It's not that simple.

In 2018, LibreSSL added EVP_CIPHER_CTX_get_iv (
https://github.com/libressl-portable/openbsd/commit/db321d7792) which
returns the current IV, and OpenSSH has been using it ever since.

In 2020, OpenSSL added a function of the same name (
https://github.com/openssl/openssl/commit/79f4417ed94) which behaves
differently. Maybe OpenSSL could change it before 3.0 instead of shipping
an incompatible API? EVP_CIPHER_CTX_get_original_iv would be consistent
with the function they deprecated. ie

EVP_CIPHER_CTX_get_iv -> EVP_CIPHER_CTX_get_original_iv
EVP_CIPHER_CTX_get_iv_state -> EVP_CIPHER_CTX_get_iv

--
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
Re: [PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0 [ In reply to ]
mkl at pengutronix
Dec 7, 2020, 8:10 AM
Post #3 of 8 (947 views)
Permalink
On 12/4/20 3:36 AM, Darren Tucker wrote:
> Thanks for the investigation.
>
>> The issue is that openssh needs the "current" IV state (which the
>> now-deprecated EVP_CIPHER_CTX_iv() used to return), but it's calling the
>> wrong openssl function to obtain it.
>
> It's not that simple.
>
> In 2018, LibreSSL added EVP_CIPHER_CTX_get_iv (
> https://github.com/libressl-portable/openbsd/commit/db321d7792) which
> returns the current IV, and OpenSSH has been using it ever since.
>
> In 2020, OpenSSL added a function of the same name (
> https://github.com/openssl/openssl/commit/79f4417ed94) which behaves
> differently. Maybe OpenSSL could change it before 3.0 instead of shipping
> an incompatible API? EVP_CIPHER_CTX_get_original_iv would be consistent
> with the function they deprecated. ie
>
> EVP_CIPHER_CTX_get_iv -> EVP_CIPHER_CTX_get_original_iv
> EVP_CIPHER_CTX_get_iv_state -> EVP_CIPHER_CTX_get_iv

I tried to wrap up the problem and created an openssl issue:

https://github.com/openssl/openssl/issues/13631

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

Attached Files:

Re: [PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0 [ In reply to ]
mkl at pengutronix
Dec 8, 2020, 8:41 AM
Post #4 of 8 (947 views)
Permalink
On 12/7/20 5:10 PM, Marc Kleine-Budde wrote:
> On 12/4/20 3:36 AM, Darren Tucker wrote:
>> Thanks for the investigation.
>>
>>> The issue is that openssh needs the "current" IV state (which the
>>> now-deprecated EVP_CIPHER_CTX_iv() used to return), but it's calling the
>>> wrong openssl function to obtain it.
>>
>> It's not that simple.
>>
>> In 2018, LibreSSL added EVP_CIPHER_CTX_get_iv (
>> https://github.com/libressl-portable/openbsd/commit/db321d7792) which
>> returns the current IV, and OpenSSH has been using it ever since.
>>
>> In 2020, OpenSSL added a function of the same name (
>> https://github.com/openssl/openssl/commit/79f4417ed94) which behaves
>> differently. Maybe OpenSSL could change it before 3.0 instead of shipping
>> an incompatible API? EVP_CIPHER_CTX_get_original_iv would be consistent
>> with the function they deprecated. ie
>>
>> EVP_CIPHER_CTX_get_iv -> EVP_CIPHER_CTX_get_original_iv
>> EVP_CIPHER_CTX_get_iv_state -> EVP_CIPHER_CTX_get_iv
>
> I tried to wrap up the problem and created an openssl issue:
>
> https://github.com/openssl/openssl/issues/13631

Which turned out to be a duplicate of

https://github.com/openssl/openssl/issues/13411

Are you interested in another take on a patch to fix OpenSSH with the current
OpenSSL-3.0?

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

Attached Files:

Re: [PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0 [ In reply to ]
djm at mindrot
Dec 9, 2020, 3:29 PM
Post #5 of 8 (947 views)
Permalink
On Tue, 8 Dec 2020, Marc Kleine-Budde wrote:

> On 12/7/20 5:10 PM, Marc Kleine-Budde wrote:
> > On 12/4/20 3:36 AM, Darren Tucker wrote:
> >> Thanks for the investigation.
> >>
> >>> The issue is that openssh needs the "current" IV state (which the
> >>> now-deprecated EVP_CIPHER_CTX_iv() used to return), but it's calling the
> >>> wrong openssl function to obtain it.
> >>
> >> It's not that simple.
> >>
> >> In 2018, LibreSSL added EVP_CIPHER_CTX_get_iv (
> >> https://github.com/libressl-portable/openbsd/commit/db321d7792) which
> >> returns the current IV, and OpenSSH has been using it ever since.
> >>
> >> In 2020, OpenSSL added a function of the same name (
> >> https://github.com/openssl/openssl/commit/79f4417ed94) which behaves
> >> differently. Maybe OpenSSL could change it before 3.0 instead of shipping
> >> an incompatible API? EVP_CIPHER_CTX_get_original_iv would be consistent
> >> with the function they deprecated. ie
> >>
> >> EVP_CIPHER_CTX_get_iv -> EVP_CIPHER_CTX_get_original_iv
> >> EVP_CIPHER_CTX_get_iv_state -> EVP_CIPHER_CTX_get_iv
> >
> > I tried to wrap up the problem and created an openssl issue:
> >
> > https://github.com/openssl/openssl/issues/13631
>
> Which turned out to be a duplicate of
>
> https://github.com/openssl/openssl/issues/13411
>
> Are you interested in another take on a patch to fix OpenSSH with the current
> OpenSSL-3.0?

It looks like OpenSSL are considering renaming the API. I think we can wait
for them to make up their minds before proceeding. If the go ahead with
the plan to rename EVP_CIPHER_CTX_get_iv_state to get_running_iv then the
fix will be a single #define in one of our compat headers AFAIK.

-d
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0 [ In reply to ]
mkl at pengutronix
Dec 9, 2020, 11:43 PM
Post #6 of 8 (947 views)
Permalink
On 12/10/20 12:29 AM, Damien Miller wrote:
>> Are you interested in another take on a patch to fix OpenSSH with the current
>> OpenSSL-3.0?
>
> It looks like OpenSSL are considering renaming the API. I think we can wait
> for them to make up their minds before proceeding. If the go ahead with
> the plan to rename EVP_CIPHER_CTX_get_iv_state to get_running_iv then the
> fix will be a single #define in one of our compat headers AFAIK.

Yes, but the name of EVP_CIPHER_CTX_get_iv_state(),
EVP_CIPHER_CTX_get_running_iv() or something else is not the only problem. The
other problem is the going-to-be-renamed OpenSSL function EVP_CIPHER_CTX_get_iv().

See my patch on trying to work around this with the current situation:

https://lists.mindrot.org/pipermail/openssh-unix-dev/2020-December/039008.html

Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

Attached Files:

Re: [PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0 [ In reply to ]
t.glaser at tarent
Dec 10, 2020, 12:15 AM
Post #7 of 8 (947 views)
Permalink
On Thu, 10 Dec 2020, Marc Kleine-Budde wrote:

> See my patch on trying to work around this with the current situation:

AIUI the “current situation” is an unreleased beta.

If the OpenSSL people are going to fix this before the
release, no need to even consider doing these acrobatics.

bye,
//mirabilos
--
15:41?<Lo-lan-do:#fusionforge> Somebody write a testsuite for helloworld :-)
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH] cipher: fix dhgex for non-GCM ciphers for OpenSSL 3.0 [ In reply to ]
mkl at pengutronix
Jan 18, 2021, 5:37 AM
Post #8 of 8 (897 views)
Permalink
On 12/10/20 9:15 AM, Thorsten Glaser wrote:
> On Thu, 10 Dec 2020, Marc Kleine-Budde wrote:
>
>> See my patch on trying to work around this with the current situation:
>
> AIUI the “current situation” is an unreleased beta.
>
> If the OpenSSL people are going to fix this before the
> release, no need to even consider doing these acrobatics.

OpenSSL just closed the issue. It should be fixed with:

https://github.com/openssl/openssl/commit/0d83b7b9036feea680ba45751df028ff5e86cd63

> Rename EVP_CIPHER_CTX_get_iv and EVP_CIPHER_CTX_get_iv_state for clarity
>
> To clarify the purpose of these two calls rename them to
> EVP_CIPHER_CTX_get_original_iv and EVP_CIPHER_CTX_get_updated_iv.
>
> Also rename the OSSL_CIPHER_PARAM_IV_STATE to OSSL_CIPHER_PARAM_UPDATED_IV
> to better align with the function name.

regards,
Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

Attached Files:

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal