maradns reproducibility fixes and the merits of picking a random number

maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 5664 bytes --]

So, I've got a fix for the reproducibility issues for maradns... part of
the fixes are fairly obvious, setting a specific date and setting the
version to be, well, the version...

But there's one nervous-making issue this revealed; maradns embeds a
random number at build time ... allegedly for systems that don't have
/dev/urandom... see
maradns-3.5.0020/deadwood-3.5.0020/src/Makefile.ubuntu2004:

  # Since some systems may not have /dev/urandom (Windows, *cough* *cough*), we
  # keep a randomly generated prime around

So it's got some code to generate a random number at build time and
embed it in the binary. Now, if there's anything I know about good
practices about random numbers, this sort of thing is generally a very
large red flag! It also makes the package build differently every time!

So, Debian's maradns package just removes this embedding of a "random"
number, and I've basically adapted their patches to build reproducibly
on guix too... by basically embedding the same "random" number every
single build!

That said, hopefully it actually uses /dev/urandom and this is just a
fallback for when /dev/urandom is missing? Is that actually how this is
supposed to work? Is that actually how the code does work? If that's the
case, I think the following patch should work ok on Guix.

But I wanted some extra eyes on this before pushing...

live well,
  vagrant

p.s. Obviously, I picked the best random number.

From a2e10d39de37c363b25f06dbb275e2bf6d614b7c Mon Sep 17 00:00:00 2001
From: Vagrant Cascadian <vagrant@reproducible-builds.org>
Date: Sun, 5 Jun 2022 13:57:27 -0700
Subject: [PATCH 3/3] gnu: maradns: Build reproducibly.

* gnu/packages/patches/maradns-deadwood-do-not-embed-random-number.patch: New
  file.
* gnu/local.mk (dist_patch_DATA): Add patch.
* gnu/package/dns.scm (maradns)[source]: Add patch.
  [arguments]: Pass VERSION and COMPILED via makeflags.
---
 gnu/local.mk                                  |  1 +
 gnu/packages/dns.scm                          |  7 +++-
 ...-deadwood-do-not-embed-random-number.patch | 38 +++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/maradns-deadwood-do-not-embed-random-number.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 68b317b32a..ff1135e48e 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1471,6 +1471,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/lvm2-static-link.patch			\
   %D%/packages/patches/mailutils-variable-lookup.patch		\
   %D%/packages/patches/make-impure-dirs.patch			\
+  %D%/packages/patches/maradns-deadwood-do-not-embed-random-number.patch	\
   %D%/packages/patches/mariadb-link-libatomic.patch		\
   %D%/packages/patches/mars-install.patch			\
   %D%/packages/patches/mars-sfml-2.3.patch			\
diff --git a/gnu/packages/dns.scm b/gnu/packages/dns.scm
index fea255c930..39268ddfcf 100644
--- a/gnu/packages/dns.scm
+++ b/gnu/packages/dns.scm
@@ -1302,7 +1302,10 @@ (define-public maradns
                            (version-major+minor version) "/"
                            version "/maradns-" version ".tar.xz"))
        (sha256
-        (base32 "1qgabw6y2bwy6y88dikis62k789i0xh7iwxan8jmqpzvksqwjfgw"))))
+        (base32 "1qgabw6y2bwy6y88dikis62k789i0xh7iwxan8jmqpzvksqwjfgw"))
+       (patches
+        (search-patches
+         "maradns-deadwood-do-not-embed-random-number.patch"))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? #f                      ; need to be root to run tests
@@ -1310,6 +1313,8 @@ (define-public maradns
        (list
         ,(string-append "CC=" (cc-for-target))
         (string-append "PREFIX=" %output)
+        (string-append "COMPILED=" "2012-04-18")
+        (string-append "VERSION=" ,version)
         (string-append "RPM_BUILD_ROOT=" %output))
        #:phases
        (modify-phases %standard-phases
diff --git a/gnu/packages/patches/maradns-deadwood-do-not-embed-random-number.patch b/gnu/packages/patches/maradns-deadwood-do-not-embed-random-number.patch
new file mode 100644
index 0000000000..7e51e79259
--- /dev/null
+++ b/gnu/packages/patches/maradns-deadwood-do-not-embed-random-number.patch
@@ -0,0 +1,38 @@
+Adapted from https://sources.debian.org/src/maradns/2.0.13-1.4/debian/patches/deadwood_makefile.patch/
+
+diff --git a/deadwood-3.5.0020/src/Makefile.ubuntu2004 b/deadwood-3.5.0020/src/Makefile.ubuntu2004
+index 62868aa..2c8c094 100644
+--- a/deadwood-3.5.0020/src/Makefile.ubuntu2004
++++ b/deadwood-3.5.0020/src/Makefile.ubuntu2004
+@@ -29,13 +29,10 @@ all:	Deadwood version.h
+ #
+ 
+ clean:
+-	rm -f Test DwMain DwTcp *.exe *.o a.out RandomPrime writehash_test* \
+-		Deadwood foo* dw_cache DwHash DwCompress *stackdump \
+-		core ; \
+-		./make.version.h ; if [ -e /dev/urandom ] ; \
+-			then rm DwRandPrime.h  ; \
+-			cc RandomPrime.c ; ./a.out > DwRandPrime.h ; rm a.out \
+-		; fi 
++	rm -f Test DwMain DwTcp *.exe *.o a.out writehash_test* \
++		Deadwood foo* dw_cache DwHash DwCompress *stackdump core
++
++
+ 
+ version.h:	
+ 	./make.version.h
+@@ -67,11 +64,8 @@ DwSocket.o:	DwSocket.c DwStr.h DwSocket.h
+ DwSys.o:	DwSys.c DwStr.h version.h
+ 	$(CC) $(FLAGS) -Wall -c -o DwSys.o DwSys.c
+ 
+-RandomPrime:	RandomPrime.c
+-	$(CC) -O3 -o RandomPrime RandomPrime.c
+-
+-DwRandPrime.h: RandomPrime
+-	if [ -e /dev/urandom ] ; then ./RandomPrime > DwRandPrime.h ; fi
++DwRandPrime.h:
++	echo '#define MUL_CONSTANT 1238145941' > DwRandPrime.h
+ 
+ DwHash.o:	DwHash.c DwStr.h DwRandPrime.h DwHash.h
+ 	$(CC) $(FLAGS) -Wall -c -o DwHash.o DwHash.c
-- 
2.35.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Felix Lechner]

Hi,

On Mon, Jun 6, 2022 at 6:50 PM Vagrant Cascadian
<vagrant@reproducible-builds.org> wrote:
>
> So, Debian's maradns package just removes this embedding of a "random"
> number, and I've basically adapted their patches to build reproducibly
> on guix too... by basically embedding the same "random" number every
> single build!

There may be more than one opinion, but as the maintainer of a TLS
library in Debian I think it is a questionable tradeoff. At a minimum,
it would be preferable to use the version number instead of a fixed
constant for all releases.

MaraDNS does not support DNSSEC so the program may not use entropy for
keys. Either way, I'd rather use an unreproducible build than,
accidentally, a known number series to encrypt secrets. Can one patch
out the constant entirely so it is no longer available?

The upstream website says: "People like MaraDNS because it’s ...
remarkably secure." [1] Since many distributions have the same issue,
upstream could perhaps offer the patch as a build switch to enable a
build-time seed only when needed.

Thank you for your hard work on Guix! As a newbie I'll say, what a
great distro. Thanks, everyone!

Kind regards,
Felix Lechner

[1] https://maradns.samiam.org/

Re: maradns reproducibility fixes and the merits of picking a random number

[Julien Lepiller]

On June 7, 2022 5:24:22 AM GMT+02:00, Felix Lechner <felix.lechner@gmail.com> wrote:
>Hi,
>
>On Mon, Jun 6, 2022 at 6:50 PM Vagrant Cascadian
><vagrant@reproducible-builds.org> wrote:
>>
>> So, Debian's maradns package just removes this embedding of a "random"
>> number, and I've basically adapted their patches to build reproducibly
>> on guix too... by basically embedding the same "random" number every
>> single build!
>
>There may be more than one opinion, but as the maintainer of a TLS
>library in Debian I think it is a questionable tradeoff. At a minimum,
>it would be preferable to use the version number instead of a fixed
>constant for all releases.

Consider that even without the patch, each distro will build maradns once and distribute the package to their user. Every user gets the same binary with the same "random" number. So even if it's chosen at build time, it won't really help.

In our case, it only means users who don't use substitutes get a random number, others get the same number that the build farm picked at random. Fixing a number doesn't sound like it's gonna change a lot for these users.

>
>MaraDNS does not support DNSSEC so the program may not use entropy for
>keys. Either way, I'd rather use an unreproducible build than,
>accidentally, a known number series to encrypt secrets. Can one patch
>out the constant entirely so it is no longer available?
>
>The upstream website says: "People like MaraDNS because it’s ...
>remarkably secure." [1] Since many distributions have the same issue,
>upstream could perhaps offer the patch as a build switch to enable a
>build-time seed only when needed.

Sounds like the safest option. Maybe we could change the code that uses that number to naise an exception or abort?

>
>Thank you for your hard work on Guix! As a newbie I'll say, what a
>great distro. Thanks, everyone!
>
>Kind regards,
>Felix Lechner
>
>[1] https://maradns.samiam.org/
>

Re: maradns reproducibility fixes and the merits of picking a random number

[Brian Cully via Development of GNU Guix and the GNU System distribution.]

>>The upstream website says: "People like MaraDNS because it’s ...
>>remarkably secure." [1] Since many distributions have the same 
>>issue,
>>upstream could perhaps offer the patch as a build switch to 
>>enable a
>>build-time seed only when needed.
>
> Sounds like the safest option. Maybe we could change the code 
> that uses that number to naise an exception or abort?

This seems like the best option to me, as well: either add a flag 
to explicitly enable embedding a constant, or remove the code 
entirely and replace it with a build failure (or runtime failure, 
if a build failure is not possible). It seems like a mis-feature 
to me to embed a constant seed, and invites silent 
misconfiguration which will lead to security breaches.

-bjc

Re: maradns reproducibility fixes and the merits of picking a random number

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

On Tue, Jun 07, 2022 at 08:11:54AM -0400, Brian Cully via Development of GNU Guix and the GNU System distribution. wrote:
> 
> > > The upstream website says: "People like MaraDNS because it’s ...
> > > remarkably secure." [1] Since many distributions have the same
> > > issue,
> > > upstream could perhaps offer the patch as a build switch to enable a
> > > build-time seed only when needed.
> > 
> > Sounds like the safest option. Maybe we could change the code that uses
> > that number to naise an exception or abort?
> 
> This seems like the best option to me, as well: either add a flag to
> explicitly enable embedding a constant, or remove the code entirely and
> replace it with a build failure (or runtime failure, if a build failure is
> not possible). It seems like a mis-feature to me to embed a constant seed,
> and invites silent misconfiguration which will lead to security breaches.
> 
> -bjc

I like the idea of forcing the program to segfault if it looks for
/dev/urandom and it isn't there more than distributing a randomized
prime number.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Tobias Geerinckx-Rice]

[-- Attachment #1: Type: text/plain, Size: 288 bytes --]

Efraim Flashner 写道：
> I like the idea of forcing the program to segfault if it looks 
> for
> /dev/urandom and it isn't there more than distributing a 
> randomized
> prime number.

+4

Or error out nicely.  Don't let's ship such ‘features’.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

On Tue, Jun 07, 2022 at 07:20:25AM +0200, Julien Lepiller wrote:
> 
> 
> On June 7, 2022 5:24:22 AM GMT+02:00, Felix Lechner <felix.lechner@gmail.com> wrote:
> >Hi,
> >
> >On Mon, Jun 6, 2022 at 6:50 PM Vagrant Cascadian
> ><vagrant@reproducible-builds.org> wrote:
> >>
> >> So, Debian's maradns package just removes this embedding of a "random"
> >> number, and I've basically adapted their patches to build reproducibly
> >> on guix too... by basically embedding the same "random" number every
> >> single build!

I have to say I was shocked to not see 4 as the random number¹.

> >There may be more than one opinion, but as the maintainer of a TLS
> >library in Debian I think it is a questionable tradeoff. At a minimum,
> >it would be preferable to use the version number instead of a fixed
> >constant for all releases.
> 
> Consider that even without the patch, each distro will build maradns once and distribute the package to their user. Every user gets the same binary with the same "random" number. So even if it's chosen at build time, it won't really help.
> 
> In our case, it only means users who don't use substitutes get a random number, others get the same number that the build farm picked at random. Fixing a number doesn't sound like it's gonna change a lot for these users.

This is something we can work with. We can just mark the package as
'#:substitutable? #f' and then everyone will have to build it
themselves. It still won't really be reproducible, but everyone will
actually have their own special random number.

> >
> >MaraDNS does not support DNSSEC so the program may not use entropy for
> >keys. Either way, I'd rather use an unreproducible build than,
> >accidentally, a known number series to encrypt secrets. Can one patch
> >out the constant entirely so it is no longer available?
> >
> >The upstream website says: "People like MaraDNS because it’s ...
> >remarkably secure." [1] Since many distributions have the same issue,
> >upstream could perhaps offer the patch as a build switch to enable a
> >build-time seed only when needed.
> 
> Sounds like the safest option. Maybe we could change the code that uses that number to naise an exception or abort?
> >
> >Thank you for your hard work on Guix! As a newbie I'll say, what a
> >great distro. Thanks, everyone!
> >
> >Kind regards,
> >Felix Lechner
> >
> >[1] https://maradns.samiam.org/
> >
> 

¹ https://xkcd.com/221/

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 2726 bytes --]

On 2022-06-08, Efraim Flashner wrote:
> On Tue, Jun 07, 2022 at 07:20:25AM +0200, Julien Lepiller wrote:
>> On June 7, 2022 5:24:22 AM GMT+02:00, Felix Lechner <felix.lechner@gmail.com> wrote:
>> >On Mon, Jun 6, 2022 at 6:50 PM Vagrant Cascadian
>> ><vagrant@reproducible-builds.org> wrote:
>> >>
>> >> So, Debian's maradns package just removes this embedding of a "random"
>> >> number, and I've basically adapted their patches to build reproducibly
>> >> on guix too... by basically embedding the same "random" number every
>> >> single build!
>
> I have to say I was shocked to not see 4 as the random number¹.
...
> ¹ https://xkcd.com/221/

In an alternate reality, I did consider 4 for the reasons you alluded
to, but ruled it out because it was not a random prime number. :)


>> >There may be more than one opinion, but as the maintainer of a TLS
>> >library in Debian I think it is a questionable tradeoff. At a minimum,
>> >it would be preferable to use the version number instead of a fixed
>> >constant for all releases.
>> 
>> Consider that even without the patch, each distro will build maradns once and distribute the package to their user. Every user gets the same binary with the same "random" number. So even if it's chosen at build time, it won't really help.
>> 
>> In our case, it only means users who don't use substitutes get a random number, others get the same number that the build farm picked at random. Fixing a number doesn't sound like it's gonna change a lot for these users.
>
> This is something we can work with. We can just mark the package as
> '#:substitutable? #f' and then everyone will have to build it
> themselves. It still won't really be reproducible, but everyone will
> actually have their own special random number.

This actually seems like the best approach in the short term! Leaving
time to work out a better fix long-term, probably by working with
upstream...

Thoughts?


>> >MaraDNS does not support DNSSEC so the program may not use entropy for
>> >keys. Either way, I'd rather use an unreproducible build than,
>> >accidentally, a known number series to encrypt secrets. Can one patch
>> >out the constant entirely so it is no longer available?
>> >
>> >The upstream website says: "People like MaraDNS because it’s ...
>> >remarkably secure." [1] Since many distributions have the same issue,
>> >upstream could perhaps offer the patch as a build switch to enable a
>> >build-time seed only when needed.
>> 
>> Sounds like the safest option. Maybe we could change the code that uses that number to naise an exception or abort?

Yeah, seems worth taking this or similar ideas upstream...


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]

On 2022-06-08, Vagrant Cascadian wrote:
> On 2022-06-08, Efraim Flashner wrote:
>> On Tue, Jun 07, 2022 at 07:20:25AM +0200, Julien Lepiller wrote:
>>> On June 7, 2022 5:24:22 AM GMT+02:00, Felix Lechner <felix.lechner@gmail.com> wrote:
>>> >On Mon, Jun 6, 2022 at 6:50 PM Vagrant Cascadian
>>> ><vagrant@reproducible-builds.org> wrote:
>> This is something we can work with. We can just mark the package as
>> '#:substitutable? #f' and then everyone will have to build it
>> themselves. It still won't really be reproducible, but everyone will
>> actually have their own special random number.
>
> This actually seems like the best approach in the short term! Leaving
> time to work out a better fix long-term, probably by working with
> upstream...
>
> Thoughts?

Should I just push that part for the short-term workaround? Or does
someone else want to push that?


>>> >MaraDNS does not support DNSSEC so the program may not use entropy for
>>> >keys. Either way, I'd rather use an unreproducible build than,
>>> >accidentally, a known number series to encrypt secrets. Can one patch
>>> >out the constant entirely so it is no longer available?
>>> >
>>> >The upstream website says: "People like MaraDNS because it’s ...
>>> >remarkably secure." [1] Since many distributions have the same issue,
>>> >upstream could perhaps offer the patch as a build switch to enable a
>>> >build-time seed only when needed.
>>> 
>>> Sounds like the safest option. Maybe we could change the code that uses that number to naise an exception or abort?
>
> Yeah, seems worth taking this or similar ideas upstream...

And, this was the best place I found to mention this issue upstream,
will see what kind of response I get:

  https://github.com/samboy/MaraDNS/discussions/101#discussioncomment-3006487


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]

On 2022-06-22, Vagrant Cascadian wrote:
> On 2022-06-08, Vagrant Cascadian wrote:
>> On 2022-06-08, Efraim Flashner wrote:
>>> On Tue, Jun 07, 2022 at 07:20:25AM +0200, Julien Lepiller wrote:
>>>> On June 7, 2022 5:24:22 AM GMT+02:00, Felix Lechner <felix.lechner@gmail.com> wrote:
>>>> >On Mon, Jun 6, 2022 at 6:50 PM Vagrant Cascadian
>>>> ><vagrant@reproducible-builds.org> wrote:
>>> This is something we can work with. We can just mark the package as
>>> '#:substitutable? #f' and then everyone will have to build it
>>> themselves. It still won't really be reproducible, but everyone will
>>> actually have their own special random number.
>>
>> This actually seems like the best approach in the short term! Leaving
>> time to work out a better fix long-term, probably by working with
>> upstream...
>>
>> Thoughts?
>
> Should I just push that part for the short-term workaround? Or does
> someone else want to push that?
>
>
>>>> >MaraDNS does not support DNSSEC so the program may not use entropy for
>>>> >keys. Either way, I'd rather use an unreproducible build than,
>>>> >accidentally, a known number series to encrypt secrets. Can one patch
>>>> >out the constant entirely so it is no longer available?
>>>> >
>>>> >The upstream website says: "People like MaraDNS because it’s ...
>>>> >remarkably secure." [1] Since many distributions have the same issue,
>>>> >upstream could perhaps offer the patch as a build switch to enable a
>>>> >build-time seed only when needed.
>>>> 
>>>> Sounds like the safest option. Maybe we could change the code that uses that number to naise an exception or abort?
>>
>> Yeah, seems worth taking this or similar ideas upstream...
>
> And, this was the best place I found to mention this issue upstream,
> will see what kind of response I get:
>
>   https://github.com/samboy/MaraDNS/discussions/101#discussioncomment-3006487

Upstream appears to think it is mostly ok to actually embed a specific
random prime... and not have it be different across all the builds, as
the number is mixed with other randomness from /dev/urandom.

It is expensive to generate the random prime on some hardware, so doing
so at runtime might not be feasible in some cases...

So, where do we go from here, knowing what we now know? :)


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Efraim Flashner]

[-- Attachment #1: Type: text/plain, Size: 3039 bytes --]

On Mon, Jun 27, 2022 at 06:31:41PM -0700, Vagrant Cascadian wrote:
> On 2022-06-22, Vagrant Cascadian wrote:
> > On 2022-06-08, Vagrant Cascadian wrote:
> >> On 2022-06-08, Efraim Flashner wrote:
> >>> On Tue, Jun 07, 2022 at 07:20:25AM +0200, Julien Lepiller wrote:
> >>>> On June 7, 2022 5:24:22 AM GMT+02:00, Felix Lechner <felix.lechner@gmail.com> wrote:
> >>>> >On Mon, Jun 6, 2022 at 6:50 PM Vagrant Cascadian
> >>>> ><vagrant@reproducible-builds.org> wrote:
> >>> This is something we can work with. We can just mark the package as
> >>> '#:substitutable? #f' and then everyone will have to build it
> >>> themselves. It still won't really be reproducible, but everyone will
> >>> actually have their own special random number.
> >>
> >> This actually seems like the best approach in the short term! Leaving
> >> time to work out a better fix long-term, probably by working with
> >> upstream...
> >>
> >> Thoughts?
> >
> > Should I just push that part for the short-term workaround? Or does
> > someone else want to push that?
> >
> >
> >>>> >MaraDNS does not support DNSSEC so the program may not use entropy for
> >>>> >keys. Either way, I'd rather use an unreproducible build than,
> >>>> >accidentally, a known number series to encrypt secrets. Can one patch
> >>>> >out the constant entirely so it is no longer available?
> >>>> >
> >>>> >The upstream website says: "People like MaraDNS because it’s ...
> >>>> >remarkably secure." [1] Since many distributions have the same issue,
> >>>> >upstream could perhaps offer the patch as a build switch to enable a
> >>>> >build-time seed only when needed.
> >>>> 
> >>>> Sounds like the safest option. Maybe we could change the code that uses that number to naise an exception or abort?
> >>
> >> Yeah, seems worth taking this or similar ideas upstream...
> >
> > And, this was the best place I found to mention this issue upstream,
> > will see what kind of response I get:
> >
> >   https://github.com/samboy/MaraDNS/discussions/101#discussioncomment-3006487
> 
> Upstream appears to think it is mostly ok to actually embed a specific
> random prime... and not have it be different across all the builds, as
> the number is mixed with other randomness from /dev/urandom.
> 
> It is expensive to generate the random prime on some hardware, so doing
> so at runtime might not be feasible in some cases...
> 
> So, where do we go from here, knowing what we now know? :)
> 
> 
> live well,
>   vagrant

I looked back at the original email. I think we should not embed a
static random prime and mark it as non-substitutable. Then with that
flag add a note that it generates a prime during building and everyone
having a unique prime is more important to us than everyone having the
same reproducible prime.


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Jack Hill]

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

On Tue, 28 Jun 2022, Efraim Flashner wrote:
> On Mon, Jun 27, 2022 at 06:31:41PM -0700, Vagrant Cascadian wrote:
>>>
>>>   https://github.com/samboy/MaraDNS/discussions/101#discussioncomment-3006487
>>
>> Upstream appears to think it is mostly ok to actually embed a specific
>> random prime... and not have it be different across all the builds, as
>> the number is mixed with other randomness from /dev/urandom.
>>
>> It is expensive to generate the random prime on some hardware, so doing
>> so at runtime might not be feasible in some cases...
>>
>> So, where do we go from here, knowing what we now know? :)
>>
>>
>> live well,
>>   vagrant
>
> I looked back at the original email. I think we should not embed a
> static random prime and mark it as non-substitutable. Then with that
> flag add a note that it generates a prime during building and everyone
> having a unique prime is more important to us than everyone having the
> same reproducible prime.

Hi, I think I lost something in the reasoning here. It sounded to me like 
upstream was OK with embedding a fixed prime, and even suggested one:

> If one’s coding style can’t have a random 32-bit number in a build, make 
> MUL_CONSTANT an unchanging number like 1748294267 (the number I use for 
> Windows builds of Deadwood). Deadwood will still be secure from hash 
> bucket collision attacks except in the very rare corner case of 
> /dev/urandom not giving out good random numbers.

Best,
Jack

Re: maradns reproducibility fixes and the merits of picking a random number

[Tobias Geerinckx-Rice]

Hi,

Vagrant said:
> It is expensive to generate the random prime on some hardware, so doing
> so at runtime might not be feasible in some cases...

But in the same reply you're paraphrasing, upstream also says:

> In 2010, I updated that homegrown hash compression
> algorithm to also add a random number when compressing
> the input, and calculating another 32-bit random number
> when Deadwood starts.
^^^^^^^^^^^^^^^^^^^^^^^

and

> I believe the hash compression algorithm is protected from hash
> bucket collision attacks, even if Deadwood is patched to make 
> MUL_CONSTANT a constant number, since the add constant
> remains random.

so their 'too computationally expensive' does not make sense to me.  Do they bail out if generating the truly random part 'takes too long'?  Surely not.

Neither does the 'ah, but your urandom might be broken' argument for silently substituting a still less random number.

I don't think this alone justifies the scheme, or disabling substitutes.

Kind regards,

T G-R

Sent on the go.  Excuse or enjoy my brevity.

Re: maradns reproducibility fixes and the merits of picking a random number

[Gábor Boskovits]

[-- Attachment #1: Type: text/plain, Size: 1620 bytes --]

Hi,

Tobias Geerinckx-Rice <me@tobias.gr> ezt írta (időpont: 2022. jún. 28., K
18:07):

> Hi,
>
> Vagrant said:
> > It is expensive to generate the random prime on some hardware, so doing
> > so at runtime might not be feasible in some cases...
>
> But in the same reply you're paraphrasing, upstream also says:
>
> > In 2010, I updated that homegrown hash compression
> > algorithm to also add a random number when compressing
> > the input, and calculating another 32-bit random number
> > when Deadwood starts.
> ^^^^^^^^^^^^^^^^^^^^^^^
>
> and
>
> > I believe the hash compression algorithm is protected from hash
> > bucket collision attacks, even if Deadwood is patched to make
> > MUL_CONSTANT a constant number, since the add constant
> > remains random.
>
> so their 'too computationally expensive' does not make sense to me.  Do
> they bail out if generating the truly random part 'takes too long'?  Surely
> not.
>
> Neither does the 'ah, but your urandom might be broken' argument for
> silently substituting a still less random number.
>
> I don't think this alone justifies the scheme, or disabling substitutes.
>
I tend to agree.
Afaics this can be solved in a workaround way. I don't think this random
number is picked up by the build in any way. Upstream could just provide it
as an optional config value. That would be better in every respect.  Then
they could just give a build flag to move to the new model. Do you think
such a proposal would be accepted upstream?

>
> Kind regards,
>
> T G-R
>
> Sent on the go.  Excuse or enjoy my brevity.
>
>

[-- Attachment #2: Type: text/html, Size: 2260 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

On 2022-06-28, Gábor Boskovits wrote:
> Tobias Geerinckx-Rice <me@tobias.gr> ezt írta (időpont: 2022. jún. 28., K
> 18:07):
>> Vagrant said:
>> > It is expensive to generate the random prime on some hardware, so doing
>> > so at runtime might not be feasible in some cases...
>>
>> But in the same reply you're paraphrasing, upstream also says:
>>
>> > In 2010, I updated that homegrown hash compression
>> > algorithm to also add a random number when compressing
>> > the input, and calculating another 32-bit random number
>> > when Deadwood starts.
>> ^^^^^^^^^^^^^^^^^^^^^^^
>>
>> and
>>
>> > I believe the hash compression algorithm is protected from hash
>> > bucket collision attacks, even if Deadwood is patched to make
>> > MUL_CONSTANT a constant number, since the add constant
>> > remains random.
>>
>> so their 'too computationally expensive' does not make sense to me.  Do
>> they bail out if generating the truly random part 'takes too long'?  Surely
>> not.
>>
>> Neither does the 'ah, but your urandom might be broken' argument for
>> silently substituting a still less random number.

Yeah, the response was a bit confusing to me, at least. :)


>> I don't think this alone justifies the scheme, or disabling substitutes.

I am at a loss as to what to do then ... nothing and just have it be
unreproducible? embed a specific random number? come up with better
upstreamable patches?


> I tend to agree.
> Afaics this can be solved in a workaround way. I don't think this random
> number is picked up by the build in any way.

What do you mean? The whole reason I discovered this issue is that it
embeds a random number in the build, making it not build reproducibly...


> Upstream could just provide it as an optional config value. That would
> be better in every respect.  Then they could just give a build flag to
> move to the new model.

That sounds reasonable to me...


> Do you think such a proposal would be accepted upstream?

I hope something better would be accepted upstream, but I don't myself
use maradns, and have a grand total of precisely one interaction with
maradns upstream so far... :)


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Tobias Geerinckx-Rice]

>I am at a loss as to what to do then ... nothing and just have it be
>unreproducible? embed a specific random number? come up with better
>upstreamable patches?

From upstream's response and my own biases and my reading of the room here, I'd say #2.


Kind regards,

T G-R

Sent on the go.  Excuse or enjoy my brevity.

Re: maradns reproducibility fixes and the merits of picking a random number

[Tobias Geerinckx-Rice]

...I mean: as long as there's this second, run-time random term added to it as upstream says there is.  I didn't audit the code to verify that.

Kind regards,

T G-R

Sent on the go.  Excuse or enjoy my brevity.

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 1029 bytes --]

On 2022-06-28, Tobias Geerinckx-Rice wrote:
>>I am at a loss as to what to do then ... nothing and just have it be
>>unreproducible? embed a specific random number? come up with better
>>upstreamable patches?
>
> From upstream's response and my own biases and my reading of the room here, I'd say #2.

Hrm.

I hear Efraim say better to have unique randomness and no substitutes,
and I hear Tobias say more or less it's ok as long as upstream is right
about it being ok to embed a specific prime as other random numbers get
mixed in at runtime...

I have a slight inclination towards making it non-substituteable, but I
may just be enamored of this as an interesting solution that most
distributions do not really have the option of taking. :)

Anyone else able to weigh in?

Or actually review the code?

If we can't find someone to review the code, seems like
non-substitutable is the safest approach ... at the guaranteed loss of
reproducibility. :/

Would love to resolve this issue one way or another.


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 538 bytes --]

On 2022-07-11, Vagrant Cascadian wrote:
> I hear Efraim say better to have unique randomness and no substitutes,
> and I hear Tobias say more or less it's ok as long as upstream is right
> about it being ok to embed a specific prime as other random numbers get
> mixed in at runtime...

Well, now that I hit send already, I guess another option is ... to have
both?

One package without patches that is not substitutable and not
reproducible, and one with patches that is verifyably reproducible and
substitutable?


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Ludovic Courtès]

Hi,

Vagrant Cascadian <vagrant@reproducible-builds.org> skribis:

> On 2022-07-11, Vagrant Cascadian wrote:
>> I hear Efraim say better to have unique randomness and no substitutes,
>> and I hear Tobias say more or less it's ok as long as upstream is right
>> about it being ok to embed a specific prime as other random numbers get
>> mixed in at runtime...
>
> Well, now that I hit send already, I guess another option is ... to have
> both?
>
> One package without patches that is not substitutable and not
> reproducible, and one with patches that is verifyably reproducible and
> substitutable?

Honestly, I don’t think it’s worth bothering about the non-substitutable
trick.  In practice, maradns should be able to rely on /dev/urandom at
run time, right?

Ludo’.

Re: maradns reproducibility fixes and the merits of picking a random number

[Tobias Geerinckx-Rice]

[-- Attachment #1: Type: text/plain, Size: 276 bytes --]

Ludovic Courtès 写道：
> Honestly, I don’t think it’s worth bothering about the 
> non-substitutable
> trick.

Agreed.

> In practice, maradns should be able to rely on /dev/urandom at
> run time, right?

That is my understanding.

Kind regards,

T G-R

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Ludovic Courtès]

Hi,

Vagrant Cascadian <vagrant@reproducible-builds.org> skribis:

> But there's one nervous-making issue this revealed; maradns embeds a
> random number at build time ... allegedly for systems that don't have
> /dev/urandom... see
> maradns-3.5.0020/deadwood-3.5.0020/src/Makefile.ubuntu2004:
>
>   # Since some systems may not have /dev/urandom (Windows, *cough* *cough*), we
>   # keep a randomly generated prime around
>
> So it's got some code to generate a random number at build time and
> embed it in the binary. Now, if there's anything I know about good
> practices about random numbers, this sort of thing is generally a very
> large red flag! It also makes the package build differently every time!

Woow.  Reproducibility work lets you uncover issues you wouldn’t think of!

Ludo’.

Re: maradns reproducibility fixes and the merits of picking a random number

[Arun Isaac]

Hi Vagrant,

> But there's one nervous-making issue this revealed; maradns embeds a
> random number at build time ... allegedly for systems that don't have
> /dev/urandom... see
> maradns-3.5.0020/deadwood-3.5.0020/src/Makefile.ubuntu2004:
>
>   # Since some systems may not have /dev/urandom (Windows, *cough* *cough*), we
>   # keep a randomly generated prime around
>
> So it's got some code to generate a random number at build time and
> embed it in the binary. Now, if there's anything I know about good
> practices about random numbers, this sort of thing is generally a very
> large red flag! It also makes the package build differently every
> time!

Wow, great find! Has this issue been reported to maradns upstream? If
upstream fixes it or provides us a compile flag to disable this
"feature", it would be even better in the long run.

Regards,
Arun

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

On 2022-06-09, Arun Isaac wrote:
> Hi Vagrant,
>
>> But there's one nervous-making issue this revealed; maradns embeds a
>> random number at build time ... allegedly for systems that don't have
>> /dev/urandom... see
>> maradns-3.5.0020/deadwood-3.5.0020/src/Makefile.ubuntu2004:
>>
>>   # Since some systems may not have /dev/urandom (Windows, *cough* *cough*), we
>>   # keep a randomly generated prime around
>>
>> So it's got some code to generate a random number at build time and
>> embed it in the binary. Now, if there's anything I know about good
>> practices about random numbers, this sort of thing is generally a very
>> large red flag! It also makes the package build differently every
>> time!
>
> Wow, great find! Has this issue been reported to maradns upstream? If
> upstream fixes it or provides us a compile flag to disable this
> "feature", it would be even better in the long run.

That does sound like the best long-term approach, definitely!

Will take the issue upstream...


live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

Re: maradns reproducibility fixes and the merits of picking a random number

[Philip McGrath]

Hi,

On 6/8/22 16:25, Vagrant Cascadian wrote:
> On 2022-06-09, Arun Isaac wrote:
>> Hi Vagrant,
>>
>>> But there's one nervous-making issue this revealed; maradns embeds a
>>> random number at build time ... allegedly for systems that don't have
>>> /dev/urandom... see
>>> maradns-3.5.0020/deadwood-3.5.0020/src/Makefile.ubuntu2004:
>>>
>>>    # Since some systems may not have /dev/urandom (Windows, *cough* *cough*), we
>>>    # keep a randomly generated prime around
>>>
>>> So it's got some code to generate a random number at build time and
>>> embed it in the binary. Now, if there's anything I know about good
>>> practices about random numbers, this sort of thing is generally a very
>>> large red flag! It also makes the package build differently every
>>> time!
>>
>> Wow, great find! Has this issue been reported to maradns upstream? If
>> upstream fixes it or provides us a compile flag to disable this
>> "feature", it would be even better in the long run.
> 
> That does sound like the best long-term approach, definitely!
> 
> Will take the issue upstream...
> 

Not sure if this is useful—I've never used MaraDNS, and I know just 
barely enough about Windows to try to keep free software portable—but, 
on Windows XP and later, you can get cryptographic-quality randomness by 
dynamically loading SystemFunction036 from Advapi32.dll, sometimes known 
as RtlGenRandom. Racket uses this to implement crypto-random-bytes, 
which uses /dev/urandom on Unix-like systems. It's also apparently used 
by Chromium, Firefox, and many others.

Racket example: 
https://github.com/racket/racket/blob/master/racket/collects/racket/private/windows-rand.rkt

C example from Microsoft: 
http://web.archive.org/web/20180929235240/https://blogs.msdn.microsoft.com/michael_howard/2005/01/14/cryptographically-secure-random-number-on-windows-without-using-cryptoapi/

Documentation, such as it is: 
https://docs.microsoft.com/en-us/windows/win32/api/ntsecapi/nf-ntsecapi-rtlgenrandom

Mozilla thread explaining why the warnings in that documentation are 
irrelevant: https://bugzilla.mozilla.org/show_bug.cgi?id=504270#c4

-Philip

Re: maradns reproducibility fixes and the merits of picking a random number

[Liliana Marie Prikler]

Am Montag, dem 06.06.2022 um 18:49 -0700 schrieb Vagrant Cascadian:
> p.s. Obviously, I picked the best random number.
I beg to differ.
> +-RandomPrime:  RandomPrime.c
> +-      $(CC) -O3 -o RandomPrime RandomPrime.c
> +-
> +-DwRandPrime.h: RandomPrime
> +-      if [ -e /dev/urandom ] ; then ./RandomPrime > DwRandPrime.h ;
> fi
> ++DwRandPrime.h:
> ++      echo '#define MUL_CONSTANT 1238145941' > DwRandPrime.h
This does not satisfy requirement #221: chosen by a fair dice roll. 
Randomness can therefore not be guaranteed.

Re: maradns reproducibility fixes and the merits of picking a random number

[Vagrant Cascadian]

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

On 2022-06-08, Liliana Marie Prikler wrote:
> Am Montag, dem 06.06.2022 um 18:49 -0700 schrieb Vagrant Cascadian:
>> p.s. Obviously, I picked the best random number.
> I beg to differ.
>> +-RandomPrime:  RandomPrime.c
>> +-      $(CC) -O3 -o RandomPrime RandomPrime.c
>> +-
>> +-DwRandPrime.h: RandomPrime
>> +-      if [ -e /dev/urandom ] ; then ./RandomPrime > DwRandPrime.h ;
>> fi
>> ++DwRandPrime.h:
>> ++      echo '#define MUL_CONSTANT 1238145941' > DwRandPrime.h
> This does not satisfy requirement #221: chosen by a fair dice roll. 
> Randomness can therefore not be guaranteed.

I will admit to "Obviously, I picked the best random number." as a
joke. Hard-coding any supposedly random number seems awfully wrong to
me!

This is a not particularly great patch to make it compile reproducibly,
on the *assumption* that number will not actually be used in practice,
because it *supposed* to only be used when /dev/urandom is not
available. I would love to see better patches that make fewer
assumptions!

FWIW, This is effectively the same embedded random number used in the
Debian patch, although the maradns packaging in Debian basically comes
to the same result by copying files around rather than patching them
directly.

live well,
  vagrant

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]