Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions elbepack/daemons/soap/esoap.py
Comment thread
t-8ch marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,20 @@ def start_cdrom(self, builddir):
def finish_cdrom(self, builddir):
self.app.pm.set_upload_cdrom(builddir, ValidationMode.NO_CHECK)

@rpc(String, _returns=String)
def start_base_image(self, builddir):
fname = 'uploaded_base_image.img'

with open(os.path.join(builddir, fname), 'w'):
# Now write empty File
pass

return fname

@rpc(String)
def finish_base_image(self, builddir):
self.app.pm.set_upload_base_image(builddir, ValidationMode.NO_CHECK)

@rpc(String, _returns=String)
def start_pdebuild(self, builddir):
fname = 'current_pdebuild.tar.gz'
Expand Down
42 changes: 25 additions & 17 deletions elbepack/efilesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,25 +443,9 @@ def write_fstab(self, xml):
f.close()

def part_target(self, targetdir, grub_version, grub_fw_type):
from elbepack.hdimg import do_hdimg

# create target images and copy the rfs into them
hdimages = do_hdimg(self.xml,
targetdir,
self,
grub_version,
grub_fw_type)

for i in hdimages:
self.images.append(i)
self.image_packers[i] = default_packer

if self.xml.has('target/package/tar'):
targz_name = self.xml.text('target/package/tar/name')
def _make_tarball(options, targz_name):
try:
options = ''
if self.xml.has('target/package/tar/options'):
options = self.xml.text('target/package/tar/options')
cmd = 'tar cfz %(dest)s/%(fname)s -C %(sdir)s %(options)s .'
args = dict(
options=options,
Expand All @@ -476,6 +460,30 @@ def part_target(self, targetdir, grub_version, grub_fw_type):
# error was logged; continue creating cpio image
pass

from elbepack.hdimg import do_hdimg

# create target images and copy the rfs into them
hdimages = do_hdimg(self.xml,
targetdir,
self,
grub_version,
grub_fw_type)

for i in hdimages:
self.images.append(i)
self.image_packers[i] = default_packer

if self.xml.has('target/package/tar'):
targz_name = self.xml.text('target/package/tar/name')
options = ''
if self.xml.has('target/package/tar/options'):
options = self.xml.text('target/package/tar/options')
_make_tarball(options, targz_name)

if self.xml.has('target/package/base-image'):
targz_name = self.xml.text('target/package/base-image/name')
_make_tarball('', targz_name)

if self.xml.has('target/package/cpio'):
oldwd = os.getcwd()
cpio_name = self.xml.text('target/package/cpio/name')
Expand Down
4 changes: 4 additions & 0 deletions elbepack/elbexml.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.
Can we not use 'uploaded_base_image.img' directly and avoid this new XML element?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.
While it will never be used when set by the user, those users who set it anyways for whatever reason will suddenly have their build break if we removed it from the schema again.

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)
Expand Down
18 changes: 13 additions & 5 deletions elbepack/initvmaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,17 @@ def _attach(args):
_initvm_from_args(args).attach()


def _submit_with_repodir_and_dl_result(control, xmlfile, cdrom, args):
def _submit_with_repodir_and_dl_result(control, xmlfile, cdrom, base_image, args):
fname = f'elbe-repodir-{time.time_ns()}.xml'
preprocess_xmlfile = os.path.join(os.path.dirname(xmlfile), fname)
try:
with Repodir(xmlfile, preprocess_xmlfile):
_submit_and_dl_result(control, preprocess_xmlfile, cdrom, args)
_submit_and_dl_result(control, preprocess_xmlfile, cdrom, base_image, args)
except RepodirError as err:
raise with_cli_details(err, 127, 'elbe repodir failed')


def _submit_and_dl_result(control, xmlfile, cdrom, args):
def _submit_and_dl_result(control, xmlfile, cdrom, base_image, args):

with preprocess_file(xmlfile, variants=args.variants, sshport=args.sshport,
soapport=args.soapport) as xmlfile:
Expand All @@ -117,6 +117,11 @@ def _submit_and_dl_result(control, xmlfile, cdrom, args):
control.set_cdrom(prjdir, cdrom)
print('Upload finished')

if base_image is not None:
print('Uploading base image. This might take a while')
control.set_base_image(prjdir, base_image)
print('Upload finished')

control.service.build(prjdir, args.build_bin, args.build_sources, bool(cdrom))

print('Build started, waiting till it finishes')
Expand Down Expand Up @@ -278,6 +283,9 @@ def _add_submit_arguments(f):
f = add_argument('--build-sdk', dest='build_sdk', action='store_true', default=False,
help="Also make 'initvm submit' build an SDK.")(f)

f = add_argument('--base-image', dest='base_image',
help='Use a base image instead of debootstrap (experimental)')(f)

return f


Expand Down Expand Up @@ -368,7 +376,7 @@ def _create(args):
elif cdrom is not None:
xmlfile = tmp.fname('source.xml')

_submit_with_repodir_and_dl_result(initvm.control, xmlfile, cdrom, args)
_submit_with_repodir_and_dl_result(initvm.control, xmlfile, cdrom, args.base_image, args)


@_add_initvm_from_args_arguments
Expand All @@ -393,7 +401,7 @@ def _submit(args):
else:
args.parser.error('Unknown file ending (use either xml or iso)')

_submit_with_repodir_and_dl_result(initvm.control, xmlfile, cdrom, args)
_submit_with_repodir_and_dl_result(initvm.control, xmlfile, cdrom, args.base_image, args)


@add_argument_sshport
Expand Down
11 changes: 11 additions & 0 deletions elbepack/projectmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ def set_upload_cdrom(self, builddir, url_validation):
# Make db reload the xml file
self.db.set_xml(ep.builddir, None)

def set_upload_base_image(self, builddir, url_validation):
ep = self.open_project(builddir, url_validation, allow_busy=False)
ep.xml.set_base_image(
path.join(
ep.builddir,
'uploaded_base_image.img'))
Comment thread
t-8ch marked this conversation as resolved.
ep.sync_xml_to_disk()

# Make db reload the xml file
self.db.set_xml(ep.builddir, None)

def build_project(
self,
builddir,
Expand Down
49 changes: 35 additions & 14 deletions elbepack/rfs.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import logging
import os
import subprocess
import tarfile
from urllib.parse import urlsplit

from elbepack.efilesystem import ChRootFilesystem, dpkg_architecture
Expand Down Expand Up @@ -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:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should use numeric_owner=True.
Also this will break our usecase with Python 3.14 as suid bits will be stripped.

At some point we will need some test to make sure that features such as suid bits, absolute links etc work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does initvm guarantee at least python >= 3.12?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alas, no:

root@elbe-daemon:~# python3
Python 3.11.2 (main, Apr 28 2025, 14:11:48) [GCC 12.2.0] on linux

I'm not sure how the initvm debian version is decided, but the code could be conditional to python version.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably go ahead and use the fully trusted filter?

Probably, but please test what happens to a file with a leading /. It should still up end in the target directory and not the root of the initvm.
Maybe we need a custom filter to strip those leading /.

Does initvm guarantee at least python >= 3.12?

We should keep supporting Debian bookworm. This only provides 3.11.
(And no backport for the filter functionality)

As this is a new feature anyways we could restrict its usage to Debian trixie initvms.
Which will become default with the next release anyways.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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?
This will break with the next Debian upgrade and we'll need to debug this then again.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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):

PermissionError: [Errno 13] Permission denied: '/boot/grub/.background_cache.png'

So one could conceivably attack the initvm itself with a malicious tarball.

On the other hand, using 'tar' filter is problematic because

Clear high mode bits (setuid, setgid, sticky) and group/other write bits ([S_IWGRP](https://docs.python.org/3/library/stat.html#stat.S_IWGRP) | [S_IWOTH](https://docs.python.org/3/library/stat.html#stat.S_IWOTH)).

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?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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')
Expand Down
37 changes: 35 additions & 2 deletions elbepack/schema/dbsfed.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ SPDX-FileCopyrightText: Linutronix GmbH
</documentation>
</annotation>
</element>
<element name="base-image-in-initvm" type="rfs:string" minOccurs="0" maxOccurs="1">
<annotation>
<documentation>
Base image file inside initvm to be used instead of debootstrap as initial root filesystem.
</documentation>
</annotation>
</element>
<element name="src-cdrom" type="rfs:src-cdrom" minOccurs="0" maxOccurs="1">
<annotation>
<documentation>
Expand Down Expand Up @@ -1214,7 +1221,7 @@ SPDX-FileCopyrightText: Linutronix GmbH
<element name="type" type="rfs:bootstrap_type" minOccurs="0" maxOccurs="1">
<annotation>
<documentation>
Bootstrapper type. Either debootstrap (the default) or mmdebstrap.
Bootstrapper type. Either debootstrap (the default) or mmdebstrap or (experimental) base-image.
</documentation>
</annotation>
</element>
Expand All @@ -1225,6 +1232,7 @@ SPDX-FileCopyrightText: Linutronix GmbH
<restriction base="string">
<enumeration value="debootstrap" />
<enumeration value="mmdebstrap" />
<enumeration value="base-image" />
</restriction>
</simpleType>

Expand Down Expand Up @@ -1802,7 +1810,7 @@ SPDX-FileCopyrightText: Linutronix GmbH
<complexType name="package">
<annotation>
<documentation>
list of packages, each contains the hole rootfilesystem
list of packages, each contains the whole rootfilesystem
</documentation>
</annotation>
<all>
Expand All @@ -1813,6 +1821,13 @@ SPDX-FileCopyrightText: Linutronix GmbH
</documentation>
</annotation>
</element>
<element name="base-image" type="rfs:base-image" minOccurs="0">
<annotation>
<documentation>
base image package of the rootfilesystem (experimental)
</documentation>
</annotation>
</element>
<element name="cpio" type="rfs:cpio" minOccurs="0">
<annotation>
<documentation>
Expand Down Expand Up @@ -1856,6 +1871,24 @@ SPDX-FileCopyrightText: Linutronix GmbH
<attributeGroup ref="xml:specialAttrs"/>
</complexType>

<complexType name="base-image">
<annotation>
<documentation>
describes a base image package
</documentation>
</annotation>
<all>
<element name="name" type="rfs:string" minOccurs="0">
<annotation>
<documentation>
filename of the base image package
</documentation>
</annotation>
</element>
</all>
<attributeGroup ref="xml:specialAttrs"/>
</complexType>

<complexType name="cpio">
<annotation>
<documentation>
Expand Down
5 changes: 5 additions & 0 deletions elbepack/soapclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ def set_cdrom(self, builddir, cdrom_file):
self._upload_file(builddir, fname, cdrom_file)
self.service.finish_cdrom(builddir)

def set_base_image(self, builddir, base_image_file):
fname = self.service.start_base_image(builddir)
self._upload_file(builddir, fname, base_image_file)
self.service.finish_base_image(builddir)
Comment thread
t-8ch marked this conversation as resolved.

def get_files(self, builddir, outdir, *, pbuilder_only=False, wildcard=None):
files = self.service.get_files(builddir)

Expand Down
1 change: 1 addition & 0 deletions newsfragments/+base-images.feature.rst
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.
57 changes: 57 additions & 0 deletions tests/simple-validation-image-base.xml
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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?
Right now it is hard to see what is necessary and what just copied.

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>
Loading
Loading