Re: [Openvpn-devel] [PATCH v3] tapctl: generate driver-specific adapter names

2023-05-19 Thread Selva Nair 
Acked-by: Selva Nair 

On Fri, May 19, 2023 at 4:27 AM Lev Stipakov  wrote:

> From: Lev Stipakov 
>
> At the moment if --name is not specified, adapter names
> are generated by Windows and they look a bit confusing
> like "Local Area Connection 2".
>
> This is also behavior of "Add a new  virtual network
> adapter" shortcuts.
>
> This makes tapctl generate driver-specific names for adapters
> if --name is missing, inclusing resolving duplicates. For instance
> following commands:
>
>   tapctl.exe create --hwid ovpn-dco
>
> will create an adapter named
>
>   OpenVPN Data Channel Offload
>
> If the name is taken, the next one will be
>
>   OpenVPN Data Channel Offload #1
>
> and so on up to 100.
>
> Fixes https://github.com/OpenVPN/openvpn/issues/337
>
> Change-Id: Ic5afb470d14ac7b231d91f0f5de0a0046043a7e0
> Signed-off-by: Lev Stipakov 
> ---
>  v3:
>   - use _stprintf_s instead of _tcscat_s
>   - make functions static
>   - ensure iResult is always assigned
>
>  v2: fix MinGW compilation
>
>  src/tapctl/main.c | 132 --
>  1 file changed, 105 insertions(+), 27 deletions(-)
>
> diff --git a/src/tapctl/main.c b/src/tapctl/main.c
> index 1194036f..d76d553c 100644
> --- a/src/tapctl/main.c
> +++ b/src/tapctl/main.c
> @@ -126,6 +126,85 @@ usage(void)
>title_string);
>  }
>
> +/**
> + * Checks if adapter with given name doesn't already exist
> + */
> +static BOOL
> +is_adapter_name_available(LPCTSTR name, struct tap_adapter_node
> *adapter_list, BOOL log)
> +{
> +for (struct tap_adapter_node *a = adapter_list; a; a = a->pNext)
> +{
> +if (_tcsicmp(name, a->szName) == 0)
> +{
> +if (log)
> +{
> +LPOLESTR adapter_id = NULL;
> +StringFromIID((REFIID)>guid, _id);
> +_ftprintf(stderr, TEXT("Adapter \"%") TEXT(PRIsLPTSTR)
> TEXT("\" already exists (GUID %")
> +  TEXT(PRIsLPOLESTR) TEXT(").\n"), a->szName,
> adapter_id);
> +CoTaskMemFree(adapter_id);
> +}
> +
> +return FALSE;
> +}
> +}
> +
> +return TRUE;
> +}
> +
> +/**
> + * Returns unique adapter name based on hwid or NULL if name cannot be
> generated.
> + * Caller is responsible for freeing it.
> + */
> +static LPTSTR
> +get_unique_adapter_name(LPCTSTR hwid, struct tap_adapter_node
> *adapter_list)
> +{
> +if (hwid == NULL)
> +{
> +return NULL;
> +}
> +
> +LPCTSTR base_name;
> +if (_tcsicmp(hwid, TEXT("ovpn-dco")) == 0)
> +{
> +base_name = TEXT("OpenVPN Data Channel Offload");
> +}
> +else if (_tcsicmp(hwid, TEXT("wintun")) == 0)
> +{
> +base_name = TEXT("OpenVPN Wintun");
> +}
> +else if (_tcsicmp(hwid, TEXT("root\\") TEXT(TAP_WIN_COMPONENT_ID)) ==
> 0)
> +{
> +base_name = TEXT("OpenVPN TAP-Windows6");
> +}
> +else
> +{
> +return NULL;
> +}
> +
> +if (is_adapter_name_available(base_name, adapter_list, FALSE))
> +{
> +return _tcsdup(base_name);
> +}
> +
> +size_t name_len = _tcslen(base_name) + 10;
> +LPTSTR name = malloc(name_len * sizeof(TCHAR));
> +if (name == NULL)
> +{
> +return NULL;
> +}
> +for (int i = 1; i < 100; ++i)
> +{
> +_stprintf_s(name, name_len, TEXT("%ls #%d"), base_name, i);
> +
> +if (is_adapter_name_available(name, adapter_list, FALSE))
> +{
> +return name;
> +}
> +}
> +
> +return NULL;
> +}
>
>  /**
>   * Program entry point
> @@ -210,50 +289,49 @@ _tmain(int argc, LPCTSTR argv[])
>  iResult = 1; goto quit;
>  }
>
> -if (szName)
> +/* Get existing network adapters. */
> +struct tap_adapter_node *pAdapterList = NULL;
> +dwResult = tap_list_adapters(NULL, NULL, );
> +if (dwResult != ERROR_SUCCESS)
>  {
> -/* Get existing network adapters. */
> -struct tap_adapter_node *pAdapterList = NULL;
> -dwResult = tap_list_adapters(NULL, NULL, );
> -if (dwResult != ERROR_SUCCESS)
> -{
> -_ftprintf(stderr, TEXT("Enumerating adapters failed
> (error 0x%x).\n"), dwResult);
> -iResult = 1; goto create_delete_adapter;
> -}
> +_ftprintf(stderr, TEXT("Enumerating adapters failed (error
> 0x%x).\n"), dwResult);
> +iResult = 1;
> +goto create_delete_adapter;
> +}
>
> -/* Check for duplicates. */
> -for (struct tap_adapter_node *pAdapter = pAdapterList;
> pAdapter; pAdapter = pAdapter->pNext)
> +LPTSTR adapter_name = szName ? _tcsdup(szName) :
> get_unique_adapter_name(szHwId, pAdapterList);
> +if (adapter_name)
> +{
> +/* Check for duplicates when name was specified,
> + * otherwise get_adapter_default_name() takes care of it */
> +

Re: [Openvpn-devel] [PATCH 2/2] Fix CR_RESPONSE mangaement message using wrong key_id

2023-05-19 Thread Selva Nair 
Hi,

While this bugfix should be merged, I'm a conflicted about the way these
two patches are split up. It just makes reviewing harder than it should be.
They actually form two independent changes but with one half intersecting
with the other for no reason.

On Wed, May 17, 2023 at 7:03 AM Arne Schwabe  wrote:

> the management interface expects the management key id instead
> of the openvpn key id. In the past they often were the same for low ids
> which hid the bug quite well.
>
> Also do not pick uninitialised keystates (management key_id is not valid
> in these) and provide better logging when a key state is not found.
>
> Change-Id: If9fa1165a0e886b570b3738546ed810a32367cbe
> Signed-off-by: Arne Schwabe 
> ---
>  src/openvpn/push.c   | 4 ++--
>  src/openvpn/ssl_common.h | 2 +-
>  src/openvpn/ssl_verify.c | 5 +
>  3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/openvpn/push.c b/src/openvpn/push.c
> index 8e9627199..8f0a534ac 100644
> --- a/src/openvpn/push.c
> +++ b/src/openvpn/push.c
> @@ -267,9 +267,9 @@ receive_cr_response(struct context *c, const struct
> buffer *buffer)
>  struct tls_session *session = >c2.tls_multi->session[TM_ACTIVE];
>  struct man_def_auth_context *mda = session->opt->mda_context;
>  struct env_set *es = session->opt->es;
> -int key_id = get_primary_key(c->c2.tls_multi)->key_id;
> +unsigned int mda_key_id =
> get_primary_key(c->c2.tls_multi)->mda_key_id;
>
> -management_notify_client_cr_response(key_id, mda, es, m);
> +management_notify_client_cr_response(mda_key_id, mda, es, m);
>  #endif
>  #if ENABLE_PLUGIN

 verify_crresponse_plugin(c->c2.tls_multi, m);
>

I think, this patch could end here the bugfix is complete without the need
for 1/2.

The rest of the patch depends on 1/2, but is not required for the above. It
could be combined with 1/2 and submitted as a follow up refactoring patch.


> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
> index ebfd25432..be0f18746 100644
> --- a/src/openvpn/ssl_common.h
> +++ b/src/openvpn/ssl_common.h
> @@ -733,7 +733,7 @@ get_key_by_management_key_id(struct tls_multi *multi,
> unsigned int mda_key_id)
>  for (int i = 0; i < KEY_SCAN_SIZE; ++i)
>  {
>  struct key_state *ks = get_key_scan(multi, i);
> -if (ks->mda_key_id == mda_key_id)
> +if (ks->mda_key_id == mda_key_id && ks->state > S_UNDEF)
>  {
>  return ks;
>  }
> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index 4c78c2b2c..567f55ea3 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/openvpn/ssl_verify.c
> @@ -1279,6 +1279,11 @@ tls_authenticate_key(struct tls_multi *multi, const
> unsigned int mda_key_id, con
>  {
>  ks->mda_status = auth ? ACF_SUCCEEDED : ACF_FAILED;
>  }
> +else
> +{
> +msg(D_TLS_DEBUG_LOW, "%s: no key state found for management
> key id "
> +"%d", __func__, mda_key_id);
> +}
>  }
>  return (bool) ks;
>  }


 Selva
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/2] Remove contribution from Jason A. Donenfeld

2023-05-19 Thread Arne Schwabe 
This reverts commit 423ced962db3129b4ed551c489624faba4340652, which
has Jason A. Donenfeld as author.

Jason has expressed that he does not want to be bothered with the
license change of OpenVPN and unfortunately that leaves us no alternative
other than to remove his contribution from OpenVPN in order to be able to
go forward with the license change.

Change-Id: I8142753928498169032450c56d0497a5042bdc9b
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c   |  1 -
 src/openvpn/options.c| 26 +-
 src/openvpn/options.h|  1 -
 src/openvpn/ssl_common.h |  1 -
 src/openvpn/ssl_verify_mbedtls.c | 16 
 src/openvpn/ssl_verify_openssl.c |  2 +-
 6 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d358ad003..c023b33c6 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3347,7 +3347,6 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
 to.verify_hash_depth = options->verify_hash_depth;
-to.verify_hash_no_ca = options->verify_hash_no_ca;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e4c596b89..fe9285384 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2991,11 +2991,21 @@ options_postprocess_verify_ce(const struct options 
*options,
 else
 {
 #ifdef ENABLE_CRYPTO_MBEDTLS
+if (!(options->ca_file))
+{
+msg(M_USAGE, "You must define CA file (--ca)");
+}
+
 if (options->ca_path)
 {
 msg(M_USAGE, "Parameter --capath cannot be used with the mbed 
TLS version version of OpenVPN.");
 }
-#endif  /* ifdef ENABLE_CRYPTO_MBEDTLS */
+#else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
+if ((!(options->ca_file)) && (!(options->ca_path)))
+{
+msg(M_USAGE, "You must define CA file (--ca) or CA path 
(--capath)");
+}
+#endif
 if (pull)
 {
 
@@ -3727,13 +3737,6 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 options_postprocess_http_proxy_override(o);
 }
 #endif
-if (!o->ca_file && !o->ca_path && o->verify_hash
-&& o->verify_hash_depth == 0)
-{
-msg(M_INFO, "Using certificate fingerprint to verify peer (no CA "
-"option set). ");
-o->verify_hash_no_ca = true;
-}
 
 if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP)
 {
@@ -4029,11 +4032,8 @@ options_postprocess_filechecks(struct options *options)
 errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE,
  options->dh_file, R_OK, "--dh");
 
-if (!options->verify_hash_no_ca)
-{
-errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
- options->ca_file, R_OK, "--ca");
-}
+errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
+ options->ca_file, R_OK, "--ca");
 
 errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,
  options->ca_path, R_OK, "--capath");
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index f5890b90f..95f1158a4 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -604,7 +604,6 @@ struct options
 struct verify_hash_list *verify_hash;
 hash_algo_type verify_hash_algo;
 int verify_hash_depth;
-bool verify_hash_no_ca;
 unsigned int ssl_flags; /* set to SSLF_x flags from ssl.h */
 
 #ifdef ENABLE_PKCS11
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 27b029479..c0b3caa71 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -345,7 +345,6 @@ struct tls_options
 const char *remote_cert_eku;
 struct verify_hash_list *verify_hash;
 int verify_hash_depth;
-bool verify_hash_no_ca;
 hash_algo_type verify_hash_algo;
 #ifdef ENABLE_X509ALTUSERNAME
 char *x509_username_field[MAX_PARMS];
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index e3437f740..c9ef7a171 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -62,22 +62,6 @@ verify_callback(void *session_obj, mbedtls_x509_crt *cert, 
int cert_depth,
 struct buffer cert_fingerprint = x509_get_sha256_fingerprint(cert, );
 cert_hash_remember(session, cert_depth, _fingerprint);
 
-if (session->opt->verify_hash_no_ca)
-{
-/*
- * If we decide to verify the peer certificate based on the fingerprint
- * we ignore wrong dates and the certificate not being

[Openvpn-devel] [PATCH 2/2] Implement using --peer-fingerprint without CA certificates

2023-05-19 Thread Arne Schwabe 
This is implements --peer-fingerprint command to support OpenVPN authentication
without involving a PKI.

The current implementation in OpenVPN for peer fingerprint has been already
extensively rewritten from the original submission from Jason. The commit
preserved the original author since it was based on Jason code/idea.

The code uses two commits to prepare the --peer-fingerprint solution as
which choose to use a simple to use --peer-fingerprint directive instead
of using using a --tls-verify script like the v1 of the patch proposed.
The two commit preparing this are:

 - Extend verify-hash to allow multiple hashes
 - Implement peer-fingerprint to check fingerprint of peer certificate

This perparing patches make this actual patch quite short. There are some
lines in this patch that bear some similarity to the ones like

if (!preverify_ok && !session->opt->verify_hash_no_ca)

vs

if (!preverify_ok && !session->opt->ca_file_none)

But these similarities are one line fragments and dictated by the
surrounding style and program flow, so even a complete black box
implementation will likely end up with the same lines.

Patch V2: Changes in V2 (by Arne Schwabe):
  - Only check peer certificates, not all cert levels, if you need
multiple levels of certificate you should use a real CA
  - Use peer-fingerprint instead tls-verify on server side in example.
  - rename variable ca_file_none to verify_hash_no_ca
  - do no require --ca none but allow --ca simply
to be absent when --peer-fingprint is present
  - adjust warnings/errors messages to also point to
peer-fingerprint as valid verification method.
  - Fix mbed TLS version of not requiring CA
not working

Patch v3: Fix minor style. Remove unessary check of verify_hash_no_ca in
ssl.c.

Patch v4: remove the last parts of Jason's original patch.

Change-Id: Ie74c3d606c5429455c293c367462244566a936e3
Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c   |  1 +
 src/openvpn/options.c| 26 +-
 src/openvpn/options.h|  1 +
 src/openvpn/ssl_common.h |  1 +
 src/openvpn/ssl_verify_mbedtls.c | 16 
 src/openvpn/ssl_verify_openssl.c |  2 +-
 6 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c023b33c6..d358ad003 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -3347,6 +3347,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
 to.verify_hash_depth = options->verify_hash_depth;
+to.verify_hash_no_ca = options->verify_hash_no_ca;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fe9285384..e4c596b89 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2991,21 +2991,11 @@ options_postprocess_verify_ce(const struct options 
*options,
 else
 {
 #ifdef ENABLE_CRYPTO_MBEDTLS
-if (!(options->ca_file))
-{
-msg(M_USAGE, "You must define CA file (--ca)");
-}
-
 if (options->ca_path)
 {
 msg(M_USAGE, "Parameter --capath cannot be used with the mbed 
TLS version version of OpenVPN.");
 }
-#else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
-if ((!(options->ca_file)) && (!(options->ca_path)))
-{
-msg(M_USAGE, "You must define CA file (--ca) or CA path 
(--capath)");
-}
-#endif
+#endif  /* ifdef ENABLE_CRYPTO_MBEDTLS */
 if (pull)
 {
 
@@ -3737,6 +3727,13 @@ options_postprocess_mutate(struct options *o, struct 
env_set *es)
 options_postprocess_http_proxy_override(o);
 }
 #endif
+if (!o->ca_file && !o->ca_path && o->verify_hash
+&& o->verify_hash_depth == 0)
+{
+msg(M_INFO, "Using certificate fingerprint to verify peer (no CA "
+"option set). ");
+o->verify_hash_no_ca = true;
+}
 
 if (o->config && streq(o->config, "stdin") && o->remap_sigusr1 == SIGHUP)
 {
@@ -4032,8 +4029,11 @@ options_postprocess_filechecks(struct options *options)
 errs |= check_file_access_inline(options->dh_file_inline, CHKACC_FILE,
  options->dh_file, R_OK, "--dh");
 
-errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
- options->ca_file, R_OK, "--ca");
+if (!options->verify_hash_no_ca)
+{
+errs |= check_file_access_inline(options->ca_file_inline, CHKACC_FILE,
+ options->ca_file, R_OK, "--ca");
+}
 
 errs |= check_file_access_chroot(options->chroot_dir, CHKACC_FILE,

[Openvpn-devel] [PATCH v3] tapctl: generate driver-specific adapter names

2023-05-19 Thread Lev Stipakov 
From: Lev Stipakov 

At the moment if --name is not specified, adapter names
are generated by Windows and they look a bit confusing
like "Local Area Connection 2".

This is also behavior of "Add a new  virtual network
adapter" shortcuts.

This makes tapctl generate driver-specific names for adapters
if --name is missing, inclusing resolving duplicates. For instance
following commands:

  tapctl.exe create --hwid ovpn-dco

will create an adapter named

  OpenVPN Data Channel Offload

If the name is taken, the next one will be

  OpenVPN Data Channel Offload #1

and so on up to 100.

Fixes https://github.com/OpenVPN/openvpn/issues/337

Change-Id: Ic5afb470d14ac7b231d91f0f5de0a0046043a7e0
Signed-off-by: Lev Stipakov 
---
 v3:
  - use _stprintf_s instead of _tcscat_s
  - make functions static
  - ensure iResult is always assigned

 v2: fix MinGW compilation

 src/tapctl/main.c | 132 --
 1 file changed, 105 insertions(+), 27 deletions(-)

diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 1194036f..d76d553c 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -126,6 +126,85 @@ usage(void)
   title_string);
 }
 
+/**
+ * Checks if adapter with given name doesn't already exist
+ */
+static BOOL
+is_adapter_name_available(LPCTSTR name, struct tap_adapter_node *adapter_list, 
BOOL log)
+{
+for (struct tap_adapter_node *a = adapter_list; a; a = a->pNext)
+{
+if (_tcsicmp(name, a->szName) == 0)
+{
+if (log)
+{
+LPOLESTR adapter_id = NULL;
+StringFromIID((REFIID)>guid, _id);
+_ftprintf(stderr, TEXT("Adapter \"%") TEXT(PRIsLPTSTR) 
TEXT("\" already exists (GUID %")
+  TEXT(PRIsLPOLESTR) TEXT(").\n"), a->szName, 
adapter_id);
+CoTaskMemFree(adapter_id);
+}
+
+return FALSE;
+}
+}
+
+return TRUE;
+}
+
+/**
+ * Returns unique adapter name based on hwid or NULL if name cannot be 
generated.
+ * Caller is responsible for freeing it.
+ */
+static LPTSTR
+get_unique_adapter_name(LPCTSTR hwid, struct tap_adapter_node *adapter_list)
+{
+if (hwid == NULL)
+{
+return NULL;
+}
+
+LPCTSTR base_name;
+if (_tcsicmp(hwid, TEXT("ovpn-dco")) == 0)
+{
+base_name = TEXT("OpenVPN Data Channel Offload");
+}
+else if (_tcsicmp(hwid, TEXT("wintun")) == 0)
+{
+base_name = TEXT("OpenVPN Wintun");
+}
+else if (_tcsicmp(hwid, TEXT("root\\") TEXT(TAP_WIN_COMPONENT_ID)) == 0)
+{
+base_name = TEXT("OpenVPN TAP-Windows6");
+}
+else
+{
+return NULL;
+}
+
+if (is_adapter_name_available(base_name, adapter_list, FALSE))
+{
+return _tcsdup(base_name);
+}
+
+size_t name_len = _tcslen(base_name) + 10;
+LPTSTR name = malloc(name_len * sizeof(TCHAR));
+if (name == NULL)
+{
+return NULL;
+}
+for (int i = 1; i < 100; ++i)
+{
+_stprintf_s(name, name_len, TEXT("%ls #%d"), base_name, i);
+
+if (is_adapter_name_available(name, adapter_list, FALSE))
+{
+return name;
+}
+}
+
+return NULL;
+}
 
 /**
  * Program entry point
@@ -210,50 +289,49 @@ _tmain(int argc, LPCTSTR argv[])
 iResult = 1; goto quit;
 }
 
-if (szName)
+/* Get existing network adapters. */
+struct tap_adapter_node *pAdapterList = NULL;
+dwResult = tap_list_adapters(NULL, NULL, );
+if (dwResult != ERROR_SUCCESS)
 {
-/* Get existing network adapters. */
-struct tap_adapter_node *pAdapterList = NULL;
-dwResult = tap_list_adapters(NULL, NULL, );
-if (dwResult != ERROR_SUCCESS)
-{
-_ftprintf(stderr, TEXT("Enumerating adapters failed (error 
0x%x).\n"), dwResult);
-iResult = 1; goto create_delete_adapter;
-}
+_ftprintf(stderr, TEXT("Enumerating adapters failed (error 
0x%x).\n"), dwResult);
+iResult = 1;
+goto create_delete_adapter;
+}
 
-/* Check for duplicates. */
-for (struct tap_adapter_node *pAdapter = pAdapterList; pAdapter; 
pAdapter = pAdapter->pNext)
+LPTSTR adapter_name = szName ? _tcsdup(szName) : 
get_unique_adapter_name(szHwId, pAdapterList);
+if (adapter_name)
+{
+/* Check for duplicates when name was specified,
+ * otherwise get_adapter_default_name() takes care of it */
+if (szName && !is_adapter_name_available(adapter_name, 
pAdapterList, TRUE))
 {
-if (_tcsicmp(szName, pAdapter->szName) == 0)
-{
-StringFromIID((REFIID)>guid, );
-_ftprintf(stderr, TEXT("Adapter \"%") TEXT(PRIsLPTSTR) 
TEXT("\" already exists (GUID %")
-  TEXT(PRIsLPOLESTR)

[Openvpn-devel] [PATCH applied] Re: Interactive service: do not force a target desktop for openvpn.exe

2023-05-19 Thread Gert Doering 
Acked-by: Gert Doering 

This is all voodoo to me, but it doesn't break compilation, doesn't
bring in unsafe pointer constructs, and people say "it makes their use
case work", so it sounds like a good fix.

Microsoft documentation on

  
https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessasuserw

says interesting things...

   "The user must have full access to both the specified window
station and desktop. If you want the process to be interactive,
specify winsta0\default. If the lpDesktop member is NULL, the
new process inherits the desktop and window station of its
parent process. If this member is an empty string, "", the new
process connects to a window station using the rules described
in Process Connection to a Window Station."

.. so we're moving from "winsta0\default" to "NULL" (startup_info is
ZeroMemory'ed), which sounds like "now it will run in the desktop
environment of the iservice", whatever *that* might be...  but it's
certainly a valid and documented choice...  and since openvpn.exe
"as run from iservice" doesn't want any windows etc., whatever makes
it succeed sounds good to me.

Your patch has been applied to the master and release/2.6 branch.

(Not sure this is a "bugfix" or a "mini feature", but it certainly
warrants a bit more thorough testing across Windows variants before
releasing 2.6.5)

commit 244d9b7942dabf0297c8f689457eeb0f9fa0aa1e (master)
commit 9e112be5dde043cdc1f073f33ada0a1810b730a6 (release/2.6)
Author: Selva Nair
Date:   Thu May 18 13:33:45 2023 -0400

 Interactive service: do not force a target desktop for openvpn.exe

 Signed-off-by: Selva Nair 
 Acked-by: Gert Doering 
 Message-Id: <20230518173345.2722530-1-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26705.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] tapctl: generate driver-specific adapter names

2023-05-19 Thread Lev Stipakov 
Hi,

> It looks much simpler to write the above 5 lines as
>
> _stprintf_s(name, name_len, TEXT("%ls #%d"), base_name, i)

Agreed.

> If (adapter_name) is false, we reach here with iResult not set, but it gets 
> referenced below. Add an else { iResult = 1; } or initialize iResult to 1 at 
> top?

Normally this should not happen, adapter_name is currently set either
by --name or using a hwid-generated name. But surely let's handle this
case too.

I think iResult should be set to 0, since if we don't set the name
this is not an error - same behavior has been before.

Agreed with the rest, V3 is coming.


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH applied] Re: dco-win: support for --dev-node

2023-05-19 Thread Gert Doering 
Acked-by: Gert Doering 

The patch is actually quite trivial - just prevent the "if (dco)"
option checker from unsetting --dev-node...

I have not tested it beyond "stare-at-code" and "do a MinGW test compile".

Your patch has been applied to the master and release/2.6 branch.

commit c543cf464e97866e20345feb46c82752fedc9d71 (master)
commit ea9382d7b41849a92a9d18fa3f5420925277144e (release/2.6)
Author: Lev Stipakov
Date:   Thu May 18 14:00:58 2023 +0300

 dco-win: support for --dev-node

 Signed-off-by: Lev Stipakov 
 Acked-by: Gert Doering 
 Message-Id: <20230518110058.1382-1-lstipa...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26702.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH applied] Re: src/openvpn/dco_freebsd.c: handle malloc failure

2023-05-19 Thread Gert Doering 
Acked-by: Gert Doering 

As discussed.

Tested with a basic client run on FreeBSD14 with DCO.

Your patch has been applied to the master and release/2.6 branch.

commit 5e79aed439d4e1b101c768aabfd695cd1c0a54ce (master)
commit 73ce6ac984e3ab496f97979e41f2a27569a432fd (release/2.6)
Author: Ilya Shipitsin
Date:   Thu May 18 23:21:39 2023 +0200

 src/openvpn/dco_freebsd.c: handle malloc failure

 Signed-off-by: Ilya Shipitsin 
 Acked-by: Gert Doering 
 Message-Id: <20230518212139.1261-1-chipits...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26707.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