public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] fedora: Replace pasta hard links by copies, mangle Build-IDs
@ 2023-08-23 13:48 Stefano Brivio
  2023-08-23 13:53 ` Richard W.M. Jones
  0 siblings, 1 reply; 5+ messages in thread
From: Stefano Brivio @ 2023-08-23 13:48 UTC (permalink / raw)
  To: passt-dev; +Cc: 'Richard W . M . Jones'

The hard link trick didn't actually fix the issue with SELinux file
contexts properly: as opposed to symbolic links, SELinux now
correctly associates types to the labels that are set -- except that
those labels are now shared, so we can end up (depending on how
rpm(8) extracts the archives) with /usr/bin/passt having a
pasta_exec_t context.

This got rather confusing as running restorecon(8) seemed to fix up
labels -- but that's simply toggling between passt_exec_t and
pasta_exec_t for both links, because each invocation will just "fix"
the file with the mismatching context.

Replace the hard links with copies. AppArmor's attachment, instead,
works with hard links, and if there's no LSM, we can keep symbolic
links, so keep symbolic links in the Makefile.

With copies, rpmbuild(8) will warn about duplicate Build-IDs in the
same package. Mangle them in pasta binaries by summing one to the
last byte, modulo one byte, using xxd (provided by vim-common) and
disable the automatic rehashing by find-debuginfo(1) -- we already
have per-release Build-IDs thanks to $VERSION passed on 'make'.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 contrib/fedora/passt.spec | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/contrib/fedora/passt.spec b/contrib/fedora/passt.spec
index d0c6895..51bf5a8 100644
--- a/contrib/fedora/passt.spec
+++ b/contrib/fedora/passt.spec
@@ -9,6 +9,10 @@
 
 %global git_hash {{{ git_head }}}
 %global selinuxtype targeted
+# Different Build-IDs for passt and pasta: don't let find-debuginfo touch them
+%undefine _unique_build_ids
+%global _no_recompute_build_ids 1
+
 
 Name:		passt
 Version:	{{{ git_version }}}
@@ -19,7 +23,7 @@ Group:		System Environment/Daemons
 URL:		https://passt.top/
 Source:		https://passt.top/passt/snapshot/passt-%{git_hash}.tar.xz
 
-BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel
+BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel, binutils, vim-common
 Requires:	(%{name}-selinux = %{version}-%{release} if selinux-policy-%{selinuxtype})
 
 %description
@@ -56,15 +60,28 @@ This package adds SELinux enforcement to passt(1) and pasta(1).
 %install
 
 %make_install DESTDIR=%{buildroot} prefix=%{_prefix} bindir=%{_bindir} mandir=%{_mandir} docdir=%{_docdir}/%{name}
-# The Makefile creates symbolic links for pasta, but we need hard links for
+# The Makefile creates symbolic links for pasta, but we need actual copies for
 # SELinux file contexts to work as intended. Same with pasta.avx2 if present.
-ln -f %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
+#
+# To avoid duplicate Build-IDs in the same package, we increase the last byte of
+# the value for pasta binaries by one (modulo one byte). Note that we already
+# have differentiated Build-IDs per release, courtesy of $VERSION, so we don't
+# need find-debuginfo(1) to recalculate them.
+rm %{buildroot}%{_bindir}/pasta
+objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt
+printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
+objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
+rm %{buildroot}/build_id
+
 %ifarch x86_64
-ln -f %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
+rm %{buildroot}%{_bindir}/pasta.avx2
+objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2
+printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
+objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
+rm %{buildroot}/build_id
 
 ln -sr %{buildroot}%{_mandir}/man1/passt.1 %{buildroot}%{_mandir}/man1/passt.avx2.1
 ln -sr %{buildroot}%{_mandir}/man1/pasta.1 %{buildroot}%{_mandir}/man1/pasta.avx2.1
-install -p -m 755 %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
 %endif
 
 pushd contrib/selinux
-- 
@@ -9,6 +9,10 @@
 
 %global git_hash {{{ git_head }}}
 %global selinuxtype targeted
+# Different Build-IDs for passt and pasta: don't let find-debuginfo touch them
+%undefine _unique_build_ids
+%global _no_recompute_build_ids 1
+
 
 Name:		passt
 Version:	{{{ git_version }}}
@@ -19,7 +23,7 @@ Group:		System Environment/Daemons
 URL:		https://passt.top/
 Source:		https://passt.top/passt/snapshot/passt-%{git_hash}.tar.xz
 
-BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel
+BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel, binutils, vim-common
 Requires:	(%{name}-selinux = %{version}-%{release} if selinux-policy-%{selinuxtype})
 
 %description
@@ -56,15 +60,28 @@ This package adds SELinux enforcement to passt(1) and pasta(1).
 %install
 
 %make_install DESTDIR=%{buildroot} prefix=%{_prefix} bindir=%{_bindir} mandir=%{_mandir} docdir=%{_docdir}/%{name}
-# The Makefile creates symbolic links for pasta, but we need hard links for
+# The Makefile creates symbolic links for pasta, but we need actual copies for
 # SELinux file contexts to work as intended. Same with pasta.avx2 if present.
-ln -f %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
+#
+# To avoid duplicate Build-IDs in the same package, we increase the last byte of
+# the value for pasta binaries by one (modulo one byte). Note that we already
+# have differentiated Build-IDs per release, courtesy of $VERSION, so we don't
+# need find-debuginfo(1) to recalculate them.
+rm %{buildroot}%{_bindir}/pasta
+objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt
+printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
+objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
+rm %{buildroot}/build_id
+
 %ifarch x86_64
-ln -f %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
+rm %{buildroot}%{_bindir}/pasta.avx2
+objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2
+printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
+objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
+rm %{buildroot}/build_id
 
 ln -sr %{buildroot}%{_mandir}/man1/passt.1 %{buildroot}%{_mandir}/man1/passt.avx2.1
 ln -sr %{buildroot}%{_mandir}/man1/pasta.1 %{buildroot}%{_mandir}/man1/pasta.avx2.1
-install -p -m 755 %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
 %endif
 
 pushd contrib/selinux
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] fedora: Replace pasta hard links by copies, mangle Build-IDs
  2023-08-23 13:48 [PATCH] fedora: Replace pasta hard links by copies, mangle Build-IDs Stefano Brivio
@ 2023-08-23 13:53 ` Richard W.M. Jones
  2023-08-23 13:59   ` Stefano Brivio
  0 siblings, 1 reply; 5+ messages in thread
From: Richard W.M. Jones @ 2023-08-23 13:53 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, devel


On Wed, Aug 23, 2023 at 03:48:44PM +0200, Stefano Brivio wrote:
> The hard link trick didn't actually fix the issue with SELinux file
> contexts properly: as opposed to symbolic links, SELinux now
> correctly associates types to the labels that are set -- except that
> those labels are now shared, so we can end up (depending on how
> rpm(8) extracts the archives) with /usr/bin/passt having a
> pasta_exec_t context.
> 
> This got rather confusing as running restorecon(8) seemed to fix up
> labels -- but that's simply toggling between passt_exec_t and
> pasta_exec_t for both links, because each invocation will just "fix"
> the file with the mismatching context.
> 
> Replace the hard links with copies. AppArmor's attachment, instead,
> works with hard links, and if there's no LSM, we can keep symbolic
> links, so keep symbolic links in the Makefile.
> 
> With copies, rpmbuild(8) will warn about duplicate Build-IDs in the
> same package. Mangle them in pasta binaries by summing one to the
> last byte, modulo one byte, using xxd (provided by vim-common) and
> disable the automatic rehashing by find-debuginfo(1) -- we already
> have per-release Build-IDs thanks to $VERSION passed on 'make'.

Right, this ^ was going to be my comment.  RPM doesn't like having two
identical copies of a file.  Hacking the binary to "fix" the problem
doesn't sound like a solution.

I'm CC-ing Fedora-devel-list to find out we can properly fix this issue.

Rich.

> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  contrib/fedora/passt.spec | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/contrib/fedora/passt.spec b/contrib/fedora/passt.spec
> index d0c6895..51bf5a8 100644
> --- a/contrib/fedora/passt.spec
> +++ b/contrib/fedora/passt.spec
> @@ -9,6 +9,10 @@
>  
>  %global git_hash {{{ git_head }}}
>  %global selinuxtype targeted
> +# Different Build-IDs for passt and pasta: don't let find-debuginfo touch them
> +%undefine _unique_build_ids
> +%global _no_recompute_build_ids 1
> +
>  
>  Name:		passt
>  Version:	{{{ git_version }}}
> @@ -19,7 +23,7 @@ Group:		System Environment/Daemons
>  URL:		https://passt.top/
>  Source:		https://passt.top/passt/snapshot/passt-%{git_hash}.tar.xz
>  
> -BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel
> +BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel, binutils, vim-common
>  Requires:	(%{name}-selinux = %{version}-%{release} if selinux-policy-%{selinuxtype})
>  
>  %description
> @@ -56,15 +60,28 @@ This package adds SELinux enforcement to passt(1) and pasta(1).
>  %install
>  
>  %make_install DESTDIR=%{buildroot} prefix=%{_prefix} bindir=%{_bindir} mandir=%{_mandir} docdir=%{_docdir}/%{name}
> -# The Makefile creates symbolic links for pasta, but we need hard links for
> +# The Makefile creates symbolic links for pasta, but we need actual copies for
>  # SELinux file contexts to work as intended. Same with pasta.avx2 if present.
> -ln -f %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
> +#
> +# To avoid duplicate Build-IDs in the same package, we increase the last byte of
> +# the value for pasta binaries by one (modulo one byte). Note that we already
> +# have differentiated Build-IDs per release, courtesy of $VERSION, so we don't
> +# need find-debuginfo(1) to recalculate them.
> +rm %{buildroot}%{_bindir}/pasta
> +objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt
> +printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
> +objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
> +rm %{buildroot}/build_id
> +
>  %ifarch x86_64
> -ln -f %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> +rm %{buildroot}%{_bindir}/pasta.avx2
> +objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2
> +printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
> +objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> +rm %{buildroot}/build_id
>  
>  ln -sr %{buildroot}%{_mandir}/man1/passt.1 %{buildroot}%{_mandir}/man1/passt.avx2.1
>  ln -sr %{buildroot}%{_mandir}/man1/pasta.1 %{buildroot}%{_mandir}/man1/pasta.avx2.1
> -install -p -m 755 %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
>  %endif
>  
>  pushd contrib/selinux
> -- 
> 2.39.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fedora: Replace pasta hard links by copies, mangle Build-IDs
  2023-08-23 13:53 ` Richard W.M. Jones
@ 2023-08-23 13:59   ` Stefano Brivio
  2023-08-23 14:31     ` Richard W.M. Jones
  0 siblings, 1 reply; 5+ messages in thread
From: Stefano Brivio @ 2023-08-23 13:59 UTC (permalink / raw)
  To: Richard W.M. Jones; +Cc: devel, passt-dev

On Wed, 23 Aug 2023 14:53:27 +0100
"Richard W.M. Jones" <rjones@redhat.com> wrote:

> On Wed, Aug 23, 2023 at 03:48:44PM +0200, Stefano Brivio wrote:
> > The hard link trick didn't actually fix the issue with SELinux file
> > contexts properly: as opposed to symbolic links, SELinux now
> > correctly associates types to the labels that are set -- except that
> > those labels are now shared, so we can end up (depending on how
> > rpm(8) extracts the archives) with /usr/bin/passt having a
> > pasta_exec_t context.
> > 
> > This got rather confusing as running restorecon(8) seemed to fix up
> > labels -- but that's simply toggling between passt_exec_t and
> > pasta_exec_t for both links, because each invocation will just "fix"
> > the file with the mismatching context.
> > 
> > Replace the hard links with copies. AppArmor's attachment, instead,
> > works with hard links, and if there's no LSM, we can keep symbolic
> > links, so keep symbolic links in the Makefile.
> > 
> > With copies, rpmbuild(8) will warn about duplicate Build-IDs in the
> > same package. Mangle them in pasta binaries by summing one to the
> > last byte, modulo one byte, using xxd (provided by vim-common) and
> > disable the automatic rehashing by find-debuginfo(1) -- we already
> > have per-release Build-IDs thanks to $VERSION passed on 'make'.  
> 
> Right, this ^ was going to be my comment.  RPM doesn't like having two
> identical copies of a file.

In which other way, though? cpio(1) is fine with it, and I tried to
install the package on both ext4 and xfs -- the only warning I got was
the (semi-reasonable) one from rpmbuild about duplicate Build-IDs.

> Hacking the binary to "fix" the problem doesn't sound like a solution.
> 
> I'm CC-ing Fedora-devel-list to find out we can properly fix this issue.
> 
> Rich.
> 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  contrib/fedora/passt.spec | 27 ++++++++++++++++++++++-----
> >  1 file changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/contrib/fedora/passt.spec b/contrib/fedora/passt.spec
> > index d0c6895..51bf5a8 100644
> > --- a/contrib/fedora/passt.spec
> > +++ b/contrib/fedora/passt.spec
> > @@ -9,6 +9,10 @@
> >  
> >  %global git_hash {{{ git_head }}}
> >  %global selinuxtype targeted
> > +# Different Build-IDs for passt and pasta: don't let find-debuginfo touch them
> > +%undefine _unique_build_ids
> > +%global _no_recompute_build_ids 1
> > +
> >  
> >  Name:		passt
> >  Version:	{{{ git_version }}}
> > @@ -19,7 +23,7 @@ Group:		System Environment/Daemons
> >  URL:		https://passt.top/
> >  Source:		https://passt.top/passt/snapshot/passt-%{git_hash}.tar.xz
> >  
> > -BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel
> > +BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel, binutils, vim-common
> >  Requires:	(%{name}-selinux = %{version}-%{release} if selinux-policy-%{selinuxtype})
> >  
> >  %description
> > @@ -56,15 +60,28 @@ This package adds SELinux enforcement to passt(1) and pasta(1).
> >  %install
> >  
> >  %make_install DESTDIR=%{buildroot} prefix=%{_prefix} bindir=%{_bindir} mandir=%{_mandir} docdir=%{_docdir}/%{name}
> > -# The Makefile creates symbolic links for pasta, but we need hard links for
> > +# The Makefile creates symbolic links for pasta, but we need actual copies for
> >  # SELinux file contexts to work as intended. Same with pasta.avx2 if present.
> > -ln -f %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
> > +#
> > +# To avoid duplicate Build-IDs in the same package, we increase the last byte of
> > +# the value for pasta binaries by one (modulo one byte). Note that we already
> > +# have differentiated Build-IDs per release, courtesy of $VERSION, so we don't
> > +# need find-debuginfo(1) to recalculate them.
> > +rm %{buildroot}%{_bindir}/pasta
> > +objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt
> > +printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
> > +objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
> > +rm %{buildroot}/build_id
> > +
> >  %ifarch x86_64
> > -ln -f %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> > +rm %{buildroot}%{_bindir}/pasta.avx2
> > +objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2
> > +printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
> > +objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> > +rm %{buildroot}/build_id
> >  
> >  ln -sr %{buildroot}%{_mandir}/man1/passt.1 %{buildroot}%{_mandir}/man1/passt.avx2.1
> >  ln -sr %{buildroot}%{_mandir}/man1/pasta.1 %{buildroot}%{_mandir}/man1/pasta.avx2.1
> > -install -p -m 755 %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> >  %endif
> >  
> >  pushd contrib/selinux
> > -- 
> > 2.39.2  

-- 
Stefano


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fedora: Replace pasta hard links by copies, mangle Build-IDs
  2023-08-23 13:59   ` Stefano Brivio
@ 2023-08-23 14:31     ` Richard W.M. Jones
  2023-09-07  0:21       ` Stefano Brivio
  0 siblings, 1 reply; 5+ messages in thread
From: Richard W.M. Jones @ 2023-08-23 14:31 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: devel, passt-dev

On Wed, Aug 23, 2023 at 03:59:39PM +0200, Stefano Brivio wrote:
> On Wed, 23 Aug 2023 14:53:27 +0100
> "Richard W.M. Jones" <rjones@redhat.com> wrote:
> 
> > On Wed, Aug 23, 2023 at 03:48:44PM +0200, Stefano Brivio wrote:
> > > The hard link trick didn't actually fix the issue with SELinux file
> > > contexts properly: as opposed to symbolic links, SELinux now
> > > correctly associates types to the labels that are set -- except that
> > > those labels are now shared, so we can end up (depending on how
> > > rpm(8) extracts the archives) with /usr/bin/passt having a
> > > pasta_exec_t context.
> > > 
> > > This got rather confusing as running restorecon(8) seemed to fix up
> > > labels -- but that's simply toggling between passt_exec_t and
> > > pasta_exec_t for both links, because each invocation will just "fix"
> > > the file with the mismatching context.
> > > 
> > > Replace the hard links with copies. AppArmor's attachment, instead,
> > > works with hard links, and if there's no LSM, we can keep symbolic
> > > links, so keep symbolic links in the Makefile.
> > > 
> > > With copies, rpmbuild(8) will warn about duplicate Build-IDs in the
> > > same package. Mangle them in pasta binaries by summing one to the
> > > last byte, modulo one byte, using xxd (provided by vim-common) and
> > > disable the automatic rehashing by find-debuginfo(1) -- we already
> > > have per-release Build-IDs thanks to $VERSION passed on 'make'.  
> > 
> > Right, this ^ was going to be my comment.  RPM doesn't like having two
> > identical copies of a file.
> 
> In which other way, though? cpio(1) is fine with it, and I tried to
> install the package on both ext4 and xfs -- the only warning I got was
> the (semi-reasonable) one from rpmbuild about duplicate Build-IDs.

I'm fairly sure I've seen an error when you have two identical files;
it might only happen in Koji.  Anyway, hacking the binary is surely
wrong, but let's hear the opinions of the Fedora / SELinux developers.

Rich.

> > Hacking the binary to "fix" the problem doesn't sound like a solution.
> > 
> > I'm CC-ing Fedora-devel-list to find out we can properly fix this issue.
> > 
> > Rich.
> > 
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > > ---
> > >  contrib/fedora/passt.spec | 27 ++++++++++++++++++++++-----
> > >  1 file changed, 22 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/contrib/fedora/passt.spec b/contrib/fedora/passt.spec
> > > index d0c6895..51bf5a8 100644
> > > --- a/contrib/fedora/passt.spec
> > > +++ b/contrib/fedora/passt.spec
> > > @@ -9,6 +9,10 @@
> > >  
> > >  %global git_hash {{{ git_head }}}
> > >  %global selinuxtype targeted
> > > +# Different Build-IDs for passt and pasta: don't let find-debuginfo touch them
> > > +%undefine _unique_build_ids
> > > +%global _no_recompute_build_ids 1
> > > +
> > >  
> > >  Name:		passt
> > >  Version:	{{{ git_version }}}
> > > @@ -19,7 +23,7 @@ Group:		System Environment/Daemons
> > >  URL:		https://passt.top/
> > >  Source:		https://passt.top/passt/snapshot/passt-%{git_hash}.tar.xz
> > >  
> > > -BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel
> > > +BuildRequires:	gcc, make, checkpolicy, selinux-policy-devel, binutils, vim-common
> > >  Requires:	(%{name}-selinux = %{version}-%{release} if selinux-policy-%{selinuxtype})
> > >  
> > >  %description
> > > @@ -56,15 +60,28 @@ This package adds SELinux enforcement to passt(1) and pasta(1).
> > >  %install
> > >  
> > >  %make_install DESTDIR=%{buildroot} prefix=%{_prefix} bindir=%{_bindir} mandir=%{_mandir} docdir=%{_docdir}/%{name}
> > > -# The Makefile creates symbolic links for pasta, but we need hard links for
> > > +# The Makefile creates symbolic links for pasta, but we need actual copies for
> > >  # SELinux file contexts to work as intended. Same with pasta.avx2 if present.
> > > -ln -f %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
> > > +#
> > > +# To avoid duplicate Build-IDs in the same package, we increase the last byte of
> > > +# the value for pasta binaries by one (modulo one byte). Note that we already
> > > +# have differentiated Build-IDs per release, courtesy of $VERSION, so we don't
> > > +# need find-debuginfo(1) to recalculate them.
> > > +rm %{buildroot}%{_bindir}/pasta
> > > +objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt
> > > +printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
> > > +objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt %{buildroot}%{_bindir}/pasta
> > > +rm %{buildroot}/build_id
> > > +
> > >  %ifarch x86_64
> > > -ln -f %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> > > +rm %{buildroot}%{_bindir}/pasta.avx2
> > > +objcopy --dump-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2
> > > +printf '\x'$(printf %02x $(( ( 0x$(xxd -ps -s 35 %{buildroot}/build_id) + 1 ) % 0xff )) ) | dd of=%{buildroot}/build_id seek=35 bs=1 count=1 conv=notrunc
> > > +objcopy --update-section .note.gnu.build-id=%{buildroot}/build_id %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> > > +rm %{buildroot}/build_id
> > >  
> > >  ln -sr %{buildroot}%{_mandir}/man1/passt.1 %{buildroot}%{_mandir}/man1/passt.avx2.1
> > >  ln -sr %{buildroot}%{_mandir}/man1/pasta.1 %{buildroot}%{_mandir}/man1/pasta.avx2.1
> > > -install -p -m 755 %{buildroot}%{_bindir}/passt.avx2 %{buildroot}%{_bindir}/pasta.avx2
> > >  %endif
> > >  
> > >  pushd contrib/selinux
> > > -- 
> > > 2.39.2  
> 
> -- 
> Stefano

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] fedora: Replace pasta hard links by copies, mangle Build-IDs
  2023-08-23 14:31     ` Richard W.M. Jones
@ 2023-09-07  0:21       ` Stefano Brivio
  0 siblings, 0 replies; 5+ messages in thread
From: Stefano Brivio @ 2023-09-07  0:21 UTC (permalink / raw)
  To: Richard W.M. Jones; +Cc: devel, passt-dev

On Wed, 23 Aug 2023 15:31:46 +0100
"Richard W.M. Jones" <rjones@redhat.com> wrote:

> On Wed, Aug 23, 2023 at 03:59:39PM +0200, Stefano Brivio wrote:
> > On Wed, 23 Aug 2023 14:53:27 +0100
> > "Richard W.M. Jones" <rjones@redhat.com> wrote:
> >   
> > > On Wed, Aug 23, 2023 at 03:48:44PM +0200, Stefano Brivio wrote:  
> > > > The hard link trick didn't actually fix the issue with SELinux file
> > > > contexts properly: as opposed to symbolic links, SELinux now
> > > > correctly associates types to the labels that are set -- except that
> > > > those labels are now shared, so we can end up (depending on how
> > > > rpm(8) extracts the archives) with /usr/bin/passt having a
> > > > pasta_exec_t context.
> > > > 
> > > > This got rather confusing as running restorecon(8) seemed to fix up
> > > > labels -- but that's simply toggling between passt_exec_t and
> > > > pasta_exec_t for both links, because each invocation will just "fix"
> > > > the file with the mismatching context.
> > > > 
> > > > Replace the hard links with copies. AppArmor's attachment, instead,
> > > > works with hard links, and if there's no LSM, we can keep symbolic
> > > > links, so keep symbolic links in the Makefile.
> > > > 
> > > > With copies, rpmbuild(8) will warn about duplicate Build-IDs in the
> > > > same package. Mangle them in pasta binaries by summing one to the
> > > > last byte, modulo one byte, using xxd (provided by vim-common) and
> > > > disable the automatic rehashing by find-debuginfo(1) -- we already
> > > > have per-release Build-IDs thanks to $VERSION passed on 'make'.    
> > > 
> > > Right, this ^ was going to be my comment.  RPM doesn't like having two
> > > identical copies of a file.  
> > 
> > In which other way, though? cpio(1) is fine with it, and I tried to
> > install the package on both ext4 and xfs -- the only warning I got was
> > the (semi-reasonable) one from rpmbuild about duplicate Build-IDs.  
> 
> I'm fairly sure I've seen an error when you have two identical files;

...but those are not identical, exactly because I change the Build-ID.

> it might only happen in Koji.  Anyway, hacking the binary is surely
> wrong, but let's hear the opinions of the Fedora / SELinux developers.

I don't see why it would be "surely wrong".

It's surely ugly, though, and David suggested that two separate builds
might be slightly less ugly, albeit (more) wasteful, but passt takes
seconds to build, after all. New patch posted.

-- 
Stefano


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-09-07  0:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-23 13:48 [PATCH] fedora: Replace pasta hard links by copies, mangle Build-IDs Stefano Brivio
2023-08-23 13:53 ` Richard W.M. Jones
2023-08-23 13:59   ` Stefano Brivio
2023-08-23 14:31     ` Richard W.M. Jones
2023-09-07  0:21       ` Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).