Re: [PATCH] defaultsetup.conf: Enable security flags+pie by default

[Peter Kjellerstedt <peter.kjellerstedt@axis.com>]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org <openembedded-
> core-bounces@lists.openembedded.org> On Behalf Of Khem Raj
> Sent: den 24 juli 2018 16:12
> To: ChenQi <Qi.Chen@windriver.com>
> Cc: Patches and discussions about the oe-core layer <openembedded-
> core@lists.openembedded.org>
> Subject: Re: [OE-core] [PATCH] defaultsetup.conf: Enable security
> flags+pie by default
> 
> On Tue, Jul 24, 2018 at 12:30 AM ChenQi <Qi.Chen@windriver.com> wrote:
> >
> > Hi Khem,
> >
> > The comments in security-flags.inc also needs to be modified to
> remove
> > 'poky-lsb' info.
> >
> > I'd suggest we still put it into distro conf file (poky.conf) instead
> of
> > defaultsetup.conf, because defaultsetup.conf is included by
> > bitbake.conf. I think things in defaultsetup.conf should be necessary
> > default values to build things out. I don't think security flags is
> > necessary to build things out.
> 
> this is the default setup, even non-poky users will get consistent
> experience.

I have to agree with Chen here. I think requiring security_flags.inc from 
defaultsetup.conf is the wrong thing to do. We use security_flags.inc in 
our setup, and I know how much trouble it has brought. To me, using it 
should be a distro decision, not something that is enforced by the use 
of bitbake.

> > Also, I got a question when I just looked at this file.
> > Do you think we should adjust CFLAGS and LDFALGS in security_flags.inc
> > instead of the current TARGET_CC_ARCH and TARGET_LDFLAGS?
> 
> in many cases packages do not honor CFLAGS/LDFLAGS say during configure
> 
> > We are naming
> > variables to SECURITY_CFLAGS and SECURITY_LDFLAGS, it seems that they
> > belong to CFLAGS and LDFLAGS naturally. But I'm not sure about it.
> >
> yes they do, but this makes it easy to override the setting for
> packages where these options are needed to be overridden or modified.

Actually, with the changes introduced in Pyro, SECURITY_CFLAGS became a 
mess. Before Pyro, you either set SECURITY_CFLAGS to 
"${SECURITY_NO_PIE_CFLAGS}" (to disable the use of -fpie), or you set it 
to the empty string (to disable all security options). With Pyro and later, 
you instead have to set SECURITY_CFLAGS to "${SECURITY_NO_PIE_CFLAGS} 
${SECURITY_NOPIE_CFLAGS}" to make sure -fpie is disabled, or set it to 
"${SECURITY_NOPIE_CFLAGS}" to disable everything. Alternatively you can 
set SECURITY_PIE_CFLAGS to "${SECURITY_NOPIE_CFLAGS}" to only disable 
-fpie.

I have considered to suggest changing the definition of 
SECURITY_NOPIE_CFLAGS to:

SECURITY_NOPIE_CFLAGS ?= "${@'-no-pie -fno-PIE' if '${GCCPIE}' else ''}"

and then change SECURITY_NO_PIE_CFLAGS to:

SECURITY_NO_PIE_CFLAGS ?= "-fstack-protector-strong ${SECURITY_NOPIE_CFLAGS} ${lcl_maybe_fortify} ${SECURITY_STRINGFORMAT}"

That would better have matched the situation before Pyro, in that one yet 
again would set SECURITY_CFLAGS to "${SECURITY_NO_PIE_CFLAGS}" to disable 
-fpie. Unfortunately one would still have to set SECURITY_CFLAGS to 
"${SECURITY_NOPIE_CFLAGS}" to disable everything.

> > Best Regards,
> > Chen Qi
> >
> >
> > On 07/24/2018 03:09 AM, Khem Raj wrote:
> > > This has been an opt-in for so long, some distributions e.g.
> > > poky-lsb uses it by default however, since most of linux
> > > distros have started to default to these settings for security
> > > enhancements, time has come for OE to make it default too
> > >
> > > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > ---
> > >   meta/conf/distro/defaultsetup.conf | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/meta/conf/distro/defaultsetup.conf
> b/meta/conf/distro/defaultsetup.conf
> > > index ca2f9178d2..352e279596 100644
> > > --- a/meta/conf/distro/defaultsetup.conf
> > > +++ b/meta/conf/distro/defaultsetup.conf
> > > @@ -1,6 +1,7 @@
> > >   include conf/distro/include/default-providers.inc
> > >   include conf/distro/include/default-versions.inc
> > >   include conf/distro/include/default-distrovars.inc
> > > +require conf/distro/include/security_flags.inc
> > >   include conf/distro/include/world-broken.inc
> > >
> > >   TCMODE ?= "default"

//Peter