Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reproducibility problems with libcrypto and libtss2 #1616

Closed
miczyg1 opened this issue Feb 26, 2024 · 12 comments · Fixed by #1630
Closed

Reproducibility problems with libcrypto and libtss2 #1616

miczyg1 opened this issue Feb 26, 2024 · 12 comments · Fixed by #1630

Comments

@miczyg1
Copy link
Contributor

miczyg1 commented Feb 26, 2024

Please describe the problem

Describe the bug
libcrypto and libtss2 are not reproducible. A build made by two different people on the same revision showed that libcrypto and libtss2 libraries do not have the same hashes (by checking that hashes.txt file after build).

Expected behavior
libcrypto and libtss2 should be reproducible.

Additional context
Checking the affected libraries with diffoscope shows that there are paths and dates included in these libraries. For reference, full diffoscope output is available here (pastebin cannot handle nearly 500k lines file).

Most notable are:

  1. libcrypto strings:
│ ├── strings --all --bytes=8 {}
│ │ @@ -10042,22 +10042,22 @@
│ │  secure malloc failure
│ │  too many bytes
│ │  too small buffer
│ │  unknown name in random section
│ │  zero length number
│ │  OPENSSL_ia32cap
│ │  %s:%d: OpenSSL internal error: %s
│ │ -built on: Wed Feb 21 13:03:19 2024 UTC
│ │ +built on: Mon Feb 26 11:23:16 2024 UTC
│ │  platform: linux-x86_64
│ │  OPENSSLDIR: "/ssl"
│ │  ENGINESDIR: "//lib64/engines-3"
│ │  MODULESDIR: "//lib64/ossl-modules"
│ │  OpenSSL 3.0.8 7 Feb 2023
│ │  CPUINFO: N/A
│ │ -compiler: /home/michal/Development/Dasharo/heads/crossgcc/x86/bin/x86_64-linux-musl-gcc -fdebug-prefix-map=/home/michal/Development/Dasharo/heads=heads -gno-record-gcc-switches -D__MUSL__ -isystem /home/michal/Development/Dasharo/heads/install/x86/include -L/home/michal/Development/Dasharo/heads/install/x86/lib  -fPIC -pthread -m64 -Wa,--noexecstack -Os -DOPENSSL_USE_NODELETE -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_BUILDING_OPENSSL -DNDEBUG
│ │ +compiler: /home/coreboot/coreboot/crossgcc/x86/bin/x86_64-linux-musl-gcc -fdebug-prefix-map=/home/coreboot/coreboot=heads -gno-record-gcc-switches -D__MUSL__ -isystem /home/coreboot/coreboot/install/x86/include -L/home/coreboot/coreboot/install/x86/lib  -fPIC -pthread -m64 -Wa,--noexecstack -Os -DOPENSSL_USE_NODELETE -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_BUILDING_OPENSSL -DNDEBUG
│ │  crypto/ex_data.c
│ │  CRYPTO_set_ex_data
│ │  CRYPTO_dup_ex_data
│ │  ossl_crypto_new_ex_data_ex
│ │  ossl_crypto_get_ex_new_index_ex
│ │  get_and_lock
│ │  CPUINFO: OPENSSL_ia32cap=0x%llx:0x%llx
  1. libtss2 strings:

The libs seems to put the full path into the shared object file. Either the prefix is
/home/michal/Development/Dasharo/heads/build/x86/ or /home/coreboot/coreboot/build/x86/. The difference here results from using the same docker image as base environment to build heads, but the parameter for mounting the directory with source was different, i.e. -v $PWD:$PWD vs -v $PWD:/home/coreboot/coreboot.

While the paths are easy to workaround (simply enforce the same mounting paths to docker), these libraries should give the same hashes no matter the paths where they have been built.

@miczyg1
Copy link
Contributor Author

miczyg1 commented Feb 26, 2024

Found out that problem with libtss2 is the RPATHs: https://reproducible-builds.org/docs/deterministic-build-systems/

For openssl, there is a perl script that generates crypto/buildinf.h with date and compiler. So we could probably patch it during openssl configure step.

@tlaurion
Copy link
Collaborator

tlaurion commented Mar 28, 2024

@miczyg1 some progress on this yet nothing official.

Openssl is now reproducible under master...tlaurion:heads:reproducible_openssl_libtss2 for patches/openssl-3.0.8.patch

(Feel free to borrow, hack, open a PR if working!)

Trying a sed approach to patch configure on the fly after bootstrap/autoreconf there. No more hardcode nor rpath references under configure scripts as a result, but still I get

user@heads-tests-deb12:~/heads$ strings build/x86/tpm2-tools-5.2/tools/tpm2 | grep heads | sort | uniq
heads/build/x86/tpm2-tools-5.2
heads/crossgcc/x86/x86_64-linux-musl/include
heads/crossgcc/x86/x86_64-linux-musl/include/bits
heads/crossgcc/x86/x86_64-linux-musl/include/sys
heads/install/x86/include/openssl
heads/install/x86/include/tss2
user@heads-tests-deb12:~/heads$ strings build/x86/tpm2-tss-3.2.0/src/tss2-sys/.libs/libtss2-sys.so.1.0.0 | grep heads | sort | uniq
heads/build/x86/tpm2-tss-3.2.0
heads/crossgcc/x86/x86_64-linux-musl/include
heads/crossgcc/x86/x86_64-linux-musl/include/bits
heads/crossgcc/x86/x86_64-linux-musl/include/netinet
heads/crossgcc/x86/x86_64-linux-musl/include/sys

Not sure what to push forward there, this is messy. Will retry later :/
@JonathonHall-Purism?

@JonathonHall-Purism
Copy link
Collaborator

@tlaurion If I understand what you're looking for correctly, the output you have looks good. The output shows that -fdebug-prefix-map was correctly applied, substituting heads for the beginning of the path (the part that may vary on different systems).

So instead of /home/tlaurion/work/heads/build/x86/tpm2-tools-5.2 you get heads/build/x86/tpm2-tools-5.2, and I should get the same thing.

Also, I started looking over the seds in the WIP commit to see if there's a better way to control that behavior - it looks like a lot of them probably are not actually relevant, e.g. hardcode_runpath_var=yes only occurs for a host OS like sysv4*MP*, some of them were for things like AmigaOS on m68k, etc., a lot of them do not appear to apply to Linux.

I'd start by eliminating the substitutions that aren't having any effect until we find the minimal changes that do fix the problem (which IIRC were rpaths, readelf on both versions could confirm). Then we can look at those individually and see if there is a proper parameter we can give to configure or something else we can do that's more reliable.

@tlaurion
Copy link
Collaborator

@JonathonHall-Purism under Circleci heads is project :/

Maybe I went too far and should replace dest dir for CircleCI... Will recheck how to force output dir there and clean WiP commit. Thanks for checking this out!

@JonathonHall-Purism
Copy link
Collaborator

@JonathonHall-Purism under Circleci heads is project :/

What do you mean, how does that relate to the path? The -fdebug-prefix-map should apply there too regardless of where the build occurs, and it looks like it's doing that from the output above.

If it's still different, could you post the differences between a CircleCI build and a local build?

@tlaurion
Copy link
Collaborator

tlaurion commented Apr 1, 2024

@JonathonHall-Purism Issue vanished with PoC under master...tlaurion:heads:reproducible_openssl_libtss2

But i'm even more challenged now with the behavior I observe:
repro.txt

More specifically tpm2 binary hardcoded strings:

│ ├── strings --all --bytes=8 {}
│ │ @@ -2541,15 +2541,15 @@
│ │  Where <options> are:
│ │  [ -%c | --%s%s]
│ │  [ --%s%s]
│ │  enable-errata
│ │  T:h::vVQZ
│ │  --help=no-man
│ │  -h=no-man
│ │ -v0.2.0-2047-gad91210b
│ │ +v0.2.0-2047-gad91210bd2b
│ │  Unknown option found: %c
│ │  TPM2TOOLS_TCTI
│ │  TPM2TOOLS_ENABLE_ERRATA
│ │  Could not open /dev/null
│ │  tcti-help(%s): %s
│ │  %s: tool doesn't support the TCTI option
│ │  Unknown help argument, got: "%s"

Where the hell this git version is coming from?!?!
@JonathonHall-Purism : I understand that tpm2-tools and tpm2-tss are bootstrapped prior of being configured following upstream instructions. But doing so calls a bunch of host called scripts which reconfigures mostly eveything that we then need to patch.... But that one, wow.
For context, we hardode git abbrev 7 in Makefile. Here we have 8 and 12. Fun :)

I was hoping #1269 to permit to fix it al together, but we

  • don't seem to be there yet
  • Nix comes with other complications, even with my tests with docker image which should eventually work and fix all this
  • Meanwhile, running things under CircleCI develop env brings even more things in terms of reproducibility...

So we will need to fix this one more cleanly prior of nix to come to the rescue.

I also attempted to hack around PACKAGE_STRING and PACKAGE_VERSION under https://github.com/tlaurion/heads/tree/reproducible_openssl_libtss2_version_embedded_into_tpm2tools_more_chars_then_git_abbrev-failed but as the name of the branch says, that was not successful either.

@JonathonHall-Purism Any idea here?

@tlaurion
Copy link
Collaborator

tlaurion commented Apr 1, 2024

@JonathonHall-Purism under Circleci heads is project :/

Maybe I went too far and should replace dest dir for CircleCI... Will recheck how to force output dir there and clean WiP commit. Thanks for checking this out!

project is the name of the output directory that CircleCI creates and checks out git under.
So if on local machine the pwd is heads, under CircleCI it was project.
That was fixed under latest commit of working branch by fixing CircleCI config.

@JonathonHall-Purism
Copy link
Collaborator

Where the hell this git version is coming from?!?!

The top of configure.ac (https://github.com/tpm2-software/tpm2-tools/blob/5.2/configure.ac#L1):

AC_INIT([tpm2-tools],
    [m4_esyscmd_s([git describe --tags --always --dirty])])

The second argument is a version string, which becomes AC_PACKAGE_VERSION. That's used to set the VERSION macro, which is used in lib/tpm2_options.c.

Since we are building tpm2-tools from a tarball, and bootstrapping ourselves rather than running the pre-generated configure script, it happens to pick up a version from the Heads repo.

Putting aside a bit of a rant about autotools, I think two options are:

  • Build from a Git clone instead of a tarball (would pick up the tag in git --describe as intended)
  • Patch configure.ac (perhaps add --git-dir=. to prevent it from walking up to Heads - might make sense upstream as well, or patch in a fixed version?)

@tlaurion
Copy link
Collaborator

tlaurion commented Apr 2, 2024

@JonathonHall-Purism I fixed it by changing the configure script itself which might not be the best.
Considering how Heads works for patches, it might be beneficial to switch master...tlaurion:heads:reproducible_openssl_libtss2#diff-08ff2866b921be31a83456caa713dfdae6f70e545ec475fec1e15a5a284b4188R31 into a patch for configure.ac but i'm not sure the patch would apply cleanly unless #1269 is merged.

Will go forward and try to clean master...tlaurion:heads:reproducible_openssl_libtss2 to open PR

@JonathonHall-Purism
Copy link
Collaborator

Glad that produces the result we want, it's a step in the right direction. IMO, try to trim down the seds to just the few that are actually needed (most of them don't seem to be relevant), then I could help make it a proper patch.

@tlaurion

This comment has been minimized.

@tlaurion
Copy link
Collaborator

tlaurion commented Apr 3, 2024

All tools/libs packed under tools.cpio reproducible per #1630.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants