Repository: osbuild/osbuild
Pull request #966 (2022-02-09 17:40:28)

stages/sfdisk: support changing GPT partition attribute bits
The GPT attribute field is a 64-bit field that contains two subfields. The higher field is interpreted only in the context of the partition ID, while the lower field is common to all partition IDs. sfdisk supports to change this fields, and also a osbuild user might be interested on change these bits. This patchset adds support to change these bits in the sfdisk stage. 

The meaning of these bits (from Wikipedia) :

- 0 Platform required (required by the computer to function properly, OEM partition for example, disk partitioning utilities must preserve the partition as is)
- 1 EFI firmware should ignore the content of the partition and not try to read from it
- 2 Legacy BIOS bootable (equivalent to active flag (typically bit 7 set) at offset +0h in partition entries of the MBR partition table)
- 3–47 Reserved for future use
- 48–63 Defined and used by the individual partition type




Patch:
diff --git a/.github/workflows/create-tag.yml b/.github/workflows/create-tag.yml
new file mode 100644
index 00000000..3ec46bd8
--- /dev/null
+++ b/.github/workflows/create-tag.yml
@@ -0,0 +1,24 @@
+# This action creates a release every second Wednesday
+name: "Create and push release tag"
+
+on:
+  workflow_dispatch:
+  schedule:
+    - cron: "0 13 * * 3"
+
+jobs:
+  tag-and-push:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Even or odd week
+        run: if [ `expr \`date +\%s\` / 86400 \% 2` -eq 0 ]; then echo "WEEK=odd" >> $GITHUB_ENV; else echo "WEEK=even" >> $GITHUB_ENV; fi
+        shell: bash
+
+      - name: Upstream tag
+        uses: osbuild/release-action@create-tag
+        if: ${{ env.WEEK == 'even' || github.event_name != 'schedule' }}
+        with:
+          token: "${{ secrets.SCHUTZBOT_GITHUB_ACCESS_TOKEN }}"
+          username: "imagebuilder-bot"
+          email: "imagebuilder-bots+imagebuilder-bot@redhat.com"
diff --git a/.packit.yaml b/.packit.yaml
index f38576e1..92d79f56 100644
--- a/.packit.yaml
+++ b/.packit.yaml
@@ -13,6 +13,7 @@ copy_upstream_release_description: true
 
 upstream_tag_template: v{version}
 
+srpm_build_deps: []
 actions:
   get-current-version: bash -c "git describe --tags --abbrev=0 | sed 's|v||'"
 
diff --git a/devices/org.osbuild.lvm2.lv b/devices/org.osbuild.lvm2.lv
index bfcaa9e8..af8eedaa 100755
--- a/devices/org.osbuild.lvm2.lv
+++ b/devices/org.osbuild.lvm2.lv
@@ -67,12 +67,12 @@ class LVService(devices.DeviceService):
         ]
         res = subprocess.run(cmd,
                              check=False,
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.STDOUT,
+                             stdout=subprocess.DEVNULL,
+                             stderr=subprocess.PIPE,
                              encoding="UTF-8")
 
         if res.returncode != 0:
-            data = res.stdout.strip()
+            data = res.stderr.strip()
             msg = f"Failed to set LV device ({fullname}) status: {data}"
             raise RuntimeError(msg)
 
@@ -90,7 +90,7 @@ class LVService(devices.DeviceService):
             res = subprocess.run(cmd,
                                  check=False,
                                  stdout=subprocess.PIPE,
-                                 stderr=subprocess.STDOUT,
+                                 stderr=subprocess.PIPE,
                                  encoding="UTF-8")
 
             if res.returncode == 5:
@@ -101,7 +101,7 @@ class LVService(devices.DeviceService):
                 continue
 
             if res.returncode != 0:
-                json.dump({"error": res.stdout.strip()}, sys.stdout)
+                json.dump({"error": res.stderr.strip()}, sys.stdout)
                 return 1
 
             vg_name = res.stdout.strip()
@@ -125,11 +125,11 @@ class LVService(devices.DeviceService):
         res = subprocess.run(cmd,
                              check=False,
                              stdout=subprocess.PIPE,
-                             stderr=subprocess.STDOUT,
+                             stderr=subprocess.PIPE,
                              encoding="UTF-8")
 
         if res.returncode != 0:
-            raise RuntimeError(res.stdout.strip())
+            raise RuntimeError(res.stderr.strip())
 
         data = res.stdout.strip()
         devnum = list(map(int, data.split(";")))
diff --git a/osbuild.spec b/osbuild.spec
index da867b0b..6718940a 100644
--- a/osbuild.spec
+++ b/osbuild.spec
@@ -1,7 +1,7 @@
 %global         forgeurl https://github.com/osbuild/osbuild
 %global         selinuxtype targeted
 
-Version:        52
+Version:        53
 
 %forgemeta
 
diff --git a/osbuild/util/selinux.py b/osbuild/util/selinux.py
index 26ad5995..46c1e6a2 100644
--- a/osbuild/util/selinux.py
+++ b/osbuild/util/selinux.py
@@ -1,10 +1,14 @@
 """SELinux utility functions"""
 
+import errno
 import os
 import subprocess
 
 from typing import Dict, TextIO
 
+# Extended attribute name for SELinux labels
+XATTR_NAME_SELINUX = b"security.selinux"
+
 
 def parse_config(config_file: TextIO):
     """Parse an SELinux configuration file"""
@@ -51,6 +55,30 @@ def setfiles(spec_file: str, root: str, *paths):
 
 def getfilecon(path: str) -> str:
     """Get the security context associated with `path`"""
-    label = os.getxattr(path, b"security.selinux",
+    label = os.getxattr(path, XATTR_NAME_SELINUX,
                         follow_symlinks=False)
     return label.decode().strip('\n\0')
+
+
+def setfilecon(path: str, context: str) -> None:
+    """
+    Set the security context associated with `path`
+
+    Like `setfilecon`(3), but does not attempt to translate
+    the context via `selinux_trans_to_raw_context`.
+    """
+
+    try:
+        os.setxattr(path, XATTR_NAME_SELINUX,
+                    context.encode(),
+                    follow_symlinks=True)
+    except OSError as err:
+        # in case we get a not-supported error, check if
+        # the context we want to set is already set and
+        # ignore the error in that case. This follows the
+        # behavior of `setfilecon(3)`.
+        if err.errno == errno.ENOTSUP:
+            have = getfilecon(path)
+            if have == context:
+                return
+        raise
diff --git a/setup.py b/setup.py
index 3ae3c0b1..a02774f7 100644
--- a/setup.py
+++ b/setup.py
@@ -2,7 +2,7 @@ import setuptools
 
 setuptools.setup(
     name="osbuild",
-    version="52",
+    version="53",
     description="A build system for OS images",
     packages=["osbuild", "osbuild.formats", "osbuild.util"],
     license='Apache-2.0',
diff --git a/sources/org.osbuild.curl b/sources/org.osbuild.curl
index dbc7d3df..d46cd6a0 100755
--- a/sources/org.osbuild.curl
+++ b/sources/org.osbuild.curl
@@ -15,12 +15,10 @@ up the download.
 
 import concurrent.futures
 import itertools
-import math
 import os
 import subprocess
 import sys
 import tempfile
-import time
 
 from osbuild import sources
 
@@ -93,17 +91,13 @@ def fetch(url, checksum, directory):
     with tempfile.TemporaryDirectory(prefix="osbuild-unverified-file-", dir=directory) as tmpdir:
         # some mirrors are sometimes broken. retry manually, because we could be
         # redirected to a different, working, one on retry.
-        start_time = time.monotonic()
         return_code = 0
-        for _ in range(20):
-            elapsed_time = time.monotonic() - start_time
-            if elapsed_time >= 300:
-                continue
+        for _ in range(10):
             curl_command = [
                 "curl",
                 "--silent",
-                "--max-time", f"{int(math.ceil(300 - elapsed_time))}",
-                "--connect-timeout", "60",
+                "--speed-limit", "1000",
+                "--connect-timeout", "30",
                 "--fail",
                 "--location",
                 "--output", checksum,
diff --git a/stages/org.osbuild.firewall b/stages/org.osbuild.firewall
index f2f53056..544373ce 100755
--- a/stages/org.osbuild.firewall
+++ b/stages/org.osbuild.firewall
@@ -84,13 +84,14 @@ def main(tree, options):
 
     # The options below are "lokkit" compatibility options and can not be used
     # with other options.
-    subprocess.run(["chroot",
-                    tree,
-                    "firewall-offline-cmd"] +
-                   list(map(lambda x: f"--port={x}", ports)) +
-                   list(map(lambda x: f"--service={x}", enabled_services)) +
-                   list(map(lambda x: f"--remove-service={x}", disabled_services)),
-                   check=True)
+    if ports or enabled_services or disabled_services:
+        subprocess.run(["chroot",
+                        tree,
+                        "firewall-offline-cmd"] +
+                      list(map(lambda x: f"--port={x}", ports)) +
+                      list(map(lambda x: f"--service={x}", enabled_services)) +
+                      list(map(lambda x: f"--remove-service={x}", disabled_services)),
+                      check=True)
 
     return 0
 
diff --git a/stages/org.osbuild.grub2 b/stages/org.osbuild.grub2
index 140e592b..0b4c11da 100755
--- a/stages/org.osbuild.grub2
+++ b/stages/org.osbuild.grub2
@@ -49,6 +49,9 @@ to be selected as default. The contents of variable needs to match the
 corresponding loader entry, which currently is a combination of the
 machine id and kernel NVRA, like e.g.:
   `ffffffffffffffffffffffffffffffff-5.6.6-300.fc32.x86_64`
+If `saved_entry` is set it is advisable to set `config.default` to
+`saved` so that any re-creation of the grub configuration by the
+user will preserve that functionality.
 
 Support for "greenboot" can be turned on via the `greenboot` option.
 Greenboot is the idea of automatically rolling back bad updates,
@@ -191,6 +194,10 @@ SCHEMA = """
     "type": "object",
     "additionalProperties": false,
     "properties": {
+      "default": {
+        "description": "Default boot entry",
+        "type": "string"
+      },
       "terminal_input": {
          "$ref": "#/definitions/terminal"
       },
@@ -350,11 +357,13 @@ def copy_efi_data(tree, vendor):
                         symlinks=False)
 
 
+# pylint: disable=too-many-instance-attributes
 class GrubConfig:
     def __init__(self, rootfs, bootfs):
         self.rootfs = rootfs
         self.bootfs = bootfs
         self.path = "boot/grub2/grub.cfg"
+        self.default_entry = None
         self.ignition = False
         self.greenboot = False
         self.kernel_opts = ""
@@ -470,6 +479,9 @@ class GrubConfig:
             val = " ".join(self.terminal_output)
             data += f'GRUB_TERMINAL_OUTPUT="{val}"\n'
 
+        if self.default_entry is not None:
+            data += f'GRUB_DEFAULT={self.default_entry}\n'
+
         return data
 
 
@@ -513,6 +525,7 @@ def main(tree, options):
     config.terminal_input = cfg.get("terminal_input")
     config.terminal_output = cfg.get("terminal_output")
     config.timeout = cfg.get("timeout", 0)
+    config.default_entry = cfg.get("default")
 
     # Create the configuration file that determines how grub.cfg is generated.
     if write_defaults:
diff --git a/stages/org.osbuild.rpm b/stages/org.osbuild.rpm
index ca3671fc..22dbb36a 100755
--- a/stages/org.osbuild.rpm
+++ b/stages/org.osbuild.rpm
@@ -5,7 +5,9 @@ Verify, and install RPM packages
 The `exclude.docs` option can be used to tell rpm to not install docs.
 
 `gpgkeys` should be an array of strings containing each GPG key to be used
-to verify the packages.
+to verify the packages. Alternatively, the keys can be imported via files
+located in the tree via `gpgkeys.fromtree`. This is done after the packages
+are installed so it is possible to import keys packaged in rpms.
 
 `packages` is an array of objects representing RPMs. Each RPM is identified by
 its checksums. Specifically, the content hash of the rpm, not the checksums
@@ -65,6 +67,11 @@ SCHEMA = """
     "type": "array",
     "items": { "type": "string" }
   },
+  "gpgkeys.fromtree": {
+    "description": "Array of files in the tree with GPG keys to import",
+    "type": "array",
+    "items": { "type": "string" }
+  },
   "packages": {
     "description": "Array of RPM content hashes",
     "type": "array",
@@ -107,6 +114,11 @@ SCHEMA_2 = """
       "type": "array",
       "items": { "type": "string" }
     },
+    "gpgkeys.fromtree": {
+      "description": "Array of files in the tree with GPG keys to import",
+      "type": "array",
+      "items": { "type": "string" }
+    },
     "disable_dracut": {
       "description": "Prevent dracut from running",
       "type": "boolean"
@@ -278,12 +290,22 @@ def main(tree, inputs, options):
             "--verbose",
             "--root", tree,
             *extra_args,
-            # The content hash of the rpms has been verified, default to not
-            # verifying again (see /usr/lib/rpm/macros for more info)
-            "--define", "_pkgverify_level none",
+            # All digests and signatures of the rpms has been verified,
+            # default to not verifying package or header signatures when
+            # reading.
+            "--nosignature",
             "--install", manifest.name
         ], cwd=pkgpath, check=True)
 
+    for key in options.get("gpgkeys.fromtree", []):
+        path = os.path.join(tree, key.lstrip("/"))
+        subprocess.run([
+            "rpmkeys",
+            "--root", tree,
+            "--import", path
+        ], check=True)
+        print(f"imported gpg keys from '{key}'")
+
     # re-enabled dracut
     if no_dracut:
         enable_dracut(masked_files)
diff --git a/stages/org.osbuild.selinux b/stages/org.osbuild.selinux
index 2ca277e7..b7e0ffea 100755
--- a/stages/org.osbuild.selinux
+++ b/stages/org.osbuild.selinux
@@ -26,6 +26,7 @@ import subprocess
 import sys
 
 import osbuild.api
+from osbuild.util import selinux
 
 
 SCHEMA = """
@@ -60,7 +61,7 @@ def main(tree, options):
 
     for path, label in labels.items():
         fullpath = os.path.join(tree, path.lstrip("/"))
-        subprocess.run(["chcon", "-v", label, fullpath], check=True)
+        selinux.setfilecon(fullpath, label)
 
     if options.get("force_autorelabel", False):
         stamp = pathlib.Path(tree, ".autorelabel")
diff --git a/stages/org.osbuild.sfdisk b/stages/org.osbuild.sfdisk
index 4f541826..568a9e7c 100755
--- a/stages/org.osbuild.sfdisk
+++ b/stages/org.osbuild.sfdisk
@@ -66,6 +66,17 @@ SCHEMA_2 = r"""
           "uuid": {
             "description": "UUID of the partition (GPT)",
             "type": "string"
+          },
+          "attrs": {
+            "description": "Attributes of the partition (GPT)",
+            "type": "array",
+            "maxItems": 64,
+            "uniqueItems": true,
+            "items": {
+              "type": "integer",
+              "minimum": 0,
+              "maximum": 63
+            }
           }
         }
       }
@@ -82,7 +93,8 @@ class Partition:
                  size: int = None,
                  bootable: bool = False,
                  name: str = None,
-                 uuid: str = None):
+                 uuid: str = None,
+                 attrs: int = None):
         self.type = pttype
         self.start = start
         self.size = size
@@ -90,6 +102,7 @@ class Partition:
         self.name = name
         self.uuid = uuid
         self.index = None
+        self.attrs = attrs
 
     @property
     def start_in_bytes(self):
@@ -135,9 +148,13 @@ class PartitionTable:
         command = f"label: {self.label}\nlabel-id: {self.uuid}"
         for partition in self.partitions:
             fields = []
-            for field in ["start", "size", "type", "name", "uuid"]:
+            for field in ["start", "size", "type", "name", "uuid", "attrs"]:
                 value = getattr(partition, field)
                 if value:
+                    if field == "attrs":
+                        # make a list into a comma-separated string
+                        attr_list = [str(element) for element in value]
+                        value = ",".join(attr_list)
                     fields += [f'{field}="{value}"']
             if partition.bootable:
                 fields += ["bootable"]
@@ -169,6 +186,7 @@ class PartitionTable:
             part.size = disk_parts[i]["size"]
             part.type = disk_parts[i].get("type")
             part.name = disk_parts[i].get("name")
+            part.attrs = disk_parts[i].get("attrs")
 
 
 def partition_from_json(js) -> Partition:
@@ -177,7 +195,8 @@ def partition_from_json(js) -> Partition:
                   size=js.get("size"),
                   bootable=js.get("bootable"),
                   name=js.get("name"),
-                  uuid=js.get("uuid"))
+                  uuid=js.get("uuid"),
+                  attrs=js.get("attrs"))
     return p
 
 


Test patch:
diff --git a/test/data/stages/firewall/b.json b/test/data/stages/firewall/b.json
index a95161ab..61f381c6 100644
--- a/test/data/stages/firewall/b.json
+++ b/test/data/stages/firewall/b.json
@@ -475,6 +475,12 @@
           ]
         }
       },
+      {
+        "name": "org.osbuild.firewall",
+        "options": {
+          "default_zone": "trusted"
+        }
+      },
       {
         "name": "org.osbuild.firewall",
         "options": {
@@ -488,8 +494,7 @@
           ],
           "disabled_services": [
             "telnet"
-          ],
-          "default_zone": "trusted"
+          ]
         }
       }
     ]
diff --git a/test/data/stages/firewall/b.mpp.json b/test/data/stages/firewall/b.mpp.json
index 5d2b83fb..9a75c2a4 100644
--- a/test/data/stages/firewall/b.mpp.json
+++ b/test/data/stages/firewall/b.mpp.json
@@ -29,6 +29,12 @@
           }
         }
       },
+      {
+        "name": "org.osbuild.firewall",
+        "options": {
+          "default_zone": "trusted"
+        }
+      },
       {
         "name": "org.osbuild.firewall",
         "options": {
@@ -42,8 +48,7 @@
           ],
           "disabled_services": [
             "telnet"
-          ],
-          "default_zone": "trusted"
+          ]
         }
       }
     ]
diff --git a/test/data/stages/grub2/b.json b/test/data/stages/grub2/b.json
index e0adf3db..8c08cd43 100644
--- a/test/data/stages/grub2/b.json
+++ b/test/data/stages/grub2/b.json
@@ -376,7 +376,9 @@
                 "console"
               ],
               "serial": "serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1"
-            }
+            },
+            "saved_entry": "fff-1.0.0",
+            "write_defaults": true
           }
         }
       ]
diff --git a/test/data/stages/grub2/b.mpp.json b/test/data/stages/grub2/b.mpp.json
index 21c0e76a..0d5bad27 100644
--- a/test/data/stages/grub2/b.mpp.json
+++ b/test/data/stages/grub2/b.mpp.json
@@ -34,7 +34,9 @@
                 "console"
               ],
               "serial": "serial --speed=115200 --unit=0 --word=8 --parity=no --stop=1"
-            }
+            },
+            "saved_entry": "fff-1.0.0",
+            "write_defaults": true
           }
         }
       ]
diff --git a/test/mod/test_util_lvm2.py b/test/mod/test_util_lvm2.py
index 81fd8f34..53a50923 100644
--- a/test/mod/test_util_lvm2.py
+++ b/test/mod/test_util_lvm2.py
@@ -81,13 +81,13 @@ def list_vgs():
     res = subprocess.run(cmd,
                          check=False,
                          stdout=subprocess.PIPE,
-                         stderr=subprocess.STDOUT,
+                         stderr=subprocess.PIPE,
                          encoding="UTF-8")
 
     data = res.stdout.strip()
 
     if res.returncode != 0:
-        msg = f"vgs: {data}"
+        msg = f"vgs: {res.stderr.strip()}"
         raise RuntimeError(msg)
 
     data = json.loads(data)
diff --git a/test/mod/test_util_selinux.py b/test/mod/test_util_selinux.py
index 74023723..c8524aa6 100644
--- a/test/mod/test_util_selinux.py
+++ b/test/mod/test_util_selinux.py
@@ -2,41 +2,59 @@
 # Tests for the 'osbuild.util.selinux' module.
 #
 
+import errno
 import io
-import unittest
+from unittest import mock
 
 from osbuild.util import selinux
 
 
-class TestObjectStore(unittest.TestCase):
-
-    def test_selinux_config(self):
-        f = io.StringIO()
-        cfg = selinux.parse_config(f)
-        self.assertIsNotNone(cfg)
-        policy = selinux.config_get_policy(cfg)
-        self.assertIsNone(policy)
-
-        example_good = """
-        # This file controls the state of SELinux on the system.
-        # SELINUX= can take one of these three values:
-        #     enforcing - SELinux security policy is enforced.
-        #     permissive - SELinux prints warnings instead of enforcing.
-        #     disabled - No SELinux policy is loaded.
-        SELINUX=enforcing
-        # SELINUXTYPE= can take one of these three values:
-        #     targeted - Targeted processes are protected,
-        #     minimum - Modification of targeted policy.
-        #     mls - Multi Level Security protection.
-        SELINUXTYPE=targeted
-        """
-
-        f = io.StringIO(example_good)
-        cfg = selinux.parse_config(f)
-        self.assertIn('SELINUX', cfg)
-        self.assertIn('SELINUXTYPE', cfg)
-        self.assertEqual(cfg['SELINUX'], 'enforcing')
-        self.assertEqual(cfg['SELINUXTYPE'], 'targeted')
-
-        policy = selinux.config_get_policy(cfg)
-        self.assertEqual(policy, 'targeted')
+def test_selinux_config():
+    f = io.StringIO()
+    cfg = selinux.parse_config(f)
+    assert cfg is not None
+    policy = selinux.config_get_policy(cfg)
+    assert policy is None
+
+    example_good = """
+    # This file controls the state of SELinux on the system.
+    # SELINUX= can take one of these three values:
+    #     enforcing - SELinux security policy is enforced.
+    #     permissive - SELinux prints warnings instead of enforcing.
+    #     disabled - No SELinux policy is loaded.
+    SELINUX=enforcing
+    # SELINUXTYPE= can take one of these three values:
+    #     targeted - Targeted processes are protected,
+    #     minimum - Modification of targeted policy.
+    #     mls - Multi Level Security protection.
+    SELINUXTYPE=targeted
+    """
+
+    f = io.StringIO(example_good)
+    cfg = selinux.parse_config(f)
+    assert 'SELINUX' in cfg
+    assert 'SELINUXTYPE' in cfg
+    assert cfg['SELINUX'] == 'enforcing'
+    assert cfg['SELINUXTYPE'] == 'targeted'
+
+    policy = selinux.config_get_policy(cfg)
+    assert policy == 'targeted'
+
+
+def test_setfilecon():
+    with mock.patch("os.setxattr") as setxattr:
+
+        selinux.setfilecon("/path", "context")
+        setxattr.assert_called_once_with("/path", selinux.XATTR_NAME_SELINUX,
+                                         b"context", follow_symlinks=True)
+
+    with mock.patch("os.getxattr") as getxattr:
+        with mock.patch("os.setxattr") as setxattr:
+
+            def raise_error(*_args, **_kwargs):
+                raise OSError(errno.ENOTSUP, "Not supported")
+
+            getxattr.return_value = b"context"
+            setxattr.side_effect = raise_error
+
+            selinux.setfilecon("path", "context")