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

Local Build-System produces diferrent roms than CircleCI #1829

Closed
8 tasks
Oessel opened this issue Nov 1, 2024 · 12 comments · Fixed by #1833
Closed
8 tasks

Local Build-System produces diferrent roms than CircleCI #1829

Oessel opened this issue Nov 1, 2024 · 12 comments · Fixed by #1833

Comments

@Oessel
Copy link

Oessel commented Nov 1, 2024

Context of the Build

1. What board are you trying to build?

t430-hotp-maximized

2. What repository:branch are you using to build from?

  • [ x ] Heads:Master
  • Other (please specify)

3. What version of coreboot are you trying to build

produced ROM's have no commit ID, source tree is up-to-date, so it should be 2399-g515ca5d

4. In building the rom where did you get the blobs?

  • No blobs required
  • Provided by the company that installed Heads on the device
  • Extracted from a backup rom taken from this device
  • Extracted from another backup rom taken from another device (please identify the board model)
  • [ x ] Extracted from the online bios using the automated tools provided in Heads
  • I don't know

5. If using the automated tools to get the blobs did you run the relevant scripts in the blobs directory

  • Yes
  • No

6. What operating system are you using

Debian-12 AppVM on QubesOS 4.2.3 ( with Docker and Nix installed after provided guides)

Please describe the problem

My build system produces Roms with different hashes than CircleCi, the Roms are only named e.g. t430-hotp-maximized-.rom (no commit-id). I even built a fresh docker image on a new created VM and built from a fresh cloned git-tree with the same result

To Reproduce
Steps to reproduce the behavior:

1.Create a build environment in a qubes AppVM after the guide in the documentation
2. clone https://github.com/linuxboot/heads
3. run docker run -e DISPLAY=$DISPLAY --network host --rm -ti -v $(pwd):$(pwd) -w $(pwd) linuxboot/heads:dev-env -- make BOARD=t430-hotp-maximized

Expected behavior
System produces a reproductible rom

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Please excuse my incomplete description, i'll try to provide any information you need

@Oessel Oessel changed the title Local Build-System produces difer Local Build-System produces diferrent roms than CircleCI Nov 1, 2024
@Oessel
Copy link
Author

Oessel commented Nov 1, 2024

hashes.txt

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 2, 2024

From : linuxboot/heads-wiki#70 (comment)

#Download CircleCI hashes.txt and put it in local dir:
HASH1=hashes.txt
#point to local build hashes.txt for same build:
HASH2=~/heads/build/x86/t430-hotp/maximized/hashes.txt
egrep '^[0-9a-f]{64}' $HASHES1 | while read line; do HASH_REF=$(echo $line|awk -F " " {'print $1'}); FILE_REF=$(echo $line|awk -F "/" {'print $NF'}); if ! grep -q "$HASH_REF" $HASHES2; then echo "$FILE_REF doesn't match";fi; done

Same hashes.
There is a problem though with git reporting git error which makes the build system not fill git commit ID etc

ie: fatal: detected dubious ownership in repository at '/home/user/heads'

That is, Makefile git operations are failing in container because of ID mismatches somewhere, and I cannot find where for the moment:

user@heads-tests-deb12-nix:~/heads$ docker run -e DISPLAY=$DISPLAY --network host --rm -ti -v $(pwd):$(pwd) -w $(pwd) tlaurion/heads-dev-env:v0.2.3 -- make BOARD=t430-hotp-maximized
fatal: detected dubious ownership in repository at '/home/user/heads'
To add an exception for this directory, call:

	git config --global --add safe.directory /home/user/heads
warning: Not a git repository. Use --no-index to compare two paths outside a working tree
usage: git diff --no-index [<options>] <path> <path>

Diff output format options
    -p, --patch           generate patch
    -s, --no-patch        suppress diff output
    -u                    generate patch
    -U, --unified[=<n>]   generate diffs with <n> lines context
    -W, --[no-]function-context
                          generate diffs with <n> lines context
    --raw                 generate the diff in raw format
    --patch-with-raw      synonym for '-p --raw'
    --patch-with-stat     synonym for '-p --stat'
    --numstat             machine friendly --stat
    --shortstat           output only the last line of --stat
    -X, --dirstat[=<param1>,<param2>...]
                          output the distribution of relative amount of changes for each sub-directory
    --cumulative          synonym for --dirstat=cumulative
    --dirstat-by-file[=<param1>,<param2>...]
                          synonym for --dirstat=files,<param1>,<param2>...
    --check               warn if changes introduce conflict markers or whitespace errors
    --summary             condensed summary such as creations, renames and mode changes
    --name-only           show only names of changed files
    --name-status         show only names and status of changed files
    --stat[=<width>[,<name-width>[,<count>]]]
                          generate diffstat
    --stat-width <width>  generate diffstat with a given width
    --stat-name-width <width>
                          generate diffstat with a given name width
    --stat-graph-width <width>
                          generate diffstat with a given graph width
    --stat-count <count>  generate diffstat with limited lines
    --[no-]compact-summary
                          generate compact summary in diffstat
    --binary              output a binary diff that can be applied
    --[no-]full-index     show full pre- and post-image object names on the "index" lines
    --[no-]color[=<when>] show colored diff
    --ws-error-highlight <kind>
                          highlight whitespace errors in the 'context', 'old' or 'new' lines in the diff
    -z                    do not munge pathnames and use NULs as output field terminators in --raw or --numstat
    --[no-]abbrev[=<n>]   use <n> digits to display object names
    --src-prefix <prefix> show the given source prefix instead of "a/"
    --dst-prefix <prefix> show the given destination prefix instead of "b/"
    --line-prefix <prefix>
                          prepend an additional prefix to every line of output
    --no-prefix           do not show any source or destination prefix
    --default-prefix      use default prefixes a/ and b/
    --inter-hunk-context <n>
                          show context between diff hunks up to the specified number of lines
    --output-indicator-new <char>
                          specify the character to indicate a new line instead of '+'
    --output-indicator-old <char>
                          specify the character to indicate an old line instead of '-'
    --output-indicator-context <char>
                          specify the character to indicate a context instead of ' '

Diff rename options
    -B, --break-rewrites[=<n>[/<m>]]
                          break complete rewrite changes into pairs of delete and create
    -M, --find-renames[=<n>]
                          detect renames
    -D, --irreversible-delete
                          omit the preimage for deletes
    -C, --find-copies[=<n>]
                          detect copies
    --[no-]find-copies-harder
                          use unmodified files as source to find copies
    --no-renames          disable rename detection
    --[no-]rename-empty   use empty blobs as rename source
    --[no-]follow         continue listing the history of a file beyond renames
    -l <n>                prevent rename/copy detection if the number of rename/copy targets exceeds given limit

Diff algorithm options
    --minimal             produce the smallest possible diff
    -w, --ignore-all-space
                          ignore whitespace when comparing lines
    -b, --ignore-space-change
                          ignore changes in amount of whitespace
    --ignore-space-at-eol ignore changes in whitespace at EOL
    --ignore-cr-at-eol    ignore carrier-return at the end of line
    --ignore-blank-lines  ignore changes whose lines are all blank
    -I, --[no-]ignore-matching-lines <regex>
                          ignore changes whose all lines match <regex>
    --[no-]indent-heuristic
                          heuristic to shift diff hunk boundaries for easy reading
    --patience            generate diff using the "patience diff" algorithm
    --histogram           generate diff using the "histogram diff" algorithm
    --diff-algorithm <algorithm>
                          choose a diff algorithm
    --anchored <text>     generate diff using the "anchored diff" algorithm
    --word-diff[=<mode>]  show word diff, using <mode> to delimit changed words
    --word-diff-regex <regex>
                          use <regex> to decide what a word is
    --color-words[=<regex>]
                          equivalent to --word-diff=color --word-diff-regex=<regex>
    --[no-]color-moved[=<mode>]
                          moved lines of code are colored differently
    --[no-]color-moved-ws <mode>
                          how white spaces are ignored in --color-moved

Other diff options
    --[no-]relative[=<prefix>]
                          when run from subdir, exclude changes outside and show relative paths
    -a, --[no-]text       treat all files as text
    -R                    swap two inputs, reverse the diff
    --[no-]exit-code      exit with 1 if there were differences, 0 otherwise
    --[no-]quiet          disable all output of the program
    --[no-]ext-diff       allow an external diff helper to be executed
    --[no-]textconv       run external text conversion filters when comparing binary files
    --ignore-submodules[=<when>]
                          ignore changes to submodules in the diff generation
    --submodule[=<format>]
                          specify how differences in submodules are shown
    --ita-invisible-in-index
                          hide 'git add -N' entries from the index
    --ita-visible-in-index
                          treat 'git add -N' entries as real in the index
    -S <string>           look for differences that change the number of occurrences of the specified string
    -G <regex>            look for differences that change the number of occurrences of the specified regex
    --pickaxe-all         show all changes in the changeset with -S or -G
    --pickaxe-regex       treat <string> in -S as extended POSIX regular expression
    -O <file>             control the order in which files appear in the output
    --rotate-to <path>    show the change in the specified path first
    --skip-to <path>      skip the output to the specified path
    --find-object <object-id>
                          look for differences that change the number of occurrences of the specified object
    --diff-filter [(A|C|D|M|R|T|U|X|B)...[*]]
                          select files by diff type
    --output <file>       output to a specific file

fatal: detected dubious ownership in repository at '/home/user/heads'
To add an exception for this directory, call:

	git config --global --add safe.directory /home/user/heads
----------------------------------------------------------------------
!!!!!! BUILD SYSTEM INFO !!!!!!
System CPUS: 12
System Available Memory: 15146 GB
System Load Average: 0.98
----------------------------------------------------------------------
Used **CPUS**: 12
Used **LOADAVG**: 18
Used **AVAILABLE_MEM_GB**: 15146 GB
----------------------------------------------------------------------
**MAKE_JOBS**: -j12 --load-average=18 

Variables available for override (use 'make VAR_NAME=value'):
**CPUS** (default: number of processors, e.g., 'make CPUS=4')
**LOADAVG** (default: 1.5 times CPUS, e.g., 'make LOADAVG=54')
**AVAILABLE_MEM_GB** (default: memory available on the system in GB, e.g., 'make AVAILABLE_MEM_GB=4')
**MEM_PER_JOB_GB** (default: 1GB per job, e.g., 'make MEM_PER_JOB_GB=2')
----------------------------------------------------------------------
!!!!!! Build starts !!!!!!

Problem vanishes if sudo chown root:root ~/heads but that is not a real solution. Investigating.

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 3, 2024

Problem was safedir in docker image (flake.nix) not creating gitconfig under /root dir. Testing fix under #1833

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 3, 2024

Reasoning of discrepancies for prosperity: git commands under Makefile were not able to get git commit/clean state, resulting in differences in produced binaries because version exported and not reused under kernel modules (modules.cpio) and tools/libraries packed (tools.cpio) packed in initrd.cpio.xz, resulting in payload (kernel+initramfs) packed as payload, resulting in coreboot image being different locally/ci (where CI runs docker in privileged mode and check for git was bypassed per how CircleCI runs docker images resulting in different behavior). Details:

  • GIT_HASH := $(shell git rev-parse HEAD) call failed under local docker image usage, resulting in no versioning info reused under Heads buildsystem using Heads hash as version. GIT_HASH exported and reused was null.
  • HEADS_GIT_VERSION := $(shell git describe --abbrev=7 --tags --dirty) call failed under local docker image usage, resulting in no versioning user Heads buildsystem using Heads as version. This variable wass used in rom filename and was also null.

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 3, 2024

@Oessel #1833 fixes the root cause of the issue, but not README.md which seems to need improvement so that users wanting to do local builds that matches hashes of CircleCI can do so arriving to same results.

Can you repro and also provide PR/comment here what is missing under README.md so that it's clearer that CircleCI used versioned docker image is needed to produce reproducible builds that match from CircleCI and local builds? I seem to have failed writing docs that permit others, like you to arrive to same results.

I would love to fix docs seperately, since I would merge #1833 as bugfix now and address docs seperately.
Thanks.

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 3, 2024

Needs README.md fix, reopening issue until other PR fixes README.md with end user reported needed fixes

@tlaurion tlaurion reopened this Nov 3, 2024
@Oessel
Copy link
Author

Oessel commented Nov 3, 2024

I think i accidently caused some trouble here. I applied changes to some files in my local branch, so i usually don't check all hashes, but when i noticed the missing commit-id, i tried to figure out if this is caused by my changes. After some testing, i tried to build with a fresh build-system and fresh cloned source, but i simply forgot to run the right command and then checked the hashes. I caused the wrong hashes with my incorrect testing, so i am really sorry for this! I mixed up this two different problems and opened the issue, you're docs are correctly saying what is needed to build reproductible Roms. I would like to change the title of this issue and close it as completed if you are OK with this, because the dubious ownership is fixed and the different hashes are not an heads-issue

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 3, 2024

@Oessel please replicate with master :)
I pushed a fix that was needed following this issue and pushed docker v0.2.4 used by Circleci.

@Oessel
Copy link
Author

Oessel commented Nov 3, 2024

Now i got proper named output files and the same hashes as CircleCI, but both hashes.txt contain the hash of the bottom.rom twice and no hash of the combined rom.

@Oessel
Copy link
Author

Oessel commented Nov 3, 2024

Not related to this issue, but another thing came to my mind. Of course this is a very specific scenario, but if a high skilled attacker would be able to manipulate my local source files, wouldn't it be possible that this attacker is able to replace the hashes.txt in my build-directory with a downloaded file from CircleCI? Does it make sense to compute the hashes of you local built roms on your own system (e.g. in a disposable VM on qubes) and check them against CircleCI too? But as i said, not related to this issue at all and very paranoid ;)

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 4, 2024

Now i got proper named output files and the same hashes as CircleCI, but both hashes.txt contain the hash of the bottom.rom twice and no hash of the combined rom.

This is seperate, known issue #1534, i'm attemtping to fix with #1834

@tlaurion
Copy link
Collaborator

tlaurion commented Nov 4, 2024

Not related to this issue, but another thing came to my mind. Of course this is a very specific scenario, but if a high skilled attacker would be able to manipulate my local source files, wouldn't it be possible that this attacker is able to replace the hashes.txt in my build-directory with a downloaded file from CircleCI? Does it make sense to compute the hashes of you local built roms on your own system (e.g. in a disposable VM on qubes) and check them against CircleCI too? But as i said, not related to this issue at all and very paranoid ;)

This is why reproducible builds exists, hashes.txt are produced for discrepancy introspection/issue opening and final ROM hash is the only thing that matters at the end.

Your concerns on github/git/TLS seems related to #1794 which as of now i'm not sure what to reply into.

@Oessel Feel free to rename and close this issue.

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.

2 participants