-
Notifications
You must be signed in to change notification settings - Fork 73
elbepack: add support for base images #461
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
base: master
Are you sure you want to change the base?
Changes from all commits
f4e0c61
9e01669
fe13a83
d433270
45795a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -498,6 +498,10 @@ def set_cdrom_mirror(self, abspath): | |
| cdrom = mirror.ensure_child('cdrom') | ||
| cdrom.set_text(abspath) | ||
|
|
||
| def set_base_image(self, base_image_path): | ||
| base_image = self.xml.ensure_child('base-image-in-initvm') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an implementation detail of the initvm which will now leak back to the user.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This follows the cdrom implementation, which similarly sets an 'internal' xml property so that the uploaded cdrom iso file can be found inside the initvm from that property. I'd rather be consistent, and refactor all of it at once.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we add the new XML field, we will have to keep allowing it. Adding clutter to the schema which we already know will be useless soonish anyways is something I'd like to avoid. |
||
| base_image.set_text(base_image_path) | ||
|
|
||
| def dump_elbe_version(self): | ||
| version = self.xml.ensure_child('elbe_version') | ||
| version.set_text(elbe_version) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| import logging | ||
| import os | ||
| import subprocess | ||
| import tarfile | ||
| from urllib.parse import urlsplit | ||
|
|
||
| from elbepack.efilesystem import ChRootFilesystem, dpkg_architecture | ||
|
|
@@ -73,26 +74,46 @@ def __init__(self, xml, path, build_sources=False, | |
| if clean: | ||
| self.rfs.rmtree('') | ||
|
|
||
| self.fresh_debootstrap = False | ||
| self.need_dumpdebootstrap = False | ||
| # TODO think about reinitialization if elbe_version differs | ||
| if not self.rfs.isfile('etc/elbe_version'): | ||
| # avoid starting daemons inside the buildenv | ||
| self.rfs.mkdir_p('usr/sbin') | ||
| # grub-legacy postinst will fail if /boot/grub does not exist | ||
| self.rfs.mkdir_p('boot/grub') | ||
| self.rfs.write_file( | ||
| 'usr/sbin/policy-rc.d', | ||
| 0o755, | ||
| '#!/bin/sh\nexit 101\n') | ||
| self.debootstrap(arch) | ||
| self.fresh_debootstrap = True | ||
| self.need_dumpdebootstrap = True | ||
| else: | ||
| self.fresh_debootstrap = False | ||
| self.need_dumpdebootstrap = False | ||
| use_base_image = self.xml.has('target/debootstrap/type') and \ | ||
| self.xml.text('target/debootstrap/type') == 'base-image' | ||
| if use_base_image: | ||
| if self.xml.has('base-image-in-initvm'): | ||
| self.copy_base_image(self.xml.text('base-image-in-initvm')) | ||
| else: | ||
| raise Exception('XML specifies the use of base image, ' | ||
| 'but none has been provided on the command line ' | ||
| '(use --base-image "path/to/image")') | ||
| else: | ||
| if self.xml.has('base-image-in-initvm'): | ||
| raise Exception('Base image was provided on the command line,' | ||
| ' but XML file specifies the use of debootstap') | ||
| self.prepare_and_run_debootstrap() | ||
|
|
||
| self.initialize_dirs(build_sources=build_sources) | ||
| create_apt_prefs(self.xml, self.rfs) | ||
|
|
||
| def prepare_and_run_debootstrap(self): | ||
| # avoid starting daemons inside the buildenv | ||
| self.rfs.mkdir_p('usr/sbin') | ||
| # grub-legacy postinst will fail if /boot/grub does not exist | ||
| self.rfs.mkdir_p('boot/grub') | ||
| self.rfs.write_file( | ||
| 'usr/sbin/policy-rc.d', | ||
| 0o755, | ||
| '#!/bin/sh\nexit 101\n') | ||
| self.debootstrap(self.arch) | ||
| self.fresh_debootstrap = True | ||
| self.need_dumpdebootstrap = True | ||
|
|
||
| def copy_base_image(self, base_image_file): | ||
| with tarfile.open(base_image_file) as tar: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO this should use At some point we will need some test to make sure that features such as suid bits, absolute links etc work.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can probably go ahead and use the fully trusted filter? https://docs.python.org/3/library/tarfile.html#tarfile.fully_trusted_filter Since this is writing out a full root filesystem, no tar filter will protect against supply chain attacks with a malicious tarball, so we can just drop the restrictions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does initvm guarantee at least python >= 3.12?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alas, no: I'm not sure how the initvm debian version is decided, but the code could be conditional to python version.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Probably, but please test what happens to a file with a leading
We should keep supporting Debian bookworm. This only provides 3.11. As this is a new feature anyways we could restrict its usage to Debian trixie initvms.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the numeric_owner tweak, but not the filter tweak (conditional to python version).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add the forward-compatibility with the filters and Python 3.14, too?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fully trusted filter indeed will write to / if the tarball has absolute paths (the tarballs coming out of elbe do not have that, but such a tarball is easy to make manually with -P): So one could conceivably attack the initvm itself with a malicious tarball. On the other hand, using 'tar' filter is problematic because So I'd suggest that the tarball members are checked before extraction so that none of them start with a /, and if they do, that's a supply chain attack, and the build stops there and then. Otherwise, the fully trusted filter is used if python >= 3.14 and default filter (or no filter) otherwise. Are you ok with it?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for testing. The approach sounds good. I think we can error out (with a custom exception type) if the Python version is too old for filters or if the filter detects an absolute path. That should be the easiest implementation. |
||
| logging.info(f'Extracting {base_image_file} into {self.path}') | ||
| tar.extractall(path=self.path, numeric_owner=True) | ||
|
|
||
| def cdrom_umount(self): | ||
| if self.xml.prj.has('mirror/cdrom'): | ||
| cdrompath = self.rfs.fname('cdrom') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| (experimental) Add support for producing base images and using them to build extended images without relying on debootstrap. |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you trim down these test images to the bare minimum to show and test the feature? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| <!-- | ||
| SPDX-License-Identifier: GPL-3.0-or-later | ||
| SPDX-FileCopyrightText: Linutronix GmbH | ||
| --> | ||
| <ns0:RootFileSystem xmlns:ns0="https://www.linutronix.de/projects/Elbe" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| created="2009-05-20T08:50:56" revision="6" | ||
| xsi:schemaLocation="https://www.linutronix.de/projects/Elbe dbsfed.xsd"> | ||
|
|
||
| <project> | ||
| <name>simple-validation-image</name> | ||
| <version>1.0</version> | ||
| <suite>bookworm</suite> | ||
| <buildtype>amd64</buildtype> | ||
|
|
||
| <description> | ||
| Image used to test ELBE features | ||
| </description> | ||
|
|
||
| <mirror> | ||
| <primary_host>deb.debian.org</primary_host> | ||
| <primary_path>/debian</primary_path> | ||
| <primary_proto>http</primary_proto> | ||
| <primary_key> | ||
| -----BEGIN PGP PUBLIC KEY BLOCK----- | ||
|
|
||
| mDMEY865UxYJKwYBBAHaRw8BAQdAd7Z0srwuhlB6JKFkcf4HU4SSS/xcRfwEQWzr | ||
| crf6AEq0SURlYmlhbiBTdGFibGUgUmVsZWFzZSBLZXkgKDEyL2Jvb2t3b3JtKSA8 | ||
| ZGViaWFuLXJlbGVhc2VAbGlzdHMuZGViaWFuLm9yZz6IlgQTFggAPhYhBE1k/sEZ | ||
| wgKQZ9bnkfjSWFuHg9SBBQJjzrlTAhsDBQkPCZwABQsJCAcCBhUKCQgLAgQWAgMB | ||
| Ah4BAheAAAoJEPjSWFuHg9SBSgwBAP9qpeO5z1s5m4D4z3TcqDo1wez6DNya27QW | ||
| WoG/4oBsAQCEN8Z00DXagPHbwrvsY2t9BCsT+PgnSn9biobwX7bDDg== | ||
| =5NZE | ||
| -----END PGP PUBLIC KEY BLOCK----- | ||
| </primary_key> | ||
| </mirror> | ||
|
|
||
| </project> | ||
|
|
||
| <archivedir>simple-validation-image-archive1</archivedir> | ||
|
|
||
| <target> | ||
| <hostname>validation-image</hostname> | ||
| <package> | ||
| <base-image> | ||
| <name>base-rootfs.tgz</name> | ||
| </base-image> | ||
| </package> | ||
|
|
||
| <pkg-list> | ||
| <pkg>unzip</pkg> | ||
| <pkg>linux-image-amd64</pkg> | ||
| <pkg>grub-pc</pkg> | ||
| </pkg-list> | ||
|
|
||
| </target> | ||
| </ns0:RootFileSystem> |
Uh oh!
There was an error while loading. Please reload this page.