Re: [Openvpn-devel] [PATCH v2] Fix best gateway selection over netlink

2020-09-07 Thread Vladislav Grishenko
Sorry, comment typo:
- /* kernel cat return 0.0.0.0/128 host route */
+ /* kernel can return ::/128 host route */

--
Best Regards, Vladislav Grishenko

> -Original Message-
> From: Vladislav Grishenko 
> Sent: Tuesday, September 8, 2020 7:54 AM
> To: openvpn-devel@lists.sourceforge.net
> Subject: [Openvpn-devel] [PATCH v2] Fix best gateway selection over
netlink
> 
> Netlink route request with NLM_F_DUMP flag set means to return all entries
> matching criteria passed in message content - matching supplied family &
dst
> address in our case.
> So, gateway from the first ipv4 route was always used.
> 
> On kernels earlier than 2.6.38 default routes are the last ones, so some
host/net
> route w/o gateway is likely be returned as first, causing gateway to be
invalid or
> empty.
> After refactoring in 2.6.38 kernel first routes are the default (or more
specific
> 0.0.0.0/n), so it's gateway is usually valid, hiding the problem.
> 
> Fix this behavior by requesting exact route with corresponding full prefix
size,
> and filter dumped routes against default route /0 if dst was not set.
Empty dst is
> not valid address, so it's handled as default route request too.
> 
> If there's several default routes dumped with different metrics, the first
one will
> be less-numbered, so we can stop w/o additional iteration for metric
> comparison.
> 
> Tested on 5.4.0, 4.1.51 and 2.6.36 kernels.
> 
> Signed-off-by: Vladislav Grishenko 
> ---
>  src/openvpn/networking_sitnl.c | 47 +-
>  1 file changed, 41 insertions(+), 6 deletions(-)
> 
> diff --git a/src/openvpn/networking_sitnl.c
b/src/openvpn/networking_sitnl.c
> index 713a213a..81d52710 100644
> --- a/src/openvpn/networking_sitnl.c
> +++ b/src/openvpn/networking_sitnl.c
> @@ -90,7 +90,7 @@ struct sitnl_route_req {
>  char buf[256];
>  };
> 
> -typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *msg, void *arg);
> +typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *req, struct
> +nlmsghdr *msg, void *arg);
> 
>  /**
>   * Object returned by route request operation @@ -345,6 +345,13 @@
> sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned int groups,
>   *   continue;
>   *   }
>   */
> +
> +if (h->nlmsg_type == NLMSG_DONE)
> +{
> +ret = 0;
> +goto out;
> +}
> +
>  if (h->nlmsg_type == NLMSG_ERROR)
>  {
>  err = (struct nlmsgerr *)NLMSG_DATA(h); @@ -360,7 +367,11
@@
> sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned int groups,
>  ret = 0;
>  if (cb)
>  {
> -ret = cb(h, arg_cb);
> +int r = cb(payload, h, arg_cb);
> +if (r <= 0)
> +{
> +ret = r;
> +}
>  }
>  }
>  else
> @@ -375,8 +386,12 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer,
> unsigned int groups,
> 
>  if (cb)
>  {
> -ret = cb(h, arg_cb);
> -goto out;
> +int r = cb(payload, h, arg_cb);
> +if (r <= 0)
> +{
> +ret = r;
> +goto out;
> +}
>  }
>  else
>  {
> @@ -413,14 +428,21 @@ typedef struct {
>  } route_res_t;
> 
>  static int
> -sitnl_route_save(struct nlmsghdr *n, void *arg)
> +sitnl_route_save(struct nlmsghdr *req, struct nlmsghdr *n, void *arg)
>  {
>  route_res_t *res = arg;
> +struct rtmsg *q = NLMSG_DATA(req);
>  struct rtmsg *r = NLMSG_DATA(n);
>  struct rtattr *rta = RTM_RTA(r);
>  int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
>  unsigned int ifindex = 0;
> 
> +/* if dumped, filter out non-default routes */
> +if (q->rtm_dst_len == 0 && r->rtm_dst_len)
> +{
> +return 1;
> +}
> +
>  while (RTA_OK(rta, len))
>  {
>  switch (rta->rta_type)
> @@ -477,11 +499,24 @@ sitnl_route_best_gw(sa_family_t af_family, const
> inet_address_t *dst,
>  {
>  case AF_INET:
>  res.addr_size = sizeof(in_addr_t);
> -req.n.nlmsg_flags |= NLM_F_DUMP;
> +/*
> + * kernel can't return 0.0.0.0/32 host route, therefore
> + * need to dump all the routes and filter them in cb()
> + */
> +if (!dst || dst->ipv4 == htonl(INADDR_ANY))
> +{
> +req.n.nlmsg_flags |= NLM_F_DUMP;
> +}
> +else
> +{
> +req.r.rtm_dst_len = 32;
> +}
>  break;
> 
>  case AF_INET6:
>  res.addr_size = sizeof(struct in6_addr);
> +/* kernel cat return 0.0.0.0/128 host route */
> +

[Openvpn-devel] [PATCH v2] Fix best gateway selection over netlink

2020-09-07 Thread Vladislav Grishenko
Netlink route request with NLM_F_DUMP flag set means to return
all entries matching criteria passed in message content -
matching supplied family & dst address in our case.
So, gateway from the first ipv4 route was always used.

On kernels earlier than 2.6.38 default routes are the last ones,
so some host/net route w/o gateway is likely be returned as first,
causing gateway to be invalid or empty.
After refactoring in 2.6.38 kernel first routes are the default
(or more specific 0.0.0.0/n), so it's gateway is usually valid,
hiding the problem.

Fix this behavior by requesting exact route with corresponding
full prefix size, and filter dumped routes against default
route /0 if dst was not set. Empty dst is not valid address,
so it's handled as default route request too.

If there's several default routes dumped with different
metrics, the first one will be less-numbered, so we can stop
w/o additional iteration for metric comparison.

Tested on 5.4.0, 4.1.51 and 2.6.36 kernels.

Signed-off-by: Vladislav Grishenko 
---
 src/openvpn/networking_sitnl.c | 47 +-
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 713a213a..81d52710 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -90,7 +90,7 @@ struct sitnl_route_req {
 char buf[256];
 };
 
-typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *msg, void *arg);
+typedef int (*sitnl_parse_reply_cb)(struct nlmsghdr *req, struct nlmsghdr 
*msg, void *arg);
 
 /**
  * Object returned by route request operation
@@ -345,6 +345,13 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned 
int groups,
  *   continue;
  *   }
  */
+
+if (h->nlmsg_type == NLMSG_DONE)
+{
+ret = 0;
+goto out;
+}
+
 if (h->nlmsg_type == NLMSG_ERROR)
 {
 err = (struct nlmsgerr *)NLMSG_DATA(h);
@@ -360,7 +367,11 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned 
int groups,
 ret = 0;
 if (cb)
 {
-ret = cb(h, arg_cb);
+int r = cb(payload, h, arg_cb);
+if (r <= 0)
+{
+ret = r;
+}
 }
 }
 else
@@ -375,8 +386,12 @@ sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned 
int groups,
 
 if (cb)
 {
-ret = cb(h, arg_cb);
-goto out;
+int r = cb(payload, h, arg_cb);
+if (r <= 0)
+{
+ret = r;
+goto out;
+}
 }
 else
 {
@@ -413,14 +428,21 @@ typedef struct {
 } route_res_t;
 
 static int
-sitnl_route_save(struct nlmsghdr *n, void *arg)
+sitnl_route_save(struct nlmsghdr *req, struct nlmsghdr *n, void *arg)
 {
 route_res_t *res = arg;
+struct rtmsg *q = NLMSG_DATA(req);
 struct rtmsg *r = NLMSG_DATA(n);
 struct rtattr *rta = RTM_RTA(r);
 int len = n->nlmsg_len - NLMSG_LENGTH(sizeof(*r));
 unsigned int ifindex = 0;
 
+/* if dumped, filter out non-default routes */
+if (q->rtm_dst_len == 0 && r->rtm_dst_len)
+{
+return 1;
+}
+
 while (RTA_OK(rta, len))
 {
 switch (rta->rta_type)
@@ -477,11 +499,24 @@ sitnl_route_best_gw(sa_family_t af_family, const 
inet_address_t *dst,
 {
 case AF_INET:
 res.addr_size = sizeof(in_addr_t);
-req.n.nlmsg_flags |= NLM_F_DUMP;
+/*
+ * kernel can't return 0.0.0.0/32 host route, therefore
+ * need to dump all the routes and filter them in cb()
+ */
+if (!dst || dst->ipv4 == htonl(INADDR_ANY))
+{
+req.n.nlmsg_flags |= NLM_F_DUMP;
+}
+else
+{
+req.r.rtm_dst_len = 32;
+}
 break;
 
 case AF_INET6:
 res.addr_size = sizeof(struct in6_addr);
+/* kernel cat return 0.0.0.0/128 host route */
+req.r.rtm_dst_len = 128;
 break;
 
 default:
-- 
2.17.1



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


[Openvpn-devel] [PATCH 3/5] Allow running a default configuration with TLS libraries without BF-CBC

2020-09-07 Thread Arne Schwabe
Modern TLS libraries might drop Blowfish by default or distributions
might disable Blowfish in OpenSSL/mbed TLS. We still signal OCC
options with BF-CBC compatible strings. To avoid requiring BF-CBC
for this, special case this one usage of BF-CBC enough to avoid a hard
requirement on Blowfish in the default configuration.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/init.c| 18 +--
 src/openvpn/options.c | 51 ---
 2 files changed, 54 insertions(+), 15 deletions(-)

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index dff090b1..1e0baf2a 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2752,14 +2752,18 @@ do_init_crypto_tls_c1(struct context *c)
 #endif /* if P2MP */
 }
 
-/* Do not warn if we only have BF-CBC in options->ciphername
- * because it is still the default cipher */
-bool warn = !streq(options->ciphername, "BF-CBC")
- || options->enable_ncp_fallback;
-/* Get cipher & hash algorithms */
-init_key_type(>c1.ks.key_type, options->ciphername, 
options->authname,
-  options->keysize, true, warn);
 
+if (!options->ncp_enabled || options->enable_ncp_fallback
+|| !streq(options->ciphername, "BF-CBC"))
+{
+/* Get cipher & hash algorithms
+ * skip BF-CBC for NCP setups when cipher as this is the default
+ * and is also special cased later to allow it to be not available
+ * as we need to construct a fake BF-CBC occ string
+ */
+init_key_type(>c1.ks.key_type, options->ciphername, 
options->authname,
+  options->keysize, true, true);
+}
 /* Initialize PRNG with config-specified digest */
 prng_init(options->prng_hash, options->prng_nonce_secret_len);
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 90e78a7b..01da88ad 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3640,11 +3640,32 @@ calc_options_string_link_mtu(const struct options *o, 
const struct frame *frame)
 {
 struct frame fake_frame = *frame;
 struct key_type fake_kt;
-init_key_type(_kt, o->ciphername, o->authname, o->keysize, true,
-  false);
+
 frame_remove_from_extra_frame(_frame, crypto_max_overhead());
-crypto_adjust_frame_parameters(_frame, _kt, o->replay,
-   cipher_kt_mode_ofb_cfb(fake_kt.cipher));
+
+/* o->ciphername can be still BF-CBC and our SSL library might not like
+ * like it, workaround this important corner case in the name of
+ * compatibility and not stopping openvpn on our default configuration
+ */
+if ((strcmp(o->ciphername, "BF-CBC") == 0)
+&& cipher_kt_get(o->ciphername) == NULL)
+{
+init_key_type(_kt, "none", o->authname, o->keysize, true,
+  false);
+
+crypto_adjust_frame_parameters(_frame, _kt, o->replay,
+   
cipher_kt_mode_ofb_cfb(fake_kt.cipher));
+/* 64 bit block size, 64 bit IV size */
+frame_add_to_extra_frame(_frame, 64/8 + 64/8);
+}
+else
+{
+init_key_type(_kt, o->ciphername, o->authname, o->keysize, 
true,
+  false);
+
+crypto_adjust_frame_parameters(_frame, _kt, o->replay,
+   
cipher_kt_mode_ofb_cfb(fake_kt.cipher));
+}
 frame_finalize(_frame, o->ce.link_mtu_defined, o->ce.link_mtu,
o->ce.tun_mtu_defined, o->ce.tun_mtu);
 msg(D_MTU_DEBUG, "%s: link-mtu %u -> %d", __func__, (unsigned int) 
link_mtu,
@@ -3812,18 +3833,32 @@ options_string(const struct options *o,
+ (TLS_SERVER == true)
<= 1);
 
-init_key_type(, o->ciphername, o->authname, o->keysize, true,
-  false);
+/* Skip resolving BF-CBC to allow SSL libraries without BF-CBC
+ * to work here in the default configuration */
+const char *ciphername = o->ciphername;
+int keysize;
+
+if (strcmp(o->ciphername, "BF-CBC") == 0) {
+init_key_type(, "none", o->authname, o->keysize, true,
+  false);
+ciphername = cipher_kt_name(kt.cipher);
+keysize = 128;
+}
+else
+{
+init_key_type(, o->ciphername, o->authname, o->keysize, true,
+  false);
+keysize = kt.cipher_length * 8;
+}
 /* Only announce the cipher to our peer if we are willing to
  * support it */
-const char *ciphername = cipher_kt_name(kt.cipher);
 if (p2p_nopull || !o->ncp_enabled
 || tls_item_in_cipher_list(ciphername, o->ncp_ciphers))
 {
 buf_printf(, 

[Openvpn-devel] [PATCH 5/5] Prefer TLS libraries TLS PRF function, fix OpenVPN in FIPS mode

2020-09-07 Thread Arne Schwabe
This moves from using our own copy of the TLS1 PRF function to using
TLS library provided function where possible. This includes currently
OpenSSL 1.1.0+ and mbed TLS 2.18+.

For the libraries where it is not possible to use the library's own
function, we still use our own implementation. mbed TLS will continue
to use our own old PRF function while for OpenSSL we will use a
adapted version from OpenSSL 1.0.2t code. The version allows to be
used in a FIPS enabled environment.

The old OpenSSL and mbed TLS implementation could have shared some
more code but as we will eventually drop support for older TLS
libraries, the separation makes it easier it remove that code
invdidually.

No FIPS conformitiy testing etc has been done, this is only about
allowing OpenVPN on a system where FIPS mode has been enabled system
wide (e.g. on RHEL derivates).

Signed-off-by: Arne Schwabe 
---
 src/openvpn/crypto_backend.h   |  19 +++
 src/openvpn/crypto_mbedtls.c   | 143 
 src/openvpn/crypto_openssl.c   | 172 -
 src/openvpn/ssl.c  | 130 +--
 tests/unit_tests/openvpn/test_crypto.c |  32 +
 5 files changed, 366 insertions(+), 130 deletions(-)

diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 85cb084a..b72684ce 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -699,4 +699,23 @@ const char *translate_cipher_name_from_openvpn(const char 
*cipher_name);
  */
 const char *translate_cipher_name_to_openvpn(const char *cipher_name);
 
+
+/**
+ * Calculates the TLS 1.0-1.1 PRF function. For the exact specification of the
+ * fun ction definition see the TLS RFCs like RFC 4346.
+ *
+ * @param seed  seed to use
+ * @param seed_len  length of the seed
+ * @param secretsecret to use
+ * @param secret_lenlength of the secret
+ * @param outputoutput destination
+ * @param output_lenlength of output/number of bytes to generate
+ */
+void
+ssl_tls1_PRF(const uint8_t *seed,
+ int seed_len,
+ const uint8_t *secret,
+ int secret_len,
+ uint8_t *output,
+ int output_len);
 #endif /* CRYPTO_BACKEND_H_ */
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index fbb1f120..dcdd964a 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -54,6 +54,7 @@
 #include 
 
 #include 
+#include 
 
 
 /*
@@ -984,4 +985,146 @@ memcmp_constant_time(const void *a, const void *b, size_t 
size)
 
 return diff;
 }
+/* mbedtls-2.18.0 or newer */
+#ifdef HAVE_MBEDTLS_SSL_TLS_PRF
+void
+ssl_tls1_PRF(const uint8_t *seed,
+int seed_len,
+const uint8_t *secret,
+int secret_len,
+uint8_t *output,
+int output_len)
+{
+mbedtls_ssl_tls_prf(MBEDTLS_SSL_TLS_PRF_TLS1, secret, secret_len, "", seed,
+seed_len, output, output_len);
+}
+#else
+/*
+ * Generate the hash required by for the \c tls1_PRF function.
+ *
+ * @param md_kt Message digest to use
+ * @param sec   Secret to base the hash on
+ * @param sec_len   Length of the secret
+ * @param seed  Seed to hash
+ * @param seed_len  Length of the seed
+ * @param out   Output buffer
+ * @param olen  Length of the output buffer
+ */
+static void
+tls1_P_hash(const md_kt_t *md_kt,
+const uint8_t *sec,
+int sec_len,
+const uint8_t *seed,
+int seed_len,
+uint8_t *out,
+int olen)
+{
+struct gc_arena gc = gc_new();
+uint8_t A1[MAX_HMAC_KEY_LENGTH];
+
+#ifdef ENABLE_DEBUG
+const int olen_orig = olen;
+const uint8_t *out_orig = out;
+#endif
+
+hmac_ctx_t *ctx = hmac_ctx_new();
+hmac_ctx_t *ctx_tmp = hmac_ctx_new();
+
+dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash sec: %s", format_hex(sec, sec_len, 0, 
));
+dmsg(D_SHOW_KEY_SOURCE, "tls1_P_hash seed: %s", format_hex(seed, seed_len, 
0, ));
+
+int chunk = md_kt_size(md_kt);
+unsigned int A1_len = md_kt_size(md_kt);
+
+hmac_ctx_init(ctx, sec, sec_len, md_kt);
+hmac_ctx_init(ctx_tmp, sec, sec_len, md_kt);
+
+hmac_ctx_update(ctx,seed,seed_len);
+hmac_ctx_final(ctx, A1);
+
+for (;; )
+{
+hmac_ctx_reset(ctx);
+hmac_ctx_reset(ctx_tmp);
+hmac_ctx_update(ctx,A1,A1_len);
+hmac_ctx_update(ctx_tmp,A1,A1_len);
+hmac_ctx_update(ctx,seed,seed_len);
+
+if (olen > chunk)
+{
+hmac_ctx_final(ctx, out);
+out += chunk;
+olen -= chunk;
+hmac_ctx_final(ctx_tmp, A1); /* calc the next A1 value */
+}
+else/* last one */
+{
+hmac_ctx_final(ctx, A1);
+memcpy(out,A1,olen);
+break;
+}
+}
+hmac_ctx_cleanup(ctx);
+hmac_ctx_free(ctx);
+hmac_ctx_cleanup(ctx_tmp);
+

[Openvpn-devel] [PATCH 4/5] Check return values in md_ctx_init and hmac_ctx_init

2020-09-07 Thread Arne Schwabe
Without this OpenVPN will later segfault on a FIPS enabled system due
to the algorithm available but not allowed.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/crypto_openssl.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index c60d4a54..75557cca 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -954,7 +954,10 @@ md_ctx_init(EVP_MD_CTX *ctx, const EVP_MD *kt)
 ASSERT(NULL != ctx && NULL != kt);
 
 EVP_MD_CTX_init(ctx);
-EVP_DigestInit(ctx, kt);
+if (EVP_DigestInit(ctx, kt) != 1)
+{
+crypto_msg(M_FATAL, "EVP_DigestInit failed");
+}
 }
 
 void
@@ -1011,7 +1014,10 @@ hmac_ctx_init(HMAC_CTX *ctx, const uint8_t *key, int 
key_len,
 ASSERT(NULL != kt && NULL != ctx);
 
 HMAC_CTX_reset(ctx);
-HMAC_Init_ex(ctx, key, key_len, kt, NULL);
+if  (HMAC_Init_ex(ctx, key, key_len, kt, NULL) != 1)
+{
+crypto_msg(M_FATAL, "HMAC_Init_ex failed");
+}
 
 /* make sure we used a big enough key */
 ASSERT(HMAC_size(ctx) <= key_len);
-- 
2.26.2



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


[Openvpn-devel] [PATCH] Ignore --cipher for cipher negotiation in server client mode

2020-09-07 Thread Arne Schwabe
OpenVPN will ignore --cipher in lieu of the replacement data-ciphers
for cipher negioation.

Signed-off-by: Arne Schwabe 
---
 doc/man-sections/protocol-options.rst |  6 --
 src/openvpn/options.c | 26 --
 2 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/doc/man-sections/protocol-options.rst 
b/doc/man-sections/protocol-options.rst
index e9d5d63d..ca1407b9 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -57,8 +57,10 @@ configured in a compatible way between both the local and 
remote side.
   http://www.cs.ucsd.edu/users/mihir/papers/hmac.html
 
 --cipher alg
-  This option is deprecated for server-client mode. ``--data-ciphers``
-  or possibly `--data-ciphers-fallback`` should be used instead.
+  This option is ignored for server-client mode cipher selection.
+  ``--data-ciphers`` or possibly ``--data-ciphers-fallback`` must be used
+  instead.  It only determines which cipher is send in the
+  OCC string (see ``opt-verify``) for compatbility with old peers.
 
   Encrypt data channel packets with cipher algorithm ``alg``.
 
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 01da88ad..7dc3e3eb 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3074,12 +3074,6 @@ options_postprocess_cipher(struct options *o)
  "--data-ciphers-fallback config option");
 }
 
-msg(M_WARN, "--cipher is not set. Previous OpenVPN version defaulted 
to "
-"BF-CBC as fallback when cipher negotiation failed in this case. "
-"If you need this fallback please add '--data-ciphers-fallback "
-"BF-CBC' to your configuration and/or add BF-CBC to "
-"--data-ciphers.");
-
 /* We still need to set the ciphername to BF-CBC since various other
  * parts of OpenVPN assert that the ciphername is set */
 o->ciphername = "BF-CBC";
@@ -3087,22 +3081,10 @@ options_postprocess_cipher(struct options *o)
 else if (!o->enable_ncp_fallback
  && !tls_item_in_cipher_list(o->ciphername, o->ncp_ciphers))
 {
-msg(M_WARN, "DEPRECATED OPTION: --cipher set to '%s' but missing in"
-" --data-ciphers (%s). Future OpenVPN version will "
-"ignore --cipher for cipher negotiations. "
-"Add '%s' to --data-ciphers or change --cipher '%s' to "
-"--data-ciphers-fallback '%s' to silence this warning.",
-o->ciphername, o->ncp_ciphers, o->ciphername,
-o->ciphername, o->ciphername);
-o->enable_ncp_fallback = true;
-
-/* Append the --cipher to ncp_ciphers to allow it in NCP */
-size_t newlen = strlen(o->ncp_ciphers) + 1 + strlen(o->ciphername) + 1;
-char *ncp_ciphers = gc_malloc(newlen, false, >gc);
-
-ASSERT(openvpn_snprintf(ncp_ciphers, newlen, "%s:%s", o->ncp_ciphers,
-o->ciphername));
-o->ncp_ciphers = ncp_ciphers;
+msg(M_WARN, "Note: --cipher set to '%s' but missing in"
+" --data-ciphers (%s). OpenVPN 2.6+ ignores --cipher for "
+"cipher negiotiation.",
+o->ciphername, o->ncp_ciphers);
 }
 }
 
-- 
2.26.2



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


[Openvpn-devel] [PATCH] Cleanup print_details and add signature/ED certificate print

2020-09-07 Thread Arne Schwabe
This commit cleans up the logic in the function a bit. It also makes it
more clear the the details printed in the second part of the message are
details about the peer certificate and not the TLS connection as such.
Also print the signature algorithm as this might help to identify
peer certificate that still use SHA1.

The new format with for TLS 1.3 and an EC certificate.

Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer 
certificate: 384 bit EC, curve secp384r1, signature: ecdsa-with-SHA256

Using the more generic OpenSSL functions also allows use to correctly
print details about ED certificates:

Control Channel: TLSv1.3, cipher TLSv1.3 TLS_AES_256_GCM_SHA384, peer 
certificate: 253 bit ED25519, signature: ED25519

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_openssl.c | 118 +-
 1 file changed, 78 insertions(+), 40 deletions(-)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index f52c7c39..c602c625 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -2031,6 +2031,80 @@ key_state_read_plaintext(struct key_state_ssl *ks_ssl, 
struct buffer *buf,
 return ret;
 }
 
+/**
+ * Print human readable information about the certifcate into buf
+ * @param cert  the certificate being used
+ * @param buf   output buffer
+ * @param buflenoutput buffer length
+ */
+static void
+print_cert_details(X509 *cert, char *buf, size_t buflen)
+{
+const char *curve = "";
+const char *type = "(error getting type)";
+EVP_PKEY *pkey = X509_get_pubkey(cert);
+
+if (pkey == NULL)
+{
+buf[0] = 0;
+return;
+}
+
+int typeid = EVP_PKEY_id(pkey);
+
+#ifndef OPENSSL_NO_EC
+if (typeid == EVP_PKEY_EC && EVP_PKEY_get0_EC_KEY(pkey) != NULL)
+{
+EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+const EC_GROUP *group = EC_KEY_get0_group(ec);
+
+int nid = EC_GROUP_get_curve_name(group);
+if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
+{
+curve = "(error getting curve name)";
+}
+}
+#endif
+if (EVP_PKEY_id(pkey) != 0)
+{
+int typeid = EVP_PKEY_id(pkey);
+type = OBJ_nid2sn(typeid);
+
+/* OpenSSL reports rsaEncryption, dsaEncryption and
+* id-ecPublicKey, map these values to nicer ones */
+if (typeid == EVP_PKEY_RSA)
+{
+type = "RSA";
+}
+else if (typeid == EVP_PKEY_DSA)
+{
+type = "DSA";
+}
+else if (typeid == EVP_PKEY_EC)
+{
+/* EC gets the curve appended after the type */
+type = "EC, curve ";
+}
+else if (type == NULL)
+{
+type = "unknown type";
+}
+}
+
+char sig[128];
+int signature_nid = X509_get_signature_nid(cert);
+if (signature_nid != 0)
+{
+openvpn_snprintf(sig, sizeof(sig), ", signature: %s",
+ OBJ_nid2sn(signature_nid));
+}
+
+openvpn_snprintf(buf, buflen, ", peer certificate: %d bit %s%s%s",
+ EVP_PKEY_bits(pkey), type, curve, sig);
+
+EVP_PKEY_free(pkey);
+}
+
 /* **
  *
  * Information functions
@@ -2042,7 +2116,6 @@ void
 print_details(struct key_state_ssl *ks_ssl, const char *prefix)
 {
 const SSL_CIPHER *ciph;
-X509 *cert;
 char s1[256];
 char s2[256];
 
@@ -2053,48 +2126,13 @@ print_details(struct key_state_ssl *ks_ssl, const char 
*prefix)
  SSL_get_version(ks_ssl->ssl),
  SSL_CIPHER_get_version(ciph),
  SSL_CIPHER_get_name(ciph));
-cert = SSL_get_peer_certificate(ks_ssl->ssl);
-if (cert != NULL)
-{
-EVP_PKEY *pkey = X509_get_pubkey(cert);
-if (pkey != NULL)
-{
-if ((EVP_PKEY_id(pkey) == EVP_PKEY_RSA) && 
(EVP_PKEY_get0_RSA(pkey) != NULL))
-{
-RSA *rsa = EVP_PKEY_get0_RSA(pkey);
-openvpn_snprintf(s2, sizeof(s2), ", %d bit RSA",
- RSA_bits(rsa));
-}
-else if ((EVP_PKEY_id(pkey) == EVP_PKEY_DSA) && 
(EVP_PKEY_get0_DSA(pkey) != NULL))
-{
-DSA *dsa = EVP_PKEY_get0_DSA(pkey);
-openvpn_snprintf(s2, sizeof(s2), ", %d bit DSA",
- DSA_bits(dsa));
-}
-#ifndef OPENSSL_NO_EC
-else if ((EVP_PKEY_id(pkey) == EVP_PKEY_EC) && 
(EVP_PKEY_get0_EC_KEY(pkey) != NULL))
-{
-EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
-const EC_GROUP *group = EC_KEY_get0_group(ec);
-const char *curve;
+X509 *cert = SSL_get_peer_certificate(ks_ssl->ssl);
 
-int nid = EC_GROUP_get_curve_name(group);
-if (nid == 0 || (curve = OBJ_nid2sn(nid)) == NULL)
-{
-curve = "Error getting curve 

Re: [Openvpn-devel] [PATCH] Fix best gateway selection over netlink

2020-09-07 Thread Gert Doering
Hi,

On Mon, Sep 07, 2020 at 06:17:34PM +0500, Vladislav Grishenko wrote:
>  {
>  case AF_INET:
>  res.addr_size = sizeof(in_addr_t);
> -req.n.nlmsg_flags |= NLM_F_DUMP;
> +req.r.rtm_dst_len = 32;
>  break;

For the record, this actually doesn't work (long discussion on IRC),
so "NAK, followup patch coming".

Thanks for your work on this.

And perfectly on time to go into beta4 :-)

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


[Openvpn-devel] [PATCH] Fix best gateway selection over netlink

2020-09-07 Thread Vladislav Grishenko
Netlink route request with NLM_F_DUMP flag set means to
return all entries matching criteria passed in message
content - matching supplied dst address in our case.
So, gateway from the first returned route was always used
even there were more specific routes present.
By a chance, after route refactoring in ~2.6.38 first route
is the default route, so default gateway was always used,
hiding the problem.
On earlier kernels default route is the last one, so
route w/o gateway is likely be returned as first causes
gateway always to be 0.0.0.0.

Fix this behavior by requesting exact route, not dump along
with specifying correct dst perfix size.

Tested on 5.4.0, 4.1.51 and 2.6.36 kernels.

Signed-off-by: Vladislav Grishenko 
---
 src/openvpn/networking_sitnl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/networking_sitnl.c b/src/openvpn/networking_sitnl.c
index 713a213a..150dfa5c 100644
--- a/src/openvpn/networking_sitnl.c
+++ b/src/openvpn/networking_sitnl.c
@@ -477,11 +477,12 @@ sitnl_route_best_gw(sa_family_t af_family, const 
inet_address_t *dst,
 {
 case AF_INET:
 res.addr_size = sizeof(in_addr_t);
-req.n.nlmsg_flags |= NLM_F_DUMP;
+req.r.rtm_dst_len = 32;
 break;
 
 case AF_INET6:
 res.addr_size = sizeof(struct in6_addr);
+req.r.rtm_dst_len = 128;
 break;
 
 default:
-- 
2.17.1



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