Re: [Openvpn-devel] [PATCH 1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry

[Arne Schwabe]

Am 25.08.20 um 01:51 schrieb Eric Thorpe:
> Hi Selva,
> 
>> In multi_connection_estableished, we have
>>
>> #ifdef MANAGEMENT_DEF_AUTH
>>     if (management)
>>     {
>>         management_connection_established(management,
>>                                           &mi->context.c2.mda_context,
>> mi->context.c2.es );
>>     }
>> #endif
>> I do not see why this requires --management-client-auth or any
>> management related options to be in use. 
> 
> management is set to NULL unless a management address is defined
> (--management), so the above is not called for a plugin or script
> 
> bool
> open_management(struct context *c)
> {
>     /* initialize management layer */
>     if (management)
>     {
>     if (c->options.management_addr) <
>     {
>  
>         {
>     else
>     {
>     close_management(); <
>     }
> 
> I'm not trying to be resistant in finding another way here to avoid this
> extra state you guys don't want, this is a real bug in re-auth/reneg
> that we have seen occur multiple times in various setups that we want to
> see fixed, and it effects management, 

Management goes another code path and management_client_auth directly
calls send_auth_failed.

> script and plugin authentication,
> however I have had a good walk through the code and I can't see another way.
>
> The other alternative here is to refactor up the call chain so the
> context_2 is passed to each of these functions instead of just multi &
> session, however this is about a 500 line refactor that I really don't
> want to do.

Well the tls session should basically only care about its own session
and the outer logic that ties the tls session together should handle
this. The current code is not as clean as it could be/should be. But I
also haven't digged deep enough to actually understand if your is
actually fixing the problem correctly. We have a working scenario (I
assume) with management and client-auth

- Do we send the AUTH_FAILED over the new or the old tls session?
- Do we establish the data channel (key_method_2_write) before writing
the key?

(and the same for the new approach)

You haven't described the bug/symptons you are seeing yet but I assume
you get desynced keys etc?

I understand that is frustrating from your end since you have a quite
simple fix and fixes your problem but I am here maintaining OpenVPN and
it took me 1-2 weeks to get rid of 2-3 similar variables to the one you
want to introduce.

Arne



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

[Openvpn-devel] [PATCH] Fix compilation with older mbed TLS versions (mbedtls_tls_prf_types undefined)

[Arne Schwabe]

The usage of the new keying material methods was not properly guarded.

To avoid a number of ifdefs this commit uses a dummy struct and function.
When we eventually drop support for non-EKM mbed TLS version we can remove
these.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_mbedtls.c | 10 ++
 src/openvpn/ssl_mbedtls.h |  5 -
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 4287b59e..4ec355a9 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -253,6 +253,16 @@ key_state_export_keying_material(struct tls_session 
*session,
 return  NULL;
 }
 }
+#else
+unsigned char*
+key_state_export_keying_material(struct tls_session *session,
+ const char* label, size_t label_size,
+ size_t ekm_size,
+ struct gc_arena *gc)
+{
+/* Dummy function to avoid ifdefs in the common code */
+return NULL;
+}
 #endif /* HAVE_EXPORT_KEYING_MATERIAL */
 
 bool
diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index 17aae551..ff64e17c 100644
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -82,6 +82,7 @@ struct external_context {
 void *sign_ctx;
 };
 
+#ifdef HAVE_EXPORT_KEYING_MATERIAL
 /** struct to cache TLS secrets for keying material exporter (RFC 5705).
  * The constants (64 and 48) are inherent to TLS version and
  * the whole keying material export will likely change when they change */
@@ -90,6 +91,9 @@ struct tls_key_cache {
 mbedtls_tls_prf_types tls_prf_type;
 unsigned char master_secret[48];
 };
+#else
+struct tls_key_cache { };
+#endif
 
 /**
  * Structure that wraps the TLS context. Contents differ depending on the
@@ -124,7 +128,6 @@ struct key_state_ssl {
 bio_ctx *bio_ctx;
 
 struct tls_key_cache tls_key_cache;
-
 };
 
 /**
-- 
2.26.2



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

Re: [Openvpn-devel] [PATCH] Adds client-auth-pending-extra management functionality.

[Eric Thorpe]

Hi Arne,


- to avoid the 256 byte management limit and multiple commands use maybe
the same approach as client-auth that allows a longer frame, you can
still limit that to 1024.
To be clear here, it isn't so much the limitation of the management or 
control channel, it's situations where a tun-mtu may be set to say 750 
for example, meaning the data size can only realistically be about 730 
minus INFO_PRE, flags, etc, or OpenVPN or the network will drop the 
control channel message. I would not recommend making any changes to 
management or the control channel here, simply just have the ability to 
send the data in smaller parts.



CR_TEXT is challenge response text and those clients that currently
implement it only allow a single line. I would suggest the following to
get the patch in a acceptable state:

- Document your authentication method also in management.txt and also
what IV_SSO flag indicates this authentication method.
Are you suggesting that I do not use CR_TEXT, and instead introduce a 
new Challenge Response type (CR_DATA for example)?


Cheers,
Eric

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
supp...@sparklabs.com

On 25/08/2020 1:02 am, Arne Schwabe wrote:

Am 24.08.20 um 09:59 schrieb Eric Thorpe:

Hi Arne,

The main scenario this addresses is 2FA authentication which needs to
transmit very long responses such as those requiring keys. In these
cases, the responses can be upwards of 1500 bytes. Management is
restricted (currently) to 256 bytes and the control channel I believe to
1024, however the larger problem is low MTU connections or poorly
configured connection with fragmentation problems would refuse to
transmit packets of this size over the control channel, so we need the
ability to break these up.

At the moment I'm experimenting with the following layout which has
shown success:

client-auth-pending  CR_TEXT:R,C:2
client-auth-pending-extra  CR_TEXT:CN,1:
client-auth-pending-extra  CR_TEXT:CN,2:

Where flag C=Chunked data and 2 is the number of chunks to expect. Then
CN,:.

The other issue this addresses is there are scenario where more than one
challenge reply is required. A simple example of this is resident key
registration, where the username is pulled from the clients certificate,
a password is queried, then attestation is queried, and then
authentication is queried.

Depends on if these should asked sequentially, the  you just send
another CR_TEXT challenge after the first is answer or in parallel.


client-auth-pending-extra is a bit of a catch-all addition where it
effectively opens up the client-auth-pending spec to allow the control
channel to be used without having to constantly drop the connection like
the current AUTH_DENY method pre-2.5, but without spamming the user with
AUTH_PENDING notifications, and allows a connected management script to
handle it without adding further complexity internally to OpenVPN.

CR_TEXT is challenge response text and those clients that currently
implement it only allow a single line. I would suggest the following to
get the patch in a acceptable state:

- Document your authentication method also in management.txt and also
what IV_SSO flag indicates this authentication method.

- to avoid the 256 byte management limit and multiple commands use maybe
the same approach as client-auth that allows a longer frame, you can
still limit that to 1024.

Otherwise I think this the right direction.

Arne




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

Re: [Openvpn-devel] [PATCH 1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry

[Eric Thorpe]

Hi Selva,


In multi_connection_estableished, we have

#ifdef MANAGEMENT_DEF_AUTH
    if (management)
    {
        management_connection_established(management,
&mi->context.c2.mda_context, mi->context.c2.es );
    }
#endif
I do not see why this requires --management-client-auth or any 
management related options to be in use. 


management is set to NULL unless a management address is defined 
(--management), so the above is not called for a plugin or script


bool
open_management(struct context *c)
{
    /* initialize management layer */
    if (management)
    {
    if (c->options.management_addr) <
    {
 
        {
    else
    {
    close_management(); <
    }

I'm not trying to be resistant in finding another way here to avoid this 
extra state you guys don't want, this is a real bug in re-auth/reneg 
that we have seen occur multiple times in various setups that we want to 
see fixed, and it effects management, script and plugin authentication, 
however I have had a good walk through the code and I can't see another way.


The other alternative here is to refactor up the call chain so the 
context_2 is passed to each of these functions instead of just multi & 
session, however this is about a 500 line refactor that I really don't 
want to do.


Cheers,
Eric

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
supp...@sparklabs.com

On 25/08/2020 12:25 am, Selva Nair wrote:



On Mon, Aug 24, 2020 at 3:49 AM Eric Thorpe > wrote:


Hi Selva,


my suggestion would be to make
this conditional on MANAGEMNET_DEF_AUTH so that we can
then get it from  session->opt->mda_context just as we do it when
auth is done via the management. In practice, that would cover
most builds where this is really useful.

Unfortunately this doesn't help. The mda_context flags are only
set when --management-client-auth is in use, meaning this patch
would not cover plugin or script authentication, which are the
more commonly used, and this patch set specifically addresses
plugin authentication.


Are you sure of that?

In multi_connection_estableished, we have

#ifdef MANAGEMENT_DEF_AUTH
    if (management)
    {
        management_connection_established(management,
&mi->context.c2.mda_context, mi->context.c2.es );
    }
#endif

management_connection_established will set DAF_CONENCTION_ESATBLISHED 
in mdac->flags and that's what's used to determine REAUTH.


I do not see why this requires --management-client-auth or any 
management related options to be in use. Sure, management support and 
deferred auth support have to be enabled but restricting the 
usefulness of your patch to those cases is not really a limitation.


What am I missing?

Selva

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com  
https://twitter.com/sparklabs  
supp...@sparklabs.com  

On 23/08/2020 12:13 pm, Selva Nair wrote:

Hi,

On Thu, Aug 13, 2020 at 4:37 AM Eric Thorpe mailto:e...@sparklabs.com>> wrote:

Hi Arne,

The issue is your state is not accessible from where that
boolean needs
to be used unless I am missing something? Please advise if
I'm mistaken
or of another route.


I agree with Arne that duplicating a state machine variable is
not a good approach. But we have to somehow get the REAUTH (reneg)
info in here.

This has stalled for too long, so my suggestion would be to make
this conditional on MANAGEMNET_DEF_AUTH so that we can
then get it from session->opt->mda_context just as we do it when
auth is done via the management. In practice, that would cover
most builds where this is really useful.

In fact, I think we should always enable MANAGEMENT_DEF_AUTH
when management is enabled. That also gets rid of a lot of IFDEFs and
allow the use of useful bits like CID more widely in the code. I see
no compelling reason for such fine-grained build options.

A marginal increase in code size is of little consequence all but
embedded devices which can continue to cope without this
as they do now.

Selva


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

[Openvpn-devel] [PATCH] Add DNS SRV host discovery support

[Vladislav Grishenko]

DNS SRV (rfc2782) support allows to use several OpenVPN servers for a single
domain w/o explicit profile enumerating, to move services from host to host
with little fuss, and to designate some hosts as primary servers for a service
and others as backups.
OpenVPN client ask for a specific service/protocol for a specific domain and
get back the names & ports of available servers to connect to, ordered by
priority and relative weight. Note, implemented server selection is intended
to express static information such as "this server has a faster CPU than that
one" or "this server has a much better network connection than that one" at
the moment client asks for a records.
Feature has been asked several times already, can be useful in case of for huge
ammounts of client & servers deployed.

For example, instead of multiple remotes in profile:
remote server.example.com 1194 udp
remote server1.example.com 1195 udp
remote server2.example.org 1196 udp
remote server.example.com 443 tcp
remote server1.example.tld 8443 tcp

Now it's possible to specify just static domain(s) and enable discovery:
remote server.example.com 1194 udp
remote server.example.com 443 tcp
remote-discovery

When discovery is enabled, OpenVPN will first try to resolve
_openvpn._udp.server.example.com SRV records and use resolved targets (see
example below) as servers for the subsequent connection attempts. If there's
no records or resolve error, OpenVPN will fallback to direct connection
using specified server.example.com host and 1194 port. Next connection
attempt will be done with next configures connection/remote entry - using
_openvpn._tcp.server.example.com records (if any) and server.example.com:443
as last resort.

DNS zone can look like below (arbitrary prio & weight values):
_openvpn._udp.server.example.com IN SRV 10 70 1194 server.example.com
_openvpn._udp.server.example.com IN SRV 10 30 1195 server1.example.com
_openvpn._udp.server.example.com IN SRV 20 0 1196 server2.example.org
_openvpn._tcp.server.example.com IN SRV 10 10 443 server.example.com
_openvpn._tcp.server.example.com IN SRV 10 200 443 server.example.com

Currently only "openvpn" service is supported, usage of per-connection
service names is up to syntax decision (either intead of fallback port, or
as remote-discovery argument, etc), almost all the required mechanics
is implemented for that.

References:
 https://tools.ietf.org/html/rfc2782
 https://en.wikipedia.org/wiki/SRV_record
 https://sourceforge.net/p/openvpn/mailman/message/34364911/
 https://forums.openvpn.net/viewtopic.php?f=10&t=13660

Signed-off-by: Vladislav Grishenko 
---
 configure.ac|   2 +-
 doc/man-sections/client-options.rst |  26 ++
 src/openvpn/Makefile.am |   2 +-
 src/openvpn/buffer.h|   5 -
 src/openvpn/init.c  |   9 +-
 src/openvpn/manage.c|   2 +-
 src/openvpn/openvpn.vcxproj |   8 +-
 src/openvpn/options.c   |  11 +
 src/openvpn/options.h   |   1 +
 src/openvpn/ps.c|   2 +-
 src/openvpn/socket.c| 530 ++--
 src/openvpn/socket.h|  15 +
 src/openvpn/syshead.h   |   5 +
 13 files changed, 580 insertions(+), 38 deletions(-)

diff --git a/configure.ac b/configure.ac
index f8279924..67251bed 100644
--- a/configure.ac
+++ b/configure.ac
@@ -477,7 +477,7 @@ SOCKET_INCLUDES="
 "
 
 AC_CHECK_HEADERS(
-   [net/if.h netinet/ip.h resolv.h sys/un.h net/if_utun.h 
sys/kern_control.h],
+   [net/if.h netinet/ip.h arpa/nameser.h resolv.h sys/un.h net/if_utun.h 
sys/kern_control.h],
,
,
[[${SOCKET_INCLUDES}]]
diff --git a/doc/man-sections/client-options.rst 
b/doc/man-sections/client-options.rst
index ec1e3b11..53e6580e 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -299,6 +299,32 @@ configuration.
   specification (4/6 suffix), OpenVPN will try both IPv4 and IPv6
   addresses, in the order getaddrinfo() returns them.
 
+--remote-discovery
+  Perform rfc2782 remote host discovery using specified ``host`` and
+  ``proto`` values.
+  OpenVPN will try to resolve `` _openvpn._proto.host`` name via DNS
+  and use returned DNS SRV target and port records as OpenVPN servers
+  addresses in the order specified by Priority and Weight of that
+  records. If one record resolves to multiple IP addresses, OpenVPN
+  will try them in the order that the system getaddrinfo() presents
+  them. If discovery is failed, usual ``host`` and ``port`` connection
+  will be tried as a fallback.
+
+  Example:
+  ::
+
+ remote example.net 1194 udp
+ remote example.net 443 tcp
+ remote-discovery
+
+  DNS zone:
+  ::
+
+ _openvpn._udp.example.net IN SRV 10 60 1194 server1.example.net
+ _openvpn._udp.example.net IN SRV 10 40 1194 server2.example.net
+ _openvpn._udp.example.net IN SRV 20  0 1194 

Re: [Openvpn-devel] [PATCH applied] Re: Refactor key_state_export_keying_material functions

[Gert Doering]

Hi,

this commit seems to require a more recent mbedTLS version - it fails
a number of my buildslaves with

gcc -DHAVE_CONFIG_H -I. -I../.. -I../../include  -I../../include  
-I../../src/compat
-DPLUGIN_LIBDIR=\"/usr/local/lib/openvpn/plugins\"  -Wall -Wno-unused-parameter 
-Wno-unused-function -Wno-stringop-truncation -g -O2 -std=c99 -MT auth_token.o 
-MD -MP -MF .deps/auth_token.Tpo -c -o auth_token.o auth_token.c
In file included from ssl_backend.h:41:0,
 from ssl_common.h:38,
 from ssl.h:43,
 from openvpn.h:31,
 from auth_token.c:12:
ssl_mbedtls.h:90:5: error: unknown type name 'mbedtls_tls_prf_types'
 mbedtls_tls_prf_types tls_prf_type;
 ^

- I think it *should* fail the configure phase if mbedTLS is too old,
and clearly specify which version is required now (we have a check, but
it's fairly unspecific, and only says "mbed TLS 2.y.z required").

I've opened a trac ticket to track this

  https://community.openvpn.net/openvpn/ticket/1319

gert


On Sun, Aug 23, 2020 at 10:01:29PM +0200, Gert Doering wrote:
> Your patch has been applied to the master branch.
> 
> I have no idea what this all does (and not enough brain power to develop
> interest in finding out :-/ ) but if Steffan says it's good, so it is.
> 
> Also, t_client agrees :-)
> 
> commit 10abd656a3ae279cea7344055ce23637b7a62f6b (master)
> Author: Arne Schwabe
> Date:   Fri Aug 14 16:51:53 2020 +0200
> 
>  Refactor key_state_export_keying_material functions
> 
>  Signed-off-by: Arne Schwabe 
>  Signed-off-by: Arne Schwabe 
>  Acked-by: Steffan Karger 
>  Message-Id: <20200814145153.12895-1-a...@rfc2549.org>
>  URL: 
> https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20739.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
> 

-- 
"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] Adds client-auth-pending-extra management functionality.

[Arne Schwabe]

Am 24.08.20 um 09:59 schrieb Eric Thorpe:
> Hi Arne,
> 
> The main scenario this addresses is 2FA authentication which needs to
> transmit very long responses such as those requiring keys. In these
> cases, the responses can be upwards of 1500 bytes. Management is
> restricted (currently) to 256 bytes and the control channel I believe to
> 1024, however the larger problem is low MTU connections or poorly
> configured connection with fragmentation problems would refuse to
> transmit packets of this size over the control channel, so we need the
> ability to break these up.
> 
> At the moment I'm experimenting with the following layout which has
> shown success:
> 
> client-auth-pending  CR_TEXT:R,C:2
> client-auth-pending-extra  CR_TEXT:CN,1:
> client-auth-pending-extra  CR_TEXT:CN,2:
> 
> Where flag C=Chunked data and 2 is the number of chunks to expect. Then
> CN,:.
> 
> The other issue this addresses is there are scenario where more than one
> challenge reply is required. A simple example of this is resident key
> registration, where the username is pulled from the clients certificate,
> a password is queried, then attestation is queried, and then
> authentication is queried.

Depends on if these should asked sequentially, the  you just send
another CR_TEXT challenge after the first is answer or in parallel.

> client-auth-pending-extra is a bit of a catch-all addition where it
> effectively opens up the client-auth-pending spec to allow the control
> channel to be used without having to constantly drop the connection like
> the current AUTH_DENY method pre-2.5, but without spamming the user with
> AUTH_PENDING notifications, and allows a connected management script to
> handle it without adding further complexity internally to OpenVPN.

CR_TEXT is challenge response text and those clients that currently
implement it only allow a single line. I would suggest the following to
get the patch in a acceptable state:

- Document your authentication method also in management.txt and also
what IV_SSO flag indicates this authentication method.

- to avoid the 256 byte management limit and multiple commands use maybe
the same approach as client-auth that allows a longer frame, you can
still limit that to 1024.

Otherwise I think this the right direction.

Arne



signature.asc
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 1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry

[Selva Nair]

On Mon, Aug 24, 2020 at 3:49 AM Eric Thorpe  wrote:

> Hi Selva,
>
> my suggestion would be to make
> this conditional on MANAGEMNET_DEF_AUTH so that we can
> then get it from  session->opt->mda_context just as we do it when
> auth is done via the management. In practice, that would cover
> most builds where this is really useful.
>
> Unfortunately this doesn't help. The mda_context flags are only set when
> --management-client-auth is in use, meaning this patch would not cover
> plugin or script authentication, which are the more commonly used, and this
> patch set specifically addresses plugin authentication.
>

Are you sure of that?

In multi_connection_estableished, we have

#ifdef MANAGEMENT_DEF_AUTH
if (management)
{
management_connection_established(management,
  &mi->context.c2.mda_context, mi->
context.c2.es);
}
#endif

management_connection_established will set DAF_CONENCTION_ESATBLISHED in
mdac->flags and that's what's used to determine REAUTH.

I do not see why this requires --management-client-auth or any management
related options to be in use. Sure, management support and deferred auth
support have to be enabled but restricting the usefulness of your patch to
those cases is not really a limitation.

What am I missing?

Selva

---
> Eric Thorpe
> SparkLabs 
> Developerhttps://www.sparklabs.comhttps://twitter.com/sparklabssupp...@sparklabs.com
>
> On 23/08/2020 12:13 pm, Selva Nair wrote:
>
> Hi,
>
> On Thu, Aug 13, 2020 at 4:37 AM Eric Thorpe  wrote:
>
>> Hi Arne,
>>
>> The issue is your state is not accessible from where that boolean needs
>> to be used unless I am missing something? Please advise if I'm mistaken
>> or of another route.
>>
>
> I agree with Arne that duplicating a state machine variable is
> not a good approach. But we have to somehow get the REAUTH (reneg)
> info in here.
>
> This has stalled for too long, so my suggestion would be to make
> this conditional on MANAGEMNET_DEF_AUTH so that we can
> then get it from  session->opt->mda_context just as we do it when
> auth is done via the management. In practice, that would cover
> most builds where this is really useful.
>
> In fact, I think we should always enable MANAGEMENT_DEF_AUTH
> when management is enabled. That also gets rid of a lot of IFDEFs and
> allow the use of useful bits like CID more widely in the code. I see
> no compelling reason for such fine-grained build options.
>
> A marginal increase in code size is of little consequence all but
> embedded devices which can continue to cope without this
> as they do now.
>
> Selva
>
>
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Adds client-auth-pending-extra management functionality.

[Eric Thorpe]

Hi Arne,

The main scenario this addresses is 2FA authentication which needs to 
transmit very long responses such as those requiring keys. In these 
cases, the responses can be upwards of 1500 bytes. Management is 
restricted (currently) to 256 bytes and the control channel I believe to 
1024, however the larger problem is low MTU connections or poorly 
configured connection with fragmentation problems would refuse to 
transmit packets of this size over the control channel, so we need the 
ability to break these up.


At the moment I'm experimenting with the following layout which has 
shown success:


client-auth-pending  CR_TEXT:R,C:2
client-auth-pending-extra  CR_TEXT:CN,1:
client-auth-pending-extra  CR_TEXT:CN,2:

Where flag C=Chunked data and 2 is the number of chunks to expect. Then 
CN,:.


The other issue this addresses is there are scenario where more than one 
challenge reply is required. A simple example of this is resident key 
registration, where the username is pulled from the clients certificate, 
a password is queried, then attestation is queried, and then 
authentication is queried.


client-auth-pending-extra is a bit of a catch-all addition where it 
effectively opens up the client-auth-pending spec to allow the control 
channel to be used without having to constantly drop the connection like 
the current AUTH_DENY method pre-2.5, but without spamming the user with 
AUTH_PENDING notifications, and allows a connected management script to 
handle it without adding further complexity internally to OpenVPN.


Cheers,
Eric

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
supp...@sparklabs.com

On 22/08/2020 7:12 pm, Arne Schwabe wrote:

Am 21.08.20 um 08:24 schrieb Eric Thorpe:

This allows extra INFO_PRE mesasges to be sent to a client during an
authentication stage. This may be required to send additional challenges,
or allow longer messages to be sent by breaking them up and sending in parts.

Could you describe for what feature you need this? If we are extending
the protocol for some multiline AUTH_PENDING feature, we should document
how this works etc...

Arne




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

Re: [Openvpn-devel] [PATCH 1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry

[Eric Thorpe]

Hi Selva,


my suggestion would be to make
this conditional on MANAGEMNET_DEF_AUTH so that we can
then get it from  session->opt->mda_context just as we do it when
auth is done via the management. In practice, that would cover
most builds where this is really useful.
Unfortunately this doesn't help. The mda_context flags are only set when 
--management-client-auth is in use, meaning this patch would not cover 
plugin or script authentication, which are the more commonly used, and 
this patch set specifically addresses plugin authentication.


Regards,
Eric

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
supp...@sparklabs.com

On 23/08/2020 12:13 pm, Selva Nair wrote:

Hi,

On Thu, Aug 13, 2020 at 4:37 AM Eric Thorpe > wrote:


Hi Arne,

The issue is your state is not accessible from where that boolean
needs
to be used unless I am missing something? Please advise if I'm
mistaken
or of another route.


I agree with Arne that duplicating a state machine variable is
not a good approach. But we have to somehow get the REAUTH (reneg)
info in here.

This has stalled for too long, so my suggestion would be to make
this conditional on MANAGEMNET_DEF_AUTH so that we can
then get it from  session->opt->mda_context just as we do it when
auth is done via the management. In practice, that would cover
most builds where this is really useful.

In fact, I think we should always enable MANAGEMENT_DEF_AUTH
when management is enabled. That also gets rid of a lot of IFDEFs and
allow the use of useful bits like CID more widely in the code. I see
no compelling reason for such fine-grained build options.

A marginal increase in code size is of little consequence all but
embedded devices which can continue to cope without this
as they do now.

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