[asterisk-dev] [Code Review] 3709: configure.ac: Check OpenSSL for support of Elliptic Curve cryptography

[Alexander Traud]

> On July 4, 2014, 7:48 a.m., Alexander Traud wrote:
> > As I am the contributor of the original patch: Argg, I thought I double-checked all symbols. Anyway, mistakes happen.
> > I am not sure why I wasn’t invited as reviewer for this one here.
> > 
> > EC_KEY_new_by_curve_name() is the only culprit symbol, or? The first part of my patch is not related to Elliptic Curve (ECDHE). Therefore, DHE is not affected and should not be stripped. Consequently, the here introduced #ifdef HAVE_OPENSSL_EC should come later. I would have placed just before that last } else { to disable just that condition. That way, ECDHE kicks in automatically even without a re-compile, when the underlying OpenSSL library is changed to version 1.0.2.
> > 
> > As I am new here, do I have to open a review with a patch attached, or how do we handle this?
> 
> wdoekes wrote:
>     Quickest is if you open your own request. Mortals do not have the power to change other people's requests. And exchanging patches over RB is not common.
>     
>     Oh, I see that it has been submitted already. In that case: open your own request.
> 
> wdoekes wrote:
>     And generally there is a period of rest between submitting the review and committing the code, so you would normally have been in time for review.
>     
>     Unbreaking the test build agents was probably considered high priority ;)

> Unbreaking the test build agents was probably considered high priority.

Sure. No problem with that. But somehow getting an invite/notification about this, because (I think) I am related to this. I just noticed about this review by looking-up the revisions in trunk. I could have easily missed it. Isn’t it possible to invite mortals via the ‘peoples’ tag here?

Actually, I would have expected a follow-up/related bug in the issue tracker which then links to this review. To be honest, I do not get the relationship between the issue tracker and the review board quite yet.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3709/#review12483
-----------------------------------------------------------


On July 3, 2014, 4:55 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3709/
> -----------------------------------------------------------
> 
> (Updated July 3, 2014, 4:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23905
>     https://issues.asterisk.org/jira/browse/ASTERISK-23905
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Review https://reviewboard.asterisk.org/r/3647/ introduced PFS in Asterisk that depends on the elliptic curve library support being present in OpenSSL. As it turns out, some versions of OpenSSL don't have this library - notably the version running on our build agents.
> 
> This patch fixes the build by providing a configure check for the specific library calls that the PFS patch relies on.
> 
> 
> Diffs
> -----
> 
>   /trunk/main/tcptls.c 417876 
>   /trunk/include/asterisk/autoconfig.h.in 417876 
>   /trunk/configure.ac 417876 
>   /trunk/configure UNKNOWN 
> 
> Diff: https://reviewboard.asterisk.org/r/3709/diff/
> 
> 
> Testing
> -------
> 
> With ec.h present, Asterisk detects the header via the presence of the specified function and sets HAVE_OPENSSL_EC to 1.
> 
> Without ec.h present, Asterisk does not detect the file. HAVE_OPENSSL_EC is 0, and the build succeeds.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140704/f9eb54dc/attachment.html>