Re: [Openvpn-devel] [PATCH] Reformat all source files
Hi, On Fri, Mar 27, 2020 at 04:24:00PM +0100, David Sommerseth wrote: > On 16/11/2019 11:28, Arne Schwabe wrote: > > Over time some patches slipped in that were not 100% complient to uncrustify > > This rerun fixes those issues [..] > Only done quick code review and RHEL-7 build. Changes looks reasonable and is > by far closer to what I would expect our coding style to look like. > > Acked-By: David Sommerseth ... but it does not apply to current git master...? Applying: Reformat all source files error: patch failed: src/openvpn/tun.c:3418 error: src/openvpn/tun.c: patch does not apply Patch failed at 0001 Reformat all source files 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 4/5] Implement sending SSO challenge to clients
On 09/11/2019 16:13, Arne Schwabe wrote: > This implements sending AUTH_PENDING and INFO_PRE messages to clients > that indicate that the clients should be continue authentication with > a second factor. This can currently be out of band (openurl) or a normal > challenge/response 2FA like TOTP (CR_TEXT). > > Signed-off-by: Arne Schwabe > --- > doc/management-notes.txt | 26 +++ > src/openvpn/manage.c | 46 > src/openvpn/manage.h | 3 +++ > src/openvpn/multi.c | 19 + > src/openvpn/push.c | 24 + > src/openvpn/push.h | 2 ++ > 6 files changed, 120 insertions(+) Code and management notes looks reasonable. But again, it would be good to have a way to test this properly to avoid regressions later on. Since this is also a more advanced authentication method, having good test methods is even more critical. -- kind regards, David Sommerseth OpenVPN Inc ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Reformat all source files
On 16/11/2019 11:28, Arne Schwabe wrote: > Over time some patches slipped in that were not 100% complient to uncrustify > This rerun fixes those issues > > This run used Uncrustify-0.69.0_f > --- > src/openvpn/buffer.c | 2 +- > src/openvpn/crypto.h | 2 +- > src/openvpn/networking.h | 4 +-- > src/openvpn/networking_iproute2.c | 14 > src/openvpn/networking_sitnl.h| 2 +- > src/openvpn/openvpn.h | 2 +- > src/openvpn/options.c | 8 +++-- > src/openvpn/options.h | 4 +-- > src/openvpn/proto.h | 2 +- > src/openvpn/push.c| 20 ++-- > src/openvpn/route.c | 2 +- > src/openvpn/socket.h | 54 +++ > src/openvpn/ssl.c | 6 ++-- > src/openvpn/ssl.h | 1 + > src/openvpn/ssl_openssl.c | 10 +++--- > src/openvpn/ssl_verify.c | 18 +-- > src/openvpn/ssl_verify.h | 3 +- > src/openvpn/tun.c | 20 ++-- > src/openvpn/vlan.c| 4 +-- > 19 files changed, 99 insertions(+), 79 deletions(-) > Only done quick code review and RHEL-7 build. Changes looks reasonable and is by far closer to what I would expect our coding style to look like. Acked-By: David Sommerseth -- kind regards, David Sommerseth OpenVPN Inc 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 v2 1/5] Implement parsing and sending INFO and INFO_PRE control messages
On 09/11/2019 16:13, Arne Schwabe wrote:
> OpenVPN 3 implements these messages to send information during the
> authentication to the UI, implement these message also in OpenVPN 2.x
>
> Signed-off-by: Arne Schwabe
> ---
> src/openvpn/forward.c | 8
> src/openvpn/push.c| 33 +
> src/openvpn/push.h| 3 +++
> 3 files changed, 44 insertions(+)
>
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 8451706b..0f735384 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -395,6 +395,14 @@ check_incoming_control_channel_dowork(struct context *c)
> {
> server_pushed_signal(c, &buf, false, 4);
> }
> +else if (buf_string_match_head_str(&buf, "INFO_PRE"))
> +{
> +server_pushed_info(c, &buf, 8);
> +}
> +else if (buf_string_match_head_str(&buf, "INFO"))
> +{
> +server_pushed_info(c, &buf, 4);
> +}
> else
> {
> msg(D_PUSH_ERRORS, "WARNING: Received unknown control
> message: %s", BSTR(&buf));
> diff --git a/src/openvpn/push.c b/src/openvpn/push.c
> index 368b6920..ee1cb980 100644
> --- a/src/openvpn/push.c
> +++ b/src/openvpn/push.c
> @@ -178,6 +178,39 @@ server_pushed_signal(struct context *c, const struct
> buffer *buffer, const bool
> }
> }
>
> +void
> +server_pushed_info(struct context *c, const struct buffer *buffer,
> + const int adv)
> +{
> +const char *m = "";
> +struct buffer buf = *buffer;
> +
> +if (buf_advance(&buf, adv) && buf_read_u8(&buf) == ',' && BLEN(&buf))
> +{
> +m = BSTR(&buf);
> +}
> +
> +#ifdef ENABLE_MANAGEMENT
> +struct gc_arena gc;
> +if (management)
> +{
> +gc = gc_new();
> +
> +/*
> + * We use >INFOMSG here instead of plain >INFO since INFO is used to
> + * for management greeting and we don't want to confuse the client
> + */
> +struct buffer out = alloc_buf_gc(256, &gc);
> +buf_printf(&out, ">%s:%s", "INFOMSG", m);
> +management_notify_generic(management, BSTR(&out));
> +
> +gc_free(&gc);
> +}
> +#endif
The indent of this #endif is wrong. Wouldn't harm with an "/*
ENABLE_MANAGEMENT */" comment as well. This could be fixed at merge time.
Otherwise, this looks good.
Acked-By: David Sommerseth
--
kind regards,
David Sommerseth
OpenVPN Inc
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 v2 2/5] Implement support for signalling IV_SSO to server
Sorry, I'm loosing grip of my mailing-list-fu ... Managed to reply only to Arne.
On 27/03/2020 21:59, David Sommerseth wrote:
> On 09/11/2019 16:13, Arne Schwabe wrote:
>> Signed-off-by: Arne Schwabe
>> ---
>> src/openvpn/ssl.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
>> index 4455ebb8..cbb87e43 100644
>> --- a/src/openvpn/ssl.c
>> +++ b/src/openvpn/ssl.c
>> @@ -2355,7 +2355,9 @@ push_peer_info(struct buffer *buf, struct tls_session
>> *session)
>> if strncmp(e->string, "UV_", 3)==0
>> || strncmp(e->string, "IV_PLAT_VER=",
>> sizeof("IV_PLAT_VER=")-1)==0)
>>&& session->opt->push_peer_info_detail >= 2)
>> - ||
>> (strncmp(e->string,"IV_GUI_VER=",sizeof("IV_GUI_VER=")-1)==0))
>> + ||
>> (strncmp(e->string,"IV_GUI_VER=",sizeof("IV_GUI_VER=")-1)==0)
>> + ||
>> (strncmp(e->string,"IV_SSO=",sizeof("IV_SSO=")-1)==0)
>> + )
>> && buf_safe(&out, strlen(e->string)+1))
>> {
>> buf_printf(&out, "%s\n", e->string);
>>
>
> Code looks good, smoke tested on RHEL-7 against a test server without any
> issues.
>
> Test 1: openvpn --config test.conf --push-peer-info
> No IV_SSO entry found in server log.
>
> Test 2: openvpn --config test.conf --push-peer-info --setenv IV_SSO 1
> IV_SSO=1 was found in server log
>
> Test 3: openvpn --config test.conf --push-peer-info setenv IV_SSX 1
> No IV_SSO nor IV_SSX found in server log
>
Acked-by: David Sommerseth
--
kind regards,
David Sommerseth
OpenVPN Inc
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 v2 3/5] Implement sending response to challenge via CR_RESPONSE
On 09/11/2019 16:13, Arne Schwabe wrote: > When a client announces its support to support text based > challenge/response via IV_SOO=cr_text,the client needs to also Typo. IV_SOO -> IV_SSO > be able to reply to that response. > > This adds the "cr-response" management function to be able to > do this. The answer should be base64 encoded. Could we have some kind of test/sample script demonstrating this feature? It would also help test that this feature works and also useful for regression testing later on. -- kind regards, David Sommerseth OpenVPN Inc signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
