[OE-core] [PATCH] openssl10: Fix mutliple include assumptions for bn.h in opensslconf.h

Fri Mar 1 17:40:00 UTC 2019

On Fri, Mar 1, 2019 at 9:35 AM Denys Dmytriyenko <denis at denix.org> wrote:
>
> On Wed, Feb 27, 2019 at 05:51:23PM -0800, Khem Raj wrote:
> > On Mon, Feb 25, 2019 at 7:19 PM Denys Dmytriyenko <denis at denix.org> wrote:
> > >
> > > Khem, Richard,
> > >
> > > Sorry for belated reply. I haven't had time for master yet, but since this
> > > just got backported to thud, I'm seeing a similar breakage.
> > >
> > > First of all, BN_LLONG not being defined does seem to be "fixed" by this
> > > patch, but I'm not entirely sure why it now checks for OPENSSL_SYS_UEFI - this
> > > seems to be a new define in OpenSSL 1.1, and doesn't even exist in OpenSSL 1.0
> > > Is it a pure luck that it works now? Any hidden meaning I missded?
> > >
> >
> > We don't have to, but then we might have uefi to compile so thats why its
> > there.
>
> Where is OPENSSL_SYS_UEFI supposed to be defined?
>

some apps like uefi can do that.

> There are 0 references to it in OpenSSL 1.0 code base. It appears to be
> introduced only in OpenSSL 1.1, since there are several references there.
>
> Hence my earlier question about whether it should be used here in the patch
> for 1.0.2q
>
>
> > > And it also breaks exactly the same for DES_LONG due to a similar construct:
> >
> > yeah the header nesting is a nightmare in openssl 1.0, it is a bit better
> > in 1.1, a lot of these recursions are fixed. but backporting then completely
> > to 1.0 is not an option, so we have to do tweaks that sustains the build.
> >
> > >
> > >
> > > #if (defined(HEADER_NEW_DES_H) || defined(HEADER_DES_H)) && !defined(DES_LONG)
> > > /* If this is set to 'unsigned int' on a DEC Alpha, this gives about a
> > >  * %20 speed up (longs are 8 bytes, int's are 4). */
> > > #ifndef DES_LONG
> > > #define DES_LONG unsigned int
> > > #endif
> > > #endif
>
> Would you recommend doing the same change here to fix DES_LONG not being
> defined due to nested header inclusion?
>
>
> > > I was going to fix it similarly as BN_LLONG, but since I don't understand how
> > > it was supposed to be fixed, I'm not sure how to fix DES_LONG not being
> > > defined. Any ideas?
> > >
> > > Thanks.
> > >
> > > --
> > > Denys
> > >
> > >
> > > On Wed, Feb 06, 2019 at 10:25:26PM -0800, Khem Raj wrote:
> > > > After adding #pragma once to wrapper header ( opensslconf.h ) this
> > > > latent issue got to bite us, where it expect bn.h to be including
> > > > openssl.h to define BN_* defines, which is fragile. This patch removes
> > > > the contraints for nested includes for bn.h
> > > >
> > > > Signed-off-by: Khem Raj <raj.khem at gmail.com>
> > > > ---
> > > >  .../0001-Fix-BN_LLONG-breakage.patch          | 33 +++++++++++++++++++
> > > >  .../openssl/openssl10_1.0.2q.bb               |  1 +
> > > >  2 files changed, 34 insertions(+)
> > > >  create mode 100644 meta/recipes-connectivity/openssl/openssl10/0001-Fix-BN_LLONG-breakage.patch
> > > >
> > > > diff --git a/meta/recipes-connectivity/openssl/openssl10/0001-Fix-BN_LLONG-breakage.patch b/meta/recipes-connectivity/openssl/openssl10/0001-Fix-BN_LLONG-breakage.patch
> > > > new file mode 100644
> > > > index 0000000000..13d39c918c
> > > > --- /dev/null
> > > > +++ b/meta/recipes-connectivity/openssl/openssl10/0001-Fix-BN_LLONG-breakage.patch
> > > > @@ -0,0 +1,33 @@
> > > > +From 247b3188cde5f3347091cd54271127386d3aece0 Mon Sep 17 00:00:00 2001
> > > > +From: Khem Raj <raj.khem at gmail.com>
> > > > +Date: Wed, 6 Feb 2019 22:10:33 -0800
> > > > +Subject: [PATCH] Fix BN_LLONG breakage
> > > > +
> > > > +opensslconf.h is un-defining BN_LLONG only when included from bn.h which
> > > > +is not robust at all, especially when include guards are used and
> > > > +multiple inclusions of a given header is not allowed. so lets take out
> > > > +the nesting constraint and add OPENSSL_SYS_UEFI constraint instead
> > > > +
> > > > +Upstream-Status: Inappropriate [ fixed differently with OpenSSL 1.1+ ]
> > > > +
> > > > +Signed-off-by: Khem Raj <raj.khem at gmail.com>
> > > > +---
> > > > + crypto/opensslconf.h.in | 2 +-
> > > > + 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > +
> > > > +diff --git a/crypto/opensslconf.h.in b/crypto/opensslconf.h.in
> > > > +index 7a1c85d..a10c10f 100644
> > > > +--- a/crypto/opensslconf.h.in
> > > > ++++ b/crypto/opensslconf.h.in
> > > > +@@ -56,7 +56,7 @@
> > > > + #endif
> > > > + #endif
> > > > +
> > > > +-#if defined(HEADER_BN_H) && !defined(CONFIG_HEADER_BN_H)
> > > > ++#if !defined(OPENSSL_SYS_UEFI) && !defined(CONFIG_HEADER_BN_H)
> > > > + #define CONFIG_HEADER_BN_H
> > > > + #undef BN_LLONG
> > > > +
> > > > +--
> > > > +2.20.1
> > > > +
> > > > diff --git a/meta/recipes-connectivity/openssl/openssl10_1.0.2q.bb b/meta/recipes-connectivity/openssl/openssl10_1.0.2q.bb
> > > > index 809634f6c0..88aefdea4f 100644
> > > > --- a/meta/recipes-connectivity/openssl/openssl10_1.0.2q.bb
> > > > +++ b/meta/recipes-connectivity/openssl/openssl10_1.0.2q.bb
> > > > @@ -40,6 +40,7 @@ SRC_URI = "http://www.openssl.org/source/openssl-${PV}.tar.gz \
> > > >             file://0001-Fix-build-with-clang-using-external-assembler.patch \
> > > >             file://0001-openssl-force-soft-link-to-avoid-rare-race.patch \
> > > >             file://0001-allow-manpages-to-be-disabled.patch \
> > > > +           file://0001-Fix-BN_LLONG-breakage.patch \
> > > >             "
> > > >
> > > >  SRC_URI_append_class-target = " \
> > > > --
> > > > 2.20.1
> > > >
> > > > --
> > > > _______________________________________________
> > > > Openembedded-core mailing list
> > > > Openembedded-core at lists.openembedded.org
> > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> >