Re: [PATCH] openssl10: Fix mutliple include assumptions for bn.h in opensslconf.h

[Denys Dmytriyenko <denis@denix.org>]

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@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?

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@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@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@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@lists.openembedded.org
> > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
>