Re: [Openvpn-devel] [PATCH] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-06 Thread Antonio Quartulli 
On Mon, Feb 06, 2017 at 08:18:01PM +0100, Olivier W wrote:
> Should be compatible with all versions of OpenSSL and LibreSSL.
> Similar to what is done in curl:
> https://github.com/curl/curl/blob/028391df5d84d9fae3433afdee9261d565900355/lib/vtls/openssl.c#L603-L619
> 
> Error while compiling was:
> "ssl_openssl.c:512:30: error: no member named 'cert' in 'struct ssl_ctx_st'
> ssl.cert = ctx->ctx->cert;
> ~ ^
> 1 error generated.
> *** Error code 1"
> ---
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 8266595..a889332 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -508,10 +508,13 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx
> *ctx, const char *curve_name
>  const EC_GROUP *ecgrp = NULL;
>  EVP_PKEY *pkey = NULL;
> 
> -/* Little hack to get private key ref from SSL_CTX, yay OpenSSL... */
> -SSL ssl;
> -ssl.cert = ctx->ctx->cert;
> -pkey = SSL_get_privatekey(&ssl);
> +SSL *ssl = SSL_new(ctx->ctx);
> +if (!ssl)
> +{
> +crypto_msg(M_FATAL, "SSL_new failed");
> +}
> +pkey = SSL_get_privatekey(ssl);
> +SSL_free(ssl);

I have a question (sorry if I couldn't check myself): did you check that
SSL_get_privatekey() and SSL_free() won't crash when ssl is NULL ?

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

[Openvpn-devel] [PATCH] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-06 Thread Olivier W 
Should be compatible with all versions of OpenSSL and LibreSSL.
Similar to what is done in curl:
https://github.com/curl/curl/blob/028391df5d84d9fae3433afdee9261d565900355/lib/vtls/openssl.c#L603-L619

Error while compiling was:
"ssl_openssl.c:512:30: error: no member named 'cert' in 'struct ssl_ctx_st'
ssl.cert = ctx->ctx->cert;
~ ^
1 error generated.
*** Error code 1"
---
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 8266595..a889332 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -508,10 +508,13 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx
*ctx, const char *curve_name
 const EC_GROUP *ecgrp = NULL;
 EVP_PKEY *pkey = NULL;

-/* Little hack to get private key ref from SSL_CTX, yay OpenSSL... */
-SSL ssl;
-ssl.cert = ctx->ctx->cert;
-pkey = SSL_get_privatekey(&ssl);
+SSL *ssl = SSL_new(ctx->ctx);
+if (!ssl)
+{
+crypto_msg(M_FATAL, "SSL_new failed");
+}
+pkey = SSL_get_privatekey(ssl);
+SSL_free(ssl);

 msg(D_TLS_DEBUG, "Extracting ECDH curve from private key");

--
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 applied] github: Add PR template with contributor related information

2017-02-06 Thread David Sommerseth 
On 06/02/17 12:19, Илья Шипицин wrote:
> 
> it is the same as previous version of this patch, which I sent before
> your guidelines, and you did not complain earlier.
> it is not clear, how 167 chars long might be bad or good. for me it is
> just 167 chars.

You do understand and realize you just prove my earlier points and make
them really obvious? ... you seem not to read and learn ... from the
same URL [1] already provided several times now:

The seven rules of a great Git commit message
-

1. Separate subject from body with a blank line
2. Limit the subject line to 50 characters
3. Capitalize the subject line
4. Do not end the subject line with a period
5. Use the imperative mood in the subject line
6. Wrap the body at 72 characters
7. Use the body to explain what and why vs. how

[1] 

As a clever person once said: "I can only provide the information, I
cannot make you understand it".

> it is not trivial to be sure that it will fit your expectation.
> as an example, there are automatic tests. and you are ok with that.

First ... *read* URL[1], *read* the 7 guidelines above.

Code itself can be automated tested.  But the syntax of the code needs
to be correct to be possible to test it.  From my point of view, making
an analogy: You break the syntax.  It doesn't compile.  Which means the
result isn't testable.

> we should not assign machine job to a man, just involve some
> automation which will tell "your commit message is too long" without
> involving your time.

Those seven rules are not that far away from the coding style guidelines
we all are being picked at from time to time (*including* myself).

It is first and foremost the submitters responsibility to ensure that
everything is in order before doing the submission.  And we have both
on-list, off-list and IRC requests from time to time, where people ask
what kind of guidelines we use, where to find them and so on.  This is a
pretty common behaviour by most community contributors, not even
remotely unique to OpenVPN.  What you want is a way to not take proper
ownership of your contribution, moving the responsibility out of your area.

Sorry, this is not going to happen.

And I don't think I will even have much to say for the future in this
regards.


-- 
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 applied] github: Add PR template with contributor related information

2017-02-06 Thread Илья Шипицин 
2017-02-06 15:44 GMT+05:00 David Sommerseth :

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> I have ACKed and applied this, just to get this one resolved.  But I am
> very
> disappointed that you have yet again completely ignored the guidelines
> described in the URL[1] already provided to you - which describes how a
> patch
> and commit message should look like.
>

what I have learned is --annotate switch and, as you can see, now subject
contains "v2", so it is easy to distingush this patch and earlier edition.


>
> In fact - your SUBJECT line was 167 characters long, _including_ a URL.  It
> did not even have a proper body message.  *sigh*
>

it is the same as previous version of this patch, which I sent before your
guidelines, and you did not complain earlier.
it is not clear, how 167 chars long might be bad or good. for me it is just
167 chars.

if it is really bad, we might involve some automatic to submission process,
which will check how long is string.
@mattock ?

it is not trivial to be sure that it will fit your expectation.
as an example, there are automatic tests. and you are ok with that.
you do not being tedious "guys, I told you many times, write code without
bugs... why you are ignoring me..."

we should not assign machine job to a man, just involve some automation
which will tell "your commit message is too long" without involving your
time.



>
> You really need to stop wasting our [2] time and really read and truly
> learn
> what you read in a far better ways.  If you do not change your behaviour
> here,
> the result will be that we will not put much efforts into your
> contributions
> in the future - unless we have a lot of time available, which, honestly,
> is not very happening often.
>
> Just ask yourself: If you care so little to play by our guidelines, why
> should we care about your patches?
>
> [1] 
> [2] our ==> primarily developers and reviewers
>
>
> Regardless, your patch has been applied to the master branch
>
> commit 9e849c1e00a88abed65760ac7bee3df4aed2b754
> Author: Ilya Shipitsin
> Date:   Mon Feb 6 11:55:15 2017 +0500
>
>  github: Add PR template with contributor related information
>
>  Acked-by: David Sommerseth 
>  Message-Id: <1486364115-9801-1-git-send-email-chipits...@gmail.com>
>  URL: https://www.mail-archive.com/openvpn-devel@lists.
> sourceforge.net/msg14010.html
>  Signed-off-by: David Sommerseth 
>
>
> - --
> kind regards,
>
> David Sommerseth
>
> -BEGIN PGP SIGNATURE-
> Version: GnuPG v2.0.22 (GNU/Linux)
>
> iQIcBAEBAgAGBQJYmFN/AAoJEIbPlEyWcf3yQoUP+wSNXtlDXyXgsGQpwoaPkuic
> u9wrjwtAxdizLXgtfNV0hY5ouoV9gEEA0L0UY+UmCi1Dk4cgGQq1uDyxDTytyW3q
> k6ilwzf8dCsa2KnWnJgF7ZTVmq8xn/XKj8FYJcNVI3sT9fWE+xUsJUHzINQmRVKc
> q9SrKC3BrCDJX6+pgQqMXwXE/i5iwBy+wAYILVQnQ936nOap0EsJFiI7pOqADNrf
> 6JlTzRJm1N+0sF54dX9eek27s4CuUDR+XMwO3DLp+qA3oCQfldFhwWz99aUR0DM4
> wloW0B3IRvcQAUWgIU3dDrMt78AzMZxd+BbPpCvKq6V6Msbk0AoUiQ+PduYEJ1bn
> 270iGhfL8mcMscGo7H/W2tHeZOpCzODlvflCEa83oFEiVYYltaIfxLpnoDiZ0gWy
> J4jCD4fqE5gcC3J7GkcpTtuSv+eYpjvNT8vsNSVaMHjrPESgfDWx1RYc2sqwbmCP
> uBcHWjBqJb8Fkq8r9cGc36+kAdfo3ljfPKSM+dtQxse/8U8xSMoMTjDpSAnJ8w2p
> s8gWU2rvB06gijBHxC4Bt9G3BXEPhRU4GKwP0Vnf8IGCVgXMyKf8nZZzWWs7/BAk
> ZzgXTfggHZLcuV9CxxpCLLDSe2NXk5Vapz+Y1AVh/bE+oRDooevxLzrLhnrdbFwQ
> FIXn7HqzCwJoxW7xP08c
> =IYjw
> -END PGP 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 applied] github: Add PR template with contributor related information

2017-02-06 Thread David Sommerseth 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

I have ACKed and applied this, just to get this one resolved.  But I am very
disappointed that you have yet again completely ignored the guidelines
described in the URL[1] already provided to you - which describes how a patch
and commit message should look like.

In fact - your SUBJECT line was 167 characters long, _including_ a URL.  It
did not even have a proper body message.  *sigh*

You really need to stop wasting our [2] time and really read and truly learn
what you read in a far better ways.  If you do not change your behaviour here,
the result will be that we will not put much efforts into your contributions
in the future - unless we have a lot of time available, which, honestly,
is not very happening often.

Just ask yourself: If you care so little to play by our guidelines, why
should we care about your patches?

[1] 
[2] our ==> primarily developers and reviewers


Regardless, your patch has been applied to the master branch

commit 9e849c1e00a88abed65760ac7bee3df4aed2b754
Author: Ilya Shipitsin
Date:   Mon Feb 6 11:55:15 2017 +0500

 github: Add PR template with contributor related information

 Acked-by: David Sommerseth 
 Message-Id: <1486364115-9801-1-git-send-email-chipits...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg14010.html
 Signed-off-by: David Sommerseth 


- --
kind regards,

David Sommerseth

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYmFN/AAoJEIbPlEyWcf3yQoUP+wSNXtlDXyXgsGQpwoaPkuic
u9wrjwtAxdizLXgtfNV0hY5ouoV9gEEA0L0UY+UmCi1Dk4cgGQq1uDyxDTytyW3q
k6ilwzf8dCsa2KnWnJgF7ZTVmq8xn/XKj8FYJcNVI3sT9fWE+xUsJUHzINQmRVKc
q9SrKC3BrCDJX6+pgQqMXwXE/i5iwBy+wAYILVQnQ936nOap0EsJFiI7pOqADNrf
6JlTzRJm1N+0sF54dX9eek27s4CuUDR+XMwO3DLp+qA3oCQfldFhwWz99aUR0DM4
wloW0B3IRvcQAUWgIU3dDrMt78AzMZxd+BbPpCvKq6V6Msbk0AoUiQ+PduYEJ1bn
270iGhfL8mcMscGo7H/W2tHeZOpCzODlvflCEa83oFEiVYYltaIfxLpnoDiZ0gWy
J4jCD4fqE5gcC3J7GkcpTtuSv+eYpjvNT8vsNSVaMHjrPESgfDWx1RYc2sqwbmCP
uBcHWjBqJb8Fkq8r9cGc36+kAdfo3ljfPKSM+dtQxse/8U8xSMoMTjDpSAnJ8w2p
s8gWU2rvB06gijBHxC4Bt9G3BXEPhRU4GKwP0Vnf8IGCVgXMyKf8nZZzWWs7/BAk
ZzgXTfggHZLcuV9CxxpCLLDSe2NXk5Vapz+Y1AVh/bE+oRDooevxLzrLhnrdbFwQ
FIXn7HqzCwJoxW7xP08c
=IYjw
-END PGP 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