Bug#864082: Improved patch
Sorry about the noise,
The long mail was sent only to control@... and therefore invisible.
With kind regards,
Roland Clobus
Forwarded Message
Subject: Improved patch
Date: Fri, 28 Jan 2022 18:08:28 +0100
From: Roland Clobus
To: cont...@bugs.debian.org
reopen 864082 =
thanks
Hello maintainers,
Thanks for releasing a new version of fontconfig with the patch for the
reproducible cache files
(0001-Ensure-cache-checksums-are-determinstic.patch).
Unfortunately, I see side-effects of the applied patch (as shown in the
live-build images [1])
* Potential out-ouf-bounds-read issue: The function uuid_generate_sha1
is called with an incorrect second argument.
The second argument must be guaranteed to be of length 16 (or
longer), which is the size of a uuid.
E.g. /root/.fonts is only 12 bytes, which means that some random
bytes at the end of the string will be used for the sha1 sum.
The updated patch uses the null namespace as the basis for the sha1 sum.
... or should I have use one of the predefine namespaces instead?
* The patch adds new compiler warnings. I've added some casts to remove
compiler warnings
* There is a second scenario: initramfs with fonts:
plymouth-set-default-theme tribar --rebuild-initrd
or
update-initramfs -k all -u
The value for 'target' is contains a random part:
/var/tmp/mkinitramfs_ijJP8d//usr/share/fonts
This path is created by the plymouth hook in initramfs which uses
'fc-cache -s -y TEMPDIR'
The fonts in the ramdisk can be listed with:
zless /initrd.img | cpio --list --quiet | grep fontconfig | grep cache-7
For regular invocations of fc-cache, the '-y' argument is not used
and then 'target' and 'dir' are identical. The attached patch uses 'dir'
instead of 'target' and then the cache of the embedded fonts in the
ramdisk is reproducible as well.
Attached you'll find the patch that fixes all three issues mentioned above.
With kind regards,
Roland Clobus
[1] https://jenkins.debian.net/view/live/
From 70565e0f73d116a2a9523146228efcca1e76b016 Mon Sep 17 00:00:00 2001
From: Chris Lamb
Date: Mon, 29 Oct 2018 15:48:51 -0400
Subject: [PATCH] Make the cache filenames determinstic
Whilst working on the Reproducible Builds[0] effort, we noticed that
fontconfig generates cache files with unreproducible/non-deterministic
filenames.
This is a supplement to the changes added in f098adac54ab where we
ensured that the checksums themselves were determistic but the files
that were stored in the cache directory are currently being given
"random" names via uuid(3)'s uuid_generate_random function, thus
any images that generate such files have different contents on every
build.
This patch changes the behaviour of the cache directory filename
calculation to be based on the "source" directory name, rather than
being entirely random.
An alternative solution could be to continue to use the previous
uuid_generate_random function but use this alternative codepath if the
SOURCE_DATE_EPOCH[1] environment variable was determined to be
present via getenv(3).
This work was sponsored by Tails[2] and tracked in Debian in #864082[3].
[0] https://reproducible-builds.org/
[1] https://reproducible-builds.org/specs/source-date-epoch/
[2] https://tails.boum.org/
[3] https://bugs.debian.org/864082
---
src/fccache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: fontconfig-2.13.1/src/fccache.c
===
--- fontconfig-2.13.1.orig/src/fccache.c
+++ fontconfig-2.13.1/src/fccache.c
@@ -101,7 +101,13 @@ FcDirCacheCreateUUID (FcChar8 *dir,
ret = FcFalse;
goto bail3;
}
- uuid_generate_random (uuid);
+ if (getenv("SOURCE_DATE_EPOCH"))
+ {
+ const uuid_t nil = { 0 };
+ uuid_generate_sha1 (uuid, nil, (const char *)dir, strlen((const char *)dir));
+ }
+ else
+ uuid_generate_random (uuid);
if (force)
hash_add = FcHashTableReplace;
else
OpenPGP_signature
Description: OpenPGP digital signature
Bug#864082: Improved patch
Hi Roland, Quoting Roland Clobus (2022-01-28 18:22:17) > Sorry about the noise, as far as I'm concerned, you made no noise but you made a very valuable contribution instead. Thank you! :) > Thanks for releasing a new version of fontconfig with the patch for the > reproducible cache files > (0001-Ensure-cache-checksums-are-determinstic.patch). > > Unfortunately, I see side-effects of the applied patch (as shown in the > live-build images [1]) > > * Potential out-ouf-bounds-read issue: The function uuid_generate_sha1 > is called with an incorrect second argument. > > The second argument must be guaranteed to be of length 16 (or > longer), which is the size of a uuid. > E.g. /root/.fonts is only 12 bytes, which means that some random > bytes at the end of the string will be used for the sha1 sum. > > The updated patch uses the null namespace as the basis for the sha1 sum. > >... or should I have use one of the predefine namespaces instead? > > * The patch adds new compiler warnings. I've added some casts to remove > compiler warnings > > * There is a second scenario: initramfs with fonts: > plymouth-set-default-theme tribar --rebuild-initrd > or > update-initramfs -k all -u > > The value for 'target' is contains a random part: > /var/tmp/mkinitramfs_ijJP8d//usr/share/fonts > > This path is created by the plymouth hook in initramfs which uses > 'fc-cache -s -y TEMPDIR' > > The fonts in the ramdisk can be listed with: > zless /initrd.img | cpio --list --quiet | grep fontconfig | grep cache-7 > > For regular invocations of fc-cache, the '-y' argument is not used > and then 'target' and 'dir' are identical. The attached patch uses 'dir' > instead of 'target' and then the cache of the embedded fonts in the > ramdisk is reproducible as well. > > Attached you'll find the patch that fixes all three issues mentioned above. You completely rewrote the patch. I think it makes sense to replace Chris' authorship with yours. Chris, do you agree? Would you be willing to submit an updated patch containing the name and email of your choice and a commit message that explains your change? What you wrote above is a good explanation I think. Thanks! cheers, josch signature.asc Description: signature
Bug#864082: Improved patch
Hello Johannes,
On 28/01/2022 20:22, Johannes Schauer Marin Rodrigues wrote:
Would you be willing to submit an updated patch containing the name and email
of your choice and a commit message that explains your change? What you wrote
above is a good explanation I think.
Attached is the updated patch.
A commit message would be:
This patch changes the behaviour of the cache directory filename
calculation to be based on the "source" directory name, rather than
being entirely random if the SOURCE_DATE_EPOCH[1] environment variable
was determined to be present via getenv(3).
The two main scenarios are covered by this patch:
1) Invocation of 'fc-cache' as a postinst step
2) Invocation of 'update-initramfs' as a postinst step where the
initial ramdisk contains a font (e.g. when the plymouth hook calls
'fc-cache -s -y TEMPDIR')
or much shorter:
Generate deterministic cache directory filenames if SOURCE_DATE_EPOCH is
set.
With kind regards,
Roland Clobus
From: Roland Clobus
Date: Sat 29 Jan 07:58:22 UTC 2022
Subject: [PATCH] Make the cache filenames determinstic
Whilst working on the Reproducible Builds[0] effort, we noticed that
fontconfig generates cache files with unreproducible/non-deterministic
filenames.
This is a supplement to the changes added in f098adac54ab where we
ensured that the checksums themselves were determistic but the files
that were stored in the cache directory are currently being given
"random" names via uuid(3)'s uuid_generate_random function, thus
any images that generate such files have different contents on every
build.
This patch changes the behaviour of the cache directory filename
calculation to be based on the "source" directory name, rather than
being entirely random if the SOURCE_DATE_EPOCH[1] environment variable
was determined to be present via getenv(3).
The two main scenarios are covered by this patch:
1) Invocation of 'fc-cache' as a postinst step
2) Invocation of 'update-initramfs' as a postinst step where the
initial ramdisk contains a font (e.g. when the plymouth hook calls
'fc-cache -s -y TEMPDIR')
This work is based on the patch written by Chris Lamb
who is sponsored by Tails[3].
[0] https://reproducible-builds.org/
[1] https://reproducible-builds.org/specs/source-date-epoch/
[2] https://bugs.debian.org/864082
[3] https://tails.boum.org/
Bug-Debian: http://bugs.debian.org/864082
---
src/fccache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: fontconfig-2.13.1/src/fccache.c
===
--- fontconfig-2.13.1.orig/src/fccache.c
+++ fontconfig-2.13.1/src/fccache.c
@@ -101,7 +101,13 @@ FcDirCacheCreateUUID (FcChar8 *dir,
ret = FcFalse;
goto bail3;
}
- uuid_generate_random (uuid);
+ if (getenv("SOURCE_DATE_EPOCH"))
+ {
+ const uuid_t nil = { 0 };
+ uuid_generate_sha1 (uuid, nil, (const char *)dir, strlen((const char *)dir));
+ }
+ else
+ uuid_generate_random (uuid);
if (force)
hash_add = FcHashTableReplace;
else
OpenPGP_signature
Description: OpenPGP digital signature
