[Buildroot] [PATCH v1 1/2] libopenssl: do not leak the compiler path (reproducible builds)

[Buildroot] [PATCH v1 1/2] libopenssl: do not leak the compiler path (reproducible builds)

[Peter Seiderer]

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 ...roducible-build-do-not-leak-compiler-path.patch | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch

diff --git a/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch b/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
new file mode 100644
index 0000000000..eff72c548a
--- /dev/null
+++ b/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
@@ -0,0 +1,26 @@
+From 875fcad2ad84877763cba86c1265b57679b878b0 Mon Sep 17 00:00:00 2001
+From: Peter Seiderer <ps.report@gmx.net>
+Date: Tue, 24 Oct 2017 16:58:32 +0200
+Subject: [PATCH] Reproducible build: do not leak compiler path
+
+Signed-off-by: Peter Seiderer <ps.report@gmx.net>
+---
+ crypto/Makefile | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/crypto/Makefile b/crypto/Makefile
+index 7869996..7e63291 100644
+--- a/crypto/Makefile
++++ b/crypto/Makefile
+@@ -55,7 +55,7 @@ top:
+ all: shared
+ 
+ buildinf.h: ../Makefile
+-	$(PERL) $(TOP)/util/mkbuildinf.pl "$(CC) $(CFLAGS)" "$(PLATFORM)" >buildinf.h
++	$(PERL) $(TOP)/util/mkbuildinf.pl "$$(basename $(CC)) $(CFLAGS)" "$(PLATFORM)" >buildinf.h
+ 
+ x86cpuid.s:	x86cpuid.pl perlasm/x86asm.pl
+ 	$(PERL) x86cpuid.pl $(PERLASM_SCHEME) $(CFLAGS) $(PROCESSOR) > $@
+-- 
+2.11.0
+
-- 
2.14.2

[Buildroot] [PATCH v1 2/2] dhcp: disable isc assertions (reproducible builds)

[Peter Seiderer]

The isc assertions from the bundled bind dns library are
using the __FILE__ macro for debug messages (see
dhcp-4.3.5/bind/bind-9.9.9-P3/lib/isc/include/isc/assertions.h).

Disabling the assertions gains:

- reproducible builds (no built time pathes in the executable)
- space saving on the target:
  dhcpd: 1.9M -> 1.6M
  dhcrelay: 1.6M -> 1.3M

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
---
 package/dhcp/dhcp.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
index 45cae087aa..0bae61283a 100644
--- a/package/dhcp/dhcp.mk
+++ b/package/dhcp/dhcp.mk
@@ -11,7 +11,9 @@ DHCP_LICENSE = ISC
 DHCP_LICENSE_FILES = LICENSE
 DHCP_CONF_ENV = \
 	CPPFLAGS='-D_PATH_DHCPD_CONF=\"/etc/dhcp/dhcpd.conf\" \
-		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"'
+		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"' \
+	CFLAGS='$(TARGET_CFLAGS) -DISC_CHECK_NONE=1'
+
 DHCP_CONF_OPTS = \
 	--with-randomdev=/dev/random \
 	--with-srv-lease-file=/var/lib/dhcp/dhcpd.leases \
-- 
2.14.2

[Buildroot] [PATCH v1 2/2] dhcp: disable isc assertions (reproducible builds)

[Thomas Petazzoni]

Hello,

On Fri, 27 Oct 2017 21:24:24 +0200, Peter Seiderer wrote:
> The isc assertions from the bundled bind dns library are
> using the __FILE__ macro for debug messages (see
> dhcp-4.3.5/bind/bind-9.9.9-P3/lib/isc/include/isc/assertions.h).
> 
> Disabling the assertions gains:
> 
> - reproducible builds (no built time pathes in the executable)
> - space saving on the target:
>   dhcpd: 1.9M -> 1.6M
>   dhcrelay: 1.6M -> 1.3M
> 
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
>  package/dhcp/dhcp.mk | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
> index 45cae087aa..0bae61283a 100644
> --- a/package/dhcp/dhcp.mk
> +++ b/package/dhcp/dhcp.mk
> @@ -11,7 +11,9 @@ DHCP_LICENSE = ISC
>  DHCP_LICENSE_FILES = LICENSE
>  DHCP_CONF_ENV = \
>  	CPPFLAGS='-D_PATH_DHCPD_CONF=\"/etc/dhcp/dhcpd.conf\" \
> -		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"'
> +		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"' \
> +	CFLAGS='$(TARGET_CFLAGS) -DISC_CHECK_NONE=1'

I'm not sure about this one.

Do we want to remove assertions from production code ?

Regarding the __FILE__ problem, I think this is going to bite us on a
large number of packages. So instead of a per-package solution, we
should perhaps have a look at:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268, proposal for a
  -ffile-map-prefix option to gcc

  https://wiki.debian.org/ReproducibleBuilds/BuildPathProposal, also
  discussing a similar idea

Peter, Arnout, Yann, what do you think, especially about the
assertions ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v1 2/2] dhcp: disable isc assertions (reproducible builds)

[Peter Seiderer]

Hello Thomas, *,

On Mon, 8 Jan 2018 21:18:57 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> On Fri, 27 Oct 2017 21:24:24 +0200, Peter Seiderer wrote:
> > The isc assertions from the bundled bind dns library are
> > using the __FILE__ macro for debug messages (see
> > dhcp-4.3.5/bind/bind-9.9.9-P3/lib/isc/include/isc/assertions.h).
> > 
> > Disabling the assertions gains:
> > 
> > - reproducible builds (no built time pathes in the executable)
> > - space saving on the target:
> >   dhcpd: 1.9M -> 1.6M
> >   dhcrelay: 1.6M -> 1.3M
> > 
> > Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> > ---
> >  package/dhcp/dhcp.mk | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
> > index 45cae087aa..0bae61283a 100644
> > --- a/package/dhcp/dhcp.mk
> > +++ b/package/dhcp/dhcp.mk
> > @@ -11,7 +11,9 @@ DHCP_LICENSE = ISC
> >  DHCP_LICENSE_FILES = LICENSE
> >  DHCP_CONF_ENV = \
> >  	CPPFLAGS='-D_PATH_DHCPD_CONF=\"/etc/dhcp/dhcpd.conf\" \
> > -		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"'
> > +		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"' \
> > +	CFLAGS='$(TARGET_CFLAGS) -DISC_CHECK_NONE=1'  
> 
> I'm not sure about this one.
> 
> Do we want to remove assertions from production code ?

I think yes, as std::assert does in case NDEBUG is set (or many other assert
implementations which are disabled in release code)...

> 
> Regarding the __FILE__ problem, I think this is going to bite us on a
> large number of packages. So instead of a per-package solution, we
> should perhaps have a look at:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268, proposal for a
>   -ffile-map-prefix option to gcc
> 
>   https://wiki.debian.org/ReproducibleBuilds/BuildPathProposal, also
>   discussing a similar idea
> 

Which would save the reproducible problem, but lags the complete space
saving advantage...

> Peter, Arnout, Yann, what do you think, especially about the
> assertions ?

Any Comments?

Regards,
Peter

> 
> Best regards,
> 
> Thomas

[Buildroot] [PATCH v1 2/2] dhcp: disable isc assertions (reproducible builds)

[Yann E. MORIN]

Thomas, Peter S., All,

On 2018-01-08 21:18 +0100, Thomas Petazzoni spake thusly:
> On Fri, 27 Oct 2017 21:24:24 +0200, Peter Seiderer wrote:
> > The isc assertions from the bundled bind dns library are
> > using the __FILE__ macro for debug messages (see
> > dhcp-4.3.5/bind/bind-9.9.9-P3/lib/isc/include/isc/assertions.h).
> > 
> > Disabling the assertions gains:
> > 
> > - reproducible builds (no built time pathes in the executable)
> > - space saving on the target:
> >   dhcpd: 1.9M -> 1.6M
> >   dhcrelay: 1.6M -> 1.3M
> > 
> > Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> > ---
> >  package/dhcp/dhcp.mk | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/dhcp/dhcp.mk b/package/dhcp/dhcp.mk
> > index 45cae087aa..0bae61283a 100644
> > --- a/package/dhcp/dhcp.mk
> > +++ b/package/dhcp/dhcp.mk
> > @@ -11,7 +11,9 @@ DHCP_LICENSE = ISC
> >  DHCP_LICENSE_FILES = LICENSE
> >  DHCP_CONF_ENV = \
> >  	CPPFLAGS='-D_PATH_DHCPD_CONF=\"/etc/dhcp/dhcpd.conf\" \
> > -		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"'
> > +		-D_PATH_DHCLIENT_CONF=\"/etc/dhcp/dhclient.conf\"' \
> > +	CFLAGS='$(TARGET_CFLAGS) -DISC_CHECK_NONE=1'
> 
> I'm not sure about this one.
> 
> Do we want to remove assertions from production code ?
> 
> Regarding the __FILE__ problem, I think this is going to bite us on a
> large number of packages. So instead of a per-package solution, we
> should perhaps have a look at:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70268, proposal for a
>   -ffile-map-prefix option to gcc
> 
>   https://wiki.debian.org/ReproducibleBuilds/BuildPathProposal, also
>   discussing a similar idea
> 
> Peter, Arnout, Yann, what do you think, especially about the
> assertions ?

Assertions are supposed to be usefull during development, not during
production use, so I would be fine with disabling them, yes.

At least, I would hope that no sane developer uses assertions to check
for normal error conditions in normal operation mode... ;-]

Now, about reproducibility... We could at least pass -DFILE=\"\" and so
on from inside our gcc-wrapper, like we already do for __DATE__ and
__TIME__. Yes, that would give an empty filename in assertions and the
likes, but do we care in the end?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v1 2/2] dhcp: disable isc assertions (reproducible builds)

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 >> Peter, Arnout, Yann, what do you think, especially about the
 >> assertions ?

 > Assertions are supposed to be usefull during development, not during
 > production use, so I would be fine with disabling them, yes.

 > At least, I would hope that no sane developer uses assertions to check
 > for normal error conditions in normal operation mode... ;-]

Agreed, and the space savings are quite big - So committed, thanks.

 > Now, about reproducibility... We could at least pass -DFILE=\"\" and so
 > on from inside our gcc-wrapper, like we already do for __DATE__ and
 > __TIME__. Yes, that would give an empty filename in assertions and the
 > likes, but do we care in the end?

It would be nicer if we could pass basename($file), but that would break
with compilations with multiple source files passed to gcc or header
files, so yeah, perhaps that is the best way forward.

How does E.G. Debian handle it?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v1 2/2] dhcp: disable isc assertions (reproducible builds)

[Yann E. MORIN]

Peter, All,

On 2018-02-03 22:54 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
[--SNIP--]
>  > Now, about reproducibility... We could at least pass -DFILE=\"\" and so
>  > on from inside our gcc-wrapper, like we already do for __DATE__ and
>  > __TIME__. Yes, that would give an empty filename in assertions and the
>  > likes, but do we care in the end?
> 
> It would be nicer if we could pass basename($file), but that would break
> with compilations with multiple source files passed to gcc or header
> files, so yeah, perhaps that is the best way forward.
> 
> How does E.G. Debian handle it?

I could not find an authoritative source for this, but from what I could
hint from [0] (see the part about "Qt tests and __FILE__"), they do have
a patched gcc and a special dpkg which use the BUILD_PATH_PREFIX_MAP
environment variable as specified iby reproducible-builds.org [1].

Also, upstream gcc has an "in-progress" upstream solution being worked
on, based on BUILD_PATH_PREFIX_MAP, but they did not like it [2]...

[0] https://reproducible.alioth.debian.org/blog/posts/134/
[1] https://reproducible-builds.org/specs/build-path-prefix-map/
[2] https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00232.html

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v1 2/2] dhcp: disable isc assertions (reproducible builds)

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > Peter, All,
 > On 2018-02-03 22:54 +0100, Peter Korsgaard spake thusly:
 >> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
 > [--SNIP--]
 >> > Now, about reproducibility... We could at least pass -DFILE=\"\" and so
 >> > on from inside our gcc-wrapper, like we already do for __DATE__ and
 >> > __TIME__. Yes, that would give an empty filename in assertions and the
 >> > likes, but do we care in the end?
 >> 
 >> It would be nicer if we could pass basename($file), but that would break
 >> with compilations with multiple source files passed to gcc or header
 >> files, so yeah, perhaps that is the best way forward.
 >> 
 >> How does E.G. Debian handle it?

 > I could not find an authoritative source for this, but from what I could
 > hint from [0] (see the part about "Qt tests and __FILE__"), they do have
 > a patched gcc and a special dpkg which use the BUILD_PATH_PREFIX_MAP
 > environment variable as specified iby reproducible-builds.org [1].

 > Also, upstream gcc has an "in-progress" upstream solution being worked
 > on, based on BUILD_PATH_PREFIX_MAP, but they did not like it [2]...

 > [0] https://reproducible.alioth.debian.org/blog/posts/134/
 > [1] https://reproducible-builds.org/specs/build-path-prefix-map/
 > [2] https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00232.html

Thanks - Looks interesting. I wasn't aware of this
BUILD_PATH_PREFIX_MAP.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v1 1/2] libopenssl: do not leak the compiler path (reproducible builds)

[Thomas Petazzoni]

Hello,

On Fri, 27 Oct 2017 21:24:23 +0200, Peter Seiderer wrote:
> Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> ---
>  ...roducible-build-do-not-leak-compiler-path.patch | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
> 
> diff --git a/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch b/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
> new file mode 100644
> index 0000000000..eff72c548a
> --- /dev/null
> +++ b/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
> @@ -0,0 +1,26 @@
> +From 875fcad2ad84877763cba86c1265b57679b878b0 Mon Sep 17 00:00:00 2001
> +From: Peter Seiderer <ps.report@gmx.net>
> +Date: Tue, 24 Oct 2017 16:58:32 +0200
> +Subject: [PATCH] Reproducible build: do not leak compiler path
> +
> +Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> +---
> + crypto/Makefile | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/crypto/Makefile b/crypto/Makefile
> +index 7869996..7e63291 100644
> +--- a/crypto/Makefile
> ++++ b/crypto/Makefile
> +@@ -55,7 +55,7 @@ top:
> + all: shared
> + 
> + buildinf.h: ../Makefile
> +-	$(PERL) $(TOP)/util/mkbuildinf.pl "$(CC) $(CFLAGS)" "$(PLATFORM)" >buildinf.h
> ++	$(PERL) $(TOP)/util/mkbuildinf.pl "$$(basename $(CC)) $(CFLAGS)" "$(PLATFORM)" >buildinf.h

I hesitated a bit on this one, because after all it's our fault: we are
passing an absolute path as the value of CC. If we change that to pass
just the name of the compiler, then OpenSSL doesn't have a problem.

But, it really is OpenSSL choice to hardcode such compiler/flags
information into the binary, so it should sanitize that before using it.

Even though I believe there's probably not much hope, could you try to
submit this patch upstream?

In the mean time, I've applied to master. Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v1 1/2] libopenssl: do not leak the compiler path (reproducible builds)

[Peter Seiderer]

Hello Thomas,

On Mon, 8 Jan 2018 21:10:15 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:

> Hello,
> 
> On Fri, 27 Oct 2017 21:24:23 +0200, Peter Seiderer wrote:
> > Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> > ---
> >  ...roducible-build-do-not-leak-compiler-path.patch | 26 ++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
> > 
> > diff --git a/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch b/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
> > new file mode 100644
> > index 0000000000..eff72c548a
> > --- /dev/null
> > +++ b/package/libopenssl/0003-Reproducible-build-do-not-leak-compiler-path.patch
> > @@ -0,0 +1,26 @@
> > +From 875fcad2ad84877763cba86c1265b57679b878b0 Mon Sep 17 00:00:00 2001
> > +From: Peter Seiderer <ps.report@gmx.net>
> > +Date: Tue, 24 Oct 2017 16:58:32 +0200
> > +Subject: [PATCH] Reproducible build: do not leak compiler path
> > +
> > +Signed-off-by: Peter Seiderer <ps.report@gmx.net>
> > +---
> > + crypto/Makefile | 2 +-
> > + 1 file changed, 1 insertion(+), 1 deletion(-)
> > +
> > +diff --git a/crypto/Makefile b/crypto/Makefile
> > +index 7869996..7e63291 100644
> > +--- a/crypto/Makefile
> > ++++ b/crypto/Makefile
> > +@@ -55,7 +55,7 @@ top:
> > + all: shared
> > + 
> > + buildinf.h: ../Makefile
> > +-	$(PERL) $(TOP)/util/mkbuildinf.pl "$(CC) $(CFLAGS)" "$(PLATFORM)" >buildinf.h
> > ++	$(PERL) $(TOP)/util/mkbuildinf.pl "$$(basename $(CC)) $(CFLAGS)" "$(PLATFORM)" >buildinf.h  
> 
> I hesitated a bit on this one, because after all it's our fault: we are
> passing an absolute path as the value of CC. If we change that to pass
> just the name of the compiler, then OpenSSL doesn't have a problem.
> 
> But, it really is OpenSSL choice to hardcode such compiler/flags
> information into the binary, so it should sanitize that before using it.
> 
> Even though I believe there's probably not much hope, could you try to
> submit this patch upstream?

Finally found some spare time and submitted upstream, see [1]...

Regards,
Peter

[1] https://github.com/openssl/openssl/pull/5218

> 
> In the mean time, I've applied to master. Thanks!
> 
> Thomas