Hi,
TL;DR: I don't think this should be merged yet. My primary concern is
that we don't have any means to limit key usage to a safe value. I
raised this concern back in December 2023:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg27791.html
If we allow for packet counte
We have D_ROUTE for route addition/deletion messages, which prints at
loglevel 3. Use that for IPv6, like we do for IPv4 to reduce terminal
spam for non-legacy-networking setups. Prvious code would print the
messages at --verb 1.
Signed-off-by: Steffan Karger
---
src/openvpn/route.c | 6
Hi,
I've been just lurking for a while, but you've managed to nerd-snipe me
in responding.
On 11-12-2023 13:31, Arne Schwabe wrote:
with DCO and possible future hardware assisted OpenVPN acceleration we
are approaching the point where 32 bit IVs are not cutting it any more.
Agreed. Though t
can
replace t_cltsrv.sh (as a much faster alternative), but does require
python3 and pytest to run.
Signed-off-by: Steffan Karger
---
This is an RFC to get comments on whether you believe this approach is a
useful addition to our current test suite. We can add many more useful
tests once we agree
Hi,
On Mon, 15 Aug 2022 at 12:50, Gert Doering wrote:
> On Mon, Aug 15, 2022 at 12:40:55PM +0200, Timo Rothenpieler wrote:
> > or don't even try to retain capabilities, since
> > they're not needed either way. I'd prefer the later, since fewer
> > capabilities is generally better.
>
> I could see
Hi,
On 30-12-2021 18:28, Arne Schwabe wrote:
> That BF-CBC seems have an extra 8 bytes that I somehow missed. CBC is a
> odd since it always gives you a multiple of the blocksize (64 bit or 8
> byte) and if you evenly divide by the blocksize you get an extra block
> just for the padding. I need to
0 +912,7 @@ port_share_open(const char *host,
>
> /* no blocking on control channel back to parent */
> set_nonblock(fd[1]);
> -
> - /* initialize prng */
> -prng_init(NULL, 0);
> -
> +
> /* execute the event loop */
Traili
Hi,
Just a nit:
On 19-10-2021 20:31, Arne Schwabe wrote:
> +if (!SSL_CTX_set0_tmp_dh_pkey(ctx->ctx, dh))
> +{
> +crypto_msg(M_FATAL, "SSL_CTX_set_tmp_dh");
> +}
This error message looks incorrect and incomplete.
-Steffan
___
Open
Hi,
On Fri, 2 Jul 2021 at 12:12, Gert Doering wrote:
> On Thu, Jul 01, 2021 at 07:14:58PM +0200, Max Fillinger wrote:
> > Replace open...@fox-it.com with open...@foxcrypto.com.
> >
> > Signed-off-by: Max Fillinger
>
> Generally speaking, this is fine, I just have a small issue with
> "who can re
Hi,
On 06-04-2021 12:55, Maximilian Fillinger wrote:
>> Am 02.04.21 um 15:26 schrieb Max Fillinger:
>>> From: Uipko Berghuis
>>>
>>> In mbedtls 2.16.0 mbedtls_ctr_drbg_update() changed to
>>> mbedtls_ctr_drbg_update_ret(). Change the function name and handle the
>>> new return value error code.
>
Hi,
On 07-04-2021 17:50, Antonio Quartulli wrote:
> On 26/07/2020 15:31, Arne Schwabe wrote:
>> Am 26.07.20 um 02:01 schrieb Arne Schwabe:
>>> Am 17.07.20 um 19:10 schrieb David Sommerseth:
The --no-replay feature is considered to be a security weakness, which
was also highlighed during
Hi,
On 02-04-2021 20:16, Gert Doering wrote:
> Your patch has been applied to the master branch.
>
> I have never looked into this reliable stuff before, and do not have
> a test environment with a) significant amounts of control plane traffic,
> and b) "just the right amount" of packet loss. So
Hi,
On 10-04-2021 01:42, Arne Schwabe wrote:
>
> Am 09.04.2021 um 18:28 schrieb Gert Doering:
>> Hi,
>>
>> there was a big discussion on the IRC channel today about interactions
>> between "--chroot" and "--persist-key" and how and when stuff is reloaded
>> or not.
>>
>> Now, we all seem to agree
or.h
> @@ -67,7 +67,6 @@ bool dont_mute(unsigned int flags);
> #endif
> #define EXIT_FATAL(flags) do { if ((flags) & M_FATAL) {_exit(1);}} while
> (false)
>
> -#define HAVE_VARARG_MACROS
> #define msg(flags, ...) do { if (msg_test(flags)) {x_msg((flags),
> __VA_ARGS__);} EXIT_FATAL(flags); } while (false)
> #ifdef ENABLE_DEBUG
> #define dmsg(flags, ...) do { if (msg_test(flags)) {x_msg((flags),
> __VA_ARGS__);} EXIT_FATAL(flags); } while (false)
>
Thanks for doing the research. I wrote a patch like this a while ago,
but ever submitted it because I "just had to double check"...
Patch looks good and passes a configure-make-check cycle.
Acked-by: Steffan Karger
-Steffan
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
e8..cf9714593 100644
> --- a/src/openvpn/syshead.h
> +++ b/src/openvpn/syshead.h
> @@ -392,8 +392,6 @@ typedef int MIB_TCP_STATE;
> #ifdef PEDANTIC
> #undef HAVE_CPP_VARARG_MACRO_GCC
> #undef HAVE_CPP_VARARG_MACRO_ISO
> -#undef EMPTY_ARRAY_SIZE
> -#
generating debug log if Curve25591 is
> + * selected for DH (https://github.com/ARMmbed/mbedtls/issues/4208) */
> +if (session->opt->ssl_flags & SSLF_TLS_DEBUG_ENABLED)
> +{
> + mbedtls_debug_set_threshold(3);
> +}
> +else
> +{
> +
Hi,
On 08-03-2021 15:21, Arne Schwabe wrote:
> mbed TLS 2.25 has a nasty bug that the print function for Montgomery style
> EC curves (Curve25519 and Curve448) does segfault. See also the issue
> reported here: https://github.com/ARMmbed/mbedtls/issues/4208
>
> We request always debug level 3 fro
Hi,
On 21-12-2020 21:25, Arne Schwabe wrote:
> Am 21.12.20 um 20:11 schrieb Gert Doering:
>> On Mon, Dec 21, 2020 at 06:24:36PM +, Greg Cox wrote:
>>> If the software were
>>> to contain a mechanism to make certain failure cases automatically more
>>> prominent, particularly for 'simple' users
t;)
> -|| strprefix(p1, "tls-auth ")
> +|| streq(p1, "tls-auth")
> || strprefix(p1, "tun-ipv6")
> || strprefix(p1, "cipher "))
> {
>
Thanks for fixing this.
Acked-by: Steffan Karger
-Steffan
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
with tls-crypt-v2, connect a client, stop the server.
Signed-off-by: Steffan Karger
---
v2: remove now-redundant free_key_ctx() from key_schedule_free()
src/openvpn/init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 27a
-crypt-v2 patches were never amended to
implement the pre-loading.
Also as with the previous patch, it would be nicer if servers would not
reload the tls-crypt-v2 server key for each connecting client. But let's
first fix the issue, and see if we can improve later.
Signed-off-by: Steffan K
with tls-crypt-v2, connect a client, stop the server.
Signed-off-by: Steffan Karger
---
src/openvpn/init.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 27a4170d..5cde8a4b 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3619,6 +3
> P_OPCODE_SHIFT));
> +}
> }
> else if (len >= 2)
> {
> -return p[0] == 0 && p[1] >= 14;
> +int plen = (p[0] << 8) | p[1];
> +return plen >= 14 && plen <= 255;
> }
> else
>
Hi,
On 20-11-2020 17:04, Arne Schwabe wrote:
> The port-share option assumed that all openvpn initial reset packets
> are between 14 and 255 bytes long. This is not true for tls-crypt-v2.
Good catch, I totally missed this in the tls-crypt-v2 patch set.
> ---
> src/openvpn/ps.c | 34
> /* 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
___
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: St
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
ial exporter [RFC5705] */
> +#define IV_PROTO_TLS_KEY_EXPORT (1<<3)
>
> /* Default field in X509 to be username */
> #define X509_USERNAME_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
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 of
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 for
> {
> @@ -1891,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(&ks->crypto_options.key_ctx_bi,
> -&session->opt->key_type, ks->key_src,
> client_sid, server_sid,
> -session->opt->server))
> +
> +if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi, session))
Since you're passing session now, shouldn't you remove the key_ctx_bi
argument too? That's also part of session: (struct key_state) *ks =
&session->key[KS_PRIMARY].
> {
> msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed");
> goto cleanup;
>
Otherwise this looks good. I can also live with leaving key_ctx_bi in
for now. The current version of the patch is a clear improvement (and
paving the way for 2/2, ofc.)
So, as long as at least the whitespace is fixed:
Acked-by: Steffan Karger
-Steffan
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
t; +{
> +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 &&
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 V2
gt; - session->opt->ekm_label,
> - 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, &gc);
> -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(&gc);
> +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
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 s
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 ex
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 must_ne
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.
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 m
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(-)
di
11,7 +111,7 @@ mutate_ncp_cipher_list(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_W
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 decid
S */
> -ASSERT(0);
> -#endif
> }
>
> 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 @@ ty
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 h
gt; -msg(D_TLS_DEBUG, "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
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
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, &ad_start))
> {
> /* Restore pre-NCP frame parameters */
> -
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 suppor
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:
> Ev
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
>> k
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 K
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 (session->
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
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
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
>>>
0ce0 100644
> --- a/src/openvpn/ssl_ncp.c
> +++ 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(&new_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
c = 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
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 ciphe
avior would happen if generate_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
_wrap_buffer_write_file, pem, test_client_key,
> + strlen(test_client_key));
> will_return(__wrap_buffer_write_file, true);
>
> /* Key generation re-reads the created file as a sanity check */
> @@ -580,7 +582,8 @@ test_tls_crypt_v2_write_client_key_fil
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
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) {
> +main(
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
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 inst
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 uncrustify
test_server_key);
> }
>
> +
> int
> main(void) {
> const struct CMUnitTest tests[] = {
> @@ -576,6 +613,7 @@ main(void) {
> test_tls_crypt_v2_teardown),
> cmocka_unit_tes
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
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
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
>
Good catch. And apologies for the silly bug. Patch 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
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
if (cipher_kt_insecure(kt->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.
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 init_key_
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
Openvpn-devel@l
Hi,
On 01-02-2020 13:43, Илья Шипицин wrote:
> https://travis-ci.org/chipitsine/openvpn/jobs/644745481?utm_medium=notification&utm_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 n
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 w
e and link 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
--
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 rej
t-code goes, 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
_
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 cl
.
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
ff-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-
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 opt
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 so
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 to
> index ff7a5bb..0a24e5e 100644
> --- 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
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 patches.
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 c
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, otherwi
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
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
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 wou
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
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
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)
> {
>
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 ol
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-d
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
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
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 o
1 - 100 of 1428 matches
Mail list logo