[1/1] ntp: add patch to support for libressl

Message ID 20171108121143.5411-1-aduskett@gmail.com
State Rejected, archived
Headers show
Series
  • [1/1] ntp: add patch to support for libressl
Related show

Commit Message

Adam Duskett Nov. 8, 2017, 12:11 p.m.
NTP currently fails to compile against LibreSSL because of checks used
to determine the SSL library version.

Upstream-Status: Pending
http://bugs.ntp.org/show_bug.cgi?id=3401#c3

Signed-off-by: Adam Duskett <aduskett@gmail.com>
---
 package/ntp/0004-libressl-support.patch | 107 ++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 package/ntp/0004-libressl-support.patch

Comments

Thomas Petazzoni Nov. 23, 2017, 9:51 p.m. | #1
Hello,

On Wed,  8 Nov 2017 07:11:43 -0500, Adam Duskett wrote:
> NTP currently fails to compile against LibreSSL because of checks used
> to determine the SSL library version.
> 
> Upstream-Status: Pending
> http://bugs.ntp.org/show_bug.cgi?id=3401#c3
> 
> Signed-off-by: Adam Duskett <aduskett@gmail.com>

Arnout, Peter, Yann, I think we discussed this topic during the
Buildroot meeting, and concluded we didn't want patches in Buildroot to
enable LibreSSL compatibility with a package. Do we stand on this
position, and reject Adam's contribution on ntp?

> + #ifndef OPENSSL_VERSION_NUMBER
> ++#ifndef LIBRESSL_VERSION_NUMBER

In addition, this continue to use the LIBRESSL_VERSION_NUMBER approach,
which will fail when libressl gains support for new APIs.

Thanks in advance for giving your feedback on this patch. If nobody
complains, I'll merge :-)

Thanks!

Thomas
Arnout Vandecappelle (Essensium/Mind) Nov. 23, 2017, 10:27 p.m. | #2
On 23-11-17 22:51, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed,  8 Nov 2017 07:11:43 -0500, Adam Duskett wrote:
>> NTP currently fails to compile against LibreSSL because of checks used
>> to determine the SSL library version.
>>
>> Upstream-Status: Pending
>> http://bugs.ntp.org/show_bug.cgi?id=3401#c3
>>
>> Signed-off-by: Adam Duskett <aduskett@gmail.com>
> 
> Arnout, Peter, Yann, I think we discussed this topic during the
> Buildroot meeting, and concluded we didn't want patches in Buildroot to
> enable LibreSSL compatibility with a package. Do we stand on this
> position, and reject Adam's contribution on ntp?

 I don't think the conclusion was that we would reject patches to enable
LibreSSL compatibility outright, only:

> 
>> + #ifndef OPENSSL_VERSION_NUMBER
>> ++#ifndef LIBRESSL_VERSION_NUMBER
> 
> In addition, this continue to use the LIBRESSL_VERSION_NUMBER approach,
> which will fail when libressl gains support for new APIs.

 we would reject this approach because I believe it is not upstreamable.

 I think upstreamable patches are acceptable. And maybe even the
LIBRESSL_VERSION_NUMBER approach is OK - but then I first want to see a reliable
upstream accept it.


 Regards,
 Arnout

> 
> Thanks in advance for giving your feedback on this patch. If nobody
> complains, I'll merge :-)
> 
> Thanks!
> 
> Thomas
>
Thomas Petazzoni Nov. 23, 2017, 10:29 p.m. | #3
Hello,

On Thu, 23 Nov 2017 23:27:18 +0100, Arnout Vandecappelle wrote:

> > Arnout, Peter, Yann, I think we discussed this topic during the
> > Buildroot meeting, and concluded we didn't want patches in Buildroot to
> > enable LibreSSL compatibility with a package. Do we stand on this
> > position, and reject Adam's contribution on ntp?  
> 
>  I don't think the conclusion was that we would reject patches to enable
> LibreSSL compatibility outright, only:
> 
> >   
> >> + #ifndef OPENSSL_VERSION_NUMBER
> >> ++#ifndef LIBRESSL_VERSION_NUMBER  
> > 
> > In addition, this continue to use the LIBRESSL_VERSION_NUMBER approach,
> > which will fail when libressl gains support for new APIs.  
> 
>  we would reject this approach because I believe it is not upstreamable.
> 
>  I think upstreamable patches are acceptable. And maybe even the
> LIBRESSL_VERSION_NUMBER approach is OK - but then I first want to see a reliable
> upstream accept it.

The issue here is that Adam has submitted the patch upstream a while
ago (see bugs.ntp.org/show_bug.cgi?id=3401#c3), and upstream has
reacted.

Thomas
Arnout Vandecappelle (Essensium/Mind) Nov. 23, 2017, 10:39 p.m. | #4
On 23-11-17 23:29, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 23 Nov 2017 23:27:18 +0100, Arnout Vandecappelle wrote:
> 
>>> Arnout, Peter, Yann, I think we discussed this topic during the
>>> Buildroot meeting, and concluded we didn't want patches in Buildroot to
>>> enable LibreSSL compatibility with a package. Do we stand on this
>>> position, and reject Adam's contribution on ntp?  
>>
>>  I don't think the conclusion was that we would reject patches to enable
>> LibreSSL compatibility outright, only:
>>
>>>   
>>>> + #ifndef OPENSSL_VERSION_NUMBER
>>>> ++#ifndef LIBRESSL_VERSION_NUMBER  
>>>
>>> In addition, this continue to use the LIBRESSL_VERSION_NUMBER approach,
>>> which will fail when libressl gains support for new APIs.  
>>
>>  we would reject this approach because I believe it is not upstreamable.
>>
>>  I think upstreamable patches are acceptable. And maybe even the
>> LIBRESSL_VERSION_NUMBER approach is OK - but then I first want to see a reliable
>> upstream accept it.
> 
> The issue here is that Adam has submitted the patch upstream a while
> ago (see bugs.ntp.org/show_bug.cgi?id=3401#c3), and upstream has
> reacted.

 Maybe I missed something, but I don't see the reaction?

 Regards,
 Arnout
Thomas Petazzoni Nov. 24, 2017, 7:50 a.m. | #5
Hello,

On Thu, 23 Nov 2017 23:39:57 +0100, Arnout Vandecappelle wrote:
> On 23-11-17 23:29, Thomas Petazzoni wrote:
> > Hello,
> > 
> > On Thu, 23 Nov 2017 23:27:18 +0100, Arnout Vandecappelle wrote:
> >   
> >>> Arnout, Peter, Yann, I think we discussed this topic during the
> >>> Buildroot meeting, and concluded we didn't want patches in Buildroot to
> >>> enable LibreSSL compatibility with a package. Do we stand on this
> >>> position, and reject Adam's contribution on ntp?    
> >>
> >>  I don't think the conclusion was that we would reject patches to enable
> >> LibreSSL compatibility outright, only:
> >>  
> >>>     
> >>>> + #ifndef OPENSSL_VERSION_NUMBER
> >>>> ++#ifndef LIBRESSL_VERSION_NUMBER    
> >>>
> >>> In addition, this continue to use the LIBRESSL_VERSION_NUMBER approach,
> >>> which will fail when libressl gains support for new APIs.    
> >>
> >>  we would reject this approach because I believe it is not upstreamable.
> >>
> >>  I think upstreamable patches are acceptable. And maybe even the
> >> LIBRESSL_VERSION_NUMBER approach is OK - but then I first want to see a reliable
> >> upstream accept it.  
> > 
> > The issue here is that Adam has submitted the patch upstream a while
> > ago (see bugs.ntp.org/show_bug.cgi?id=3401#c3), and upstream has
> > reacted.  
> 
>  Maybe I missed something, but I don't see the reaction?

It was too late when I replied to you. I obviously wanted to say:
"upstream has *not* reacted". I.e, your request to "see a reliable
upstream accept it" is hard to achieve, because even if Adam did the
effort of submitting the patch upstream, there was no reaction, either
positive or negative.

Best regards,

Thomas
Yann E. MORIN Nov. 25, 2017, 12:43 p.m. | #6
Thomas, All,

On 2017-11-23 22:51 +0100, Thomas Petazzoni spake thusly:
> On Wed,  8 Nov 2017 07:11:43 -0500, Adam Duskett wrote:
> > NTP currently fails to compile against LibreSSL because of checks used
> > to determine the SSL library version.
> > 
> > Upstream-Status: Pending
> > http://bugs.ntp.org/show_bug.cgi?id=3401#c3
> > 
> > Signed-off-by: Adam Duskett <aduskett@gmail.com>
> 
> Arnout, Peter, Yann, I think we discussed this topic during the
> Buildroot meeting, and concluded we didn't want patches in Buildroot to
> enable LibreSSL compatibility with a package. Do we stand on this
> position, and reject Adam's contribution on ntp?

My position is to avoid feature patches. Adding libresl support in a
package is adding a new feature IMHO.

As such, we should not accept it, unless it has *already* been accepted
*and* merged upstream.

Regards,
Yann E. MORIN.

> > + #ifndef OPENSSL_VERSION_NUMBER
> > ++#ifndef LIBRESSL_VERSION_NUMBER
> 
> In addition, this continue to use the LIBRESSL_VERSION_NUMBER approach,
> which will fail when libressl gains support for new APIs.
> 
> Thanks in advance for giving your feedback on this patch. If nobody
> complains, I'll merge :-)
> 
> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Arnout Vandecappelle (Essensium/Mind) Nov. 25, 2017, 4:47 p.m. | #7
On 25-11-17 13:43, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2017-11-23 22:51 +0100, Thomas Petazzoni spake thusly:
>> On Wed,  8 Nov 2017 07:11:43 -0500, Adam Duskett wrote:
>>> NTP currently fails to compile against LibreSSL because of checks used
>>> to determine the SSL library version.
>>>
>>> Upstream-Status: Pending
>>> http://bugs.ntp.org/show_bug.cgi?id=3401#c3
>>>
>>> Signed-off-by: Adam Duskett <aduskett@gmail.com>
>>
>> Arnout, Peter, Yann, I think we discussed this topic during the
>> Buildroot meeting, and concluded we didn't want patches in Buildroot to
>> enable LibreSSL compatibility with a package. Do we stand on this
>> position, and reject Adam's contribution on ntp?
> 
> My position is to avoid feature patches. Adding libresl support in a
> package is adding a new feature IMHO.

 Well, it's a bit borderline; it's more like patching a script so that it can
run with busybox ash instead of relying on a bashism. However...

> As such, we should not accept it, unless it has *already* been accepted
> *and* merged upstream.

 ... I agree with this one, since there are some doubts (at least in my mind)
that this is the right approach.

 Regards,
 Arnout

> 
> Regards,
> Yann E. MORIN.
> 
>>> + #ifndef OPENSSL_VERSION_NUMBER
>>> ++#ifndef LIBRESSL_VERSION_NUMBER
>>
>> In addition, this continue to use the LIBRESSL_VERSION_NUMBER approach,
>> which will fail when libressl gains support for new APIs.
>>
>> Thanks in advance for giving your feedback on this patch. If nobody
>> complains, I'll merge :-)
>>
>> Thanks!
>>
>> Thomas
>> -- 
>> Thomas Petazzoni, CTO, Free Electrons
>> Embedded Linux and Kernel engineering
>> http://free-electrons.com
>

Patch

diff --git a/package/ntp/0004-libressl-support.patch b/package/ntp/0004-libressl-support.patch
new file mode 100644
index 0000000000..2d046b4b09
--- /dev/null
+++ b/package/ntp/0004-libressl-support.patch
@@ -0,0 +1,107 @@ 
+From a3bd714d3028241c7546ded6ae6b93887a17a7fa Mon Sep 17 00:00:00 2001
+From: Adam Duskett <aduskett@gmail.com>
+Date: Wed, 12 Jul 2017 09:14:38 -0400
+Subject: [PATCH] add libressl support
+
+Fix some preprocessor macros to add libressl support.
+
+Upstream-Status: Pending
+http://bugs.ntp.org/show_bug.cgi?id=3401#c3
+
+Signed-off-by: Adam Duskett <aduskett@gmail.com>
+---
+ include/libssl_compat.h                | 4 +++-
+ libntp/libssl_compat.c                 | 2 +-
+ libntp/ssl_init.c                      | 2 +-
+ ports/winnt/include/msvc_ssl_autolib.h | 2 +-
+ sntp/libevent/test/regress_ssl.c       | 4 ++--
+ 5 files changed, 8 insertions(+), 6 deletions(-)
+
+diff --git a/include/libssl_compat.h b/include/libssl_compat.h
+index 2a3697c..eede47b 100644
+--- a/include/libssl_compat.h
++++ b/include/libssl_compat.h
+@@ -25,8 +25,10 @@
+ #include "openssl/rsa.h"
+ 
+ #ifndef OPENSSL_VERSION_NUMBER
++#ifndef LIBRESSL_VERSION_NUMBER
+ #define OPENSSL_VERSION_NUMBER SSLEAY_VERSION_NUMBER
+ #endif
++#endif
+ 
+ #ifndef OPENSSL_VERSION_TEXT
+ #define OPENSSL_VERSION_TEXT SSLEAY_VERSION_TEXT
+@@ -37,7 +39,7 @@
+ #endif
+ 
+ /* ----------------------------------------------------------------- */
+-#if OPENSSL_VERSION_NUMBER < 0x10100000L
++#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+ /* ----------------------------------------------------------------- */
+ 
+ # include <openssl/objects.h>
+diff --git a/libntp/libssl_compat.c b/libntp/libssl_compat.c
+index afe4d07..dae7017 100644
+--- a/libntp/libssl_compat.c
++++ b/libntp/libssl_compat.c
+@@ -26,7 +26,7 @@
+ /* ----------------------------------------------------------------- */
+ 
+ /* ----------------------------------------------------------------- */
+-#if defined(OPENSSL) && OPENSSL_VERSION_NUMBER < 0x10100000L
++#if defined(OPENSSL) && OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+ /* ----------------------------------------------------------------- */
+ 
+ #include "libssl_compat.h"
+diff --git a/libntp/ssl_init.c b/libntp/ssl_init.c
+index bebf6e1..0a27050 100644
+--- a/libntp/ssl_init.c
++++ b/libntp/ssl_init.c
+@@ -21,7 +21,7 @@
+ 
+ int ssl_init_done;
+ 
+-#if OPENSSL_VERSION_NUMBER < 0x10100000L
++#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+ 
+ static void
+ atexit_ssl_cleanup(void)
+diff --git a/ports/winnt/include/msvc_ssl_autolib.h b/ports/winnt/include/msvc_ssl_autolib.h
+index 688b5e2..393e1c9 100644
+--- a/ports/winnt/include/msvc_ssl_autolib.h
++++ b/ports/winnt/include/msvc_ssl_autolib.h
+@@ -85,7 +85,7 @@
+  * request in the object file, depending on the SSL version and the
+  * build variant.
+  */
+-# if OPENSSL_VERSION_NUMBER >= 0x10100000L
++# if OPENSSL_VERSION_NUMBER >= 0x10100000L && ! defined(LIBRESSL_VERSION_NUMBER)
+ #  pragma comment(lib, "libcrypto" LTAG_SIZE LTAG_RTLIB LTAG_DEBUG ".lib")
+ # else
+ #  pragma comment(lib, "libeay32" LTAG_RTLIB LTAG_DEBUG ".lib")
+diff --git a/sntp/libevent/test/regress_ssl.c b/sntp/libevent/test/regress_ssl.c
+index 226a2a3..dc761dc 100644
+--- a/sntp/libevent/test/regress_ssl.c
++++ b/sntp/libevent/test/regress_ssl.c
+@@ -61,7 +61,7 @@
+ 
+ #include <string.h>
+ 
+-#if OPENSSL_VERSION_NUMBER < 0x10100000L
++#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER) || defined(LIBRESSL_VERSION_NUMBER)
+ #define OpenSSL_version_num SSLeay
+ #endif /* OPENSSL_VERSION_NUMBER */
+ 
+@@ -130,7 +130,7 @@ getcert(void)
+ 	X509_set_subject_name(x509, name);
+ 	X509_set_issuer_name(x509, name);
+ 
+-#if OPENSSL_VERSION_NUMBER < 0x10100000L
++#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+ 	X509_time_adj(X509_get_notBefore(x509), 0, &now);
+ 	now += 3600;
+ 	X509_time_adj(X509_get_notAfter(x509), 0, &now);
+-- 
+2.13.0
+