Re: [Openvpn-devel] [RFC PATCH v1 04/15] OpenSSL: don't use direct access to the internal of RSA_METHOD

[Steffan Karger]

Hi,

On 17-02-17 23:00, log...@free.fr wrote:
> From: Emmanuel Deloget 
> 
> OpenSSL 1.1 does not allow us to directly access the internal of
> any data type, including RSA_METHOD. We have to use the defined
> functions to do so.
> 
> Compatibility with OpenSSL 1.0 is kept by defining the corresponding
> functions when they are not found in the library.
> 
> Signed-off-by: Emmanuel Deloget 
> ---
>  configure.ac |   9 +++
>  src/openvpn/openssl_compat.h | 186 
> +++
>  src/openvpn/ssl_openssl.c|  22 ++---
>  3 files changed, 206 insertions(+), 11 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 
> 789ad08fbaa3b3fc4c95d2b7a22332c0a93aeab4..6f31609d0aeedd2c7841d271ecadd1aa6f3b11da
>  100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -905,6 +905,15 @@ if test "${enable_crypto}" = "yes" -a 
> "${with_crypto_library}" = "openssl"; then
>   X509_STORE_get0_objects \
>   X509_OBJECT_free \
>   X509_OBJECT_get_type \
> + RSA_meth_new \
> + RSA_meth_free \
> + RSA_meth_set_pub_enc \
> + RSA_meth_set_pub_dec \
> + RSA_meth_set_priv_enc \
> + RSA_meth_set_priv_dec \
> + RSA_meth_set_init \
> + RSA_meth_set_finish \
> + RSA_meth_set0_app_data \
>   ],
>   ,
>   []
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index 
> 458a6adbe2b3fcd5ea63dcea6596cc24315d463c..b1748754f821f472cf9ed7083ade918336c9b075
>  100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -41,6 +41,8 @@
>  #include "config-msvc.h"
>  #endif
>  
> +#include "buffer.h"
> +
>  #include 
>  #include 
>  
> @@ -117,4 +119,188 @@ X509_OBJECT_get_type(const X509_OBJECT *obj)
>  }
>  #endif
>  
> +#if !defined(HAVE_RSA_METH_NEW)
> +/**
> + * Allocate a new RSA method object
> + *
> + * @param name   The object name
> + * @param flags  Configuration flags
> + * @return   A new RSA method object
> + */
> +static inline RSA_METHOD *
> +RSA_meth_new(const char *name, int flags)
> +{
> +RSA_METHOD *rsa_meth = NULL;
> +ALLOC_OBJ_CLEAR(rsa_meth, RSA_METHOD);
> +rsa_meth->name = name;
> +rsa_meth->flags = flags;
> +return rsa_meth;
> +}
> +#endif
> +
> +#if !defined(HAVE_RSA_METH_FREE)
> +/**
> + * Free an existing RSA_METHOD object
> + *
> + * @param meth   The RSA_METHOD object
> + */
> +static inline void
> +RSA_meth_free(RSA_METHOD *meth)
> +{
> +free(meth);
> +}
> +#endif

I think it would be nicer to more closely mimic the 1.1 behaviour in
RSA_meth_{new,free}(), and copy the name string in new() and free it
again in free().  That could prevent a future use-after-free that would
occur for pre-1.1.0, but not 1.1.0+:

  char *mystring = calloc(50, 1);
  RSA_METHOD *meth = RSA_meth_new(mystring, 0);
  free(mystring);

  meth.smoke();
   ^^ might cause problems

(Hint: use string_alloc(x, NULL).)

> +
> +#if !defined(HAVE_RSA_METH_SET_PUB_ENC)
> +/**
> + * Set the public encoding function of an RSA_METHOD object
> + *
> + * @param meth   The RSA_METHOD object
> + * @param pub_encthe public encoding function
> + * @return   1 on success, 0 on error
> + */
> +static inline int
> +RSA_meth_set_pub_enc(RSA_METHOD *meth,
> + int (*pub_enc) (int flen, const unsigned char *from,
> + unsigned char *to, RSA *rsa,
> + int padding))
> +{
> +if (meth)
> +{
> +meth->rsa_pub_enc = pub_enc;
> +return 1;
> +}
> +return 0;
> +}
> +#endif
> +
> +#if !defined(HAVE_RSA_METH_SET_PUB_DEC)
> +/**
> + * Set the public decoding function of an RSA_METHOD object
> + *
> + * @param meth   The RSA_METHOD object
> + * @param pub_decthe public decoding function
> + * @return   1 on success, 0 on error
> + */
> +static inline int
> +RSA_meth_set_pub_dec(RSA_METHOD *meth,
> + int (*pub_dec) (int flen, const unsigned char *from,
> + unsigned char *to, RSA *rsa,
> + int padding))
> +{
> +if (meth)
> +{
> +meth->rsa_pub_dec = pub_dec;
> +return 1;
> +}
> +return 0;
> +}
> +#endif
> +
> +#if !defined(HAVE_RSA_METH_SET_PRIV_ENC)
> +/**
> + * Set the private encoding function of an RSA_METHOD object
> + *
> + * @param meth   The RSA_METHOD object
> + * @param priv_enc   the private encoding function
> + * @return   1 on success, 0 on error
> + */
> +static inline int
> +RSA_meth_set_priv_enc(RSA_METHOD *meth,
> +  int (*priv_enc) (int f

[Openvpn-devel] [PATCH applied] Re: OpenSSL: don't use direct access to the internal of X509_STORE

[Gert Doering]

Your patch has been applied to the master and release/2.4 branch.

commit f05665df4150c6a345eec5432a02fd799bea0f2c (master)
commit 24bca7bee2ee5c48880a197ce9727bbc5a0149e5 (release/2.4)
Author: Emmanuel Deloget
Date:   Fri Feb 17 23:00:41 2017 +0100

 OpenSSL: don't use direct access to the internal of X509_STORE

 Signed-off-by: Emmanuel Deloget 
 Acked-by: Steffan Karger 
 Message-Id: 
<8e6d66e3a9a40abb3d7c99c48ba59bad1037d0ef.1487368114.git.log...@free.fr>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14076.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH applied] Re: OpenSSL: don't use direct access to the internal of SSL_CTX

[Gert Doering]

Your patch has been applied to the master and release/2.4 branch.

commit 6554ac9fed9c5680f22aa4722e6e07ebf3aa3441 (master)
commit b936ddfb631e3a4b219bd035f7110da5679b2d12 (release/2.4)
Author: Emmanuel Deloget
Date:   Fri Feb 17 23:00:40 2017 +0100

 OpenSSL: don't use direct access to the internal of SSL_CTX

 Signed-off-by: Emmanuel Deloget 
 Acked-by: Steffan Karger 
 Message-Id: 

 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14088.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH applied] Re: OpenSSL: don't use direct access to the internal of X509_OBJECT

[Gert Doering]

Your patch has been applied to the master and release/2.4 branch.

commit 47191f49890ee5c53fa78a8ce9bf96b9c8d27a82 (master)
commit d782597ede843266fd2c7854a6f90ec7ce4bb92b (release/2.4)
Author: Emmanuel Deloget
Date:   Fri Feb 17 23:00:42 2017 +0100

 OpenSSL: don't use direct access to the internal of X509_OBJECT

 Signed-off-by: Emmanuel Deloget 
 Acked-by: Steffan Karger 
 Message-Id: 

 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14080.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] Summary of today's (Wed 22nd Oct 2017) community meeting

[Samuli Seppänen]

Hi,

Here's the summary of today's IRC meeting.

---

COMMUNITY MEETING

Place: #openvpn-meeting on irc.freenode.net
Date: Wednesday 22nd Oct 2017
Time: 20:00 CET (19:00 UTC)

Planned meeting topics for this meeting were here:



The next meeting has not been scheduled yet.

Your local meeting time is easy to check from services such as



SUMMARY

chipitsine, cron2, dazo, debbie10t, eworm, mattock and syzzer
participated in this meeting

--

Discussed OpenVPN 2.4.1 release. Agreed that we should make the release
in about two weeks. The OpenSSL 1.1.x support patches will be included
if possible:



The patches should not affect OpenSSL 1.0.x support, so including the
patchset does not require us to bundle (Windows) installers with OpenSSL
1.1.x.

Eworm has done testing with the full patchset, and syzzer has tested
each patch individually.

Syzzer reviewed and ACKed a few of these patches during the meeting.

--

Discussed what to do with patches that can't be attributed properly
(name + email), such as this one:



Agreed that we can take responsibility of the smaller patches if the
author can't be reached. Larger patches that can't be attributed will be
simply ignored.

---

Full chatlog has been attached to this email.

-- 
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

(21:01:55) syzzer: meeting time!
(21:01:57) mattock: hi
(21:01:59) mattock: https://community.openvpn.net/openvpn/wiki/Topics-2017-02-22
(21:02:00) vpnHelper: Title: Topics-2017-02-22 – OpenVPN Community (at 
community.openvpn.net)
(21:03:00) mattock: I'll be back in ~5 mins, need to do something real quick
(21:03:08) cron2_: so... is there anything we can discuss without dazo?
(21:03:26) syzzer: the openssl patches?
(21:03:57) syzzer: I'm applying them to my local tree now, getting ready review
(21:04:10) cron2_: well, the two of us are in agreement anyway ("in 2.4 they 
go"), but I'd like to hear dazo's thoughts
(21:04:29) cron2_: but you can do a bit of reviewing and I do a bit of merging 
:)
(21:15:23) mattock: back
(21:16:15) eworm: I applied the openssl 1.1.0 patches to my package. Builds 
fine and 'make test' succeeds.
(21:17:41) cron2_: which is definitely welcome testing.  Please run more tests, 
connecting to actual VPNs, various cert types, etc. :-)
(21:22:45) mattock: syzzer: are you reviewing the openssl-1.1.0 patches now or 
later?
(21:23:07) syzzer: I'm doing it now, as long as we don't have anything else to 
discuss
(21:23:30) mattock: we have a discussion topic: "How to deal with (good) 
patches without proper attribution?"
(21:23:39) mattock: e.g. https://community.openvpn.net/openvpn/ticket/825
(21:23:41) vpnHelper: Title: #825 (Segfault with --tls-crypt on RHEL5/CentOS5) 
– OpenVPN Community (at community.openvpn.net)
(21:24:56) syzzer: yeah, but we need dazo...
(21:24:57) mattock: we do have a name and email address on file in LDAP, but 
the contributor might not want his full name / email published
(21:25:18) mattock: did dazo say he'd be here?
(21:25:41) syzzer: yeah, I was guessing that it would be in trac somewhere, but 
I didn't think it was a good plan to just decide to use those
(21:25:48) syzzer: yeah, but a bit later
(21:59:25) slypknot [~slypknot@unaffiliated/kettlecalling] è entrato nella 
stanza.
(22:05:13) mattock: from my PoV this is probably the most silent meeting ever
(22:05:47) chipitsine [~chipitsin@195.64.208.237] è entrato nella stanza.
(22:06:29) cron2_: definitely not a very heated discussion :)
(22:06:35) syzzer: you just /ignored everyone :p
(22:06:46) cron2_: so, what have you decided?
(22:10:00) ***dazo is here
(22:10:13) cron2_: dazo: welcome!  so let's start :)
(22:10:19) mattock: hi dazo!
(22:10:22) dazo: hey!
(22:11:27) ***dazo pokes at #825
(22:12:18) syzzer: yeah, so I actually think we all more-or-less agree
(22:12:29) syzzer: but we need to figure out what the exact policy is going to 
be
(22:12:34) cron2_: what was the question again?
(22:12:49) syzzer: "what to do with not properly attributed patches?"
(22:12:49) dazo: patch acceptance policy ... full name + email
(22:12:57) cron2_: ok
(22:13:54) slypknot: you manage to prize my name out of me .. so i think it 
should apply :)
(22:15:01) dazo: hehe :)  Yes, I do not want to deviate from this at all  
we are working on a security related product, so having proper channels to get 
in touch and to know whom contributes is valuable for the project as well
(22:15:43) dazo: plus it makes it less interesting for those "drop'n'run" 
patches ... a contribution should have a reasonable owner, IMHO
(22:16:11) slypknot: +1
(22:16:12) cron2_: so how should we handle #825, which is a bugfix (and not 
like a "here's 5000 lines of new features, thanks, bye")?
(22:17:34) dazo: If simix doesn'

Re: [Openvpn-devel] [RFC PATCH v1 03/15] OpenSSL: don't use direct access to the internal of X509_OBJECT

[Steffan Karger]

On 17-02-17 23:00, log...@free.fr wrote:
> From: Emmanuel Deloget 
> 
> OpenSSL 1.1 does not allow us to directly access the internal of
> any data type, including X509_OBJECT. We have to use the defined
> functions to do so.
> 
> Compatibility with OpenSSL 1.0 is kept by defining the corresponding
> functions when they are not found in the library.
> 
> Signed-off-by: Emmanuel Deloget 
> ---
>  configure.ac |  2 ++
>  src/openvpn/openssl_compat.h | 31 +++
>  src/openvpn/ssl_openssl.c|  5 ++---
>  src/openvpn/ssl_verify_openssl.c |  2 +-
>  4 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 
> 415128c9f8687a53e4a73419f3048d07f66b70cc..789ad08fbaa3b3fc4c95d2b7a22332c0a93aeab4
>  100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -903,6 +903,8 @@ if test "${enable_crypto}" = "yes" -a 
> "${with_crypto_library}" = "openssl"; then
>   SSL_CTX_get_default_passwd_cb \
>   SSL_CTX_get_default_passwd_cb_userdata \
>   X509_STORE_get0_objects \
> + X509_OBJECT_free \
> + X509_OBJECT_get_type \
>   ],
>   ,
>   []
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index 
> 016008bc1705a41ee0ee09fecfc0b16b282cede3..458a6adbe2b3fcd5ea63dcea6596cc24315d463c
>  100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -86,4 +86,35 @@ X509_STORE_get0_objects(X509_STORE *store)
>  }
>  #endif
>  
> +#if !defined(HAVE_X509_OBJECT_FREE)
> +/**
> + * Destroy a X509 object
> + *
> + * @param objX509 object
> + */
> +static inline void
> +X509_OBJECT_free(X509_OBJECT *obj)
> +{
> +if (obj)
> +{
> +X509_OBJECT_free_contents(obj);
> +OPENSSL_free(obj);
> +}
> +}
> +#endif
> +
> +#if !defined(HAVE_X509_OBJECT_GET_TYPE)
> +/**
> + * Get the type of an X509 object
> + *
> + * @param objX509 object
> + * @return   The underlying object type
> + */
> +static inline int
> +X509_OBJECT_get_type(const X509_OBJECT *obj)
> +{
> +return obj ? obj->type : X509_LU_FAIL;
> +}
> +#endif
> +
>  #endif /* OPENSSL_COMPAT_H_ */
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 
> e57de43a748c89ff58ea00abade0b1c317013258..bf0f643f25439f71cbfe71bf5a7e8eb834b0f012
>  100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -905,11 +905,10 @@ backend_tls_ctx_reload_crl(struct tls_root_ctx 
> *ssl_ctx, const char *crl_file,
>  {
>  X509_OBJECT *obj = sk_X509_OBJECT_value(objs, i);
>  ASSERT(obj);
> -if (obj->type == X509_LU_CRL)
> +if (X509_OBJECT_get_type(obj) == X509_LU_CRL)
>  {
>  sk_X509_OBJECT_delete(objs, i);
> -X509_OBJECT_free_contents(obj);
> -OPENSSL_free(obj);
> +X509_OBJECT_free(obj);
>  }
>  }
>  
> diff --git a/src/openvpn/ssl_verify_openssl.c 
> b/src/openvpn/ssl_verify_openssl.c
> index 
> fabbb0c370b123f54ce4a1eaf5f9650b440f47f8..07975248035b48121d1383b47f40a56042bc7380
>  100644
> --- a/src/openvpn/ssl_verify_openssl.c
> +++ b/src/openvpn/ssl_verify_openssl.c
> @@ -721,7 +721,7 @@ tls_verify_crl_missing(const struct tls_options *opt)
>  {
>  X509_OBJECT *obj = sk_X509_OBJECT_value(objs, i);
>  ASSERT(obj);
> -if (obj->type == X509_LU_CRL)
> +if (X509_OBJECT_get_type(obj) == X509_LU_CRL)
>  {
>  return false;
>  }
> 

ACK

-Steffan



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 02/15] OpenSSL: don't use direct access to the internal of X509_STORE

[Steffan Karger]

On 17-02-17 23:00, log...@free.fr wrote:
> From: Emmanuel Deloget 
> 
> OpenSSL 1.1 does not allow us to directly access the internal of
> any data type, including X509_STORE. We have to use the defined functions
> to do so.
> 
> Compatibility with OpenSSL 1.0 is kept by defining the corresponding
> functions when they are not found in the library.
> 
> Signed-off-by: Emmanuel Deloget 
> ---
>  configure.ac |  1 +
>  src/openvpn/openssl_compat.h | 15 +++
>  src/openvpn/ssl_openssl.c|  7 ---
>  src/openvpn/ssl_verify_openssl.c |  6 --
>  4 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 
> 5fe5d6046ceafa2b577296af772c347ac2ad8039..415128c9f8687a53e4a73419f3048d07f66b70cc
>  100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -902,6 +902,7 @@ if test "${enable_crypto}" = "yes" -a 
> "${with_crypto_library}" = "openssl"; then
>   [ \
>   SSL_CTX_get_default_passwd_cb \
>   SSL_CTX_get_default_passwd_cb_userdata \
> + X509_STORE_get0_objects \
>   ],
>   ,
>   []
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> index 
> 59bad9ff24d10b358419d345181a0e2e52a0c662..016008bc1705a41ee0ee09fecfc0b16b282cede3
>  100644
> --- a/src/openvpn/openssl_compat.h
> +++ b/src/openvpn/openssl_compat.h
> @@ -42,6 +42,7 @@
>  #endif
>  
>  #include 
> +#include 
>  
>  #if !defined(HAVE_SSL_CTX_GET_DEFAULT_PASSWD_CB_USERDATA)
>  /**
> @@ -71,4 +72,18 @@ SSL_CTX_get_default_passwd_cb(SSL_CTX *ctx)
>  }
>  #endif
>  
> +#if !defined(HAVE_X509_STORE_GET0_OBJECTS)
> +/**
> + * Fetch the X509 object stack from the X509 store
> + *
> + * @param store  X509 object store
> + * @return   the X509 object stack
> + */
> +static inline STACK_OF(X509_OBJECT) *
> +X509_STORE_get0_objects(X509_STORE *store)
> +{
> +return store ? store->objs : NULL;
> +}
> +#endif
> +
>  #endif /* OPENSSL_COMPAT_H_ */
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 
> 39e92f8cdae52d54d0ad95a9362e4e0e1b2289f4..e57de43a748c89ff58ea00abade0b1c317013258
>  100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -900,13 +900,14 @@ backend_tls_ctx_reload_crl(struct tls_root_ctx 
> *ssl_ctx, const char *crl_file,
>  /* Always start with a cleared CRL list, for that we
>   * we need to manually find the CRL object from the stack
>   * and remove it */
> -for (int i = 0; i < sk_X509_OBJECT_num(store->objs); i++)
> +STACK_OF(X509_OBJECT) *objs = X509_STORE_get0_objects(store);
> +for (int i = 0; i < sk_X509_OBJECT_num(objs); i++)
>  {
> -X509_OBJECT *obj = sk_X509_OBJECT_value(store->objs, i);
> +X509_OBJECT *obj = sk_X509_OBJECT_value(objs, i);
>  ASSERT(obj);
>  if (obj->type == X509_LU_CRL)
>  {
> -sk_X509_OBJECT_delete(store->objs, i);
> +sk_X509_OBJECT_delete(objs, i);
>  X509_OBJECT_free_contents(obj);
>  OPENSSL_free(obj);
>  }
> diff --git a/src/openvpn/ssl_verify_openssl.c 
> b/src/openvpn/ssl_verify_openssl.c
> index 
> 274e2bbf96b6c943ce628eab143f8c76e1c47103..fabbb0c370b123f54ce4a1eaf5f9650b440f47f8
>  100644
> --- a/src/openvpn/ssl_verify_openssl.c
> +++ b/src/openvpn/ssl_verify_openssl.c
> @@ -43,6 +43,7 @@
>  #include "ssl_openssl.h"
>  #include "ssl_verify.h"
>  #include "ssl_verify_backend.h"
> +#include "openssl_compat.h"
>  
>  #include 
>  #include 
> @@ -715,9 +716,10 @@ tls_verify_crl_missing(const struct tls_options *opt)
>  crypto_msg(M_FATAL, "Cannot get certificate store");
>  }
>  
> -for (int i = 0; i < sk_X509_OBJECT_num(store->objs); i++)
> +STACK_OF(X509_OBJECT) *objs = X509_STORE_get0_objects(store);
> +for (int i = 0; i < sk_X509_OBJECT_num(objs); i++)
>  {
> -X509_OBJECT *obj = sk_X509_OBJECT_value(store->objs, i);
> +X509_OBJECT *obj = sk_X509_OBJECT_value(objs, i);
>  ASSERT(obj);
>  if (obj->type == X509_LU_CRL)
>  {
> 

ACK

-Steffan



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 01/15] OpenSSL: don't use direct access to the internal of SSL_CTX

[Steffan Karger]

On 17-02-17 23:00, log...@free.fr wrote:
> From: Emmanuel Deloget 
> 
> OpenSSL 1.1 does not allow us to directly access the internal of
> any data type, including SSL_CTX. We have to use the defined functions
> to do so.
> 
> Compatibility with OpenSSL 1.0 is kept by defining the corresponding
> functions when they are not found in the library.
> 
> Signed-off-by: Emmanuel Deloget 
> ---
>  configure.ac |  9 ++
>  src/openvpn/openssl_compat.h | 74 
> 
>  src/openvpn/ssl_openssl.c| 13 +---
>  3 files changed, 91 insertions(+), 5 deletions(-)
>  create mode 100644 src/openvpn/openssl_compat.h
> 
> diff --git a/configure.ac b/configure.ac
> index 
> b29f8b410dfb69bce1145c3bb4a1ba011f0636ec..5fe5d6046ceafa2b577296af772c347ac2ad8039
>  100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -898,6 +898,15 @@ if test "${enable_crypto}" = "yes" -a 
> "${with_crypto_library}" = "openssl"; then
>   [have_crypto_aead_modes="no"; break]
>   )
>  
> + AC_CHECK_FUNCS(
> + [ \
> + SSL_CTX_get_default_passwd_cb \
> + SSL_CTX_get_default_passwd_cb_userdata \
> + ],
> + ,
> + []
> + )
> +
>   CFLAGS="${saved_CFLAGS}"
>   LIBS="${saved_LIBS}"
>  
> diff --git a/src/openvpn/openssl_compat.h b/src/openvpn/openssl_compat.h
> new file mode 100644
> index 
> ..59bad9ff24d10b358419d345181a0e2e52a0c662
> --- /dev/null
> +++ b/src/openvpn/openssl_compat.h
> @@ -0,0 +1,74 @@
> +/*
> + *  OpenVPN -- An application to securely tunnel IP networks
> + * over a single TCP/UDP port, with support for SSL/TLS-based
> + * session authentication and key exchange,
> + * packet encryption, packet authentication, and
> + * packet compression.
> + *
> + *  Copyright (C) 2002-2017 OpenVPN Technologies, Inc. 
> + *  Copyright (C) 2010-2017 Fox Crypto B.V. 
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2
> + *  as published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program (see the file COPYING included with this
> + *  distribution); if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + */
> +
> +/**
> + * @file OpenSSL compatibility stub
> + *
> + * This file provide compatibility stubs for the OpenSSL libraries
> + * prior to version 1.1. This version introduces many changes in the
> + * library interface, including the fact that various objects and
> + * structures are not fully opaque.
> + */
> +
> +#ifndef OPENSSL_COMPAT_H_
> +#define OPENSSL_COMPAT_H_
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#elif defined(_MSC_VER)
> +#include "config-msvc.h"
> +#endif
> +
> +#include 
> +
> +#if !defined(HAVE_SSL_CTX_GET_DEFAULT_PASSWD_CB_USERDATA)
> +/**
> + * Fetch the default password callback user data from the SSL context
> + *
> + * @param ctxSSL context
> + * @return   The password callback user data
> + */
> +static inline void *
> +SSL_CTX_get_default_passwd_cb_userdata(SSL_CTX *ctx)
> +{
> +return ctx ? ctx->default_passwd_callback_userdata : NULL;
> +}
> +#endif
> +
> +#if !defined(HAVE_SSL_CTX_GET_DEFAULT_PASSWD_CB)
> +/**
> + * Fetch the default password callback from the SSL context
> + *
> + * @param ctxSSL context
> + * @return   The password callback
> + */
> +static inline pem_password_cb *
> +SSL_CTX_get_default_passwd_cb(SSL_CTX *ctx)
> +{
> +return ctx ? ctx->default_passwd_callback : NULL;
> +}
> +#endif
> +
> +#endif /* OPENSSL_COMPAT_H_ */
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 
> abf69c91a60910e450ae6d2d49ea7e5b1cd3a535..39e92f8cdae52d54d0ad95a9362e4e0e1b2289f4
>  100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -45,6 +45,7 @@
>  #include "ssl_backend.h"
>  #include "ssl_common.h"
>  #include "base64.h"
> +#include "openssl_compat.h"
>  
>  #ifdef ENABLE_CRYPTOAPI
>  #include "cryptoapi.h"
> @@ -658,7 +659,8 @@ tls_ctx_load_pkcs12(struct tls_root_ctx *ctx, const char 
> *pkcs12_file,
>  {
>  for (i = 0; i < sk_X509_num(ca); i++)
>  {
> -if 
> (!X509_STORE_add_cert(ctx->ctx->cert_store,sk_X509_value(ca, i)))
> +X509_STORE *cert_store = SSL_CTX_get_cert_store(ctx->ctx);
> +if (!X509_STORE_add_cert(cert_store,sk

[Openvpn-devel] Should we use mbedTLS certificate profiles?

[James Yonan]

mbedTLS 2 has a new feature that allows rejection of certificates if the 
key size is too small or the signing hash is weak.

The feature is controlled via struct mbedtls_x509_crt_profile.

For example, you could specify that certificates must be at least 2048 
bits and use a SHA-2 signing alg.

Wondering if we should enable this via an option, or tie it into the 
existing tls-version-min.

The granular approach would be to have specific options for each limit, 
such as ssl-min-key-size, ssl-require-sha2

The bundled approach would be to take an existing option such as 
tls-version-min and add additional constraints onto it.  For example, if 
tls-version-min is 1.2 or higher, then also require minimum key size to 
be 2048 and certificate signing hash to be SHA-2.

James

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH applied] Re: OpenSSL: don't use direct access to the internal of X509_STORE_CTX

[Gert Doering]

Your patch has been applied to the master and release/2.4 branch.

commit 88046ad9e8e333259ae6fb4a295a9931a1a0e47f (master)
commit 58efba5013f6dae4136cc038af9ffd23796cbc0d (release/2.4)
Author: Emmanuel Deloget
Date:   Fri Feb 17 23:00:48 2017 +0100

 OpenSSL: don't use direct access to the internal of X509_STORE_CTX

 Signed-off-by: Emmanuel Deloget 
 Acked-by: Steffan Karger 
 Message-Id: 
<11477a0a3cf636572c84e0110a6f1b726bc60c2c.1487368114.git.log...@free.fr>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14085.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 09/15] OpenSSL: don't use direct access to the internal of X509_STORE_CTX

[Steffan Karger]

On 22 February 2017 at 15:47, Christian Hesse  wrote:
> Steffan Karger  on Tue, 2017/02/21 22:30:
>> ACK.  Changes look good and tested against OpenSSL 0.9.8, 1.0.0, 1.0.1
>> and 1.0.2.
>
> You answered to a patch in the middle of a series. Does this ACK apply to the
> complete series or just this patch?

Just this one.  Not much brains left last night, so I only reviewed
this rather simple and independent patch out of the series :)

-Steffan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [RFC PATCH v1 09/15] OpenSSL: don't use direct access to the internal of X509_STORE_CTX

[Christian Hesse]

Steffan Karger  on Tue, 2017/02/21 22:30:
> ACK.  Changes look good and tested against OpenSSL 0.9.8, 1.0.0, 1.0.1
> and 1.0.2.

You answered to a patch in the middle of a series. Does this ACK apply to the
complete series or just this patch?
-- 
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);}


pgpy7cO83QlgZ.pgp
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix segfault when using crypto lib without AES-256-CTR or SHA256

[David Sommerseth]

On 22/02/17 10:54, Antonio Quartulli wrote:
> On Wed, Feb 22, 2017 at 09:30:39AM +0100, Steffan Karger wrote:
>> On 22-02-17 08:39, Gert Doering wrote:
>>> On Wed, Feb 22, 2017 at 02:21:35AM +0100, David Sommerseth wrote:
>> >From d97f526a2ddbf2abe60a64260601ebd742fc00cc Mon Sep 17 00:00:00 2001
>> From: "Simon (simix)" 
>
> Do we have a policy how to handle patches with missing author info?

 I see no reason at all why we should not give proper credit with full
 name.  
>>>
>>> That was only half the question - of course I *want* to give full credits,
>>> but is "not having this information available & no SoB line" a reason
>>> for rejecting a patch?
>>>
>>> The patch in question is quite obvious, so this is not something to bring
>>> in the lawyers - more a matter of general policy.
>>
>> Same here.
>>
>> For this specific patch:  I asked the reporter on trac for full name and
>> email last night.  We can wait for a bit to see if he replies.
>>
>> In general: what do we do when we don't get a full name and email, but
>> do want to apply the patch?  Wait forever?  Claim authorship (but refer
>> to the trac ticket in the commit msg)?  Apply anyway?  ...?
> 
> IMHO somebody has to take ownership of every piece of code release under a 
> given
> license (just to avoid any future problem). So the patch should not be applied
> as is.
> 
> Then ...
> in theory, you can't take ownership of somebody else' work, but nothing 
> prevents
> you from re-writing a "very similar" patch and sign it yourself. In particular
> if the author did not show any interest in pursuing this any further.
> 

+1


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix segfault when using crypto lib without AES-256-CTR or SHA256

[Antonio Quartulli]

On Wed, Feb 22, 2017 at 09:30:39AM +0100, Steffan Karger wrote:
> On 22-02-17 08:39, Gert Doering wrote:
> > On Wed, Feb 22, 2017 at 02:21:35AM +0100, David Sommerseth wrote:
>  >From d97f526a2ddbf2abe60a64260601ebd742fc00cc Mon Sep 17 00:00:00 2001
>  From: "Simon (simix)" 
> >>>
> >>> Do we have a policy how to handle patches with missing author info?
> >>
> >> I see no reason at all why we should not give proper credit with full
> >> name.  
> > 
> > That was only half the question - of course I *want* to give full credits,
> > but is "not having this information available & no SoB line" a reason
> > for rejecting a patch?
> > 
> > The patch in question is quite obvious, so this is not something to bring
> > in the lawyers - more a matter of general policy.
> 
> Same here.
> 
> For this specific patch:  I asked the reporter on trac for full name and
> email last night.  We can wait for a bit to see if he replies.
> 
> In general: what do we do when we don't get a full name and email, but
> do want to apply the patch?  Wait forever?  Claim authorship (but refer
> to the trac ticket in the commit msg)?  Apply anyway?  ...?

IMHO somebody has to take ownership of every piece of code release under a given
license (just to avoid any future problem). So the patch should not be applied
as is.

Then ...
in theory, you can't take ownership of somebody else' work, but nothing prevents
you from re-writing a "very similar" patch and sign it yourself. In particular
if the author did not show any interest in pursuing this any further.


my 2 cents.

Cheers,


-- 
Antonio Quartulli


signature.asc
Description: Digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix segfault when using crypto lib without AES-256-CTR or SHA256

[Antonio Quartulli]

On Wed, Feb 22, 2017 at 02:07:06PM +0500, Илья Шипицин wrote:
> 2017-02-22 13:30 GMT+05:00 Steffan Karger :
> 
> > On 22-02-17 08:39, Gert Doering wrote:
> > > On Wed, Feb 22, 2017 at 02:21:35AM +0100, David Sommerseth wrote:
> >  >From d97f526a2ddbf2abe60a64260601ebd742fc00cc Mon Sep 17 00:00:00
> > 2001
> >  From: "Simon (simix)" 
> > >>>
> > >>> Do we have a policy how to handle patches with missing author info?
> > >>
> > >> I see no reason at all why we should not give proper credit with full
> > >> name.
> > >
> > > That was only half the question - of course I *want* to give full
> > credits,
> > > but is "not having this information available & no SoB line" a reason
> > > for rejecting a patch?
> > >
> > > The patch in question is quite obvious, so this is not something to bring
> > > in the lawyers - more a matter of general policy.
> >
> > Same here.
> >
> > For this specific patch:  I asked the reporter on trac for full name and
> > email last night.  We can wait for a bit to see if he replies.
> >
> > In general: what do we do when we don't get a full name and email, but
> > do want to apply the patch?  Wait forever?  Claim authorship (but refer
> > to the trac ticket in the commit msg)?  Apply anyway?  ...?
> >
> 
> if there are trac templates (I'm not very familiar with trac internals), we
> can turn on the requirement of
> 
> 1) full name
> 2) legacy agreements
> 
> on the trac side

personally I think that trac is not the place to submit patches, and this
problem could re-appear anywhere: i.e. a patch for openvpn submitted somewhere
else reporting no name/email.



-- 
Antonio Quartulli


signature.asc
Description: Digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix segfault when using crypto lib without AES-256-CTR or SHA256

[Илья Шипицин]

2017-02-22 13:30 GMT+05:00 Steffan Karger :

> On 22-02-17 08:39, Gert Doering wrote:
> > On Wed, Feb 22, 2017 at 02:21:35AM +0100, David Sommerseth wrote:
>  >From d97f526a2ddbf2abe60a64260601ebd742fc00cc Mon Sep 17 00:00:00
> 2001
>  From: "Simon (simix)" 
> >>>
> >>> Do we have a policy how to handle patches with missing author info?
> >>
> >> I see no reason at all why we should not give proper credit with full
> >> name.
> >
> > That was only half the question - of course I *want* to give full
> credits,
> > but is "not having this information available & no SoB line" a reason
> > for rejecting a patch?
> >
> > The patch in question is quite obvious, so this is not something to bring
> > in the lawyers - more a matter of general policy.
>
> Same here.
>
> For this specific patch:  I asked the reporter on trac for full name and
> email last night.  We can wait for a bit to see if he replies.
>
> In general: what do we do when we don't get a full name and email, but
> do want to apply the patch?  Wait forever?  Claim authorship (but refer
> to the trac ticket in the commit msg)?  Apply anyway?  ...?
>

if there are trac templates (I'm not very familiar with trac internals), we
can turn on the requirement of

1) full name
2) legacy agreements

on the trac side


>
> (While typing this, I realize this sounds like a topic for the meeting
> tonight.  I'll put it on the agenda.)


> -Steffan
>
>
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix segfault when using crypto lib without AES-256-CTR or SHA256

[Steffan Karger]

On 22-02-17 08:39, Gert Doering wrote:
> On Wed, Feb 22, 2017 at 02:21:35AM +0100, David Sommerseth wrote:
 >From d97f526a2ddbf2abe60a64260601ebd742fc00cc Mon Sep 17 00:00:00 2001
 From: "Simon (simix)" 
>>>
>>> Do we have a policy how to handle patches with missing author info?
>>
>> I see no reason at all why we should not give proper credit with full
>> name.  
> 
> That was only half the question - of course I *want* to give full credits,
> but is "not having this information available & no SoB line" a reason
> for rejecting a patch?
> 
> The patch in question is quite obvious, so this is not something to bring
> in the lawyers - more a matter of general policy.

Same here.

For this specific patch:  I asked the reporter on trac for full name and
email last night.  We can wait for a bit to see if he replies.

In general: what do we do when we don't get a full name and email, but
do want to apply the patch?  Wait forever?  Claim authorship (but refer
to the trac ticket in the commit msg)?  Apply anyway?  ...?

(While typing this, I realize this sounds like a topic for the meeting
tonight.  I'll put it on the agenda.)

-Steffan



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel