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.

Sign up for GitHub

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

Jump to bottom

Drop tboot #2

Closed
wants to merge 4 commits into from
Closed

Drop tboot #2

wants to merge 4 commits into from

Conversation

krystian-hebel
Copy link
Member

No description provided.

@BeataZdunczyk BeataZdunczyk requested a review from miczyg1 October 4, 2023 12:30
@krystian-hebel krystian-hebel marked this pull request as ready for review October 4, 2023 13:45
@krystian-hebel @miczyg1
etc/grub.d: modify to use TrenchBoot instead of tboot
9023e93 
Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
@krystian-hebel @miczyg1
anti-evil-maid.spec.in: remove tboot from requirements and triggers
aa0f706 
Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
@krystian-hebel @miczyg1
etc/anti-evil-maid.conf: remove PCR19, change description
f52c598 
Neither TBoot nor TrenchBoot extend PCR19, which resulted in failure
in sanity check.

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
@miczyg1 miczyg1 force-pushed the drop_tboot branch 2 times, most recently from 494d7c3 to f52c598 Compare October 4, 2023 14:37
@miczyg1
Copy link

miczyg1 commented Oct 4, 2023 •
edited
Loading

@SergiiDmytruk @TomaszAIR it seems qubes-dom0-package job does not correctly build AEM package. The resulting RPM does not contain changes from our fork (nor this PR) - checked by extracting the source from archive and by checking the RPM requirements (tboot still present). Do you have some ideas what is causing it?

@krystian-hebel krystian-hebel marked this pull request as draft October 4, 2023 15:15
@krystian-hebel krystian-hebel force-pushed the drop_tboot branch 13 times, most recently from 489812e to 241b2f5 Compare October 4, 2023 16:13
@SergiiDmytruk
Copy link
Member

SergiiDmytruk commented Oct 4, 2023 •
edited
Loading

Not sure yet, trying it out locally (update: build finished without local changes in the package). I would expect build to fail if something went wrong with patches. I do see suspicious git clean that could be dropping them (another update: git clean -X should do this).

@krystian-hebel krystian-hebel force-pushed the drop_tboot branch 4 times, most recently from c17bf5b to 6c33a39 Compare October 4, 2023 16:53
@SergiiDmytruk
Copy link
Member

I don't see patches being applied in the log, like this (from grub):

+ git am --whitespace=nowarn /home/user/qubes-src/grub2/0000-compiler-unused.patch /home/user/qubes-src/grub2/0010-re-write-.gitignore.patch /home/user/qubes-src/grub2/0014-Move-bash-completion-script-922997.patch /home/user/qubes-src/grub2/0015-Allow-fallback-to-include-entries-by-title-not-just-.patch /home/user/qubes-src/grub2/0017-Make-efi-machines-load-an-env-block-from-a-variable.patch /home/user/qubes-src/grub2/0019-Add-fw_path-variable-revised.patch /home/user/qubes-src/grub2/0024-Don-t-print-GNU-GRUB-header.patch /home/user/qubes-src/grub2/0032-Enable-pager-by-default.-985860.patch /home/user/qubes-src/grub2/0034-Don-t-say-GNU-Linux-in-generated-menus.patch /home/user/qubes-src/grub2/0037-Add-.eh_frame-to-list-of-relocations-stripped.patch /home/user/qubes-src/grub2/0038-Don-t-require-a-password-to-boot-entries-generated-b.patch /home/user/qubes-src/grub2/0040-Replace-a-lot-of-man-pages-with-slightly-nicer-ones.patch /home/user/qubes-src/grub2/0043-Generate-OS-and-CLASS-in-10_linux-from-etc-os-releas.patch /home/user/qubes-src/grub2/0047-Make-grub2-mkconfig-construct-titles-that-look-like-.patch /home/user/qubes-src/grub2/0048-Add-friendly-grub2-password-config-tool-985962.patch /home/user/qubes-src/grub2/0051-Add-grub-get-kernel-settings-and-use-it-in-10_linux.patch /home/user/qubes-src/grub2/0053-Make-grub_fatal-also-backtrace.patch /home/user/qubes-src/grub2/0055-Make-our-info-pages-say-grub2-where-appropriate.patch /home/user/qubes-src/grub2/0056-macos-just-build-chainloader-entries-don-t-try-any-x.patch /home/user/qubes-src/grub2/0057-grub2-btrfs-Add-ability-to-boot-from-subvolumes.patch /home/user/qubes-src/grub2/0058-export-btrfs_subvol-and-btrfs_subvolid.patch /home/user/qubes-src/grub2/0059-grub2-btrfs-03-follow_default.patch /home/user/qubes-src/grub2/0060-grub2-btrfs-04-grub2-install.patch /home/user/qubes-src/grub2/0061-grub2-btrfs-05-grub2-mkconfig.patch /home/user/qubes-src/grub2/0062-grub2-btrfs-06-subvol-mount.patch /home/user/qubes-src/grub2/0063-Fallback-to-old-subvol-name-scheme-to-support-old-sn.patch /home/user/qubes-src/grub2/0064-Grub-not-working-correctly-with-btrfs-snapshots-bsc-.patch /home/user/qubes-src/grub2/0071-20_linux_xen-load-xen-or-multiboot-2-modules-as-need.patch /home/user/qubes-src/grub2/0081-Make-it-possible-to-enabled-build-id-sha1.patch /home/user/qubes-src/grub2/0084-Fixup-for-newer-compiler.patch /home/user/qubes-src/grub2/0086-Fixup-for-newer-compiler.patch /home/user/qubes-src/grub2/0097-grub-editenv-Add-incr-command-to-increment-integer-v.patch /home/user/qubes-src/grub2/0098-Add-auto-hide-menu-support.patch /home/user/qubes-src/grub2/0099-Add-grub-set-bootflag-utility.patch /home/user/qubes-src/grub2/0100-docs-Add-grub-boot-indeterminate.service-example.patch /home/user/qubes-src/grub2/0101-gentpl-add-disable-support.patch /home/user/qubes-src/grub2/0102-gentpl-add-pc-firmware-type.patch /home/user/qubes-src/grub2/0105-Make-it-so-we-can-tell-configure-which-cflags-utils-.patch /home/user/qubes-src/grub2/0116-Attempt-to-fix-up-all-the-places-Wsign-compare-error.patch /home/user/qubes-src/grub2/0117-Don-t-use-Wno-sign-compare-Wno-conversion-Wno-error-.patch /home/user/qubes-src/grub2/0122-Fix-getroot.c-s-trampolines.patch /home/user/qubes-src/grub2/0123-Do-not-allow-stack-trampolines-anywhere.patch /home/user/qubes-src/grub2/0124-Reimplement-boot_counter.patch /home/user/qubes-src/grub2/0125-Fix-menu-entry-selection-based-on-ID-and-title.patch /home/user/qubes-src/grub2/0126-Make-the-menu-entry-users-option-argument-to-be-opti.patch /home/user/qubes-src/grub2/0127-Add-efi-export-env-and-efi-load-env-commands.patch /home/user/qubes-src/grub2/0128-Make-it-possible-to-subtract-conditions-from-debug.patch /home/user/qubes-src/grub2/0129-Export-all-variables-from-the-initial-context-when-c.patch /home/user/qubes-src/grub2/0130-grub.d-Split-out-boot-success-reset-from-menu-auto-h.patch /home/user/qubes-src/grub2/0136-Use-git-to-apply-gnulib-patches.patch /home/user/qubes-src/grub2/0138-grub-set-bootflag-Update-comment-about-running-as-ro.patch /home/user/qubes-src/grub2/0139-grub-set-bootflag-Write-new-env-to-tmpfile-and-then-.patch /home/user/qubes-src/grub2/0140-grub.d-Fix-boot_indeterminate-getting-set-on-boot_su.patch /home/user/qubes-src/grub2/0144-bootstrap.conf-Force-autogen.sh-to-use-python3.patch /home/user/qubes-src/grub2/0155-Fix-a-missing-return-in-efi-export-env-and-efi-load-.patch /home/user/qubes-src/grub2/0166-Add-systemd-integration-scripts-to-make-systemctl-re.patch /home/user/qubes-src/grub2/0167-systemd-integration.sh-Also-set-old-menu_show_once-g.patch /home/user/qubes-src/grub2/0168-at_keyboard-use-set-1-when-keyboard-is-in-Translate-.patch /home/user/qubes-src/grub2/0169-grub-install-disable-support-for-EFI-platforms.patch /home/user/qubes-src/grub2/0172-Introduce-function-grub_debug_is_enabled-void-return.patch /home/user/qubes-src/grub2/0173-Don-t-clear-screen-when-debugging-is-enabled.patch /home/user/qubes-src/grub2/0178-Add-at_keyboard_fallback_set-var-to-force-the-set-ma.patch /home/user/qubes-src/grub2/0210-Suppress-gettext-error-message.patch /home/user/qubes-src/grub2/0214-templates-Check-for-EFI-at-runtime-instead-of-config.patch /home/user/qubes-src/grub2/0215-efi-Print-an-error-if-boot-to-firmware-setup-is-not-.patch /home/user/qubes-src/grub2/1000-templates-linux_xen-fix-detecting-xsm-policy.patch /home/user/qubes-src/grub2/1001-Hide-os-prober-disabled-warning.patch /home/user/qubes-src/grub2/1100-i386-msr-Merge-rdmsr.h-and-wrmsr.h-into-msr.h.patch /home/user/qubes-src/grub2/1101-i386-msr-Rename-grub_msr_read-and-grub_msr_write.patch /home/user/qubes-src/grub2/1102-i386-msr-Extract-and-improve-MSR-support-detection-c.patch /home/user/qubes-src/grub2/1103-i386-memory-Rename-PAGE_SHIFT-to-GRUB_PAGE_SHIFT.patch /home/user/qubes-src/grub2/1104-i386-memory-Rename-PAGE_SIZE-to-GRUB_PAGE_SIZE-and-m.patch /home/user/qubes-src/grub2/1105-mmap-Add-grub_mmap_get_lowest-and-grub_mmap_get_high.patch /home/user/qubes-src/grub2/1106-i386-slaunch-Add-basic-platform-support-for-secure-l.patch /home/user/qubes-src/grub2/1107-i386-tpm-Rename-tpm-module-to-tpm_verifier.patch /home/user/qubes-src/grub2/1108-i386-tpm-Add-TPM-TIS-and-CRB-driver.patch /home/user/qubes-src/grub2/1109-i386-txt-Add-Intel-TXT-definitions-header-file.patch /home/user/qubes-src/grub2/1110-i386-txt-Add-Intel-TXT-core-implementation.patch /home/user/qubes-src/grub2/1111-i386-txt-Add-Intel-TXT-ACM-module-support.patch /home/user/qubes-src/grub2/1112-i386-txt-Add-Intel-TXT-verification-routines.patch /home/user/qubes-src/grub2/1113-i386-slaunch-Add-secure-launch-framework-and-command.patch /home/user/qubes-src/grub2/1114-i386-txt-Initialize-TPM-1.2-event-log-in-TXT-heap.patch /home/user/qubes-src/grub2/1115-slaunch-Make-slparams-accessible-by-other-modules.patch /home/user/qubes-src/grub2/1116-multiboot2-Implement-TXT-slaunch-support.patch /home/user/qubes-src/grub2/1117-grub-core-loader-i386-txt-txt.c-don-t-reset-MSR-iter.patch /home/user/qubes-src/grub2/1118-grub-core-loader-i386-txt-txt.c-update-capabilities-.patch /home/user/qubes-src/grub2/1119-Add-GitHub-actions-for-building-QubesOS-packages.patch /home/user/qubes-src/grub2/1120-.github-workflows-build.yml-run-when-a-tag-is-pushed.patch /home/user/qubes-src/grub2/1121-.github-workflows-build.yml-drop-pull_request_target.patch
Applying: Add UNUSED define
...

Something might be missing from spec file.

@krystian-hebel
Copy link
Member Author

This is full repo, not just a set of patches that should be applied on top of another base, so it is built differently.

I'm pursuing some hacker approach, but I think we can also modify parameters set in builder.conf, namely GIT_BASEURL and GIT_PREFIX to point to this repository instead. I don't know how BRANCH works, and if it can be substituted with any of CI's parameters, but it would be much cleaner than what I'm trying to do.

@krystian-hebel krystian-hebel force-pushed the drop_tboot branch 2 times, most recently from fbaf6c0 to 6003e20 Compare October 4, 2023 17:14
@krystian-hebel
Copy link
Member Author

OK, now I have all the Applying: ... lines, but it fails later with a lot of cp: cannot create hard link '/builder/chroot-dom0-fc37//home/user/qubes-src/antievilmaid/README' to 'qubes-src/antievilmaid/README': Operation not permitted errors. No idea where those come from, but // in path seems suspicious.

@SergiiDmytruk
Copy link
Member

Building a package hardlinks components files to rootfs directory. Hardlinking works only within file-system.

I found a way to make it apply patches:

%prep
%patch 0 -p1
...
%patch 5 -p1

@SergiiDmytruk
Copy link
Member

%patch is not a good fit given that patches modify *.spec.in as well. https://github.com/TrenchBoot/qubes-antievilmaid/actions/runs/6412163247 might be a working CI run which just does git am.

@SergiiDmytruk
Copy link
Member

I thought operation not permitted was a result of reorganization but it seems to be caused by git am as I also got it and fixed with chmod. https://github.com/TrenchBoot/qubes-antievilmaid/actions/runs/6419262868 seems to work as expected. Should I send PRs with my changes or maybe we want to somehow change approach given troubles for this component?

By the way, github.job_workflow_sha not working is a bug (actions/runner#2417). Can be replaced with an input variable for convenience.

@krystian-hebel krystian-hebel force-pushed the drop_tboot branch 2 times, most recently from e1778ae to 90d84bb Compare October 6, 2023 08:27
@krystian-hebel
test
4ee1c75 
Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
@krystian-hebel
Copy link
Member Author

Seems I've got it to build with our changes. No patches are created, most annoying thing is that I had to disable checking for commit signatures with INSECURE_SKIP_CHECKING. I would have preferred to use KEYRING_DIR_GIT with GPG key of committer, but it seems that GH doesn't give a way to do so. I also hardcoded branch name for testing, it will have to be passed through variable instead. AFAICT the reference has to be a branch and not a hash, which is good for pull_request but not for push events. More fiddling around will be needed, but it seems that it is possible.

@krystian-hebel
Copy link
Member Author

This approach is unnecessarily complicated, obsoleted by #4. CI wasn't supposed to be part of either this or #4, but review of code already happens on the other PR so I'm closing this one.

@krystian-hebel krystian-hebel deleted the drop_tboot branch January 11, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miczyg1 miczyg1 Awaiting requested review from miczyg1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@krystian-hebel @miczyg1 @SergiiDmytruk