Re: [Openvpn-devel] [PATCH v2] Fix compilation on pre-EKM mbedTLS libraries.

2020-10-10 Thread Steffan Karger
> /* Dummy function to avoid ifdefs in the common code */ > -return NULL; > +return false; > } > #endif /* HAVE_EXPORT_KEYING_MATERIAL */ > > Sorry, totally missed this. Fix looks good. Acked-by: Steffan Karger -Steffan ___

[Openvpn-devel] [PATCH] Simplify key material exporter backend API

2020-10-09 Thread Steffan Karger
Just pass pointer and length, instead of a gc and return (possibly) allocated memory. Saves us some gc instantiations and memcpy()s. Exact same functionality, 19 lines less code. (Didn't want to delay the TLS EKM reviews for this, so submitted as a patch afterwards.) Signed-off-by: Steffan

[Openvpn-devel] [PATCH] networking_iproute2: fix memory leak in net_iface_mtu_set()

2020-10-09 Thread Steffan Karger
ASAN yelled at me that someone forgot to call argv_free(). Fix that. Signed-off-by: Steffan Karger --- src/openvpn/networking_iproute2.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/openvpn/networking_iproute2.c b/src/openvpn/networking_iproute2.c index f3b9c614..3b460527 100644

Re: [Openvpn-devel] [PATCH v5] Implement generating data channel keys via EKM/RFC 5705

2020-10-09 Thread Steffan Karger
ME_FIELD_DEFAULT "CN" > diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h > index cf9fba25..4bcb3181 100644 > --- a/src/openvpn/ssl_backend.h > +++ b/src/openvpn/ssl_backend.h > @@ -389,6 +389,7 @@ void key_state_ssl_free(struct key_state_ssl *ks_ssl); > void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx, > const char *crl_file, bool crl_inline); > > +#define EXPORT_KEY_DATA_LABEL "EXPORTER-OpenVPN-datakeys" > /** > * Keying Material Exporters [RFC 5705] allows additional keying material to > be > * derived from existing TLS channel. This exported keying material can then > be > diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c > index 4ec355a9..f375e957 100644 > --- a/src/openvpn/ssl_mbedtls.c > +++ b/src/openvpn/ssl_mbedtls.c > @@ -1168,11 +1168,8 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl, > > #ifdef HAVE_EXPORT_KEYING_MATERIAL > /* Initialize keying material exporter */ > -if (session->opt->ekm_size) > -{ > -mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config, > -mbedtls_ssl_export_keys_cb, > session); > -} > +mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config, > +mbedtls_ssl_export_keys_cb, session); > #endif > > /* Initialise SSL context */ > If the above is fixed: Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v4 2/2] Implement generating data channel keys via EKM/RFC 5705

2020-10-09 Thread Steffan Karger
Hi, On 25-08-2020 09:36, Arne Schwabe wrote: > OpenVPN currently uses its own (based on TLS 1.0) key derivation > mechanism to generate the 256 bytes key data in key2 struct that > are then used used to generate encryption/hmac/iv vectors. While > this mechanism is still secure, it is not state

Re: [Openvpn-devel] Introducing the OpenVPN Data Channel Offload Linux kernel module (ovpn-dco)

2020-10-06 Thread Steffan Karger
Hi Antonio, On 22-09-2020 16:17, Antonio Quartulli wrote: > Dear all, > > OpenVPN Inc.[1] and me are happy to announce that today we have released > to the public our work-in-progress project called "ovpn-dco". > > [...] > > The idea of implementing an OpenVPN kernel module has been around

Re: [Openvpn-devel] [PATCH v4 1/2] Move openvpn specific key expansion into its own function

2020-10-04 Thread Steffan Karger
891,9 +1931,8 @@ tls_session_generate_data_channel_keys(struct > tls_session *session) > } > > ks->crypto_options.flags = session->opt->crypto_flags; > -if (!generate_key_expansion(>crypto_options.key_ctx_bi, > -&

Re: [Openvpn-devel] [PATCH] Fix client's poor man NCP fallback

2020-08-22 Thread Steffan Karger
return true; > +} > > /* We could not figure out the peer's cipher but we have fallback > * enabled */ > -if (!useremotecipher && c->options.enable_ncp_fallback) > +if (!c->c2.tls_multi->remote_ciphername && > c->options.enable_ncp_fallb

Re: [Openvpn-devel] [PATCH v2 2/3] Move openvpn specific key expansion into its own function

2020-08-21 Thread Steffan Karger
Hi, On 12-08-2020 16:01, Arne Schwabe wrote: > This moves the OpenVPN specific PRF into its own function also > simplifies the code a bit by passing tls_session directly instead of > 5 of its fields. Moves in the right direction. Some comments though: > Signed-off-by: Arne Schwabe > > Patch

Re: [Openvpn-devel] [PATCH v3 1/3] Refactor key_state_export_keying_material functions

2020-08-21 Thread Steffan Karger
session->opt->ekm_label_size, > - NULL, 0, 0)) > -{ > -unsigned int len = (size * 2) + 2; > +{ > +unsigned char *ekm = (unsigned char *) gc_malloc(ekm_size, true, gc); > > -const char *key = format_hex_ex(ekm, size, len, 0, NULL, ); > -setenv_str(session->opt->es, "exported_keying_material", key); > +SSL* ssl = session->key[KS_PRIMARY].ks_ssl.ssl; > > -dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s", > - __func__, key); > -} > -else > -{ > -msg(M_WARN, "WARNING: Export keying material failed!"); > -setenv_del(session->opt->es, "exported_keying_material"); > -} > -gc_free(); > +if (SSL_export_keying_material(ssl, ekm, ekm_size, label, > + label_size, NULL, 0, 0) == 1) > +{ > +return ekm; > +} > +else > +{ > +secure_memzero(ekm, ekm_size); > +return NULL; > } > } > > Thanks, looks good to me now. Changes wrt v2 in comments and whitespace only, so didn't re-test. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2 1/3] Refactor key_state_export_keying_material functions

2020-08-12 Thread Steffan Karger
Hi, On 12-08-2020 16:01, Arne Schwabe wrote: > This refactors the common code between mbed SSL and OpenSSL into > export_user_keying_material and also prepares the backend functions > to export more than one key. > > Also fix checking the return value of SSL_export_keying_material > only 1 is a

Re: [Openvpn-devel] [PATCH 1/3] Refactor key_state_export_keying_material functions

2020-08-12 Thread Steffan Karger
Hi, Couldn't resist giving this a quick look. Feature-ACK on the patch set, but some comments on the approach: On 12-08-2020 10:55, Arne Schwabe wrote: > This refactors the common code between mbed SSL and OpenSSL into > export_user_keying_material and also prepares the backend functions > to

Re: [Openvpn-devel] [PATCH applied] Re: Remove S_OP_NORMAL key state.

2020-08-11 Thread Steffan Karger
On 11-08-2020 10:45, Gert Doering wrote: > Acked-by: Gert Doering > > Server-side and client-side tested. > > Not sure if I understand all possible implications of S_NORMAL_OP, > but indeed it is not *used* anywere, except in ">= S_ACTIVE". > > The flow of "at which point in time we set

Re: [Openvpn-devel] [PATCH v2] Rework NCP compability logic and drop BF-CBC support by default

2020-08-05 Thread Steffan Karger
Hi, No full review yet, but this version does seem to address my previous comments. Some minor nits I noticed on my first run through v2: On 29-07-2020 13:38, Arne Schwabe wrote: > This reworks the NCP logic to be more strict about what is > considered an acceptable result of an NCP negotiation.

Re: [Openvpn-devel] [PATCH 9/9] Rework NCP compability logic and drop BF-CBC support by default

2020-07-28 Thread Steffan Karger
Hi, This is awesome in many ways. Better behaviour, better code and a nice way forward to really get rid of the BF-CBC default cipher. It's also somewhat tricky, so here goes for a review purely based on stare-at-code: On 17-07-2020 15:47, Arne Schwabe wrote: > This reworks the NCP logic to be

[Openvpn-devel] [PATCH] Gently push users towards --data-ciphers in --show-ciphers output

2020-07-27 Thread Steffan Karger
Also: * fix a typo in the openssl output ("may be use*d*") * mention GCM before CBC (we prefer AEAD modes) Signed-off-by: Steffan Karger --- src/openvpn/crypto_mbedtls.c | 5 +++-- src/openvpn/crypto_openssl.c | 10 +- 2 files changed, 8 insertions(+), 7 deletions(-)

Re: [Openvpn-devel] [PATCH 8/9] Rename ncp-ciphers to data-ciphers

2020-07-24 Thread Steffan Karger
t(const char *list, struct gc_arena > *gc) > const cipher_kt_t *ktc = cipher_kt_get(token); > if (!ktc) > { > -msg(M_WARN, "Unsupported cipher in --ncp-ciphers: %s", token); > +msg(M_WARN, "Unsupported cipher in --d

Re: [Openvpn-devel] [PATCH 8/9] Rename ncp-ciphers to data-ciphers

2020-07-24 Thread Steffan Karger
Hi, On 23-07-2020 18:09, David Sommerseth wrote: >> This was a deliberate decision. We really want to people to move towards >> ncp and putting another hurdle with having an option that works better >> on but gives a warning and a option that does not work on 2.4 does not >> help here. If we

Re: [Openvpn-devel] [PATCH v3] Require AEAD support in the crypto library

2020-07-20 Thread Steffan Karger
; } > > void > diff --git a/src/openvpn/crypto_openssl.h b/src/openvpn/crypto_openssl.h > index 4694ee08..e6f8f537 100644 > --- a/src/openvpn/crypto_openssl.h > +++ b/src/openvpn/crypto_openssl.h > @@ -61,13 +61,9 @@ typedef HMAC_CTX hmac_ctx_t; > /** Cipher is in CFB mode */ &

Re: [Openvpn-devel] [PATCH 3/9] Require AEAD support in the crypto library

2020-07-20 Thread Steffan Karger
Hi, On 17-07-2020 15:47, Arne Schwabe wrote: > All supported crypto libraries have AEAD support and with our > ncp/de facto default cipher AES-256-GCM we do not want to support > the obscure corner case of a library with disabled AEAD. Again: feature-ACK, but some comments. config-msvc.h still

Re: [Openvpn-devel] [PATCH v2 2/9] Drop support for OpenSSL 1.0.1

2020-07-20 Thread Steffan Karger
ot;Extracting ECDH curve from private key"); > - > -if (pkey != NULL && (eckey = EVP_PKEY_get1_EC_KEY(pkey)) != NULL > -&& (ecgrp = EC_KEY_get0_group(eckey)) != NULL) > -{ > -nid = EC_GROUP_get_curve_name(ecgrp); > -} > -#endif /* if OPENSSL_VERSION_NUMBER >= 0x10002000L */ > } > > /* Translate NID back to name , just for kicks */ > @@ -1462,15 +1421,7 @@ tls_ctx_use_management_external_key(struct > tls_root_ctx *ctx) > > ASSERT(NULL != ctx); > > -#if (OPENSSL_VERSION_NUMBER >= 0x10002000L && > !defined(LIBRESSL_VERSION_NUMBER)) \ > -|| LIBRESSL_VERSION_NUMBER >= 0x207fL > -/* OpenSSL 1.0.2 and up */ > X509 *cert = SSL_CTX_get0_certificate(ctx->ctx); > -#else > -/* OpenSSL 1.0.1 and earlier need an SSL object to get at the > certificate */ > -SSL *ssl = SSL_new(ctx->ctx); > -X509 *cert = SSL_get_certificate(ssl); > -#endif > > ASSERT(NULL != cert); > > @@ -1510,13 +1461,6 @@ tls_ctx_use_management_external_key(struct > tls_root_ctx *ctx) > > ret = 0; > cleanup: > -#if OPENSSL_VERSION_NUMBER < 0x10002000L \ > -|| (defined(LIBRESSL_VERSION_NUMBER) && LIBRESSL_VERSION_NUMBER < > 0x207fL) > -if (ssl) > -{ > -SSL_free(ssl); > -} > -#endif > if (ret) > { > crypto_msg(M_FATAL, "Cannot enable SSL external private key > capability"); > Otherwise this now looks good to me. So if the whitespace can fixed when committing: Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 1/9] Indicate that a client is in pull mode in IV_PROTO

2020-07-20 Thread Steffan Karger
Hi, On 17-07-2020 15:47, Arne Schwabe wrote: > This allows us to skip waiting for the first PUSH_REQUEST message from > the client to send the response. Feature-ACK, clever use of existing infra. Some comments though: This commit message could use a bit more information. In particular, it would

Re: [Openvpn-devel] [PATCH 3/3] Remove key-method 1

2020-07-15 Thread Steffan Karger
Hi On 13-07-2020 11:46, Arne Schwabe wrote: > @@ -1100,7 +1100,7 @@ process_incoming_link_part1(struct context *c, struct > link_socket_info *lsi, boo > floated, _start)) > { > /* Restore pre-NCP frame parameters */ > -

Re: [Openvpn-devel] [PATCH 1/3] Drop support for OpenSSL 1.0.1

2020-07-14 Thread Steffan Karger
Hi, Feature-ACK for sure. Some comments below. On 13-07-2020 11:46, Arne Schwabe wrote: > OpenSSL 1.0.1 was supported until 2016-12-31. Rhel6/Centos6 still > use this version but considering that RHEL7 and RHEL8 are already > out, these versions can also stay with OpenVPN 2.4. > > All the

Re: [Openvpn-devel] [PATCH 2/3] Cleanup: Remove unused code of old poor man's NCP.

2020-07-08 Thread Steffan Karger
Hi, As discusses in #openvpn-devel on IRC, this patch breaks interop with clients that don't pull, but that will be restored in a follow-up refactoring (before 2.5 rc1). I can live with that, but I think this should be mentioned in the commit message. On 07-07-2020 14:16, Arne Schwabe wrote: >

Re: [Openvpn-devel] [PATCH 3/3] Make key_state->authenticated more state machine like

2020-07-08 Thread Steffan Karger
Hi, On 07-07-2020 18:20, Antonio Quartulli wrote: > On 07/07/2020 14:16, Arne Schwabe wrote: >> This order the states from unauthenticated to authenticated and also >> changes the comparison for KS_AUTH_FALSE from != to > >> >> Also remove a now obsolete comment and two obsolete ifdefs. While >>

[Openvpn-devel] [PATCH] Make openvpn --version exit with exit code 0

2020-07-07 Thread Steffan Karger
For some reason, openvpn --version has since the beginning of time returned exit code 1. A quick sample among common unix utilities confirms that the rest of the world agrees with me that 0 makes more sense. Let's make openvpn --version exit with exit code 0 too. Signed-off-by: Steffan Karger

Re: [Openvpn-devel] [PATCH 2/2] merge key_state->authenticated and key_state->auth_deferred

2020-07-07 Thread Steffan Karger
Hi, Didn't find time to fully review, but I think this is moving into the right direction. I did notice something I'd like you to consider: On 06-07-2020 18:35, Arne Schwabe wrote: > @@ -2466,7 +2466,7 @@ key_method_2_write(struct buffer *buf, struct > tls_session *session) > if

Re: [Openvpn-devel] [PATCH] systemd: Change the default cipher to AES-256-GCM for server configs

2020-06-30 Thread Steffan Karger
On 22-06-2020 19:59, David Sommerseth wrote: > On 22/06/2020 14:43, Steffan Karger wrote: >> On 22-06-2020 14:29, David Sommerseth wrote: >>> On 22/06/2020 14:21, Arne Schwabe wrote: >>>> >>>>> PrivateTmp=true >>>>> WorkingDirectory=/et

Re: [Openvpn-devel] [PATCH] systemd: Change the default cipher to AES-256-GCM for server configs

2020-06-30 Thread Steffan Karger
Hi, On 22-06-2020 16:01, Arne Schwabe wrote: > Am 22.06.20 um 14:43 schrieb Steffan Karger: >> Maybe these should be the steps: >> >> 2.4: Use to AES-256-GCM when available (basically what NCP did) >> 2.5: Switch to AES-256-GCM as the default cipher (but allow overrid

Re: [Openvpn-devel] [PATCH] systemd: Change the default cipher to AES-256-GCM for server configs

2020-06-22 Thread Steffan Karger
Hi, On 22-06-2020 14:29, David Sommerseth wrote: > On 22/06/2020 14:21, Arne Schwabe wrote: >> >>> PrivateTmp=true >>> WorkingDirectory=/etc/openvpn/server >>> -ExecStart=@sbindir@/openvpn --status %t/openvpn-server/status-%i.log >>> --status-version 2 --suppress-timestamps --config %i.conf

Re: [Openvpn-devel] [PATCH 1/3] Make cipher_kt_name always return normalised cipher name

2020-06-11 Thread Steffan Karger
+++ b/src/openvpn/ssl_ncp.c > @@ -116,8 +116,7 @@ mutate_ncp_cipher_list(const char *list, struct gc_arena > *gc) > } > else > { > -const char *ovpn_cipher_name = > -translate_cipher_name_to_openvpn(cipher_kt_name(ktc)); > +const char *ovpn_cipher_name = cipher_kt_name(ktc); > > if (buf_len(_list)> 0) > { > Makes sense, changes look good and passes my tests. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/3] Make cipher_kt_get also accept OpenVPN config cipher name

2020-06-11 Thread Steffan Karger
t *ktc = cipher_kt_get(cipher_name); > + const cipher_kt_t *ktc = cipher_kt_get(token); > if (!ktc) > { > msg(M_WARN, "Unsupported cipher in --ncp-ciphers: %s", token); > This makes the cipher_kt_name and cipher_kt_get abstractions nicely consistent. Code looks good, and passes my tests. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [BUG] test_ncp.c failing

2020-05-29 Thread Steffan Karger
On 29-05-2020 01:46, James Bottomley wrote: > The problem seems to be openssl uses a mixed case name for the cipher > and EVP_CIPHER_name() is case sensitive. Applying the patch below > fixes this for openssl and gets make check to pass all tests, but I > rather wonder why this isn't part of

Re: [Openvpn-devel] [PATCH] Switch assertion failure to returning false

2020-05-27 Thread Steffan Karger
_key_expansion fails a few > lines below, so my assumption was it was safe to do so. However, that's > just an assumption and not even an educated guess. This change correctly turns "kill the server" into "reject the connection", which I agree is a good thing. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v4] Do not write extra 0 byte for --gen-key with auth-token/tls-crypt-v2

2020-05-15 Thread Steffan Karger
as a sanity check */ > @@ -580,7 +582,8 @@ test_tls_crypt_v2_write_client_key_file_metadata(void > **state) > > /* Test writing the client key */ > expect_string(__wrap_buffer_write_file, filename, filename); > -expect_string(__wrap_buffer_write_file, pem, test_client_key_metadata); > +expect_memory(__wrap_buffer_write_file, pem, test_client_key_metadata, > +strlen(test_client_key_metadata)); > will_return(__wrap_buffer_write_file, true); > > /* Key generation re-reads the created file as a sanity check */ > Thanks, this looks good to me now. The patch has a small merge conflict with the inline-refactoring-fixups, but that should be easy enough to resolve when applying. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3] Do not write extra 0 byte for --gen-key with auth-token/tls-crypt-v2

2020-05-03 Thread Steffan Karger
Hi, On 27-04-2020 03:26, Arne Schwabe wrote: > Change crypto_pem_encode to not put a nul-terminated terminated > string into the buffer. This was useful for printf but should > not be written into the file. > > Instead do not assume that the buffer is null terminated and > print only the number

Re: [Openvpn-devel] [PATCH] Uncrustify the tests/unit_tests/ part of our tree.

2020-04-26 Thread Steffan Karger
Hi, On 26-04-2020 11:54, Gert Doering wrote: > Apply uncrustify 0.70.1 (FreeBSD port) with our rules to that part > of the tree, which followed a more compact coding style so far. > --- > > @@ -155,20 +157,21 @@ test_packet_id_write_long_wrap(void **state) > } > > int > -main(void) { >

Re: [Openvpn-devel] [PATCH v2 1/3] Use crypto library functions for const time memcmp when possible

2020-04-26 Thread Steffan Karger
Hi, On 26-04-2020 11:34, Gert Doering wrote: > On Sun, Apr 26, 2020 at 11:25:49AM +0200, Steffan Karger wrote: >>>> well, sometimes to adhere to the codestyle, you have to re-arrange code :) >>> >>> "rearrange" and "rewrite in a not easy to understa

Re: [Openvpn-devel] [Openvpn-users] new openssl = new OpenVPN release ?

2020-04-26 Thread Steffan Karger
On 22-04-2020 10:27, Jan Just Keijser wrote: > On 22/04/20 10:13, Arne Schwabe wrote: SSL_check_chain() function". Which we don't, I just grepped through our source tree. So, unless I misunderstand something about OpenSSL intricacies, I think we're safe - no new

Re: [Openvpn-devel] [PATCH v2 1/3] Use crypto library functions for const time memcmp when possible

2020-04-26 Thread Steffan Karger
Hi, On 17-04-2020 17:36, Gert Doering wrote: > On Fri, Apr 17, 2020 at 03:42:49PM +0200, Antonio Quartulli wrote: -static inline int -memcmp_constant_time(const void *a, const void *b, size_t size) -{ >>> >>> Not sure I understand the motivation for this change. "Just so

Re: [Openvpn-devel] [PATCH] Add tls-crypt-v2 test writing metadata

2020-04-26 Thread Steffan Karger
test_server_key); > } > > + > int > main(void) { > const struct CMUnitTest tests[] = { > @@ -576,6 +613,7 @@ main(void) { > test_tls_crypt_v2_teardown), > cmocka_unit_tes

Re: [Openvpn-devel] [PATCH v2] Do not write extra 0 byte for --gen-key with auth-token/tls-crypt-v2

2020-04-25 Thread Steffan Karger
Hi, On 20-04-2020 14:06, Arne Schwabe wrote: > Change crypto_pem_encode to not put a nul-terminated terminated > string into the buffer. This was useful for printf but should > not be written into the file. > > Instead do not assume that the buffer is null terminated and > print only the number

Re: [Openvpn-devel] [PATCH] Do not write extra 0 byte for --gen-key with auth-token/tls-crypt-v2

2020-04-06 Thread Steffan Karger
On 06-04-2020 18:00, Arne Schwabe wrote: > Am 06.04.20 um 17:44 schrieb Steffan Karger: >> Hi, >> >> On 06-04-2020 15:00, Arne Schwabe wrote: >>> crypto_pem_encode put a nul-terminated terminated string into the >>> buffer. This is useful for printf but

Re: [Openvpn-devel] [PATCH] Do not write extra 0 byte for --gen-key with auth-token/tls-crypt-v2

2020-04-06 Thread Steffan Karger
Hi, On 06-04-2020 15:00, Arne Schwabe wrote: > crypto_pem_encode put a nul-terminated terminated string into the > buffer. This is useful for printf but should not be written into > the file. > > Also for static keys, we were missing the nul termination when priting > it to stadout but since the

Re: [Openvpn-devel] [PATCH] Fix off-by-one in tls-crypt-v2 client wrapping with custom metadata

2020-04-06 Thread Steffan Karger
tch looks good, but it would have been nice to add a regression (unit) test. Are you willing to write one? Otherwise I will. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix OpenSSL private key passphrase notices

2020-02-27 Thread Steffan Karger
mp; (ERR_GET_REASON(ERR_peek_error()) == > EVP_R_BAD_DECRYPT)) > Thanks, and apologies for the late repsonse. This patch does was it says, and looks good to me. I think this one should go into both master and release/2.4. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] Warn about insecure ciphers also in init_key_type

2020-02-19 Thread Steffan Karger
>cipher)) > -{ > -msg(M_WARN, "WARNING: INSECURE cipher with block size less than > 128" > -" bit (%d bit). This allows attacks like SWEET32. Mitigate > by " > -"using a --cipher with a larger block size (e.g. > AES-256-CBC).",

Re: [Openvpn-devel] [PATCH] Warn about insecure ciphers also in init_key_type

2020-02-19 Thread Steffan Karger
Hi, On 29-03-2019 13:27, Arne Schwabe wrote: > With modern Clients and server initialising the crypto cipher later > and not when reading in the config, most users never the warning when > having selected BF-CBC in the configuration. > > This patch adds the logic to print out warning to

Re: [Openvpn-devel] [PATCH v2] travis-ci: add arm64, s390x builds.

2020-02-03 Thread Steffan Karger
On 03-02-2020 09:04, Илья Шипицин wrote: > also, ARM64 builds are flaky. maybe we should add them as allow_failures. What is flaky about the ARM64 builds? Is it our build? Is it the travis infra? -Steffan ___ Openvpn-devel mailing list

Re: [Openvpn-devel] linux arm64 tests fail

2020-02-01 Thread Steffan Karger
Hi, On 01-02-2020 13:43, Илья Шипицин wrote: > https://travis-ci.org/chipitsine/openvpn/jobs/644745481?utm_medium=notification_source=github_status > > it indicates "ERROR" when running tests, however tests are ok after all. > > [ RUN ] tls_crypt_v2_wrap_too_long_metadata > > ERROR: could not

Re: [Openvpn-devel] [PATCH] Move keying material exporter check from syshead.h to configure.ac

2020-01-20 Thread Steffan Karger
On 20-01-2020 13:42, Lev Stipakov wrote: > +       have_export_keying_material="yes" > +       AC_CHECK_FUNCS( > +               [SSL_export_keying_material], > +               , > +               [have_export_keying_material="no"; break] > +       ) > > > Just wondering

[Openvpn-devel] [PATCH] Move keying material exporter check from syshead.h to configure.ac

2020-01-20 Thread Steffan Karger
against the crypto libraries. That of course breaks openvpnserv builds. To fix this, change the compile-time check in syshead.h into a configure-time check in configure.ac. That's more consistent with how we do other feature checks anyway. Signed-off-by: Steffan Karger --- configure.ac

Re: [Openvpn-devel] [PATCH 1/4] Only announce IV_NCP=2 when we are willing to support these ciphers

2020-01-06 Thread Steffan Karger
Hi, Finally found some time to start looking at this patch set. On 17-11-2019 19:12, Arne Schwabe wrote: > We currently always announce IV_NCP=2 when we support these ciphers even > when we do not accept them. This lead to a server pushing a AES-GCM-128 > cipher to clients and the client then

Re: [Openvpn-devel] [PATCH v6 4/7] wintun: ring buffers based I/O

2019-12-15 Thread Steffan Karger
s, I'm fine with this patch now. I haven't tested it, nor have I reviewed Windows-specific interaction. So this needs at least one other review from someone able to review those parts. Regarding the integration into OpenVPN: Acked-by: Steffan Karger -Steffan _

Re: [Openvpn-devel] [PATCH] fix clang warning about missing braces

2019-11-28 Thread Steffan Karger
On 28-11-2019 09:06, Lev Stipakov wrote: > A struct with subobjects should be initialized > with double braces. This is not true. {0} is a valid initializer for structs in C. Both clang and gcc used to have a bug where they incorrectly warned about this. GCC fixed this a while ago[0]. I thought

[Openvpn-devel] [PATCH] mbedtls: add RFC 5705 keying material exporter support

2019-11-10 Thread Steffan Karger
. Signed-off-by: Steffan Karger --- src/openvpn/init.c| 4 +-- src/openvpn/options.c | 4 +-- src/openvpn/options.h | 2 +- src/openvpn/ssl_mbedtls.c | 64 ++- src/openvpn/ssl_mbedtls.h | 5 +++ src/openvpn/ssl_openssl.c | 4 ++- src

[Openvpn-devel] [PATCH] Update sample configs to use modern cipher, remove static key examples

2019-11-09 Thread Steffan Karger
-by: Steffan Karger --- sample/sample-config-files/loopback-client| 1 + sample/sample-config-files/loopback-server| 1 + sample/sample-config-files/static-home.conf | 75 --- sample/sample-config-files/static-office.conf | 72 -- sample/sample-config-files

Re: [Openvpn-devel] [PATCH v3] Make compression asymmetric by default and add warnings

2019-11-09 Thread Steffan Karger
Hi, Feature-ack, and overall looks good. But some nits to tackle. On 24-10-2018 12:06, Arne Schwabe wrote: > This commit introduces the allow-compression option that allow > changing the new default to the previous default or to a stricter > version. > > Warning are not generated in the post

Re: [Openvpn-devel] [PATCH v6 1/2] Make tls_version_max return the actual maximum version

2019-11-09 Thread Steffan Karger
Hi, On 09-11-2019 13:03, Arne Schwabe wrote: > Before OpenSSL 1.1.1 there could be no mismatch between > compiled and actual OpenSSL version. With OpenSSL 1.1.1 we need > runtime detection to detect the actual best TLS version supported. > > Allowing this runtime detection also allows removing

Re: [Openvpn-devel] [PATCH v2 4/7] wintun: ring buffers based I/O

2019-11-09 Thread Steffan Karger
Hi, Some first-round review comments. I still need to fully grasp the event mechanism intricacies for a real in-depth review. As a general remark: could you try to stick to the 80 char line length limit? On 07-11-2019 18:45, Lev Stipakov wrote: > From: Lev Stipakov > > Implemented according

Re: [Openvpn-devel] [PATCH v3] wintun: add --windows-driver config option

2019-11-09 Thread Steffan Karger
00644 > --- a/src/openvpn/options.h > +++ b/src/openvpn/options.h > @@ -632,6 +632,7 @@ struct options > bool show_net_up; > int route_method; > bool block_outside_dns; > +bool wintun; > #endif > > bool use_peer_id; > diff --git a/src/openvp

Re: [Openvpn-devel] [PATCH v2 2/7] wintun: add --windows-driver config option

2019-11-08 Thread Steffan Karger
Hi, Thanks for looking into wintun support. Definitely feature-ack. On 07-11-2019 18:45, Lev Stipakov wrote: > From: Lev Stipakov > > This allows to specify which tun driver openvpn should use, > tap-windows6 (default) or wintun. > > Note than wintun support will be added in follow-up

Re: [Openvpn-devel] [PATCH 1/7] Visual Studio: upgrade project files to VS2019

2019-09-20 Thread Steffan Karger
Hi, On Fri, Sep 20, 2019, 15:47 Lev Stipakov wrote: > Wintun driver is now signed for Windows 10, > > https://staging.openvpn.net/openvpn2/wintun-0.6-signed.zip > > So if you want to try out super fast openvpn2, no need to meddle anymore > with test mode / disabling signature checks. > Out of

Re: [Openvpn-devel] [PATCH] t_net.sh: wait for NO-CARRIER bit to settle before starting test

2019-09-19 Thread Steffan Karger
Hi, On 19-09-19 09:28, Antonio Quartulli wrote: > Interfaces of type tun are marked as NO-CARRIER when no process is > attached to them. However, this bit gets set with some delay after > creation. > > For this reason, it is better to wait for the bit to settle before > starting any test,

Re: [Openvpn-devel] [PATCH] Adding support for wolfSSL backend

2019-08-24 Thread Steffan Karger
Hi, On 24-08-19 21:40, Gert Doering wrote: > On Sat, Aug 24, 2019 at 06:04:21PM +0200, Arne Schwabe wrote: >> I want to give you an honest opionion of mine to merging WolfSSL in >> OpenVPN. Please note, that this is my personal opinion and not to be >> confused to be an official OpenVPN community

Re: [Openvpn-devel] [PATCH 1/1] Start TLS after connection established without waiting.

2019-08-21 Thread Steffan Karger
Hi, On 24-07-19 13:41, Daniel Kaldor wrote: > There is a ~1s delay between establishing connection with remote > server and starting TLS handshake. > This change removes delay and improves connection time. While this sounds like something we'd like to have (i.e, feature-ACK), this doesn't really

Re: [Openvpn-devel] [PATCH 1/2] Fix check if iface name is set

2019-08-13 Thread Steffan Karger
Hi, On 13-08-19 23:31, Antonio Quartulli wrote: > On 13/08/2019 23:26, David Sommerseth wrote: >> wouldn't it be better to >> do 'if (rgi6->iface[0])' instead? Since the buffer should be NULL terminated >> and has to be NULL terminted for strlen() to function anyhow. But the >> compiled code

Re: [Openvpn-devel] [PATCH for 2.4] Correct the return value of cryptoapi RSA signature callbacks

2019-07-28 Thread Steffan Karger
unsigned > int m_len, > } > > *siglen = priv_enc_CNG(cd, alg, m, (int)m_len, sig, RSA_size(rsa), > padding); > -return (siglen == 0) ? 0 : 1; > + return (*siglen == 0) ? 0 : 1; > } > > /* decrypt */ > Acked-by: Steffan Karger

Re: [Openvpn-devel] [PATCH] Correct the return value of cryptoapi RSA signature callbacks

2019-07-28 Thread Steffan Karger
return (siglen == 0) ? 0 : 1; > +return (*siglen == 0) ? 0 : 1; > } > > #endif /* OPENSSL_VERSION >= 1.1.0 */ > Acked-by: Steffan Karger ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v3] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-25 Thread Steffan Karger
tx) > msg(M_WARN, "WARNING: Your certificate is not yet valid!"); > } > > -ret = X509_cmp_time(X509_get_notAfter(cert), NULL); > +ret = X509_cmp_time(X509_get0_notAfter(cert), NULL); > if (ret == 0) > { >

Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-24 Thread Steffan Karger
Hi all, On 14-06-19 12:38, Arne Schwabe wrote: >> +#if !defined(HAVE_EVP_CIPHER_CTX_INIT) >> +#define EVP_CIPHER_CTX_init EVP_CIPHER_CTX_reset >> +#endif >> + >> +#if !defined(HAVE_EVP_CIPHER_CTX_CLEANUP) >> +#define EVP_CIPHER_CTX_cleanup EVP_CIPHER_CTX_reset >> +#endif > > These two keep the

Re: [Openvpn-devel] [PATCH v2] Fix broken fragment/mssfix with NCP

2019-07-20 Thread Steffan Karger
nnel frame. > > The change looks good and is tested. > > Acked-By: Arne Schwabe Since Arne agrees, I'm completely convinced now too: Acked-by: Steffan Karger signature.asc Description: OpenPGP digital signature ___ Openvpn-devel

Re: [Openvpn-devel] [PATCH] Do not set pkcs11-helper "safe fork mode"

2019-07-20 Thread Steffan Karger
On 28-04-19 11:23, Steffan Karger wrote: > Together with the pkcs11-helper fixes, I do think this is the right fix. > I'll try to experiment a bit with it myself too. Finally got to some testing and staring at code. The patch resolves the issue for me, and I didn't find any other issues. Ou

Re: [Openvpn-devel] [PATCH] Rate-limit incoming P_CONTROL_HARD_RESET_* packets.

2019-07-14 Thread Steffan Karger
Hi, On 06-12-18 18:10, Gert Doering wrote: > Creation of new instances (= new incoming reset packets with different > source IP addresses / source ports) can be rate-limited in the current > code by use of the "--connect-freq" option. > > For packets sent with the same source port, OpenVPN would

Re: [Openvpn-devel] [PATCH v3] Stop state-exhaustion attacks from a single source address.

2019-07-14 Thread Steffan Karger
Hi, Sorry for the terrible response time, but here goes for initial review: On 07-12-18 21:28, Gert Doering wrote: > If an attacker sends lots of RESET packets from different source > ports (but the same source address), every packet will create a > new multi_instance, leading to resource waste

[Openvpn-devel] [PATCH] configure.ac: add lzo CFLAGS/LIBS to the test flags

2019-06-02 Thread Steffan Karger
This fixes "make check" builds on systems with lzo on a non-standard location. Signed-off-by: Steffan Karger --- configure.ac | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 59673e049..56598217f 100644 --- a/configur

[Openvpn-devel] [PATCH v2] tests: remove dependency on base64

2019-05-08 Thread Steffan Karger
From: Steffan Karger Triggered by the report from Ilya, that if base64 is missing, the tests would still report success: Testing tls-crypt-v2 key generation (max length metadata)/t_lpback.sh: base64: not found OK PASS: t_lpback.sh The easiest way to fix that, is to remove

Re: [Openvpn-devel] [PATCH] build: Package missing mock_msg.h

2019-05-02 Thread Steffan Karger
-Wl,--wrap=buffer_write_file \ > -Wl,--wrap=parse_line \ > -Wl,--wrap=rand_bytes > -tls_crypt_testdriver_SOURCES = test_tls_crypt.c mock_msg.c \ > +tls_crypt_testdriver_SOURCES = test_tls_crypt.c mock_msg.c mock_msg.h \ > $(openvpn_srcdir)/argv.c \ > $(openvp

Re: [Openvpn-devel] [PATCH] Do not set pkcs11-helper "safe fork mode"

2019-04-28 Thread Steffan Karger
Hi David, Hilko, On 07-03-19 20:14, David Sommerseth wrote: > On 18/02/2019 16:31, Hilko Bengen wrote: >> From the pkcs11-helper API documentation about pkcs11h_setForkMode(): >> >>> This funciton is releavant if PKCS11H_FEATURE_MASK_THREADING is >>> set. If safe mode is on, the child process can

[Openvpn-devel] [PATCH] tests: remove dependency on base64

2019-04-17 Thread Steffan Karger
From: Steffan Karger Triggered by the report from Ilya, that if base64 is missing, the tests would still report success: Testing tls-crypt-v2 key generation (max length metadata)/t_lpback.sh: base64: not found OK PASS: t_lpback.sh The easiest way to fix that, is to remove

Re: [Openvpn-devel] cirrus-ci: freebsd builds ?

2019-04-17 Thread Steffan Karger
On Thu, 11 Apr 2019 at 00:54, Илья Шипицин wrote: > 1) error when built with mbedtls-2.16.0 (surprizingly, build does not fail) > > [ OK ] tls_crypt_v2_wrap_unwrap_no_metadata > [ RUN ] tls_crypt_v2_wrap_unwrap_max_metadata > [ OK ] tls_crypt_v2_wrap_unwrap_max_metadata > [ RUN

Re: [Openvpn-devel] [PATCH 3/3] travis-ci: update osx to xcode9.4 and modernize brew management

2019-03-17 Thread Steffan Karger
directories: > @@ -83,10 +83,6 @@ cache: >- ${HOME}/opt >- ${HOME}/Library/Caches/Homebrew > > -before_install: > - - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then brew update ; fi > - - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then brew

Re: [Openvpn-devel] [PATCH 2/3] travis-ci: change trusty image to xenial

2019-03-17 Thread Steffan Karger
repository "deb http://archive.ubuntu.com/ubuntu xenial > main universe" > - sudo apt-get update > - sudo apt-get -y install dpkg mingw-w64 > -fi > - > # Download and build crypto lib > if [ "${SSLLIB}" = "openssl" ]; then >

Re: [Openvpn-devel] [PATCH 1/3] travis-ci: add "linux-ppc64le" to build matrix

2019-03-17 Thread Steffan Karger
gt; + os: linux-ppc64le > + compiler: gcc > - env: SSLLIB="openssl" CFLAGS="-fsanitize=address" >os: linux >compiler: clang > Looks good to me and passes tests on travis. Acked-by: Steffan Karger -Steffan ___

Re: [Openvpn-devel] Summary of the community meeting (Wed, 12th Mar 2019)

2019-03-17 Thread Steffan Karger
Hi Jan Just, On 13-03-19 13:13, Jan Just Keijser wrote: > On 13/03/19 13:00, Samuli Seppänen wrote: >> Here's the summary of the IRC meeting. >> >> Talked about release OpenVPN 2.x Windows installers with OpenSSL 1.1.1. >> Agreed that this makes sense as people (on forums for example) already >>

Re: [Openvpn-devel] [Openvpn-users] Why is the authentication tag transmitted before the encrypted data?

2019-03-17 Thread Steffan Karger
Hi Pieter, [ Adding in -devel, because this really is more of a devel topic. ] On 15-03-19 15:29, Pieter Hulshoff wrote: > I was wondering why the authentication tag is transmitted before the > encrypted data in stead of after it (like in e.g. MACsec). As far as I understand, mostly because the

Re: [Openvpn-devel] [PATCH] Fix documentation of tls-verify script argument

2019-02-23 Thread Steffan Karger
many > This is indeed what we do. Thanks for the fix. Acked-by: Steffan Karger -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix broken fragment/mssfix with NCP

2019-01-19 Thread Steffan Karger
Hi, On 12-11-18 15:16, Lev Stipakov wrote: > From: Lev Stipakov > > NCP negotiation replaces worst cast crypto overhead > with actual one in data channel frame. That frame > params are used by mssfix. > > Fragment frame still contains worst case overhead. > Because of that TCP packets are

[Openvpn-devel] [PATCH v2] Fix tls-auth/crypt in connection blocks with --persist-key

2019-01-19 Thread Steffan Karger
:01 2019 Exiting due to fatal error Fix that by loading loading the key from the current connection entry. Signed-off-by: Steffan Karger --- v2: Also fix tls-crypt, not just tls-auth. src/openvpn/options.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/openvpn

[Openvpn-devel] [PATCH] Fix tls-auth/crypt in connection blocks with --persist-key

2019-01-19 Thread Steffan Karger
:01 2019 Exiting due to fatal error Fix that by loading loading the key from the current connection entry. Signed-off-by: Steffan Karger --- src/openvpn/options.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 0cf8db767

Re: [Openvpn-devel] [PATCH 2/6] Refactor tls_crypt_v2_write_server_key_file into crypto.c

2019-01-16 Thread Steffan Karger
_pem_name, _key_pem, > - _key_buf, )) > -{ > -msg(M_WARN, "ERROR: could not PEM-encode server key"); > -goto cleanup; > -} > - > -if (!buffer_write_file(filename, _key_pem)) > -{ > - msg(M_ERR, "ERR

[Openvpn-devel] [PATCH] Extend tls-crypt-v2 unit tests

2019-01-16 Thread Steffan Karger
-by: Steffan Karger --- tests/unit_tests/openvpn/Makefile.am | 6 +- tests/unit_tests/openvpn/test_tls_crypt.c | 89 ++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index b4304e35

Re: [Openvpn-devel] [PATCH 1/6] Fix loading inline tls-crypt-v2 keys with mbed TLS

2019-01-16 Thread Steffan Karger
nse. Thanks for fixing my bugs. I think we should try to add regression tests for bugs we encounter (if reasonably doable), but let's not postpone merging the bugfix for that. I'll sent a patch with a regression test later on. Acked-by: Steffan Karger -Steffan __

Re: [Openvpn-devel] RfD: printing of port numbers on v6 addresses

2018-12-26 Thread Steffan Karger
On 19-12-18 00:09, Antonio Quartulli wrote: > I personally prefer the rfc3986 notation because it is more widespread > and, therefore, easier to understand/recognize. +1 -Steffan signature.asc Description: OpenPGP digital signature ___

Re: [Openvpn-devel] [PATCH v2 1/2] Make tls_version_max return the actual maximum version

2018-12-06 Thread Steffan Karger
On 05-12-18 15:09, Arne Schwabe wrote: > Am 05.12.18 um 11:51 schrieb Steffan Karger: >> On Wed, 31 Oct 2018 at 17:53, Arne Schwabe wrote: >>> Before OpenSSL 1.1.1 there could be no mismatch between >>> compiled and actual OpenSSL version. With OpenSSL 1.1.1 we

Re: [Openvpn-devel] [PATCH v2 1/2] Make tls_version_max return the actual maximum version

2018-12-05 Thread Steffan Karger
Hi, On Wed, 31 Oct 2018 at 17:53, Arne Schwabe wrote: > Before OpenSSL 1.1.1 there could be no mismatch between > compiled and actual OpenSSL version. With OpenSSL 1.1.1 we need > runtime detection to detect the actual best TLS version supported. > > Allowing this runtime detection also allows

Re: [Openvpn-devel] [PATCH] Add tls-crypt-v2 to the list of supported inline options

2018-12-04 Thread Steffan Karger
\-\-tls\-auth > +.B \-\-crl\-verify, \-\-http\-proxy\-user\-pass, \-\-tls\-auth, > +.B \-\-tls\-crypt, > and > -.B \-\-tls\-crypt > +.B \-\-tls\-crypt-v2 > options. > > Each inline file started by the line > Absolutely correct. Acked-by: Steffan Karger -Steffan __

Re: [Openvpn-devel] [PATCH v3] Make up/down script errors not FATAL

2018-11-11 Thread Steffan Karger
Hi, I've postponed replying to this mail a couple of times because I felt I missed something and needed to look closer, but today once again I don't get it. So here goes for a potentially stupid reply: On 05-10-18 17:30, Selva Nair wrote: > On Fri, Oct 5, 2018 at 5:44 AM Steffan Kar

[Openvpn-devel] [PATCH] tls-crypt-v2: fix client reconnect bug

2018-10-31 Thread Steffan Karger
. Once moved there, it is immediately clear that v2 didn't follow the same (new) logic. Signed-off-by: Steffan Karger --- src/openvpn/init.c | 44 +++- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/openvpn/init.c b/src/openvpn/init.c

  1   2   3   4   5   6   7   8   9   10   >