Re: [Openvpn-devel] [PATCH v2] signal --dns support in peer info

2022-05-13 Thread David Sommerseth

On 13/05/2022 13:40, Arne Schwabe wrote:

Am 13.05.22 um 13:22 schrieb David Sommerseth:

On 13/05/2022 11:37, Heiko Hund wrote:

Have clients set a bit in IV_PROTO, so that servers can make an informed
decision on whether to push --dns to the client. While unknown options
are ignored by clients when pushed, they generate a warning in the log.
That can be circumvented by server backends by checking if bit 7 is set.

Signed-off-by: Heiko Hund 

[...snip...]

+    /* support for the --dns option */
+    iv_proto |= IV_PROTO_DNS_OPTION;
+

[...snip...]

+#define IV_PROTO_DNS_OPTION  (1<<6)
+

[...snip...]

To be honest, I requested this flag but I don't think this is really 
what I want/need any more. I wanted to have a flag that tells me as a 
server that I can push --dns options instead of --dhcp-options and 
accept the client to evaluate them.


But after some digging, I found that on platforms where dhcp-option is 
NOT parsed by openvpn itself (so anything but Android and Windows) and 
scripts are used to set DNS, these scripts will always use dhcp-options 
as they rely on foreign_option support. So they end up with no DNS 
configuration if only --dns is pushed and using --dhcp-option options if 
both are pushed unless the script is updated.
As I remember it, this wasn't initially about Android and Windows 
clients.  It was about the server pushing both --dhcp-option DNS and 
--dns at the same time, and that OpenVPN 2.5 and older clients would 
complain about --dns option warnings.  By signalling to a server who 
wants to avoid this (like the Access Server), the server can chose if it 
wants to send --dns or --dhcp-options, based on this flag.


Right?

That the client side isn't parsing --dns options properly is to my 
understanding a different issue, which also needs a solution.  And if my 
memory isn't completely corrupted, we had some brief talks about v2.6 
clients adding some kind of "conversion" from --dns to --dhcp-options 
*data* for scripts/plug-ins - but only in a simplistic best-effort 
approach.  If the strict --dhcp-option behavior is needed, that can be 
used with v2.6 clients for the time being if --dns causes troubles.


But if both --dns and --dhcp-options are used, and the --dhcp-option 
provides a setting --dns support, --dns should take precedence.


This should also work fine, as the --dns option is far more flexible in 
what it supports, while --dhcp-options are just simpler by design - and 
designed around a lesser dynamic Internet world.



--
kind regards,

David Sommerseth
OpenVPN Inc



OpenPGP_signature
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] signal --dns support in peer info

2022-05-13 Thread Gert Doering
Hi,

On Fri, May 13, 2022 at 01:40:07PM +0200, Arne Schwabe wrote:
> So this flag doesn't really do what I expected it to promose (This 
> client will accept --dns and use them)

So you want something that goes hand in hand with code to actually
"do something with it", like

+#if defined(TARGET_WINDOWS)||defined(TARGET_ANDROID)
+   /* we have full backend support for --dns options */
+iv_proto |= IV_PROTO_DNS_OPTION;
+#endif

and whenever a platform gets a "real" implementation, be it "openvpn
provided --up script parsing the correct variables" or "NM plugin" or 
"directly implemented by magic", we add it to that list?

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 v2] signal --dns support in peer info

2022-05-13 Thread Arne Schwabe

Am 13.05.22 um 13:22 schrieb David Sommerseth:

On 13/05/2022 11:37, Heiko Hund wrote:

Have clients set a bit in IV_PROTO, so that servers can make an informed
decision on whether to push --dns to the client. While unknown options
are ignored by clients when pushed, they generate a warning in the log.
That can be circumvented by server backends by checking if bit 7 is set.

Signed-off-by: Heiko Hund 
---
  src/openvpn/ssl.c | 3 +++
  src/openvpn/ssl.h | 3 +++
  2 files changed, 6 insertions(+)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 61dea996..24d7f3f4 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1940,6 +1940,9 @@ push_peer_info(struct buffer *buf, struct 
tls_session *session)

  /* support for P_DATA_V2 */
  int iv_proto = IV_PROTO_DATA_V2;
+    /* support for the --dns option */
+    iv_proto |= IV_PROTO_DNS_OPTION;
+
  /* support for receiving push_reply before sending
   * push request, also signal that the client wants
   * to get push-reply messages without without requiring a round
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 0ba86d3e..c8802707 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -93,6 +93,9 @@
   * result. */
  #define IV_PROTO_NCP_P2P (1<<5)
+/** Supports the --dns option introduced in version 2.6 */
+#define IV_PROTO_DNS_OPTION  (1<<6)
+
  /* Default field in X509 to be username */
  #define X509_USERNAME_FIELD_DEFAULT "CN"


Only glared on the code and compile tested.  LGTM.

Acked-By: David Sommerseth 


To be honest, I requested this flag but I don't think this is really 
what I want/need any more. I wanted to have a flag that tells me as a 
server that I can push --dns options instead of --dhcp-options and 
accept the client to evaluate them.


But after some digging, I found that on platforms where dhcp-option is 
NOT parsed by openvpn itself (so anything but Android and Windows) and 
scripts are used to set DNS, these scripts will always use dhcp-options 
as they rely on foreign_option support. So they end up with no DNS 
configuration if only --dns is pushed and using --dhcp-option options if 
both are pushed unless the script is updated.


I think having clear preference and not knowing will make debugging logs 
from 3rd parties that have both --dns and --dhcp-option in them quite 
tedious.


So this flag doesn't really do what I expected it to promose (This 
client will accept --dns and use them)


Arne



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


Re: [Openvpn-devel] [PATCH v2] signal --dns support in peer info

2022-05-13 Thread David Sommerseth

On 13/05/2022 11:37, Heiko Hund wrote:

Have clients set a bit in IV_PROTO, so that servers can make an informed
decision on whether to push --dns to the client. While unknown options
are ignored by clients when pushed, they generate a warning in the log.
That can be circumvented by server backends by checking if bit 7 is set.

Signed-off-by: Heiko Hund 
---
  src/openvpn/ssl.c | 3 +++
  src/openvpn/ssl.h | 3 +++
  2 files changed, 6 insertions(+)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 61dea996..24d7f3f4 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1940,6 +1940,9 @@ push_peer_info(struct buffer *buf, struct tls_session 
*session)
  /* support for P_DATA_V2 */
  int iv_proto = IV_PROTO_DATA_V2;
  
+/* support for the --dns option */

+iv_proto |= IV_PROTO_DNS_OPTION;
+
  /* support for receiving push_reply before sending
   * push request, also signal that the client wants
   * to get push-reply messages without without requiring a round
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 0ba86d3e..c8802707 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -93,6 +93,9 @@
   * result. */
  #define IV_PROTO_NCP_P2P (1<<5)
  
+/** Supports the --dns option introduced in version 2.6 */

+#define IV_PROTO_DNS_OPTION  (1<<6)
+
  /* Default field in X509 to be username */
  #define X509_USERNAME_FIELD_DEFAULT "CN"
  


Only glared on the code and compile tested.  LGTM.

Acked-By: David Sommerseth 


--
kind regards,

David Sommerseth
OpenVPN Inc



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


[Openvpn-devel] [PATCH v2] signal --dns support in peer info

2022-05-13 Thread Heiko Hund
Have clients set a bit in IV_PROTO, so that servers can make an informed
decision on whether to push --dns to the client. While unknown options
are ignored by clients when pushed, they generate a warning in the log.
That can be circumvented by server backends by checking if bit 7 is set.

Signed-off-by: Heiko Hund 
---
 src/openvpn/ssl.c | 3 +++
 src/openvpn/ssl.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 61dea996..24d7f3f4 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1940,6 +1940,9 @@ push_peer_info(struct buffer *buf, struct tls_session 
*session)
 /* support for P_DATA_V2 */
 int iv_proto = IV_PROTO_DATA_V2;
 
+/* support for the --dns option */
+iv_proto |= IV_PROTO_DNS_OPTION;
+
 /* support for receiving push_reply before sending
  * push request, also signal that the client wants
  * to get push-reply messages without without requiring a round
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 0ba86d3e..c8802707 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -93,6 +93,9 @@
  * result. */
 #define IV_PROTO_NCP_P2P (1<<5)
 
+/** Supports the --dns option introduced in version 2.6 */
+#define IV_PROTO_DNS_OPTION  (1<<6)
+
 /* Default field in X509 to be username */
 #define X509_USERNAME_FIELD_DEFAULT "CN"
 
-- 
2.32.0



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