| Message ID | 20171108121143.5411-1-aduskett@gmail.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | [1/1] ntp: add patch to support for libressl | expand |
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
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 >
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
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
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
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
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 >
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 +
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