Re: [Openvpn-devel] [PATCH v3] tun: remove tun_finalize()
Hi,
On Mon, Jan 17, 2022 at 4:51 AM Lev Stipakov wrote:
> From: Lev Stipakov
>
> tun_finalize() is essentially subset of socket_finalize() apart from:
>
> - using WSAFoo() functions instead of Foo()
>
> - "from" address is not returned
>
> There is no clear official statement that one can use non-WSA
> API on handles, so let's be on a safe side and use both.
>
> Introduce sockethandle_t abstraction, which represents
> socket and handle. Add SocketHandle* routines which call
> proper API depends on underlying type in abstraction.
>
> Rename socket_finalize() to sockethandle_finalize(), take
> sockethandle_t and new routines into use and kick tun_finalize().
>
> Signed-off-by: Lev Stipakov
> ---
> v3: replace macros with inline functions
>
> v2: explicitly initialize .is_handle to false for readablity
>
>
> src/openvpn/forward.c | 3 +-
> src/openvpn/socket.c | 37 -
> src/openvpn/socket.h | 49 ++
> src/openvpn/tun.c | 94 +--
> src/openvpn/tun.h | 34 +---
> 5 files changed, 80 insertions(+), 137 deletions(-)
>
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index f82386a1..a905f993 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -1115,7 +1115,8 @@ read_incoming_tun(struct context *c)
> }
> else
> {
> -read_tun_buffered(c->c1.tuntap, >c2.buf);
> +sockethandle_t sh = { .is_handle = true, .h = c->c1.tuntap->hand
> };
> +sockethandle_finalize(sh, >c1.tuntap->reads, >c2.buf, NULL);
> }
> #else /* ifdef _WIN32 */
> ASSERT(buf_init(>c2.buf, FRAME_HEADROOM(>c2.frame)));
> diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
> index df736746..780c5cb3 100644
> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -3198,7 +3198,8 @@ link_socket_read_tcp(struct link_socket *sock,
> if (!sock->stream_buf.residual_fully_formed)
> {
> #ifdef _WIN32
> -len = socket_finalize(sock->sd, >reads, buf, NULL);
> +sockethandle_t sh = { .s = sock->sd };
> +len = sockethandle_finalize(sh, >reads, buf, NULL);
> #else
> struct buffer frag;
> stream_buf_get_next(>stream_buf, );
> @@ -3664,10 +3665,10 @@ socket_send_queue(struct link_socket *sock, struct
> buffer *buf, const struct lin
> }
>
> int
> -socket_finalize(SOCKET s,
> -struct overlapped_io *io,
> -struct buffer *buf,
> -struct link_socket_actual *from)
> +sockethandle_finalize(sockethandle_t sh,
> + struct overlapped_io *io,
> + struct buffer *buf,
> + struct link_socket_actual *from)
> {
> int ret = -1;
> BOOL status;
> @@ -3675,13 +3676,7 @@ socket_finalize(SOCKET s,
> switch (io->iostate)
> {
> case IOSTATE_QUEUED:
> -status = WSAGetOverlappedResult(
> -s,
> ->overlapped,
> ->size,
> -FALSE,
> ->flags
> -);
> +status = SocketHandleGetOverlappedResult(sh, io);
> if (status)
> {
> /* successful return for a queued operation */
> @@ -3693,18 +3688,18 @@ socket_finalize(SOCKET s,
> io->iostate = IOSTATE_INITIAL;
> ASSERT(ResetEvent(io->overlapped.hEvent));
>
> -dmsg(D_WIN32_IO, "WIN32 I/O: Socket Completion success
> [%d]", ret);
> +dmsg(D_WIN32_IO, "WIN32 I/O: Completion success [%d]",
> ret);
> }
> else
> {
> /* error during a queued operation */
> ret = -1;
> -if (WSAGetLastError() != WSA_IO_INCOMPLETE)
> +if (SocketHandleGetLastError(sh) != ERROR_IO_INCOMPLETE)
>
WSA_IO_INCOMPLETE is the same as ERROR_IO_INCOMPLETE, so using them
interchangeably looks ok.
{
> /* if no error (i.e. just not finished yet), then
> DON'T execute this code */
> io->iostate = IOSTATE_INITIAL;
> ASSERT(ResetEvent(io->overlapped.hEvent));
> -msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket
> Completion error");
> +msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion
> error");
> }
> }
> break;
> @@ -3715,9 +3710,9 @@ socket_finalize(SOCKET s,
> if (io->status)
> {
> /* error return for a non-queued operation */
> -WSASetLastError(io->status);
> +SocketHandleSetLastError(sh, io->status);
> ret = -1;
> -msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Socket Completion
> non-queued error");
> +msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion
> non-queued error");
> }
> else
>
[Openvpn-devel] [PATCH applied] Re: crypto: Fix OPENSSL_FIPS enabled builds
Acked-by: Gert Doering We'll add build tests on Fedora / CentOS, as soon as September brings the new buildbot infrastructure... so we get the "looks like FIPS but isn't" stuff tested as well. (cipher) changed as instructed on IRC. Your patch has been applied to the master branch. commit 544330fefedc87a74b4e17e105ad9151b8ad1dc9 Author: David Sommerseth Date: Wed Jan 19 19:21:26 2022 +0100 crypto: Fix OPENSSL_FIPS enabled builds Signed-off-by: David Sommerseth Acked-by: Gert Doering Message-Id: <20220119182126.56880-1-open...@sf.lists.topphemmelig.net> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23570.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add a unit test for external key provider
Combining Arne's ACK for 16+17 into this one. As far as I can see (not checked line-by-line) this is indeed the same code, just squashed into one commit, not touching configure.ac (thanks). And indeed, it now tests something :-) [==] Running 3 test(s). [ RUN ] xkey_provider_test_fetch [ OK ] xkey_provider_test_fetch [ RUN ] xkey_provider_test_mgmt_sign_cb [ OK ] xkey_provider_test_mgmt_sign_cb [ RUN ] xkey_provider_test_generic_sign_cb [ OK ] xkey_provider_test_generic_sign_cb [==] 3 test(s) run. [ PASSED ] 3 test(s). PASS: provider_testdriver and just an empty "PASS: provider_testdriver" when compiled without XKEY. Your patch has been applied to the master branch. commit 4fe50675946f4533a9a373f4332e417f1bbfeabe Author: Selva Nair Date: Thu Jan 20 11:16:16 2022 -0500 Add a unit test for external key provider Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20220120161616.13447-1-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23608.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: Enable signing via provider for management-external-key
Hi, On Thu, Jan 20, 2022 at 11:32:40AM -0500, Selva Nair wrote: > On Thu, Jan 20, 2022 at 10:18 AM Gert Doering wrote: > > > Compile and client tested on 1.1.1 and 3.0.1. > > > > Glancing at the code related to management_external_key() does > > not make me very happy... too many build time variants. > > > "Happiness" is never a word that comes to mind while reading OpenVPN code :) > ... Oh, some of the code paths are really nice these days :-) - but the #ifdef maze regarding SSL libraries / crypto features is getting truly annoying. > Even at our snail's pace, 2.7 may be out before we can break free of > OpenSSL 1, LibreSSL xyz etc. An option may be to require OpenSSL 3+ or > similar for external keys, or at least for management-external-key. > > That feature is really used by only a few platforms (only Android for > now?). That was my idea - since only Windows and Android use the "xkey" code paths today (as far as I understand), make 3.0.1 a hard requirement for Windows and Android, and disable --management-external-key for older SSL builds. Maybe this is a bit too drastic, but it would reduce code paths to be maintained and tested quite a bit. For Windows and Android, we bundle the SSL library to be used anyway, so we do not need to care what the OS might bring along. > Although it's a nifty option that could potentially be leveraged to > remove pkcs11-helper, CNG etc out of OpenVPN core. Whatever reduces #ifdef and library dependencies :-) gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: pkcs11: Interface the xkey provider with pkcs11-helper
client tested with 3.0.1 (no pkcs#11 though), and stared at the code a bit.
This change looks like it really wants an "#else" and move the #endif
to the end of the function... (though the compiler does not warn)
pkcs11_init_tls_session(pkcs11h_certificate_t certificate,
struct tls_root_ctx *const ssl_ctx)
{
+
+#ifdef HAVE_XKEY_PROVIDER
+return (xkey_load_from_pkcs11h(certificate, ssl_ctx) == 0); /* inverts the
return value */
+#endif
+
int ret = 1;
(more stuff)
This prototype looks a bit surprising
+static XKEY_EXTERNAL_SIGN_fn xkey_pkcs11h_sign;
given that the function is defined just below? Is this to ensure
XKEY_EXTERNAL_SIGN_fn matches the actual function definition?
Your patch has been applied to the master branch.
commit 6121001ed82914f336da081bb8aefaeb055450cb
Author: Selva Nair
Date: Tue Dec 14 11:59:24 2021 -0500
pkcs11: Interface the xkey provider with pkcs11-helper
Signed-off-by: Selva Nair
Acked-by: Arne Schwabe
Message-Id: <20211214165928.30676-15-selva.n...@gmail.com>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23442.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Enable signing using CNG through xkey provider
Compile-tested on Linux with OpenSSL 3.0.1, and on Ubuntu/MinGW (though with older OpenSSL) to ensure it doesn't break windows builds. Your patch has been applied to the master branch. commit 7ae282ca23e5a17cd9f2eb4801deed64ca64c704 Author: Selva Nair Date: Tue Dec 14 11:59:25 2021 -0500 Enable signing using CNG through xkey provider Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-16-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23444.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add a generic key loading helper function for xkey provider
I knew fixing that "avaiable" typo would come back and bite me :-)
Client tested with 3.0.1, and glanced at the code a bit.
Fixed a comment typo ("callng free_op").
Your patch has been applied to the master branch.
commit b64c9eb31824dd46c949d071751f8aebc008004c
Author: Selva Nair
Date: Tue Dec 14 11:59:23 2021 -0500
Add a generic key loading helper function for xkey provider
Signed-off-by: Selva Nair
Acked-by: Arne Schwabe
Message-Id: <20211214165928.30676-14-selva.n...@gmail.com>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23436.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Increase ERR_BUF_SIZE when management interface support is enabled
I seem to have seen a similar patch in Arne's series, and we didn't like it there much either :-) - so yeah, smarter fix, eventually. That said, this is "only" wasting another 2048 byte as the buffer is already at 8k if PKCS11 is enabled, and it's not static but gc_malloc(). Pretty weird to use *ERR*_BUF_SIZE in manage.c and plugin.c, though :-) (and maybe the plugin_vlog() code could be changed to be smarter about the way it builds the msg_fmt string based on plugin name + format string) Anyway, client tested with 1.1.1, just for good measure. Your patch has been applied to the master branch. commit eeb019acee57ef5b9485569ec4d3279a822c4eb0 Author: Selva Nair Date: Tue Dec 14 11:59:22 2021 -0500 Increase ERR_BUF_SIZE when management interface support is enabled Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-13-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23440.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH applied] Re: Enable signing via provider for management-external-key
Hi, On Thu, Jan 20, 2022 at 10:18 AM Gert Doering wrote: > Compile and client tested on 1.1.1 and 3.0.1. > > Glancing at the code related to management_external_key() does > not make me very happy... too many build time variants. "Happiness" is never a word that comes to mind while reading OpenVPN code :) ... Even at our snail's pace, 2.7 may be out before we can break free of OpenSSL 1, LibreSSL xyz etc. An option may be to require OpenSSL 3+ or similar for external keys, or at least for management-external-key. That feature is really used by only a few platforms (only Android for now?). Although it's a nifty option that could potentially be leveraged to remove pkcs11-helper, CNG etc out of OpenVPN core. Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Fix a potential memory leak in tls_ctx_use_management_external_key
From: Selva Nair
As pointed out by Gert Doering
Signed-off-by: Selva Nair
---
To be applied after 06/18 of xkey patchset
src/openvpn/ssl_openssl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index b48845eb..3f8c3091 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -1493,6 +1493,7 @@ tls_ctx_use_management_external_key(struct tls_root_ctx
*ctx)
if (!privkey
|| !SSL_CTX_use_PrivateKey(ctx->ctx, privkey))
{
+EVP_PKEY_free(privkey);
goto cleanup;
}
EVP_PKEY_free(privkey);
--
2.30.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Support sending DigestSign request to management client
Compile-tested on 3.0.1 and stared at the code for a bit. The "global" change is trivial enough, the xkey_helper changes look safe wrt memory overflows etc, though I lack the greater understanding on how all the wheels work together (so it's good that Arne tested and ACKed this). Your patch has been applied to the master branch. commit 0f6781fa3639d05ce1afb83a45c3bb12c6f97f1b Author: Selva Nair Date: Tue Dec 14 11:59:21 2021 -0500 Support sending DigestSign request to management client Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-12-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23435.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4 16+17/18] Add a unit test for external key provider
From: Selva Nair
Tests:
- Check SIGNATURE and KEYMGMT methods can be fetched
from the provider
- Load sample RSA and EC keys as management-external-key
and check that their sign callbacks are correctly exercised:
with and without digest support mocked in the client
capability flag.
-Test generic key load and signature
v4: 16/18 and 17/18 of v3 squashed into one patch
Signed-off-by: Selva Nair
---
tests/unit_tests/openvpn/Makefile.am | 16 +
tests/unit_tests/openvpn/test_provider.c | 403 +++
2 files changed, 419 insertions(+)
create mode 100644 tests/unit_tests/openvpn/test_provider.c
diff --git a/tests/unit_tests/openvpn/Makefile.am
b/tests/unit_tests/openvpn/Makefile.am
index 44b77cc5..6b5c94ab 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -11,6 +11,8 @@ if HAVE_LD_WRAP_SUPPORT
test_binaries += tls_crypt_testdriver
endif
+test_binaries += provider_testdriver
+
TESTS = $(test_binaries)
check_PROGRAMS = $(test_binaries)
@@ -95,6 +97,20 @@ networking_testdriver_SOURCES = test_networking.c mock_msg.c
\
$(openvpn_srcdir)/platform.c
endif
+provider_testdriver_CFLAGS = @TEST_CFLAGS@ \
+ -I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
+ $(OPTIONAL_CRYPTO_CFLAGS)
+provider_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
+ $(OPTIONAL_CRYPTO_LIBS)
+
+provider_testdriver_SOURCES = test_provider.c mock_msg.c \
+ $(openvpn_srcdir)/xkey_helper.c \
+ $(openvpn_srcdir)/xkey_provider.c \
+ $(openvpn_srcdir)/buffer.c \
+ $(openvpn_srcdir)/base64.c \
+ mock_get_random.c \
+ $(openvpn_srcdir)/platform.c
+
auth_token_testdriver_CFLAGS = @TEST_CFLAGS@ \
-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
$(OPTIONAL_CRYPTO_CFLAGS)
diff --git a/tests/unit_tests/openvpn/test_provider.c
b/tests/unit_tests/openvpn/test_provider.c
new file mode 100644
index ..0182b3b4
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -0,0 +1,403 @@
+/*
+ * OpenVPN -- An application to securely tunnel IP networks
+ * over a single UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ * Copyright (C) 2021 Selva Nair
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by the
+ * Free Software Foundation, either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#include "syshead.h"
+#include "manage.h"
+#include "xkey_common.h"
+
+#ifdef HAVE_XKEY_PROVIDER
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct management *management; /* global */
+static int mgmt_callback_called;
+
+#ifndef _countof
+#define _countof(x) sizeof((x))/sizeof(*(x))
+#endif
+
+static OSSL_PROVIDER *prov[2];
+
+/* public keys for testing -- RSA and EC */
+static const char * const pubkey1 = "-BEGIN PUBLIC KEY-\n"
+"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA7GWP6RLCGlvmVioIqYI6\n"
+"LUR4owA7sJ/nJxBAk+/xzD6gqgSigBsTqeb+gdZwkKjY1N4w2DUA0r5i8Eja/BWN\n"
+"xMZtC5nxK4MACtMqIwvlzfk130NhFXKtlZj2cyFBXqDdRyeg1ZrUQagcHVcgcReP\n"
+"9yiePgfO7NUOQk8edEeOR53SFCgnLBQQ9dGWtZN0hO/5BN6NSm/fd6vq0VjTRP5a\n"
+"BAH/BnqX9/3jV0jh8N9AE59mI1rjVVQ9VDnuAPkS8dLfdC661/CNxt0YWByTIgt1\n"
+"+qjW4LUvLbnU/rlPhuJ1SBZg+z/JtDBCKfs7syu5WYFqRvNFg7/91Rr/NwxvW/1h\n"
+"8QIDAQAB\n"
+"-END PUBLIC KEY-\n";
+
+static const char * const pubkey2 = "-BEGIN PUBLIC KEY-\n"
+"MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEO85iXW+HgnUkwlj1DohNVw0GsnGIh1gZ\n"
+"u95ff1JiUaJIkYNIkZA+hwIPFVH5aJcSCv3SPIeDS2VUAESNKHZJBQ==\n"
+"-END PUBLIC KEY-\n";
+
+static const char *pubkeys[] = {pubkey1, pubkey2};
+
+static const char *prov_name = "ovpn.xkey";
+
+static const char* test_msg = "Lorem ipsum dolor sit amet, consectetur "
+ "adipisici elit, sed eiusmod tempor incidunt "
+ "ut labore et dolore magna aliqua.";
+
+static const char* test_msg_b64 =
+"TG9yZW0gaXBzdW0gZG9sb3Igc2l0IGFtZXQsIGNvbnNlY3RldHVyIGFkaXBpc2ljaS"
+"BlbGl0LCBzZWQgZWl1c21vZCB0ZW1wb3IgaW5jaWR1bnQgdXQgbGFib3JlIGV0IGRv"
+
Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
Hi, On Thu, Jan 20, 2022 at 10:21:19AM -0500, Selva Nair wrote: > Yeah, a previous version had checking for OpenSSL version in configure.ac > and the AM_CONDITIONAL made sense only in that case. I can send a > new 16/18 or please do squash 16 with 17. If you could send a new "16+17 v4" that would make my life easier in not having to think about joint commit messages :-) thanks, gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Respect algorithm support announced by management client
Glanced a bit at the code and compile-tested on 3.0.1 - looks straightforward enough :-) (and yes to the comment about "such a check would be appropriate always", but I'm leaning more to "drop support for OpenSSL < 3.0.1 for external-key features" :-) ). Your patch has been applied to the master branch. commit 43de9f547d70cab2eb3e4478bf975e139ad966f7 Author: Selva Nair Date: Tue Dec 14 11:59:20 2021 -0500 Respect algorithm support announced by management client Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-11-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23441.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Allow management client to announce pss padding support
Client-tested with OpenSSL 1.1.1 and 3.0.1. This patch looks trivial enough, but the intricacies of all these flag bits and padding are well beyond me :-) - good that Arne tested all this for real. Fixed one remaining occurence of "hashlag" in the commit message. Your patch has been applied to the master branch. commit a04e3ac04740129bc1574fa3f1a67fdad942ff14 Author: Selva Nair Date: Tue Dec 14 11:59:19 2021 -0500 Allow management client to announce pss padding support Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-10-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23430.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add a function to encode digests with PKCS1 DigestInfo wrapper
Looked at the code, did client tests on 3.0.1, added a few spaces in code like "if(nid == NID_undef)" :-) As for the actual digest / encoding parts, no idea what that does, but the code looks safe wrt memcpy(), length of things, etc. Your patch has been applied to the master branch. commit cf704eef472515e3d6469bd5377065a1378eb5c2 Author: Selva Nair Date: Tue Dec 14 11:59:18 2021 -0500 Add a function to encode digests with PKCS1 DigestInfo wrapper Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-9-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23433.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
Hi On Thu, Jan 20, 2022 at 9:51 AM Gert Doering wrote: > Hi, > > On Tue, Dec 14, 2021 at 11:59:27AM -0500, selva.n...@gmail.com wrote: > > From: Selva Nair > > > > Signed-off-by: Selva Nair > > Is it OK if I squash 16+17 together? I dislike the "history churn" > of modifying configure.ac and Makefile.am in 16 just to remove > the AM_CONDITIONAL bits again in 17... > Yeah, a previous version had checking for OpenSSL version in configure.ac and the AM_CONDITIONAL made sense only in that case. I can send a new 16/18 or please do squash 16 with 17. Thanks, Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Enable signing via provider for management-external-key
Compile and client tested on 1.1.1 and 3.0.1. Glancing at the code related to management_external_key() does not make me very happy... too many build time variants. Maybe we should look into "external key is only supported with OpenSSL 3.0.1+ builds" for 2.7 and get rid of all the #ifdef'ed code there... Your patch has been applied to the master branch. commit 199df03bf57339661a853cb764ea41a0c8349b95 Author: Selva Nair Date: Tue Dec 14 11:59:17 2021 -0500 Enable signing via provider for management-external-key Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-8-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23428.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
Hi, On Tue, Dec 14, 2021 at 11:59:27AM -0500, selva.n...@gmail.com wrote: > From: Selva Nair > > Signed-off-by: Selva Nair Is it OK if I squash 16+17 together? I dislike the "history churn" of modifying configure.ac and Makefile.am in 16 just to remove the AM_CONDITIONAL bits again in 17... gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Add xkey_provider sources and includes to MSVC project
One should read all of the patch series before complaining about "this will break MSVC compilation" :-) - so here we go, MSVC fixed. (Applying out of order, so MSVC is repaired quickly). Your patch has been applied to the master branch. commit 57abdcfc3885b3c127bb3d07e9c8ccdbffcf2548 Author: Selva Nair Date: Tue Dec 14 11:59:28 2021 -0500 Add xkey_provider sources and includes to MSVC project Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-19-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23445.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: A helper function to import private key for management-external-key
Compile tested with 3.0.1 and glanced over the code. Not actually
tested (no management-external-key here) but I know that Arne is using
*this* in his Android app, so it got a good beating :-)
There might be a memory leak lurking here:
+#ifdef HAVE_XKEY_PROVIDER
+EVP_PKEY *privkey = xkey_load_management_key(tls_libctx, pkey);
+if (!privkey
+|| !SSL_CTX_use_PrivateKey(ctx->ctx, privkey))
+{
+goto cleanup;
+}
+EVP_PKEY_free(privkey);
+#else
if I read this right, the actual signing operation is happening
in SSL_CTX_use_PrivateKey() - so, if the key can be loaded fine
(privkey != NULL) but the actual signing fails, we "goto cleanup",
and never EVP_PKEY_free() it. But I might be misunderstanding this.
Fixed one typo in a comment ("avaialble") on the fly. Hope that
won't come back as a "context not matching" conflict later on.
Your patch has been applied to the master branch.
commit c279986bf4814aad72f9358d8509aa35f54ff662
Author: Selva Nair
Date: Tue Dec 14 11:59:16 2021 -0500
A helper function to import private key for management-external-key
Signed-off-by: Selva Nair
Acked-by: Arne Schwabe
Message-Id: <20211214165928.30676-7-selva.n...@gmail.com>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23443.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Initialize the xkey provider and use it in SSL context
This is the first truly "interesting" patch in the series, that brings
stuff to be tested :-)
Tested on OSSL 1.1.1 and mbedTLS builds ("so nothing breaks with the
old stuff"), and on 3.0.1 - but only with regular keys, so no
"key_is_external() = true" yet.
Your patch has been applied to the master branch.
commit 4b85c488ecadb3c076bc6cb605e00653cef67c94
Author: Selva Nair
Date: Tue Dec 14 11:59:15 2021 -0500
Initialize the xkey provider and use it in SSL context
Signed-off-by: Selva Nair
Acked-by: Arne Schwabe
Message-Id: <20211214165928.30676-6-selva.n...@gmail.com>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23432.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] unit-test: fix test_crypto when USE_COMP is not defined
Am 20.01.22 um 11:11 schrieb Antonio Quartulli: This unit-test did not consider the case when USE_COMP is not defined, thus generating a compiler error. Adapt the test to the case when no compression is available and while at it, decompose the expected MTU values by featureso that it is easier to understand. Cc: Arne Schwabe Signed-off-by: Antonio Quartulli --- tests/unit_tests/openvpn/test_crypto.c | 52 +++--- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c index 19ce174e..851696fe 100644 --- a/tests/unit_tests/openvpn/test_crypto.c +++ b/tests/unit_tests/openvpn/test_crypto.c @@ -255,7 +255,7 @@ test_occ_mtu_calculation(void **state) o.ciphername = "none"; o.authname = "none"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1400); +assert_int_equal(linkmtu, o.ce.tun_mtu); /* Static key OCC examples */ o.shared_secret_file = "not null"; @@ -264,44 +264,51 @@ test_occ_mtu_calculation(void **state) o.ciphername = "none"; o.authname = "none"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1408); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8); /* secret, cipher AES-128-CBC, auth none */ o.ciphername = "AES-128-CBC"; o.authname = "none"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1440); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 32); /* secret, cipher none, auth SHA256 */ o.ciphername = "none"; o.authname = "SHA256"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1440); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 32); -/* --secret, cipher BF-CBC, auth SHA1 */ +/* secret, cipher BF-CBC, auth SHA1 */ o.ciphername = "BF-CBC"; o.authname = "SHA1"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1444); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 36); if you want semantic things here that should be + 20 /* sha1 */ + 16 /*bf-cbc */ -/* --secret, cipher BF-CBC, auth SHA1, tcp-client */ +/* secret, cipher BF-CBC, auth SHA1, tcp-client */ o.ce.proto = PROTO_TCP_CLIENT; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1446); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 36 + 2); o.ce.proto = PROTO_UDP; -/* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1 */ +int comp_bytes = 0; +#if defined(USE_COMP) +/* secret, comp-lzo yes, cipher BF-CBC, auth SHA1 */ o.comp.alg = COMP_ALG_LZO; +comp_bytes = 1; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1445); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + comp_bytes + 36); +#endif why the extra variable and not just +1? -/* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1, fragment 1200 */ +/* secret, comp-lzo yes, cipher BF-CBC, auth SHA1, fragment 1200 */ o.ce.fragment = 1200; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1449); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + comp_bytes + 36 + 4); I would rather have this also under COMP_DEF than to add this different tests depending on compile flags. +comp_bytes = 0; +#if defined(USE_COMP) o.comp.alg = COMP_ALG_UNDEF; +#endif o.ce.fragment = 0; /* TLS mode */ @@ -309,32 +316,32 @@ test_occ_mtu_calculation(void **state) o.tls_client = true; o.pull = true; -/* tls client, cipher AES-128-CBC, auth SHA1, tls-auth*/ +/* tls client, cipher AES-128-CBC, auth SHA1, tls-auth */ o.authname = "SHA1"; o.ciphername = "AES-128-CBC"; o.tls_auth_file = "dummy"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1457); +assert_int_equal(linkmtu, o.ce.tun_mtu + 5 + 8 + 32 + 12); I think this one is wrong sha1 is 20 and aes-128-cbc is 16. If we really want to split the magic nubmers, we should split them in the right way, otherwise it becomes more confsuing than it is helpful. Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Implement import of custom external keys
Compile-tested only (and glanced over the code). Your patch has been applied to the master branch. commit ab3a8e5c28c433fd405f964d55bb754571191b9c Author: Selva Nair Date: Tue Dec 14 11:59:14 2021 -0500 Implement import of custom external keys Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-5-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23439.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Implement SIGNATURE operations in xkey provider
This fixes compilation again. Compile-tested, briefly glanced at the code. Your patch has been applied to the master branch. commit 25f9c47127190c487eb3b4b4a3f5553fb2d62b21 Author: Selva Nair Date: Tue Dec 14 11:59:13 2021 -0500 Implement SIGNATURE operations in xkey provider Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-4-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23437.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Implement KEYMGMT in the xkey provider
Only compile-tested on Linux / OpenSSL 3.0.1 (and briefly glanced over the code to see what happens). It breaks... xkey_provider.c:223:16: error: 'XKEY_KEYDATA' has no member named 'free' 223 | key->free = (XKEY_PRIVKEY_FREE_fn *) EVP_PKEY_free; |^~ xkey_provider.c:223:26: error: 'XKEY_PRIVKEY_FREE_fn' undeclared (first use in this function) 223 | key->free = (XKEY_PRIVKEY_FREE_fn *) EVP_PKEY_free; | ^~~~ *but* this is fixed in 03/18 - and as the whole patchset is basically a no-op before all is in (it will break compilation with 3.0.1, so might annoy someone doing bisect in the future, hitting just that commit by chance...) I'll still merge & push. Your patch has been applied to the master branch. commit 44509116da50b53a5588318fb27b2e4a3da4ab6c Author: Selva Nair Date: Tue Dec 14 11:59:12 2021 -0500 Implement KEYMGMT in the xkey provider Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-3-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23438.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: A built-in provider for using external key with OpenSSL 3.0
There is not much to test here yet - so what I did was "test that it does not break with ossl 1.x" (it doesn't), that it does not compile anything into xkey_provider.o when compiled with 3.0.0 (it doesn't) and that it *does* with 3.0.1 3.0.0$ size src/openvpn/xkey_provider.o textdata bss dec hex filename 48 0 0 48 30 src/openvpn/xkey_provider.o 3.0.1$ size src/openvpn/xkey_provider.o textdata bss dec hex filename 1534 176 01710 6ae src/openvpn/xkey_provider.o As far as I can see, the xkey stuff is not actually referenced anywhere, so "it is not compiled on MSVC builds" won't break anything *yet*, but it might as soon as actually linked-in. Your patch has been applied to the master branch. commit 5910eb6cd5308720d5e4f197b83763d572fce8a1 Author: Selva Nair Date: Tue Dec 14 11:59:11 2021 -0500 A built-in provider for using external key with OpenSSL 3.0 Signed-off-by: Selva Nair Acked-by: Arne Schwabe Message-Id: <20211214165928.30676-2-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23446.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 18/18] Add xkey_provider sources and includes to MSVC project
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair Acked-By: Arne Schwabe This could be merged/squashed into the commits that introduce those files. Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 17/18] xkey-provider: Add a test for generic key load and signature
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair Signed-off-by: Selva Nair --- configure.ac | 2 - tests/unit_tests/openvpn/Makefile.am | 4 - tests/unit_tests/openvpn/test_provider.c | 112 +-- 3 files changed, 105 insertions(+), 13 deletions(-) Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 16/18] Add a unit test for external key provider
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair Tests: - Check SIGNATURE and KEYMGMT methods can be fetched from the provider - Load sample RSA and EC keys as management-external-key and check that their sign callbacks are correctly exercised: with and without digest support mocked in the client capability flag. Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 15/18] Enable signing using CNG through xkey provider
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair - Add xkey_cng_sign() as sign_op for the provider and load the key using xkey_generic_load. - Enable/Disable old code when provider is available or not. - xkey_digest is made non-static for use in cryptoapi.c One function cng_padding_type() is moved down to reduce number of ifdef's. Acked-By: Arne Schwabe Note, I have not tested the CNG signing myself. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 14/18] pkcs11: Interface the xkey provider with pkcs11-helper
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair - Load the 'private key' handle through the provider and set it in SSL_CTX - Add a sign op function to interface provider with pkcs11-helper. Previously we used its "OpenSSL Session" which internally sets up callbacks in RSA and EC key methods. Not useful for the provider interface, so, we directly call the PKCS#11 sign operation as done with mbedTLS. - tls_libctx is made global for accessing from pkcs11_openssl.c Supports ECDSA and RSA_PKCS1_PADDING signatures. PSS support will be added when pkcs11-helper with our PR for specifying CK_MECHANISM variable in sign operations is released. (i.e., next release of pkcs11-helper). Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 12/18] Increase ERR_BUF_SIZE when management interface support is enabled
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair Sending largish messages to the management interface errors due to the limited size used for the "error" buffer in x_msg_va(). Although all intermediate steps allocate required space for the data to send, it gets truncated at the last step. This really requires a smarter fix. As a quick relief, we just increase the buffer size to 10240 when management support is compiled in. Should be enough for PK_SIGN with undigested message. Signed-off-by: Selva Nair Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 13/18] Add a generic key loading helper function for xkey provider
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair - Load keys by specifying the opaque privtae key handle, public key, sign-op and free-op required for loading keys from Windows store and pkcs11. - xkey_load_management_key is refactored to use the new function - Also make xkey_digest non-static Used in following commits to load CNG and pkcs11 keys Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 11/18] Support sending DigestSign request to management client
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair To receive undigested message for signing, indicate support for handling message digesting in the client using an argument "digest" to --management-external-key. For example, to announce pkcs1 padding and digesting support use: --management-external-key pkcs1 pss digest In PK_SIGN, the algorithm string will get data=message in addition to other relevant options. Note that it is not guaranteed that the client will be prompted with undigested message. This is possible only when OpenSSL calls our provider for DigestSign() as opposed to Sign(). In practice, signature operation always appears to result in a DigestSign() call through the provider interface. Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 10/18] Respect algorithm support announced by management client
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair Support for padding algorithms in management-client is indicated in the optional argument to --management-external-key as "pkcs1", "pss" etc. We currently use it only for an early exit based on heuristics that a required algorithm may not be handled by the client. When signature is requested we do not check whether the padding is indeed supported by the client. This leads to situations like the client announcing nopadding support but we request pss signature. Here we add a check while requesting signature as well. If the padding treat it as an error instead of submitting the request to the management-interface regardless. This change is made only when xkey provider is in use, though such a check would be appropriate always. Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 09/18] Allow management client to announce pss padding support
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair The --management-external-key option can currently indicate support for 'nopadding' or 'pkcs1' signatures in the client. Add 'pss' as an option to announce that PSS signing requests are accepted. To match, extend the algorithm string in PK_SIGN request to include the following format: - RSA_PKCS1_PSS_PADDING,hashlag=name,saltlen=[max|digest] Here 'name' is the short common name of the hash algorithm. E.g., SHA1, SHA256 etc. Existing formats 'ECDSA' and 'RSA_PKCS1_PADDING' are unchanged. v2 changes: Fix typos and other sloppiness in documentation and commit message. Signed-off-by: Selva Nair Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 08/18] Add a function to encode digests with PKCS1 DigestInfo wrapper
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair The EVP_PKEY interface as well as provider passes the raw digest to the sign() function. In case of RSA_PKCS1, our management interface expects an encoded hash, which has the DigestInfo header added as per PKCSv1.5 specs, unless the hash algorithm is legacy MD5_SHA1. Fix this by - add a function to perform the pkcs1 encoding before passing the data to sign to the management interface. The implementation is not pretty, but should work. (Unfortunately OpenSSL does not expose a function for this). Note: 1. cryptoki interface used by pkcs11-helper also requires this to be done before calling the Sign op. This will come handy there too. 2. We have a similar function in ssl_mbedtls.c but its not prettier, and require porting. v2 changes: Use hard-coded headers for known hash algorithms instead of assembling it from the ASN.1 objects. Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 07/18] Enable signing via provider for management-external-key
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair - Add a function to set as sign_op during key import. The function passes the signature request to management interface, and returns the result to the provider. v2 changes: Method to do digest added to match the changes in the provider signature callback. TODO: - Allow passing the undigested message to management interface - Add pkcs1 DigestInfo header when required Signed-off-by: Selva Nair Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 06/18] A helper function to import private key for management-external-key
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair - Leverage keymgmt_import through EVP_PKEY_new_fromdata() to import "management-external-key" - When required, use this to set SSL_CTX_use_PrivateKey The sign_op is not implemented yet. This will error out while signing with --management-external-key. The next commit fixes that. Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 05/18] Initialize the xkey provider and use it in SSL context
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair - Add function to check when external key is in use - Load xkey provider into a custom library context when required - Use the custom libctx in SSL CTX when external key is in use As no keys are yet loaded through the provider, no functionality gets delegated to it as yet. v2 changes: Provider loading is reworked to activate only when external keys are in use This was 2/9 in v1 Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 04/18] Implement import of custom external keys
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com:
From: Selva Nair
Our key object retains info about the external
key as an opaque handle to the backend. We also
need the public key as an EVP_PKEY *.
For native keys we use OpenSSL API to import
data into the key. The 'handle' representing the
private key in that case is the OpenSSL EVP_PKEY
object itself.
For importing custom keys, we define custom
parameters describing the key using OSSL_PARAM
structure. We define 4 required and 1 optional
parameters for loading the key:
Required params of type OSSL_PARAM:
{.key="xkey-origin", .data_type = OSSL_PARAM_UTF8_STRING
.data = "foobar", .data_size = 0 }
Note: data_size = 0 refer to NUL terminated string in OpenSSL.
This parameter is only used to identify that the key as non-native
with an opaque handle. We really do not check the content of
the string. Should not be NULL.
{.key="handle", .data_type = OSSL_PARAM_OCTET_PTR,
.data = , .data_size = sizeof(handle)}
{.key="pubkey", .data_type = OSSL_PARAM_OCTET_STRING,
.data = , .data_size = sizeof(pubkey)}
{.key="sign_op", .data_type = OSSL_PARAM_OCTET_PTR,
.data = _op_ptr, .data_size = sizeof(sign_op_ptr)}
Optional param:
{.key="free_op", .data_type = OSSL_PARAM_OCTET_PTR,
.data = _op_ptr, .data_size = sizeof(free_op_ptr)}
The 'handle' is opaque to us and is retained. The caller
should not free it. We will free it when no longer required
by calling 'free_op()', if provided. The 'handle' should
not be NULL as that indicates missing private key.
The 'pubkey' must be an 'EVP_PKEY *' variable, and is duplicated
by us. The caller may free it after return from import.
The 'sign_op' and 'free_op' function pointers should be of type
'XKEY_EXTERNAL_SIGN_fn' and 'XKEY_PRIVKEY_FREE_fn' defined
in xkey_common.h
For example, for management-external-key, we really do not
need any 'handle'. Pass anything that will live long and
won't dereference to NULL. We do not use it for any other
purpose. Pointer to a const string could be a choice.
In this case, free_op = NULL is the safest choice.
For a usage of keymgmt_import(), see the helper function
implemented using it to load the management key in the next commit.
v2 changes: "origin" --> "xkey-origin"
This was 5/9 in v1
Acked-By: Arne Schwabe
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 03/18] Implement SIGNATURE operations in xkey provider
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair - Basic frame work for announcing support for signature operations - DigestSign and Sign functions for native keys are also implemented. Though strictly not needed, these functions for native keys sets up the framework for signature operations. They also help loading an exportable key from a file through the provider for testing. Subsequent commits will add support for signing with external keys. v2 changes: - Remove verify operations which are no longer required with proposed changes in OpenSSL 3.0.1 that we target. - Undigested message is passed to the backend sign operation when possible. This would allow more flexibility as some backends prefer to do the hash operation internally. This was 4/9 in v1 Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 02/18] Implement KEYMGMT in the xkey provider
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair A minimal set of functions for keymgmt are implemented. No support for external key import as yet, only native keys. Support for native keys is required as keys may get imported into us for some operations as well as for comparison with unexportable external keys that we hold. Implementation of signature callbacks is in the next commit. v2 changes: This was commit 3/9 in v1 v3 changes: When OpenSSL native key is imported instead of duplicating the whole key, use only the public components for public key. Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v3 01/18] A built-in provider for using external key with OpenSSL 3.0
Am 14.12.21 um 17:59 schrieb selva.n...@gmail.com: From: Selva Nair Acked-By: Arne Schwabe ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] unit-test: fix test_crypto when USE_COMP is not defined
This unit-test did not consider the case when USE_COMP is not defined, thus generating a compiler error. Adapt the test to the case when no compression is available and while at it, decompose the expected MTU values by featureso that it is easier to understand. Cc: Arne Schwabe Signed-off-by: Antonio Quartulli --- tests/unit_tests/openvpn/test_crypto.c | 52 +++--- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c index 19ce174e..851696fe 100644 --- a/tests/unit_tests/openvpn/test_crypto.c +++ b/tests/unit_tests/openvpn/test_crypto.c @@ -255,7 +255,7 @@ test_occ_mtu_calculation(void **state) o.ciphername = "none"; o.authname = "none"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1400); +assert_int_equal(linkmtu, o.ce.tun_mtu); /* Static key OCC examples */ o.shared_secret_file = "not null"; @@ -264,44 +264,51 @@ test_occ_mtu_calculation(void **state) o.ciphername = "none"; o.authname = "none"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1408); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8); /* secret, cipher AES-128-CBC, auth none */ o.ciphername = "AES-128-CBC"; o.authname = "none"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1440); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 32); /* secret, cipher none, auth SHA256 */ o.ciphername = "none"; o.authname = "SHA256"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1440); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 32); -/* --secret, cipher BF-CBC, auth SHA1 */ +/* secret, cipher BF-CBC, auth SHA1 */ o.ciphername = "BF-CBC"; o.authname = "SHA1"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1444); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 36); -/* --secret, cipher BF-CBC, auth SHA1, tcp-client */ +/* secret, cipher BF-CBC, auth SHA1, tcp-client */ o.ce.proto = PROTO_TCP_CLIENT; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1446); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + 36 + 2); o.ce.proto = PROTO_UDP; -/* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1 */ +int comp_bytes = 0; +#if defined(USE_COMP) +/* secret, comp-lzo yes, cipher BF-CBC, auth SHA1 */ o.comp.alg = COMP_ALG_LZO; +comp_bytes = 1; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1445); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + comp_bytes + 36); +#endif -/* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1, fragment 1200 */ +/* secret, comp-lzo yes, cipher BF-CBC, auth SHA1, fragment 1200 */ o.ce.fragment = 1200; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1449); +assert_int_equal(linkmtu, o.ce.tun_mtu + 8 + comp_bytes + 36 + 4); +comp_bytes = 0; +#if defined(USE_COMP) o.comp.alg = COMP_ALG_UNDEF; +#endif o.ce.fragment = 0; /* TLS mode */ @@ -309,32 +316,32 @@ test_occ_mtu_calculation(void **state) o.tls_client = true; o.pull = true; -/* tls client, cipher AES-128-CBC, auth SHA1, tls-auth*/ +/* tls client, cipher AES-128-CBC, auth SHA1, tls-auth */ o.authname = "SHA1"; o.ciphername = "AES-128-CBC"; o.tls_auth_file = "dummy"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1457); +assert_int_equal(linkmtu, o.ce.tun_mtu + 5 + 8 + 32 + 12); /* tls client, cipher AES-128-CBC, auth SHA1 */ o.tls_auth_file = NULL; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1457); +assert_int_equal(linkmtu, o.ce.tun_mtu + 5 + 8 + 32 + 12); /* tls client, cipher none, auth none */ o.authname = "none"; o.ciphername = "none"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1405); +assert_int_equal(linkmtu, o.ce.tun_mtu + 5); /* tls client, auth none, cipher none, no-replay */ o.replay = false; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1401); +assert_int_equal(linkmtu, o.ce.tun_mtu + 1); o.replay = true; @@ -343,19 +350,22 @@ test_occ_mtu_calculation(void **state) o.authname = "SHA1"; o.ciphername = "AES-256-GCM"; linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1449); +assert_int_equal(linkmtu, o.ce.tun_mtu + 5 + 44); +o.ce.fragment = 1200; +#if defined(USE_COMP) /* tls client, auth SHA1, cipher AES-256-GCM, fragment, comp-lzo yes */ o.comp.alg = COMP_ALG_LZO; -o.ce.fragment = 1200; +comp_bytes = 1; +#endif linkmtu = calc_options_string_link_mtu(, ); -assert_int_equal(linkmtu, 1454); +
