[Openvpn-devel] [PATCH applied] Re: Propagate route error to initialization_completed()
Acked-by: Gert Doering We discussed this previously, and it makes sense to take "route addition errors" into account, even if we consciously decided (long before I got involved...) that we consider these non-fatal, unlike ifconfig errors. I have stared at the code, and it looks reasonable (we discussed the approach before). Some of the lines are now longer than 80 characters, with "short continuation lines", like this +ret = add_bypass_routes(>spec.bypass, rl->rgi.gateway.addr, tt, flags, +>rgi, es, ctx); .. while our style guide permits "longer lines if the alternativ is more ugly", in this case actually rewrapping would be nicer (not a showstopper), like this: +ret = add_bypass_routes(>spec.bypass, rl->rgi.gateway.addr, +tt, flags >rgi, es, ctx); .. so two similarily-long lines, not one very long and one quite short. The rewrappings of the add_route3() calls are welcome :-) That it's only available for Windows and Linux/SITNL today is non-ideal, but I can see the motivation - for platforms where we can't distinguish "genuine errors" and "EEXIST errors", err on the side of "keep old behaviour, no new warnings". We did want a major "routing glue rewrite" for 2.6, which did not happen, so let's try again for 2.7... I have tested "things still work" by subjecting this to the full client/server test on Linux (with SITNL, with and without DCO), and run manual tests, as suggested openvpn --config base.ovpn --route 192.168.122.0 255.255.255.0 1.1.1.1 .. which ended unexpected... 2023-01-10 13:05:15 sitnl_send: rtnl: generic error (-101): Network is unreachable 2023-01-10 13:05:15 ERROR: Linux route add command failed 2023-01-10 13:05:15 Initialization Sequence Completed .. I did expect something about "with errors" here - and I think we *should* print something, given that many (most?) *Linux* users will not have a management-client-GUI running. On the management interface, it does work fine: state 1673352459,CONNECTED,ROUTE_ERROR,10.194.2.170,199.102.77.82,51194,,,fd00:abcd:194:2::1029 (and if there are no errors, it will display "CONNECTED,SUCCESS,") I have not tested what the Windows GUI will do or display in this case. Also, the EEXIST suppression of SITNL is really a bug that needs to be fixed. If I have duplicate routes... 2023-01-10 13:11:40 net_route_v4_add: 10.194.0.0/16 via 10.194.2.169 dev [NULL] table 0 metric -1 2023-01-10 13:11:40 net_route_v4_add: 10.194.0.0/16 via 10.194.2.169 dev [NULL] table 0 metric -1 .. installation just works fine, but... 2023-01-10 13:12:00 net_route_v4_del: 10.194.0.0/16 via 10.194.2.169 dev [NULL] table 0 metric -1 2023-01-10 13:12:00 net_route_v4_del: 10.194.0.0/16 via 10.194.2.169 dev [NULL] table 0 metric -1 2023-01-10 13:12:00 sitnl_send: rtnl: generic error (-3): No such process 2023-01-10 13:12:00 ERROR: Linux route delete command failed .. it does not understand that the 2nd addition actually *failed*, so tries to remove it, which creates an error that is in the wrong place, and fully avoidable. Antonio, you listening? Your patch has been applied to the master and release/2.6 branch. commit e04c253618ce2a1bb0996a67b81af891e8607fa9 (master) commit 705a08ee5adcc74d51bc096592d561f35dc8661a (release/2.6) Author: Selva Nair Date: Wed Jan 4 21:27:18 2023 -0500 Propagate route error to initialization_completed() Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <20230105022718.1641751-3-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25884.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 v4] Introduce dynamic tls-crypt for secure soft_reset/session renegotiation
On Mon, Jan 09, 2023 at 05:36:06PM +0100, Arne Schwabe wrote:
> Am 09.01.23 um 16:01 schrieb Frank Lichtenheld:
> > On Mon, Dec 12, 2022 at 12:27:45PM +0100, Arne Schwabe wrote:
> > > Currently we have only one slot for renegotiation of the session/keys.
> > > If a replayed/faked packet is inserted by a malicous attacker, the
> > > legimate peer cannot renegotiate anymore.
> > >
> > > This commit introduces dynamic tls-crypt. When both peer support this
> > > feature, both peer create a dynamic tls-crypt key using TLS EKM (export
> > > key
> >
> > "peers"
> >
> > > material) and will enforce using that key and tls-crypt for all
> > > renegotiations. This also add an additional protection layer for
> >
> > General question about this feature:
> > We trigger using this key on key_id > 0, so if I understand the code
> > correctly, it will be used first when we want to renegotiate.
> > But will it then continued to be used? What exactly is the state after
> > the successful renegotiation?
>
> That is one of this hidden logic things of OpenVPN again. We ensure that the
> keyid will go to 1 instead 0 on key rollover, so only initial keyids are 0.
> So key-ids for renegotiations will be 1 to 7 and then rollover to 1 instead
> of 0.
>
>
> /*
> * key_id increments to KEY_ID_MASK then recycles back to 1.
> * This way you know that if key_id is 0, it is the first key.
> */
> ++session->key_id;
> session->key_id &= P_KEY_ID_MASK;
> if (!session->key_id)
> {
> session->key_id = 1;
> }
Okay, so it does roughly what I assumed it does. But strictly speaking this
is not a renegotiation key then. Once the first renegotiation happens the key
will be used for ALL control channel packets, is that correct?
So how does that avoid the replay attack? I mean it obviously avoids it for the
first renegotiation, but could you replay the first renegotiation afterwards?
Or does that not work due to increasing key_id/packet_id?
Regards,
--
Frank Lichtenheld
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] xkey_pkcs11h_sign: fix dangling pointer
Warning by GCC 12:
pkcs11_openssl.c:237:22: warning:
dangling pointer ‘tbs’ to ‘enc’ may be used [-Wdangling-pointer=]
Signed-off-by: Frank Lichtenheld
---
src/openvpn/pkcs11_openssl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/openvpn/pkcs11_openssl.c b/src/openvpn/pkcs11_openssl.c
index 60bc1c47..ecf37ba0 100644
--- a/src/openvpn/pkcs11_openssl.c
+++ b/src/openvpn/pkcs11_openssl.c
@@ -169,6 +169,9 @@ xkey_pkcs11h_sign(void *handle, unsigned char *sig,
unsigned char buf[EVP_MAX_MD_SIZE];
size_t buflen;
+unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for DigestInfo
header */
+size_t enc_len = sizeof(enc);
+
if (!strcmp(sigalg.op, "DigestSign"))
{
msg(D_XKEY, "xkey_pkcs11h_sign: computing digest");
@@ -214,9 +217,6 @@ xkey_pkcs11h_sign(void *handle, unsigned char *sig,
{
/* CMA_RSA_PKCS needs pkcs1 encoded digest */
-unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for
DigestInfo header */
-size_t enc_len = sizeof(enc);
-
if (!encode_pkcs1(enc, _len, sigalg.mdname, tbs, tbslen))
{
return 0;
--
2.34.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Update copyright year to 2023
Acked-by: Gert Doering
"Automatic and really easy to verify" ("git show -I Copyright")
The patch seems to be too big for mail-archive.com to archive it
(wat?) - it's not visible there, so pointing URL: to patchwork.
Your patch has been applied to the master and release/2.6 branch.
commit ccf9d57249acb9bc9a450aec3e613bda631415f5 (master)
commit f8b4a8e396f29ba52043379b04f17df5467c28d5 (release/2.6)
Author: Frank Lichtenheld
Date: Tue Jan 10 17:05:31 2023 +0100
Update copyright year to 2023
Signed-off-by: Frank Lichtenheld
Acked-by: Gert Doering
Message-Id: <20230110160531.81010-1-fr...@lichtenheld.com>
URL:
https://patchwork.openvpn.net/project/openvpn2/patch/20230110160531.81010-1-fr...@lichtenheld.com/
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: xkey_pkcs11h_sign: fix dangling pointer
Haven't tested this beyond "does it compile on Github?" - it looks correct, though :-) Your patch has been applied to the master branch. commit 202b34da386c8574692111bad23814602d0e09f5 (master) commit 71f3a109f9f73f0d978f58e08caed896c064767f (release/2.6) Author: Frank Lichtenheld Date: Tue Jan 10 14:19:47 2023 +0100 xkey_pkcs11h_sign: fix dangling pointer Signed-off-by: Frank Lichtenheld Acked-by: Selva Nair Message-Id: <20230110131947.59552-1-fr...@lichtenheld.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25942.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 v2 4/4] Log peer-id if loglevel is D_DCO_DEBUG and dco is enabled
Hi,
On Tue, Dec 27, 2022 at 11:12:44AM +0100, Gert Doering wrote:
> Playing around with the patch a bit, the offending piece seems to be
> "mi->context.options.verbosity >= D_DCO_DEBUG" - which is unsurprising,
> as D_DCO_DEBUG is not "6" but "LOGLEV(6, 69, M_DEBUG)", which translates
> to
>
> #define LOGLEV(log_level, mute_level, other) ((log_level) |
> ENCODE_MUTE_LEVEL(mute_level) | other)
>
> (numerically, D_DCO_DEBUG = 1157628038)
... looking at error.h, there is a "check_debug_level(flags)" thing, so
changing the condition to
+if (mi->context.c2.tls_multi
+// && mi->context.options.verbosity >= D_DCO_DEBUG
+&& check_debug_level(D_DCO_DEBUG)
+&& dco_enabled(>context.options))
+{
+buf_printf(, " peer-id=%d", mi->context.c2.tls_multi->peer_id);
+}
works - "if --verb 6 or higher, *and* DCO, then have peer-id=%d as part
if the prefix".
Can you resend a v3?
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] check_engine_keys: make pass with OpenSSL 3
Not enabled by default with OpenSSL 3, so we don't
see this in our builds.
While here add missing entries to .gitignore (which
is what made me look at engine-key test in the first
place).
Signed-off-by: Frank Lichtenheld
---
.gitignore | 4
tests/unit_tests/engine-key/check_engine_keys.sh | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/.gitignore b/.gitignore
index 7335154f..813413fe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -68,6 +68,10 @@ tests/t_client-*-20??-??/
t_client.rc
t_client_ips.rc
tests/unit_tests/**/*_testdriver
+tests/unit_tests/engine-key/client.key
+tests/unit_tests/engine-key/log.txt
+tests/unit_tests/engine-key/openssl.cnf
+tests/unit_tests/engine-key/passwd
src/openvpn/openvpn
include/openvpn-plugin.h
diff --git a/tests/unit_tests/engine-key/check_engine_keys.sh
b/tests/unit_tests/engine-key/check_engine_keys.sh
index 7e9a0e80..12dd2301 100755
--- a/tests/unit_tests/engine-key/check_engine_keys.sh
+++ b/tests/unit_tests/engine-key/check_engine_keys.sh
@@ -27,7 +27,7 @@ ${top_builddir}/src/openvpn/openvpn --cd ${top_srcdir}/sample
--config sample-co
# first off check we died because of a key mismatch. If this doesn't
# pass, suspect openssl of returning different messages and update the
# test accordingly
-loggrep '(X509_check_private_key:key values
mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not
detected"; exit 1; }
+loggrep '(x509 certificate routines:(X509_check_private_key)?:key values
mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not
detected"; exit 1; }
# now look for the engine prints (these are under our control)
loggrep 'ENGINE: engine_init called' || { echo "Engine initialization not
detected"; exit 1; }
--
2.34.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] xkey_pkcs11h_sign: fix dangling pointer
Hi,
On Tue, Jan 10, 2023 at 8:21 AM Frank Lichtenheld
wrote:
> Warning by GCC 12:
> pkcs11_openssl.c:237:22: warning:
> dangling pointer ‘tbs’ to ‘enc’ may be used [-Wdangling-pointer=]
>
> Signed-off-by: Frank Lichtenheld
> ---
> src/openvpn/pkcs11_openssl.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/openvpn/pkcs11_openssl.c b/src/openvpn/pkcs11_openssl.c
> index 60bc1c47..ecf37ba0 100644
> --- a/src/openvpn/pkcs11_openssl.c
> +++ b/src/openvpn/pkcs11_openssl.c
> @@ -169,6 +169,9 @@ xkey_pkcs11h_sign(void *handle, unsigned char *sig,
> unsigned char buf[EVP_MAX_MD_SIZE];
> size_t buflen;
>
> +unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough for
> DigestInfo header */
> +size_t enc_len = sizeof(enc);
> +
> if (!strcmp(sigalg.op, "DigestSign"))
> {
> msg(D_XKEY, "xkey_pkcs11h_sign: computing digest");
> @@ -214,9 +217,6 @@ xkey_pkcs11h_sign(void *handle, unsigned char *sig,
> {
> /* CMA_RSA_PKCS needs pkcs1 encoded digest */
>
> -unsigned char enc[EVP_MAX_MD_SIZE + 32]; /* 32 bytes enough
> for DigestInfo header */
> -size_t enc_len = sizeof(enc);
> -
> if (!encode_pkcs1(enc, _len, sigalg.mdname, tbs, tbslen))
> {
> return 0;
>
I can't believe I wrote that nonsense in the first place. Fortunately
similar code in xkey_management_sign() is okay. Hard to catch in tests as
the pointer may still point to the right data after going out of scope
(dangling).
Acked-by: Selva Nair
Selva
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
Hi,
On Thu, Dec 29, 2022 at 12:27:46PM +0500, Vladislav Grishenko wrote:
> client will move on to the next connection entry.
>
> v15:
> rebase to master (Dec 2022)
> add optional port argument to --remote and --remote-srv usage message
> fix --proto option coexisting with --remote-srv
> fix --nobind option coexisting with --remote-srv
> fix options postprocess mutation lost in v13/v14
> recover --mtu-test handling with --remote-srv
> use explicit srv resolve stub for openbsd for the future
> fix comments
Getting close... but, alas, we need another rebase - the signal handling
fixes from Selva cause conflicts. Can you do a v16, please?
Also, please have a very close look at the code now - it looks like
the previous rebase is now creating quite a bit of (undesired!) code
duplication. For example, this new hunk:
+static bool
+options_postprocess_verify_ce_proto(const struct options *options,
+const struct connection_entry *ce)
+{
+int msglevel = M_WARN|M_NOPREFIX|M_OPTERR;
+
+/*
+ * Sanity check on --local, --remote, and --ifconfig
+ */
+
+if (proto_is_net(ce->proto)
+&& string_defined_equal(ce->local, ce->remote)
+&& string_defined_equal(ce->local_port, ce->remote_port))
+{
+msg(msglevel, "--remote and --local addresses are the same");
+return false;
+}
+
+if (string_defined_equal(ce->remote, options->ifconfig_local)
+|| string_defined_equal(ce->remote, options->ifconfig_remote_netmask))
+{
+msg(msglevel,
+"--local and --remote addresses must be distinct from --ifconfig "
+"addresses");
+return false;
+}
... these checks are existing today, in options_postprocess_verify_ce(),
but your patch is not *moving* them to the new function, but *duplicating*
them. This can happen if "git --rebase" gets confused by too many
unrelated changes, but is not the desired end state.
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
Re: [Openvpn-devel] [PATCH] Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode
Hi,
On 09/01/2023 21:00, Gert Doering wrote:
p2p --tls-server with no active client/peer logs once per second
"dco_update_keys: peer_id=-1"
which does exactly nothing, except fill the disk. So skip the call to
dco_update_keys() if peer_id == -1.
Signed-off-by: Gert Doering
---
src/openvpn/forward.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index ae0512fc..2ba8b0fa 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -151,6 +151,12 @@ check_dco_key_status(struct context *c)
return;
}
+/* no active peer (p2p tls-server mode) */
+if (c->c2.tls_multi->dco_peer_id == -1 )
Please remove the space after -1 (not sure why uncrustify hasn't caught it).
+{
+return;
+}
+
if (!dco_update_keys(>c1.tuntap->dco, c->c2.tls_multi))
{
/* Something bad happened. Kill the connection to
Rest looks good. Thanks!
Acked-by: Antonio Quartulli
However, as discussed on IRC: *why* are we running the check_tls code is
the peer has gone away and we have switched the peer-id to -1?
This is the real question.
--
Antonio Quartulli
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode
Antonio, thanks for the review. Fixed the whitespace. (Uncrustify did not see it since the patch was ad-hoc written on a system that does not have the hook - but my pre-merge hook would have caught it). Patch has been applied to the master and release/2.6 branch. commit 85e0df6b493396d9d1d9030c4018f67037d2f12b (master) commit 9bc2dfea0e1f643b2a02b61455e1845ae83f7518 (release/2.6) Author: Gert Doering Date: Mon Jan 9 21:00:11 2023 +0100 Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode Signed-off-by: Gert Doering Acked-by: Antonio Quartulli Message-Id: <20230109200011.2525342-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25935.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 v2] Log peer-id if loglevel is D_DCO_DEBUG and dco is enabled
This enables logging the peer id in p2mp mode if dco is enabled
and the log level is high enough
Patch v2: use check_debug_level to check current log level
Signed-off-by: Arne Schwabe
---
src/openvpn/multi.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 26904859f..b639d3c67 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -476,6 +476,12 @@ multi_instance_string(const struct multi_instance *mi,
bool null, struct gc_aren
buf_printf(, "%s/", cn);
}
buf_printf(, "%s", mroute_addr_print(>real, gc));
+if (mi->context.c2.tls_multi
+&& check_debug_level(D_DCO_DEBUG)
+&& dco_enabled(>context.options))
+{
+buf_printf(, " peer-id=%d", mi->context.c2.tls_multi->peer_id);
+}
return BSTR();
}
else if (null)
--
2.37.1 (Apple Git-137.1)
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Log peer-id if loglevel is D_DCO_DEBUG and dco is enabled
Acked-by: Gert Doering Works and helps with DCO debugging. Some of the messages look a bit stupid now... so we might want to go and polish :-) .. gremlin14943/194.97.140.21:12404 peer-id=9 dco_update_keys: peer_id=9 .. gremlin14833/194.97.140.21:11036 peer-id=11 dco_update_keys: peer_id=11 .. gremlin14340/194.97.140.21:29500 peer-id=12 dco_update_keys: peer_id=12 Your patch has been applied to the master and release/2.6 branch. commit 533c170fb60882547152e3b3222c8f7788d6b80f (master) commit d04c2110a006c204b5753113cdbed2866b9641ed (release/2.6) Author: Arne Schwabe Date: Tue Jan 10 16:19:01 2023 +0100 Log peer-id if loglevel is D_DCO_DEBUG and dco is enabled Signed-off-by: Arne Schwabe Acked-by: Gert Doering Message-Id: <20230110151901.998479-1-a...@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25946.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] Conditions under which a connection entry could be disabled
Hi, On Tue, Jan 10, 2023 at 04:42:50PM -0500, Selva Nair wrote: > I'm trying to get this info into the GUI for handling > "--management-query-remote". Selecting a disabled entry from the UI would > lead to erratic behaviour. Ideally this info (CE_DISABLED state) should be > included in the response to "remote-entry-get" command, but too late for > that to happen in 2.6. It didn't occur to me when that command was > implemented. If you are REALLY fast we can bring it in _rc2 (planned for Thursday). This should not be a very intrusive change and the only consumer of the output today is the Windows GUI, so "compatibility" is not a problem here (right?). 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
Re: [Openvpn-devel] Conditions under which a connection entry could be disabled
On Tue, Jan 10, 2023 at 4:56 PM Gert Doering wrote: > Hi, > > On Tue, Jan 10, 2023 at 04:42:50PM -0500, Selva Nair wrote: > > I'm trying to get this info into the GUI for handling > > "--management-query-remote". Selecting a disabled entry from the UI > would > > lead to erratic behaviour. Ideally this info (CE_DISABLED state) should > be > > included in the response to "remote-entry-get" command, but too late for > > that to happen in 2.6. It didn't occur to me when that command was > > implemented. > > If you are REALLY fast we can bring it in _rc2 (planned for Thursday). > > This should not be a very intrusive change and the only consumer of > the output today is the Windows GUI, so "compatibility" is not a > problem here (right?). > There is no compat issue as this is a new management command in 2.6. The change is simple but testing takes time. Let me see if I can pull-it-off tonight. Selva > > 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 > ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] Conditions under which a connection entry could be disabled
correction: > (i) --proto-force is in effect : configs not matching with the forced protocol are disabled configs --> connection entries > (ii) --http-proxy-override : UDP profiles get disabled. profiles --> connection entries On Tue, Jan 10, 2023 at 4:42 PM Selva Nair wrote: > Hi, > > I see two situations under which a connection-entry (remote) could be > disabled while iterating through the list of remotes: > (i) --proto-force is in effect : configs not matching with the forced > protocol are disabled > (ii) --http-proxy-override : UDP profiles get disabled. > This looks like an unused option. Is it dead code no longer in use? > > Are there any other cases? > > I'm trying to get this info into the GUI for handling > "--management-query-remote". Selecting a disabled entry from the UI would > lead to erratic behaviour. Ideally this info (CE_DISABLED state) should be > included in the response to "remote-entry-get" command, but too late for > that to happen in 2.6. It didn't occur to me when that command was > implemented. > > We now have a config-parser in the GUI and can get the info from the > config file if we know for sure what conditions lead to the flag > CE_DISABLED. > > Thanks, > > Selva > ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v15] Add DNS SRV remote host discovery support
Hi, sure, will do.
Yes, I’ve noticed undesired code dup in v14 and have fixed everything found
in v15 rebase, same will be rechecked in v16 of course.
Thanks!
Ср, 11 янв. 2023 г. в 01:05, Gert Doering :
> Hi,
>
> On Thu, Dec 29, 2022 at 12:27:46PM +0500, Vladislav Grishenko wrote:
> > client will move on to the next connection entry.
> >
> > v15:
> > rebase to master (Dec 2022)
> > add optional port argument to --remote and --remote-srv usage message
> > fix --proto option coexisting with --remote-srv
> > fix --nobind option coexisting with --remote-srv
> > fix options postprocess mutation lost in v13/v14
> > recover --mtu-test handling with --remote-srv
> > use explicit srv resolve stub for openbsd for the future
> > fix comments
>
> Getting close... but, alas, we need another rebase - the signal handling
> fixes from Selva cause conflicts. Can you do a v16, please?
>
> Also, please have a very close look at the code now - it looks like
> the previous rebase is now creating quite a bit of (undesired!) code
> duplication. For example, this new hunk:
>
> +static bool
> +options_postprocess_verify_ce_proto(const struct options *options,
> +const struct connection_entry *ce)
> +{
> +int msglevel = M_WARN|M_NOPREFIX|M_OPTERR;
> +
> +/*
> + * Sanity check on --local, --remote, and --ifconfig
> + */
> +
> +if (proto_is_net(ce->proto)
> +&& string_defined_equal(ce->local, ce->remote)
> +&& string_defined_equal(ce->local_port, ce->remote_port))
> +{
> +msg(msglevel, "--remote and --local addresses are the same");
> +return false;
> +}
> +
> +if (string_defined_equal(ce->remote, options->ifconfig_local)
> +|| string_defined_equal(ce->remote,
> options->ifconfig_remote_netmask))
> +{
> +msg(msglevel,
> +"--local and --remote addresses must be distinct from
> --ifconfig "
> +"addresses");
> +return false;
> +}
>
>
> ... these checks are existing today, in options_postprocess_verify_ce(),
> but your patch is not *moving* them to the new function, but *duplicating*
> them. This can happen if "git --rebase" gets confused by too many
> unrelated changes, but is not the desired end state.
>
> 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
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
--
Best regards, Vladislav Grishenko
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] Conditions under which a connection entry could be disabled
Hi, I see two situations under which a connection-entry (remote) could be disabled while iterating through the list of remotes: (i) --proto-force is in effect : configs not matching with the forced protocol are disabled (ii) --http-proxy-override : UDP profiles get disabled. This looks like an unused option. Is it dead code no longer in use? Are there any other cases? I'm trying to get this info into the GUI for handling "--management-query-remote". Selecting a disabled entry from the UI would lead to erratic behaviour. Ideally this info (CE_DISABLED state) should be included in the response to "remote-entry-get" command, but too late for that to happen in 2.6. It didn't occur to me when that command was implemented. We now have a config-parser in the GUI and can get the info from the config file if we know for sure what conditions lead to the flag CE_DISABLED. Thanks, Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Include CE_DISABLED status of remote in "remote-entry-get" response
From: Selva Nair
- The response to the management command "remote-entry-get" is
amended to include the status of the remote entry. The status
reads "disabled" if (ce->flag & DISABLED) is true, "enabled"
otherwise.
- Update and correct the description of this option in
management-notes.txt
Example responses:
In response to "remote-entry-get 0"
0,vpn.example.com,udp,enabled
END
Or, in response to "remote-entry-get all"
0,vpn.example.org,udp,enabled
2,vpn.example.net,tcp-client,disabled
1,vpn.example.com,udp,enabled
END
This helps the management client to show only enabled remotes
to the user.
An alternative would require the UI/GUI to have knowledge of
what makes the daemon set CE_DISABLED (--proto-force,
--htttp-proxy-override etc.).
Signed-off-by: Selva Nair
---
doc/management-notes.txt | 23 +--
src/openvpn/init.c | 8 +---
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 6daa811a..34f301db 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -806,9 +806,12 @@ COMMAND -- remote-entry-get (OpenVPN 2.6+ management
version > 3)
remote-entry-get []
-Retrieve remote entry (host, port and protocol) for index
- or indices from to +1. Alternatively
- = "all" retrieves all remote entries.
+Retrieve remote entry (host, port, protocol, and status) for index
+ or indices from to -1. Alternatively
+ = "all" retrieves all remote entries. The index is 0-based.
+If the entry is disabled due to protocol or proxy restrictions
+(i.e., ce->flag & CE_DISABLED == 1), the status is returned as "disabled",
+otherwise it reads "enabled" without quotes.
Example 1:
@@ -818,8 +821,8 @@ Example 1:
OpenVPN daemon responds with
- 1,vpn.example.com,1194,udp
- END
+1,vpn.example.com,1194,udp,enabled
+END
Example 2:
@@ -829,8 +832,8 @@ Example 2:
OpenVPN daemon responds with
-1,vpn.example.com,1194,udp
-2,vpn.example.net,443,tcp-client
+1,vpn.example.com,1194,udp,enabled
+2,vpn.example.net,443,tcp-client,disabled
END
Example 3:
@@ -840,9 +843,9 @@ Example 3:
OpenVPN daemon with 3 connection entries responds with
-1,vpn.example.com,1194,udp
-2,vpn.example.com,443,tcp-client
-3,vpn.example.net,443,udp
+0,vpn.example.com,1194,udp,enabled
+1,vpn.example.com,443,tcp-client,enabled
+2,vpn.example.net,443,udp,enabled
END
COMMAND -- remote (OpenVPN AS 2.1.5/OpenVPN 2.3 or higher)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index fc1943bc..c8651232 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -353,13 +353,15 @@ management_callback_remote_entry_get(void *arg, unsigned
int index, char **remot
{
struct connection_entry *ce = l->array[index];
const char *proto = proto2ascii(ce->proto, ce->af, false);
+const char *status = (ce->flags & CE_DISABLED) ? "disabled" :
"enabled";
-/* space for output including 2 commas and a nul */
-int len = strlen(ce->remote) + strlen(ce->remote_port) + strlen(proto)
+ 2 + 1;
+/* space for output including 3 commas and a nul */
+int len = strlen(ce->remote) + strlen(ce->remote_port) + strlen(proto)
+ + strlen(status) + 3 + 1;
char *out = malloc(len);
check_malloc_return(out);
-openvpn_snprintf(out, len, "%s,%s,%s", ce->remote, ce->remote_port,
proto);
+openvpn_snprintf(out, len, "%s,%s,%s,%s", ce->remote, ce->remote_port,
proto, status);
*remote = out;
}
else
--
2.34.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Include CE_DISABLED status of remote in "remote-entry-get" response
Error in commit message: 0,vpn.example.org,udp,enabled > 2,vpn.example.net,tcp-client,disabled > 1,vpn.example.com,udp,enabled > That should have been 0,vpn.example.org,udp,enabled 1,vpn.example.net,tcp-client,disabled 2,vpn.example.com,udp,enabled with indices 0, 1, 2 ordered. ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Include CE_DISABLED status of remote in remote-entry-get response
Acked-by: Gert Doering This is really straightforward. Tested with my .ovpn full with generated "remote" lines, some of them changed to "tcp", and "--proto-force tcp-client" .. 190,1185.server.org,1185,udp,disabled 191,1186.server.org,1186,udp,disabled 192,1187.server.org,1187,udp,disabled 193,1188.server.org,1188,tcp-client,enabled 194,1189.server.org,1189,tcp-client,enabled 195,1190.server.org,1190,tcp-client,enabled (NOTE: "--proto-force tcp" will surprisingly - to me - disable *everything*, because "client side TCP profiles are 'tcp-client', not 'tcp'") This said, the code with all these strlen() and malloc() calls for every single remote looks fumbly... given that it's not exactly geared for maxmimum memory efficientcy, I'm tempted to suggest just having an on-the-stack "char remote[1000];" in man_remote_entry_get(), and pass a pointer to that + sizeof() to "...callback.remote_entry_count()". But this is something for the next round of cleanup in early 2.7. I have reordered the example lines in the commit message. Your patch has been applied to the master and release/2.6 branch. commit eafbedc583c48fd46405fa0d635c688ce59c3733 (master) commit 69383817b9805137460c0e25ede4b18573cac01d (release/2.6) Author: Selva Nair Date: Wed Jan 11 01:29:10 2023 -0500 Include CE_DISABLED status of remote in remote-entry-get response Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <20230111062910.1846688-1-selva.n...@gmail.com> URL: https://www.mail-archive.com/search?l=mid=20230111062910.1846688-1-selva.n...@gmail.com 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] check_engine_keys: make pass with OpenSSL 3
Hi,
On Tue, Jan 10, 2023 at 06:02:57PM +0100, Frank Lichtenheld wrote:
> @@ -27,7 +27,7 @@ ${top_builddir}/src/openvpn/openvpn --cd
> ${top_srcdir}/sample --config sample-co
> # first off check we died because of a key mismatch. If this doesn't
> # pass, suspect openssl of returning different messages and update the
> # test accordingly
> -loggrep '(X509_check_private_key:key values
> mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not
> detected"; exit 1; }
> +loggrep '(x509 certificate routines:(X509_check_private_key)?:key values
> mismatch|func\(128\):reason\(116\))' log.txt || { echo "Key mismatch not
> detected"; exit 1; }
This change does not convince me - assuming normal regex here, the
original grep would have found any lines with
X509_check_private_key:key values mismatch
in them, while the new one would find
X509_check_private_key::key values mismatch
X509_check_private_key:X509_check_private_key:key values mismatch
but not "the old one with just one :"
Should this be
(x509 certificate routines:(X509_check_private_key:)?key va...
with the second ":" part of the (...:)? zero-or-once match?
(... as a side note, I hate this test... it's easily the test with the
most commits to make it behave across platforms and SSL library versions)
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
