diff mbox series

[Openvpn-devel] Enable usage of TLS groups not identified by a NID in OpenSSL 3

Message ID 20220329053709.19462-1-info@baentsch.ch
State Accepted
Headers
Series [Openvpn-devel] Enable usage of TLS groups not identified by a NID in OpenSSL 3 |

Commit Message

Michael Baentsch March 28, 2022, 6:37 p.m. UTC 
  From: Michael <57787676+baentsch@users.noreply.github.com>

OpenSSL3 prefers to specify groups (including EC groups) with names
instead of NID to allow also groups provided by providers.
This commit also removes the mapping of secp256r1 to prime256v1 for
the OpenSSL3 code path as OpenSSL 3.0 recognises secp256r1.1
---
 src/openvpn/ssl_openssl.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Arne Schwabe March 28, 2022, 11:22 p.m. UTC | #1 
Am 29.03.22 um 07:37 schrieb Michael Baentsch:
> From: Michael <57787676+baentsch@users.noreply.github.com>
> 
> OpenSSL3 prefers to specify groups (including EC groups) with names
> instead of NID to allow also groups provided by providers.
> This commit also removes the mapping of secp256r1 to prime256v1 for
> the OpenSSL3 code path as OpenSSL 3.0 recognises secp256r1.

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering March 28, 2022, 11:28 p.m. UTC | #2 
Hi,

On Tue, Mar 29, 2022 at 07:37:09AM +0200, Michael Baentsch wrote:
> From: Michael <57787676+baentsch@users.noreply.github.com>
> 
> OpenSSL3 prefers to specify groups (including EC groups) with names
> instead of NID to allow also groups provided by providers.
> This commit also removes the mapping of secp256r1 to prime256v1 for
> the OpenSSL3 code path as OpenSSL 3.0 recognises secp256r1.1
> ---

If Arne is fine with the actual change, I think the commit message
is good to go.  The "From:" line (which is what git recorded as author)
is a bit weird - if you're fine with that, I'll use your e-mail From:
instead, also for the "signed-off-by:" header.

(The exact meaning of the signed-off-by: is a bit fuzzy to me - our
developer docs say "patch is ready to be applied", but they also say
"if it is missing, the value of From: is used".  Some other voices
interpret it as "yes, I've read the project guidelines and license,
and agree to them"...)

gert
Michael Baentsch March 29, 2022, 3:14 a.m. UTC | #3 
Am 29.03.22 um 12:28 schrieb Gert Doering:
> Hi,
>
> On Tue, Mar 29, 2022 at 07:37:09AM +0200, Michael Baentsch wrote:
>> From: Michael <57787676+baentsch@users.noreply.github.com>
>>
>> OpenSSL3 prefers to specify groups (including EC groups) with names
>> instead of NID to allow also groups provided by providers.
>> This commit also removes the mapping of secp256r1 to prime256v1 for
>> the OpenSSL3 code path as OpenSSL 3.0 recognises secp256r1.1
>> ---
> If Arne is fine with the actual change, I think the commit message
> is good to go.  The "From:" line (which is what git recorded as author)
> is a bit weird - if you're fine with that, I'll use your e-mail From:
> instead, also for the "signed-off-by:" header.

What do you consider weird in there? My name is Michael and I prefer to 
not use a real email address in git commit messages:

<57787676+baentsch@users.noreply.github.com> is the ID github assigned 
to my "baentsch" github ID .

But then again, OpenVPN is not using github, so linking to that ID won't 
work anyway...

So, OK, go ahead and use "Michael Baentsch (info@baentsch.ch)" as From:

>
> (The exact meaning of the signed-off-by: is a bit fuzzy to me - our
> developer docs say "patch is ready to be applied", but they also say
> "if it is missing, the value of From: is used".  Some other voices
> interpret it as "yes, I've read the project guidelines and license,
> and agree to them"...)
>
> gert

Thanks in advance,

--Michael
Gert Doering March 29, 2022, 7:08 a.m. UTC | #4 
Your patch has been applied to the master branch.

Thanks :-)

commit 711a4044a095e83bb70f4620310d385d6f5c7282
Author: Michael Baentsch
Date:   Tue Mar 29 07:37:09 2022 +0200

     Enable usage of TLS groups not identified by a NID in OpenSSL 3

     Signed-off-by: Michael Baentsch <info@baentsch.ch>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20220329053709.19462-1-info@baentsch.ch>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24012.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index b8595174..af97dabc 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -572,13 +572,15 @@  void
 tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
 {
     ASSERT(ctx);
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
     struct gc_arena gc = gc_new();
     /* This method could be as easy as
      *  SSL_CTX_set1_groups_list(ctx->ctx, groups)
-     * but OpenSSL does not like the name secp256r1 for prime256v1
+     * but OpenSSL (< 3.0) does not like the name secp256r1 for prime256v1
      * This is one of the important curves.
      * To support the same name for OpenSSL and mbedTLS, we do
      * this dance.
+     * Also note that the code is wrong in the presence of OpenSSL3 providers.
      */
 
     int groups_count = get_num_elements(groups, ':');
@@ -617,6 +619,13 @@  tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
                    groups);
     }
     gc_free(&gc);
+#else
+    if (!SSL_CTX_set1_groups_list(ctx->ctx, groups))
+    {
+        crypto_msg(M_FATAL, "Failed to set allowed TLS group list: %s",
+                   groups);
+    }
+#endif
 }
 
 void