Re: [PATCH] Fix includes for userspace tools and libraries (and possible security issue)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



To be more precise...

On Tue, 2011-09-13 at 20:33 +0200, Guido Trentalancia wrote:
> Hello Stephen.
> 
> On Tue, 2011-09-13 at 13:20 -0400, Stephen Smalley wrote:
> > On Tue, 2011-09-13 at 18:31 +0200, Guido Trentalancia wrote:
> > > But let me just try again pointing out a few further details so that
> > > everybody would better understand the issue.
> > > 
> > > It is a maintenance issue with the Makefiles.
> > > 
> > > The DESTDIR variable does not matter much because the problem is that it
> > > breaks not only the installation but also the compilation itself (prior
> > > to the installation) !
> > > 
> > > So, DESTDIR is generally only used during the installation process (what
> > > DESTINATION directory shall I write to ?).
> > 
> > That may be true, but we have been using it to perform local builds of
> > the entire source repository for a very long time.  In the case of
> > package builds, it doesn't matter because there each component is
> > separately compiled and the package dependencies ensure that the
> > required components are built and installed first.
> 
> The above depends on the distribution's build process. We do not know
> for certain how each present and future distribution is going to build
> the packages...
> 
> >   At some distant
> > point in the past, this approach of using make DESTDIR was introduced as
> > a way to allow us to build the entire tree without requiring us to
> > introduce -I and -L options relative to the current directory, as that
> > was viewed as being more painful to maintain and wrong when built
> > separately (as for packages).
> 
> DESTDIR should not be mandatory to use but it is very much desirable.
> But I do not get the connection with the two compiler flags...
> 
> > > The compilation problem being discussed here is twofold: compilation
> > > itself (from *.c source code to *.o object code) and linking (from *.o
> > > object code to the tool or shared library executable).
> > > 
> > > So the first side of the problem (compilation stage) is due to the fact
> > > that gcc is only allowed to include header files from the system-wide
> > > header repository (e.g. -I/usr/include) while unfortunately it is not
> > > allowed/configured to try picking up local header files first (as in
> > > having -I$(CURDIR)/libselinux/include -I$(CURDIR)/libsepol/include -I
> > > $(CURDIR)/libsemanage/include before any eventual system-wide
> > > -I/usr/include).
> > 
> > Build with make DESTDIR=/some/path install, and it should already add
> > -I/some/path/usr/include to your CFLAGS, which will then get picked up
> > before the system headers (as per man gcc and confirmed by experiment).
> 
> No, it doesn't currently ! If you want to try reproducing it, then you
> should do so on a system which hasn't got it already installed (or make
> sure you get temporarily rid of
> $(PREFIX)/include/{selinux,sepol,semanage} and
> $(LIBDIR)/lib{selinux,sepol,semanage}.* first).
> 
> > > The second side of the problem (linking stage) is due to a somewhat
> > > similar fact that gcc is only allowed to search dynamic (shared)
> > > libraries in system-wide repositories (such as -L/usr/lib) while it is
> > > not allowed/configured to search the local build directories first (as
> > > in having -L$(CURDIR)/libselinux/src -L$(CURDIR)/libsepol/src -L
> > > $(CURDIR)/libsemanage/src before any system-wide -L/usr/lib).
> > 
> > Likewise, make DESTDIR=/some/path install will adjust your LDFLAGS
> > correctly.
> 
> See above.
> 
> And to that add that the actual LDFLAGS possibly introduces an unwanted
> and potentially dangerous libsepol.a cache !

Please read LDLIBS instead of LDFLAGS.

At the least the following objects could be affected: checkpolicy,
semodule_deps, mcstransd and the audit2why.so python module. A security
notice might be need to be issued if confirmed.

> In fact, somewhere the LDFLAGS currently adds $(LIBDIR)/libsepol.a
> instead of the local copy of static library libsepol.a. This should be
> further investigated as it might need to be treated as a security flaw
> (binaries available from different vendors might be affected if linked
> against the existing old libsepol.a static library).
> 
> > > For reference: the CURDIR variable is expanded by the GNU make tools to
> > > the current directory (it is a GNU make functionality). I propose
> > > exploiting such variable in the top-level Makefile in order to determine
> > > the top-level SELinux-userspace directory and subsequently pass it
> > > recursively to lower-level Makefiles (so that it is used in turn as the
> > > base for the *local* -I include and -L link compiler/linker flags).
> > 
> > Ok, maybe we didn't know about CURDIR back then or thought it would
> > break separate package builds.  I don't recall.  I'm not fundamentally
> > opposed to the changes so long as they don't introduce any regressions,
> > but I wanted to be sure that people understand the history.
> > 
> > > That said, I must admit I do not know much about the history, although
> > > it seems to me from the git logs that the problem has been there since
> > > the very first git commit.
> > 
> > Yet it has worked for us for quite a long time, via make DESTDIR.  If it
> > is broken, it is a recent change that broke it.
> 
> I am missing the point again especially because I wasn't able to track
> it down in git logs.

Regards,

Guido


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.


[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux