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

Changed xen submodule to track Qubes Xen #207

Closed
wants to merge 4 commits into from

Conversation

flammit
Copy link
Collaborator

@flammit flammit commented Jun 24, 2017

Closes #159

@flammit
Copy link
Collaborator Author

flammit commented Jun 24, 2017

Compiled on Ubuntu 16.04 and Fedora VM (so didn't test reproducibility), tested okay on X230 hardware.

Prevents subsequent builds from trying to unpack/repatch
@flammit
Copy link
Collaborator Author

flammit commented Jun 25, 2017

Reproducibility checks out on the Fedora 24/25, Ubuntu 16.04, Debian 8/9:

a4f0e775754d960136f8060051b00850fcb63e94bdcc13d11a5391342300ba11  qemu.qubes-xen.rom
effbecd9193732f7559a5cefce7f59178276a7e17e588f90154a0953439b75f6  x230.qubes-xen.rom

Looks like this adds a dependency on gpg (qubes-vmm-xen uses it to verify), so the http://osresearch.net/Building docs should be updated so that fedora dnf install command includes it.

@osresearch
Copy link
Collaborator

Thank you for tracking down the dependencies and making this change!

Is there a reason to rename the submodule since it is being replaced?

@flammit
Copy link
Collaborator Author

flammit commented Jun 26, 2017

Nope not real ones. Initially I had worried about a build cache conflict, but only later realized that it shouldn't a problem since the build directory name comes from the tar.

Updated to revert to the xen module name, here are the updated hashes tested on Fedora 25/Ubuntu 16.04/Debian 9 (xen binaries are the same, probably the config file variable name changes caused the hash change):

89284a5cf26ae205b56134c13f549cd29af068f503d96831a81c1bee3900a795  qemu.qubes-xen.rom
681ff8ab974524d7fb71d6d2b9831f77b9bfa64dd7bc957ee5e0a38fa15d2237  x230.qubes-xen.rom

Let me know if you want me to squash/rebase to remove the commit noise before merge.

@osresearch
Copy link
Collaborator

I'm building and testing now to prepare to merge. Thanks for signing your commits, too!

The build config is copied into the initrd, which will cause the hash change. Do you think we should have a separate configuration file for the runtime config?

@flammit
Copy link
Collaborator Author

flammit commented Jun 26, 2017

It's probably easier for now to have a single place for platform configs, plus it could be useful to know your build config at runtime. I just needed to convince myself I hadn't broken anything with the submodule name change =)

@osresearch
Copy link
Collaborator

osresearch commented Jun 26, 2017

Is your output file named qemu.qubes-xen.rom or did you rename it? I'm getting a different hash with a clean checkout and pull from your fork:

6a9a9f5dfa504c4cc42141e643e56305f72fd110020793bfa8fa6ce73566cb13  qemu.rom
39f6938a71f7f6aad62dda3e1294182ebce74be7aaf31011496025a5c4a9495c  x230.rom
m

@flammit
Copy link
Collaborator Author

flammit commented Jun 26, 2017

I just renamed the roms before hash. I'll build flammit - qubes-xen (7f6f365) again with a clean checkout on a few systems again. Will revert.

@osresearch
Copy link
Collaborator

Building on Fedora matches your hash:

89284a5cf26ae205b56134c13f549cd29af068f503d96831a81c1bee3900a795  qemu.rom

Unpacking the initrd.cpio files it appears that the only difference is the xen.gz file. So we might have a reproducibility issue in the patch.

@flammit
Copy link
Collaborator Author

flammit commented Jun 26, 2017

Darn - just goes to show building on 3 variants isn't enough... I can try to track it down if you let me know what your first build environment looks like (and I will add it to my testing regime).

@osresearch
Copy link
Collaborator

I had difficulty making Xen reproducible the first time; some of the patches were accepted... https://lists.xen.org/archives/html/xen-devel/2016-08/msg01196.html

I'm diffing the binaries and the build/logs/xen.log files to try to see if there is an easy sign of what is different.

@osresearch
Copy link
Collaborator

Ok, looks like there are some differences..

~/build/heads-xen/build/qubes-vmm-xen-4.6.5-28/xen-4.6.5/xen: cat .banner
 __  __            _  _    __    ____  
 \ \/ /___ _ __   | || |  / /_  | ___| 
  \  // _ \ '_ \  | || |_| '_ \ |___ \ 
  /  \  __/ | | | |__   _| (_) | ___) |
 /_/\_\___|_| |_|    |_|(_)___(_)____/ 
                                       

versus

[fedora@fedora-heads-20160410-32gb-nyc3-01 xen]$ cat .banner
 Xen 4.6.5

@flammit
Copy link
Collaborator Author

flammit commented Jun 26, 2017

Wish I could help. I built from clean checkouts on 6 systems Local Ubuntu 16.04, AWS Fedora 24/25, AWS Ubuntu 16.04, AWS Debian 8/9 and get the same hash as I reported above.

Wow that's annoying. I can patch to tell Xen never use the figlet command.

@osresearch
Copy link
Collaborator

Do none of your systems have figlet installed? My Ubuntu laptop (the "official" build system, since it has the yubikey with the signing keys) does. So either we change the Xen .banner build rule to not use figlet, or we add it to the build requirements (along with gpg, as you mentioned above).

After running dnf install figlet on the fedora build host, it matches my laptop:

6a9a9f5dfa504c4cc42141e643e56305f72fd110020793bfa8fa6ce73566cb13  qemu.rom

@flammit
Copy link
Collaborator Author

flammit commented Jun 26, 2017

Yeah none of those machines (even Ubuntu 16.04 laptop) has figlet installed. I think it's probably safer to have it ignore figlet since it will successfully compile whether it is there or not and people might get confused w.r.t. reproducibility. What do you think?

Removed conditional figlet usage for banner generation
@osresearch
Copy link
Collaborator

I'm patching the xen-4.6.5/xen/Makefile to remove the figlet call now, as well as removing the redefinitions of XEN_WHOAMI, etc, since we can specify those variables in the modules/xen file instead.

@flammit
Copy link
Collaborator Author

flammit commented Jun 26, 2017

Apologies but I tested and pushed the xen-4.6.5/xen/Makefile change before your last message. I'll stop touching this branch now.

@osresearch
Copy link
Collaborator

It boots in qemu and on my testing x230, so it's been merged (along with my patch 7e5c9bf ). thanks again for tracking down what was necessary to track the Qubes build -- this is one of the more important security related issues to be fixed.

@osresearch osresearch closed this Jun 26, 2017
@flammit flammit deleted the qubes-xen branch June 26, 2017 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants