Re: [Openvpn-devel] [PATCH 1/4] Only announce IV_NCP=2 when we are willing to support these ciphers

2020-01-06 Thread Steffan Karger
Hi,

Finally found some time to start looking at this patch set.

On 17-11-2019 19:12, Arne Schwabe wrote:
> We currently always announce IV_NCP=2 when we support these ciphers even
> when we do not accept them. This lead to a server pushing a AES-GCM-128
> cipher to clients and the client then rejecting it.
> 
> Signed-off-by: Arne Schwabe 
> ---
>  doc/openvpn.8| 2 ++
>  src/openvpn/init.c   | 4 
>  src/openvpn/openvpn.h| 1 +
>  src/openvpn/ssl.c| 4 +++-
>  src/openvpn/ssl_common.h | 1 +
>  5 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/openvpn.8 b/doc/openvpn.8
> index 457c2667..ae24b6c0 100644
> --- a/doc/openvpn.8
> +++ b/doc/openvpn.8
> @@ -4392,6 +4392,8 @@ NCP server (v2.4+) with "\-\-cipher BF\-CBC" and 
> "\-\-ncp\-ciphers
>  AES\-256\-GCM:AES\-256\-CBC" set can either specify "\-\-cipher BF\-CBC" or
>  "\-\-cipher AES\-256\-CBC" and both will work.
>  
> +Note, for using NCP with a OpenVPN 2.4 server this list must include
> +the AES\-256\-GCM and AES\-128\-GCM ciphers.
>  .\"*
>  .TP
>  .B \-\-ncp\-disable
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 0bdb0a9c..8f142311 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -623,6 +623,7 @@ save_ncp_options(struct context *c)
>  c->c1.ciphername = c->options.ciphername;
>  c->c1.authname = c->options.authname;
>  c->c1.keysize = c->options.keysize;
> +c->c1.ncp_ciphers = c->options.ncp_ciphers;
>  }
>  
>  /* Restores NCP-negotiable options to original values */
> @@ -632,6 +633,7 @@ restore_ncp_options(struct context *c)
>  c->options.ciphername = c->c1.ciphername;
>  c->options.authname = c->c1.authname;
>  c->options.keysize = c->c1.keysize;
> +c->options.ncp_ciphers = c->c1.ncp_ciphers;
>  }

options->ncp_ciphers itself is not negotiable, and thus does not need
restoring, I would think.

>  void
> @@ -2827,6 +2829,7 @@ do_init_crypto_tls(struct context *c, const unsigned 
> int flags)
>  to.tcp_mode = link_socket_proto_connection_oriented(options->ce.proto);
>  to.config_ciphername = c->c1.ciphername;
>  to.config_authname = c->c1.authname;
> +to.config_ncp_ciphers = c->c1.ncp_ciphers;
>  to.ncp_enabled = options->ncp_enabled;
>  to.transition_window = options->transition_window;
>  to.handshake_window = options->handshake_window;
> @@ -4532,6 +4535,7 @@ inherit_context_child(struct context *dest,
>  dest->c1.ciphername = src->c1.ciphername;
>  dest->c1.authname = src->c1.authname;
>  dest->c1.keysize = src->c1.keysize;
> +dest->c1.ncp_ciphers = src->c1.ncp_ciphers;
>  
>  /* inherit auth-token */
>  dest->c1.ks.auth_token_key = src->c1.ks.auth_token_key;
> diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
> index 900db7e1..9e46ad6c 100644
> --- a/src/openvpn/openvpn.h
> +++ b/src/openvpn/openvpn.h
> @@ -209,6 +209,7 @@ struct context_1
>  
>  const char *ciphername; /**< Data channel cipher from config file */
>  const char *authname;   /**< Data channel auth from config file */
> +const char *ncp_ciphers;/**< NCP Ciphers */
>  int keysize;/**< Data channel keysize from config file */
>  #endif
>  };
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 4455ebb8..6ca5c79a 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -2322,7 +2322,9 @@ push_peer_info(struct buffer *buf, struct tls_session 
> *session)
>  
>  /* support for Negotiable Crypto Parameters */
>  if (session->opt->ncp_enabled
> -&& (session->opt->mode == MODE_SERVER || session->opt->pull))
> +&& (session->opt->mode == MODE_SERVER || session->opt->pull)
> +&& tls_item_in_cipher_list("AES-128-GCM", 
> session->opt->config_ncp_ciphers)
> +&& tls_item_in_cipher_list("AES-256-GCM", 
> session->opt->config_ncp_ciphers))
>  {
>  buf_printf(, "IV_NCP=2\n");
>  }
> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
> index 8dd08862..fb82f610 100644
> --- a/src/openvpn/ssl_common.h
> +++ b/src/openvpn/ssl_common.h
> @@ -290,6 +290,7 @@ struct tls_options
>  
>  const char *config_ciphername;
>  const char *config_authname;
> +const char *config_ncp_ciphers;
>  bool ncp_enabled;
>  
>  bool tls_crypt_v2;
> 

Otherwise this makes sense and looks good.

-Steffan


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


[Openvpn-devel] Community meetings in January 2020

2020-01-06 Thread Samuli Seppänen
Hi,

Our community meetings will alternate between Wed 11:30 CET and Thu
20:00 CET.

Next meetings have been scheduled to

- Wed 8th  January 11:30 CET
- Thu 16th January 20:00 CET
- Wed 22nd January 11:30 CET
- Thu 30th January 20:00 CET

The place is #openvpn-meeting IRC channel at Freenode. Meeting agendas
and summaries are in here:



Samuli








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/1] configure.ac: replace set with env

2020-01-06 Thread Christian Hesse
Tom Yan  on Mon, 2020/01/06 08:48:
> How about printenv (without grep)?

The variables are not known in advance. This needs to match all variables
starting with "enable_" and "with_".
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpLW26T7v6T8.pgp
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/1] configure.ac: replace set with env

2020-01-06 Thread Gert Doering
Hi,

On Mon, Jan 06, 2020 at 08:48:00AM +, Tom Yan wrote:
> How about printenv (without grep)?

Portability.  

"env |grep" is POSIX standardized so should work everywhere.

printenv is a BSD invention, which not all unix variants are required
to have.

(Besides, printenv on its own cannot do "give me all environment variables
that start with 'with_'", which is exactly what we want here... so you'd
end up with "printenv | grep", which is less portable and more characters)

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 1/1] configure.ac: replace set with env

2020-01-06 Thread Tom Yan
How about printenv (without grep)?

From: Christian Hesse 
Sent: Monday, January 6, 2020 4:04:26 PM
To: OpenVPN Development 
Cc: Christian Hesse 
Subject: [Openvpn-devel] [PATCH 1/1] configure.ac: replace set with env

From: Christian Hesse 

The shell builtin `set` produces different output for different shells:

bash$ set | grep '^TERM='
TERM=xterm
dash$ set | grep '^TERM='
TERM='xterm'

This may break reproducible builds depending on what shell is used.

Let's replace `set` with `env`, which is a real command and always
produces identical output.

Signed-off-by: Christian Hesse 
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index a47e0a06..f13ff7b6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1326,7 +1326,7 @@ if test "${enable_async_push}" = "yes"; then
 )
 fi

-CONFIGURE_DEFINES="`set | grep '^enable_.*=' ; set | grep '^with_.*='`"
+CONFIGURE_DEFINES="`env | grep '^enable_.*=' ; env | grep '^with_.*='`"
 AC_DEFINE_UNQUOTED([CONFIGURE_DEFINES], ["`echo ${CONFIGURE_DEFINES}`"], 
[Configuration settings])

 TAP_WIN_COMPONENT_ID="PRODUCT_TAP_WIN_COMPONENT_ID"


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


[Openvpn-devel] [PATCH 1/1] configure.ac: replace set with env

2020-01-06 Thread Christian Hesse
From: Christian Hesse 

The shell builtin `set` produces different output for different shells:

bash$ set | grep '^TERM='
TERM=xterm
dash$ set | grep '^TERM='
TERM='xterm'

This may break reproducible builds depending on what shell is used.

Let's replace `set` with `env`, which is a real command and always
produces identical output.

Signed-off-by: Christian Hesse 
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index a47e0a06..f13ff7b6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1326,7 +1326,7 @@ if test "${enable_async_push}" = "yes"; then
)
 fi
 
-CONFIGURE_DEFINES="`set | grep '^enable_.*=' ; set | grep '^with_.*='`"
+CONFIGURE_DEFINES="`env | grep '^enable_.*=' ; env | grep '^with_.*='`"
 AC_DEFINE_UNQUOTED([CONFIGURE_DEFINES], ["`echo ${CONFIGURE_DEFINES}`"], 
[Configuration settings])
 
 TAP_WIN_COMPONENT_ID="PRODUCT_TAP_WIN_COMPONENT_ID"


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