Re: [Openvpn-devel] [PATCH] Fix building with LibreSSL 2.5.1 by cleaning a hack. Similar to what is done in curl: https://github.com/curl/curl/blob/028391df5d84d9fae3433afdee9261d565900355/lib/vtls/op

2017-02-15 Thread Antonio Quartulli
On Wed, Feb 15, 2017 at 10:34:16PM +0100, Olivier W wrote:
> Hello Steffan,
> 
> 2017-02-14 23:00 GMT+01:00 Steffan Karger :
> 
> > Code still looks good, patch looks a lot better (applies cleanly now),
> > but could use an extra newline in the subject.  But that doesn't warrant
> > an extra patch iteration, so ACK.
> 
> Great, thanks!
> Yes, I don't know why my commit has a long line i the subject, when I
> used "git commit --amend" to change the message, "Similar to what..."
> was on a new line. 

You need to put an empty line between the subject and the body.
For example:


my commit message

this is the body and can be multiline
random text here...
and here...



Cheers,


-- 
Antonio Quartulli

--
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 building with LibreSSL 2.5.1 by cleaning a hack. Similar to what is done in curl: https://github.com/curl/curl/blob/028391df5d84d9fae3433afdee9261d565900355/lib/vtls/op

2017-02-15 Thread Olivier W
Hello Steffan,

2017-02-14 23:00 GMT+01:00 Steffan Karger :

> Code still looks good, patch looks a lot better (applies cleanly now),
> but could use an extra newline in the subject.  But that doesn't warrant
> an extra patch iteration, so ACK.

Great, thanks!
Yes, I don't know why my commit has a long line i the subject, when I
used "git commit --amend" to change the message, "Similar to what..."
was on a new line. Next time I'll contribute to OpenVPN, I'll try to
make less mistakes with git.

Best Regards,
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack. Similar to what is done in curl: https://github.com/curl/curl/blob/028391df5d84d9fae3433afdee9261d565900355/lib/vtls/op

2017-02-14 Thread Steffan Karger
Hi,

On 13-02-17 19:38, O2 Graphics wrote:
> Use SSL_CTX_get0_privatekey() for OpenSSL >= 1.0.2
> 
> Signed-off-by: Olivier Wahrenberger 
> ---
>  src/openvpn/ssl_openssl.c | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 8266595..abf69c9 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -508,10 +508,18 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx *ctx, 
> const char *curve_name
>  const EC_GROUP *ecgrp = NULL;
>  EVP_PKEY *pkey = NULL;
>  
> +#if OPENSSL_VERSION_NUMBER >= 0x10002000L && 
> !defined(LIBRESSL_VERSION_NUMBER)
> +pkey = SSL_CTX_get0_privatekey(ctx->ctx);
> +#else
>  /* 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_new(ctx->ctx);
> +if (!ssl)
> +{
> +crypto_msg(M_FATAL, "SSL_new failed");
> +}
> +pkey = SSL_get_privatekey(ssl);
> +SSL_free(ssl);
> +#endif
>  
>  msg(D_TLS_DEBUG, "Extracting ECDH curve from private key");
>  
> 

Code still looks good, patch looks a lot better (applies cleanly now),
but could use an extra newline in the subject.  But that doesn't warrant
an extra patch iteration, so ACK.

-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] [PATCH] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-14 Thread Olivier W
Hey :-)

2017-02-14 21:00 GMT+01:00 Gert Doering :
> Funny enough, I was already subscribed to that bug... :)

:-)

> I find it a bit weird that this not addressed upstream in git itself,
> but is burdened on the operating system maintainers...  but most likely,
> I'm missing half the story.

Of course it should be addressed upstream, I've composed the patch
from a pull request I had found: https://github.com/git/git/pull/251
dating from last June.
>From what I understood, they didn't merge it yet because it uses a too
recent version of Net::SMTP, which isn't included in Travis-CI... :-/
So for now, we have to use a patch for FreeBSD, and probably others
BSD/Linux use a patch until it's upstreamed.

> whatever - it works now!  :-) - and now to address your actual patch!
>
> Steffan...?

Yeah, that would be cool! ;-)

Best Regards,
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-14 Thread Gert Doering
Hi,

On Tue, Feb 14, 2017 at 08:32:42PM +0100, Olivier W wrote:
> So, finally I've been able to use "git-send-email" with TLS. The
> problem was a patch used by FreeBSD.
> 
> If there are any FreeBSD users on this list or for the curious, you
> can find more information on FreeBSD's bugtracker:
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214335#c10

Funny enough, I was already subscribed to that bug... :)

> Or if someday you update git, Perl or one of its components
> (Net::SMTP::SSL is deprecated) and "git-send-email" break, you may
> find a way to solve the problem in the link posted above.

I find it a bit weird that this not addressed upstream in git itself,
but is burdened on the operating system maintainers...  but most likely,
I'm missing half the story.

whatever - it works now!  :-) - and now to address your actual patch!

Steffan...?

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: 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] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-14 Thread Olivier W
Hello,

So, finally I've been able to use "git-send-email" with TLS. The
problem was a patch used by FreeBSD.

If there are any FreeBSD users on this list or for the curious, you
can find more information on FreeBSD's bugtracker:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214335#c10

Or if someday you update git, Perl or one of its components
(Net::SMTP::SSL is deprecated) and "git-send-email" break, you may
find a way to solve the problem in the link posted above.

HTH!
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread Olivier W
Hello Selva,

2017-02-13 22:34 GMT+01:00 Selva Nair :
> On Debian jessie, the following .gitconfig works fo me.
>
> [sendemail]
>   smtpEncryption = tls
>   smtpServer = smtp.gmail.com
>   smtpUser = user.n...@gmail.com
>   smtpServerPort = 587
>
> No smtpsslcertpath specified, I suppose it verifies the cert using
> /etc/ssl/certs as the capath, which is the default.

Thanks! Your configuration is what I tried, from git-send-email doc's:
https://git-scm.com/docs/git-send-email#_use_gmail_as_the_smtp_server
I've just tested on Debian and I've been able to send an email with
TLS, so the problem isn't my git or gmail setup.
It could be a FreeBSD only issue, I see they have a patched version of
git-send-email because Net::SMTP::SSL is deprecated.

> Possibly your /etc/ssl/cert.pem is to blame? I do not have such a file, so
> no idea what it contains.

My /etc/ssl/cert.pem file contains all root certificates provided by
Mozilla NSS project, it should be valid.
I'll ask FreeBSD users if they can successfully use TLS with
git-send-email and if not I'll try to debug the script.

Best Regards,
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread Selva Nair
Hi,

On Mon, Feb 13, 2017 at 3:55 PM, Olivier W  wrote:

> >> That's a not exactly helpful error message... :( - I tend to just turn
> >> off SSL on stuff that goes to public mailing lists anyway if it causes
> >> issues...
> >
> > OpenSSL errors requires quite some efforts to get used to.  And in
> > addition the git-send-email errors on top doesn't always make life
> easier.
>
> I've just tried git-send-email with "--smtp-debug=1" and the error
> isn't much useful, I'm getting:
> "...
> Net::SMTP=GLOB(0x8048189a8)<<< 250 SMTPUTF8
> Net::SMTP=GLOB(0x8048189a8)>>> STARTTLS
> Net::SMTP=GLOB(0x8048189a8)<<< 220 2.0.0 Ready to start TLS
> Net::SMTP=GLOB(0x8048189a8)>>> STARTTLS
> Net::SMTP: Net::Cmd::getline(): unexpected EOF on command channel:
> Connection reset by peer at /usr/local/libexec/git-core/git-send-email
> line 1371.
> STARTTLS failed!  at /usr/local/libexec/git-core/git-send-email line
> 1371."
>

On Debian jessie, the following .gitconfig works fo me.

[sendemail]
  smtpEncryption = tls
  smtpServer = smtp.gmail.com
  smtpUser = user.n...@gmail.com 
  smtpServerPort = 587

No smtpsslcertpath specified, I suppose it verifies the cert using
/etc/ssl/certs as the capath, which is the default.

Possibly your /etc/ssl/cert.pem is to blame? I do not have such a file, so
no idea what it contains.

Selva
--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread Olivier W
Hello David,

2017-02-13 20:37 GMT+01:00 David Sommerseth :
>
> smtpsslcertpath needs to point at a CA certificate which issued the SMTP
> server certificate.  You can easily verify that things are correct by
> grabbing the server certificate using openssl:
>
> $ openssl s_client -connect $SMTP_SERVER:$PORT -starttls smtp
>
> Copy the certificate blob printed to stdout to a file.  Then take your
> CA certificate (including full chain in a single file, where the root CA
> certificate is the last one in file) and run this command:
>
> $ openssl verify -CAfile $CA_CERT_CHAIN $SERVER_CERT
>
> The output should display the file of the server certificate and ": OK".

Thanks for your help, but I still can't use GMail on port 587 (but
everything is OK on port 465).
In my .gitconfig, I have "smtpsslcertpath = /etc/ssl/cert.pem" and the
.pem file exists, installed by the package "ca_root_nss" on FreeBSD.

So, I tried "openssl s_client -connect smtp.gmail.com:587 -starttls
smtp", copying the content from: "-BEGIN CERTIFICATE-" to
"-END CERTIFICATE-" in a file, but running "openssl verify
-CAfile /etc/ssl/cert.pem gmail.cert" gives:
"gmail.cert: C = US, ST = California, L = Mountain View, O = Google
Inc, CN = smtp.gmail.com
error 20 at 0 depth lookup:unable to get local issuer certificate"

I also tried on Debian and I'm getting the same error. Same thing with
Hotmail on "smtp.live.com:587".

Well, as long as I can use the other port with SSL, it's ok :-)

>> That's a not exactly helpful error message... :( - I tend to just turn
>> off SSL on stuff that goes to public mailing lists anyway if it causes
>> issues...
>
> OpenSSL errors requires quite some efforts to get used to.  And in
> addition the git-send-email errors on top doesn't always make life easier.

I've just tried git-send-email with "--smtp-debug=1" and the error
isn't much useful, I'm getting:
"...
Net::SMTP=GLOB(0x8048189a8)<<< 250 SMTPUTF8
Net::SMTP=GLOB(0x8048189a8)>>> STARTTLS
Net::SMTP=GLOB(0x8048189a8)<<< 220 2.0.0 Ready to start TLS
Net::SMTP=GLOB(0x8048189a8)>>> STARTTLS
Net::SMTP: Net::Cmd::getline(): unexpected EOF on command channel:
Connection reset by peer at /usr/local/libexec/git-core/git-send-email
line 1371.
STARTTLS failed!  at /usr/local/libexec/git-core/git-send-email line 1371."

>>> BTW: sorry about the previous email: "[SPAM] [PATCH] Fix building with
>>> LibreSSL 2.5.1 by cleaning a hack." :-/ I'm trying to not post anymore
>>> buggy email here.
>
> No worries!  As long as you don't spam us completely with non-sense, we
> can handle a few misfires ;-)

;-)

Best Regards,
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread David Sommerseth
On 13/02/17 18:50, Gert Doering wrote:
> Hi,
> 
> On Mon, Feb 13, 2017 at 06:46:11PM +0100, Olivier W wrote:
[...snip...]
>> Now I'm fighting with git which doesn't want to use gmail's smtp
>> server to send the email. My .gitconfigure file is similar to:
>> https://git-scm.com/docs/git-send-email#_example and I've added
>> "smtpsslcertpath = /etc/ssl/cert.pem" but I'm getting this error:
>> "STARTTLS failed!  at /usr/local/libexec/git-core/git-send-email line
>> 1371."
>> I'm currently searching for a solution.

smtpsslcertpath needs to point at a CA certificate which issued the SMTP
server certificate.  You can easily verify that things are correct by
grabbing the server certificate using openssl:

$ openssl s_client -connect $SMTP_SERVER:$PORT -starttls smtp

Copy the certificate blob printed to stdout to a file.  Then take your
CA certificate (including full chain in a single file, where the root CA
certificate is the last one in file) and run this command:

$ openssl verify -CAfile $CA_CERT_CHAIN $SERVER_CERT

The output should display the file of the server certificate and ": OK".

> That's a not exactly helpful error message... :( - I tend to just turn
> off SSL on stuff that goes to public mailing lists anyway if it causes
> issues...

OpenSSL errors requires quite some efforts to get used to.  And in
addition the git-send-email errors on top doesn't always make life easier.

>> BTW: sorry about the previous email: "[SPAM] [PATCH] Fix building with
>> LibreSSL 2.5.1 by cleaning a hack." :-/ I'm trying to not post anymore
>> buggy email here.

No worries!  As long as you don't spam us completely with non-sense, we
can handle a few misfires ;-)

> I've created my share of weird git e-mails in the past :-) - so what I've
> started to do is "send the mail to myself" (if possible, on a different
> account) and then verify if the result is what I want to see...

That's a good advice :)

-- 
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread Olivier W
Hey :-)

2017-02-13 18:50 GMT+01:00 Gert Doering :
> That's a not exactly helpful error message... :( - I tend to just turn
> off SSL on stuff that goes to public mailing lists anyway if it causes
> issues...

Thanks. I also tried without SSL, but then I had messages about git
not understanding "AUTH" :-/
Finally, for gmail, it's working with:
smtpEncryption = ssl
smtpServerPort = 465
instead of tls/587

> I've created my share of weird git e-mails in the past :-) - so what I've
> started to do is "send the mail to myself" (if possible, on a different
> account) and then verify if the result is what I want to see...

Yes, I guess many people struggle with the email configuration :-) I
had tried to first send to my personal email address and it went well,
but I'm not sure how it worked since I didn't have anything related to
SMTP configuration in ".gitconfig".

Finally, I think it's now OK :-)

Best Regards,
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread Gert Doering
Hi,

On Mon, Feb 13, 2017 at 06:46:11PM +0100, Olivier W wrote:
> 2017-02-13 9:31 GMT+01:00 Gert Doering :
> > Look at "git rebase --interactive", which will enable you to squash
> > three commits into a single one.  Then you can use "git commit --amend"
> > to work on the (combined) commit message until you're happy with it.
> 
> Thanks a lot. I've finally squashed my commits, pushed to github.

Good :-)

> Now I'm fighting with git which doesn't want to use gmail's smtp
> server to send the email. My .gitconfigure file is similar to:
> https://git-scm.com/docs/git-send-email#_example and I've added
> "smtpsslcertpath = /etc/ssl/cert.pem" but I'm getting this error:
> "STARTTLS failed!  at /usr/local/libexec/git-core/git-send-email line
> 1371."
> I'm currently searching for a solution.

That's a not exactly helpful error message... :( - I tend to just turn
off SSL on stuff that goes to public mailing lists anyway if it causes
issues...

> BTW: sorry about the previous email: "[SPAM] [PATCH] Fix building with
> LibreSSL 2.5.1 by cleaning a hack." :-/ I'm trying to not post anymore
> buggy email here.

I've created my share of weird git e-mails in the past :-) - so what I've
started to do is "send the mail to myself" (if possible, on a different
account) and then verify if the result is what I want to see...

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: 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] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread Olivier W
Hello Gert,

2017-02-13 9:31 GMT+01:00 Gert Doering :
> Look at "git rebase --interactive", which will enable you to squash
> three commits into a single one.  Then you can use "git commit --amend"
> to work on the (combined) commit message until you're happy with it.

Thanks a lot. I've finally squashed my commits, pushed to github.

Now I'm fighting with git which doesn't want to use gmail's smtp
server to send the email. My .gitconfigure file is similar to:
https://git-scm.com/docs/git-send-email#_example and I've added
"smtpsslcertpath = /etc/ssl/cert.pem" but I'm getting this error:
"STARTTLS failed!  at /usr/local/libexec/git-core/git-send-email line
1371."
I'm currently searching for a solution.

BTW: sorry about the previous email: "[SPAM] [PATCH] Fix building with
LibreSSL 2.5.1 by cleaning a hack." :-/ I'm trying to not post anymore
buggy email here.

Best Regards.

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-13 Thread Gert Doering
Hi,

On Mon, Feb 13, 2017 at 12:02:45AM +0100, Olivier W wrote:
> I'll be sending the patch with "git format-patch" + "git send-email"
> as I have three commits and I'm not sure how to send a single patch
> with only "git send-email"

Look at "git rebase --interactive", which will enable you to squash
three commits into a single one.  Then you can use "git commit --amend"
to work on the (combined) commit message until you're happy with it.

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: 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] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-12 Thread Olivier W
Hello Steffan,

2017-02-10 22:20 GMT+01:00 Steffan Karger :
>
> The code change looks good to me, so ACK to that.

Thanks!

> The commit does need a better commit message though, and somewhere in
> the process the newlines got mangled.  I think the commit message of
> your initial patch is okay, it just missed the Signed-off-by line.  To
> smoothen the patch application process, could you create a commit with
> the above change, and send it to the list using git send-email?  The
> command line would be something like:
>
> git send-email --to=openvpn-devel@lists.sourceforge.net HEAD~1
>
> Otherwise, Gert or David will have to wrestle a commit together from you
> various messages, and that will likely take more time before it gets
> applied.

Sure, I'm sorry the patch got messed up and seems GMail also added the
message I was responding to at the bottom :-/
I'll be sending the patch with "git format-patch" + "git send-email"
as I have three commits and I'm not sure how to send a single patch
with only "git send-email"

Best Regards,
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-10 Thread Steffan Karger
Hi,

On 09-02-17 21:04, Olivier W wrote:
> Hello,
> Please find the new version of the patch.
> 
> So, I added back the comment I had removed and new versions of OpenSSL
> will use SSL_CTX_get0_privatekey() instead of SSL_new() +
> SSL_get_privatekey() + SSL_free().
> 
> It successfully compile with LibreSSL 2.4.5, 2.5.1 and OpenSSL 1.0.2k.
> I've also pushed it to Github and Travis-CI is currently running:
> https://github.com/OpenVPN/openvpn/pull/82
> 
> Best Regards,
> Olivier
> 
> ---
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 8266595..abf69c9 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -508,10 +508,18 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx
> *ctx, const char *curve_name
>  const EC_GROUP *ecgrp = NULL;
>  EVP_PKEY *pkey = NULL;
> 
> +#if OPENSSL_VERSION_NUMBER >= 0x10002000L && 
> !defined(LIBRESSL_VERSION_NUMBER)
> +pkey = SSL_CTX_get0_privatekey(ctx->ctx);
> +#else
>  /* 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_new(ctx->ctx);
> +if (!ssl)
> +{
> +crypto_msg(M_FATAL, "SSL_new failed");
> +}
> +pkey = SSL_get_privatekey(ssl);
> +SSL_free(ssl);
> +#endif
> 
>  msg(D_TLS_DEBUG, "Extracting ECDH curve from private key");
>

The code change looks good to me, so ACK to that.

The commit does need a better commit message though, and somewhere in
the process the newlines got mangled.  I think the commit message of
your initial patch is okay, it just missed the Signed-off-by line.  To
smoothen the patch application process, could you create a commit with
the above change, and send it to the list using git send-email?  The
command line would be something like:

git send-email --to=openvpn-devel@lists.sourceforge.net HEAD~1

Otherwise, Gert or David will have to wrestle a commit together from you
various messages, and that will likely take more time before it gets
applied.

Thanks,
-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] [PATCH] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-09 Thread Olivier W
Hello,
Please find the new version of the patch.

So, I added back the comment I had removed and new versions of OpenSSL
will use SSL_CTX_get0_privatekey() instead of SSL_new() +
SSL_get_privatekey() + SSL_free().

It successfully compile with LibreSSL 2.4.5, 2.5.1 and OpenSSL 1.0.2k.
I've also pushed it to Github and Travis-CI is currently running:
https://github.com/OpenVPN/openvpn/pull/82

Best Regards,
Olivier

---
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 8266595..abf69c9 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -508,10 +508,18 @@ tls_ctx_load_ecdh_params(struct tls_root_ctx
*ctx, const char *curve_name
 const EC_GROUP *ecgrp = NULL;
 EVP_PKEY *pkey = NULL;

+#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)
+pkey = SSL_CTX_get0_privatekey(ctx->ctx);
+#else
 /* 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_new(ctx->ctx);
+if (!ssl)
+{
+crypto_msg(M_FATAL, "SSL_new failed");
+}
+pkey = SSL_get_privatekey(ssl);
+SSL_free(ssl);
+#endif

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

2017-02-08 23:39 GMT+01:00 Steffan Karger :
> Hi,
>
> On 06-02-17 20:18, 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_new(ctx->ctx);
>> +if (!ssl)
>> +{
>> +crypto_msg(M_FATAL, "SSL_new failed");
>> +}
>> +pkey = SSL_get_privatekey(ssl);
>> +SSL_free(ssl);
>
> The code change looks good, and passes my (manual) tests.  I'd like to
> keep the comment though, because this still is a hack/workaround to get
> the private key from the SSL_CTX object, it just does so a little nicer
> at the cost of a number of malloc/free calls.
>
> It might be even worth noting that the workaround is only needed for
> OpenSSL <= 1.0.1, because later versions do have a function to get the
> private key from a struct SSL_CTX directly.  By noting that explicitly,
> we help ourselves remember to get rid of the hack as soon as we drop
> support for these OpenSSL versions.
>
> -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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-09 Thread Olivier W
Hello,

2017-02-08 23:39 GMT+01:00 Steffan Karger :
>
> The code change looks good, and passes my (manual) tests.  I'd like to
> keep the comment though, because this still is a hack/workaround to get
> the private key from the SSL_CTX object, it just does so a little nicer
> at the cost of a number of malloc/free calls.

Thanks for the review!

The hack was because the code was accessing the cert in a strange way,
not using the OpenSSL's functions, that's why I thought it was a good
idea to remove it.
But I'll add it back.

> It might be even worth noting that the workaround is only needed for
> OpenSSL <= 1.0.1, because later versions do have a function to get the
> private key from a struct SSL_CTX directly.  By noting that explicitly,
> we help ourselves remember to get rid of the hack as soon as we drop
> support for these OpenSSL versions.

That's right, I've just looked and like Arne said, we just have to add
a check for OpenSSL >= 1.0.2 and not LibreSSL to use the new function.
I'll update my patch later today.

LibreSSL will probably also add SSL_CTX_get0_privatekey() in a later
version, so the check will be needed to be updated.

Best Regards,
Olivier

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-08 Thread Arne Schwabe
Am 08.02.17 um 23:39 schrieb Steffan Karger:
> Hi,
> 
> On 06-02-17 20:18, 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_new(ctx->ctx);
>> +if (!ssl)
>> +{
>> +crypto_msg(M_FATAL, "SSL_new failed");
>> +}
>> +pkey = SSL_get_privatekey(ssl);
>> +SSL_free(ssl);
> 
> The code change looks good, and passes my (manual) tests.  I'd like to
> keep the comment though, because this still is a hack/workaround to get
> the private key from the SSL_CTX object, it just does so a little nicer
> at the cost of a number of malloc/free calls.
> 
> It might be even worth noting that the workaround is only needed for
> OpenSSL <= 1.0.1, because later versions do have a function to get the
> private key from a struct SSL_CTX directly.  By noting that explicitly,
> we help ourselves remember to get rid of the hack as soon as we drop
> support for these OpenSSL versions.
> 
SOund like adding an ifdef for >= 1.0.2 would be a good idea when we
touch this code now anyway.

Arne


--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-08 Thread Steffan Karger
Hi,

On 07-02-17 09:45, Илья Шипицин wrote:
> 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 ?
> 
> what if we involve clang static analyzer for such things ? can we count
> on it ?
> 
> it is capable of detecting "Argument with 'nonnull' attribute passed null"
> 
> and, as I can see, after applying patch it didn't find new issues
> 
> http://chipitsine.github.io/without-patch/
> http://chipitsine.github.io/with-patch/
> 
> 
> also, it might be even automated, run clang static analyzer before and
> after applying patch and compare the result

Static analyzers are useful, but do not and probably never will replace
review by someone who knows the code.  They complement each other;
neither will detect all mistakes.

In relation to that, please stop making statements like 'it passes
travis, so the patch must be okay'.  That's pertinently not true.

-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] [PATCH] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-08 Thread Steffan Karger
Hi,

On 06-02-17 20:18, 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_new(ctx->ctx);
> +if (!ssl)
> +{
> +crypto_msg(M_FATAL, "SSL_new failed");
> +}
> +pkey = SSL_get_privatekey(ssl);
> +SSL_free(ssl);

The code change looks good, and passes my (manual) tests.  I'd like to
keep the comment though, because this still is a hack/workaround to get
the private key from the SSL_CTX object, it just does so a little nicer
at the cost of a number of malloc/free calls.

It might be even worth noting that the workaround is only needed for
OpenSSL <= 1.0.1, because later versions do have a function to get the
private key from a struct SSL_CTX directly.  By noting that explicitly,
we help ourselves remember to get rid of the hack as soon as we drop
support for these OpenSSL versions.

-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] [PATCH] Fix building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-07 Thread Olivier W
Hello,

Good question. For the test, I looked how it was done in other parts
of ssl_openssl.c, like around line 1518:
https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/ssl_openssl.c#L1518-L1522
and did the same.
And as said by Gert, crypto_msg() solve it.

I'm sorry, in my first message, I fortgot to add a link to my pull
request on Github: https://github.com/OpenVPN/openvpn/pull/82

Best Regards.

2017-02-07 5:14 GMT+01:00 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_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

--
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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-07 Thread Илья Шипицин
2017-02-07 9:14 GMT+05:00 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/028391df5d84d9fae3433afdee9261
> d565900355/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_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 ?
>


what if we involve clang static analyzer for such things ? can we count on
it ?

it is capable of detecting "Argument with 'nonnull' attribute passed null"

and, as I can see, after applying patch it didn't find new issues

http://chipitsine.github.io/without-patch/
http://chipitsine.github.io/with-patch/


also, it might be even automated, run clang static analyzer before and
after applying patch and compare the result


>
> Cheers,
>
>
> --
> Antonio Quartulli
>
> 
> --
> 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 building with LibreSSL 2.5.1 by cleaning a hack.

2017-02-07 Thread Gert Doering
Hi,

On Tue, Feb 07, 2017 at 12:14:51PM +0800, Antonio Quartulli wrote:
> > +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 ?

crypto_msg(M_FATAL, ...) will not return.

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: 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] 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_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