Repository: osbuild/osbuild
Pull request #1644 (2024-03-06 13:41:13)

sources/curl: add org.osbuild.mtls secrets support
If `org.osbuild.mtls` is passed as a secret name, look for the mtls data in the environment.

---

This is just a proposal, I'm not sure if there's a more elegant way to pass in secrets.

bcl: How is the environment setup? I know this is a draft, so questions may be premature :) But what if they are missing? curl will fail with an error that's hard to debug, so it may be good to check they aren't empty when they are set.
croissanne: > How is the environment setup? I know this is a draft, so questions may be premature :) But what if they are missing? curl will fail with an error that's hard to debug, so it may be good to check they aren't empty when they are set. ETA: Also, needs documentation someplace, probably in the module docstring since there isn't an actual org.osbuild.mtls stage.

I'll add some documentation. The environment should be set up by whoever calls osbuild. We can add some errors of course in case the secret name is specified but no actual data.

Patch:
diff --git a/osbuild/remoteloop.py b/osbuild/remoteloop.py
index 86e9c823..0544e0be 100644
--- a/osbuild/remoteloop.py
+++ b/osbuild/remoteloop.py
@@ -1,5 +1,4 @@
 import contextlib
-import errno
 import os
 
 from . import api, loop
@@ -43,25 +42,7 @@ class LoopServer(api.BaseAPI):
         self.ctl = loop.LoopControl()
 
     def _create_device(self, fd, dir_fd, offset=None, sizelimit=None):
-        while True:
-            # Getting an unbound loopback device and attaching a backing
-            # file descriptor to it is racy, so we must use a retry loop
-            lo = loop.Loop(self.ctl.get_unbound())
-            try:
-                lo.configure(fd, offset=offset, sizelimit=sizelimit, autoclear=True)
-            except BlockingIOError:
-                lo.clear_fd()
-                lo.close()
-                continue
-            except OSError as e:
-                lo.close()
-                # `configure` returns EBUSY when the pages from the previously
-                # bound file have not been fully cleared yet.
-                if e.errno == errno.EBUSY:
-                    continue
-                raise e
-            break
-
+        lo = self.ctl.loop_for_fd(fd, offset=offset, sizelimit=sizelimit, autoclear=True)
         lo.mknod(dir_fd)
         # Pin the Loop objects so they are only released when the LoopServer
         # is destroyed.
diff --git a/sources/org.osbuild.curl b/sources/org.osbuild.curl
index 086c97ba..5967867b 100755
--- a/sources/org.osbuild.curl
+++ b/sources/org.osbuild.curl
@@ -2,13 +2,19 @@
 """
 Source for downloading files from URLs.
 
-The files are indexed by their content hash. Can download files
-that require secrets. The only secret provider currently supported
-is `org.osbuild.rhsm` for downloading Red Hat content that requires
-a subscriptions.
-
-Internally use curl to download the files; the files are cached in
-an internal cache. Multiple parallel connections are used to speed
+The files are indexed by their content hash. It can download files
+that require secrets. The secret providers currently supported are:
+
+- `org.osbuild.rhsm` for downloading Red Hat content that requires
+  a subscriptions.
+- `org.osbuild.mtls` for downloading content that requires client
+  certificats. The paths to the key and cert should be set in the
+  environment in OSBUILD_SOURCES_CURL_SSL_CLIENT_KEY,
+  OSBUILD_SOURCES_CURL_SSL_CLIENT_CERT, and optionally
+  OSBUILD_SOURCES_CURL_SSL_CA_CERT.
+
+It uses curl to download the files; the files are cached in an
+internal cache. Multiple parallel connections are used to speed
 up the download.
 """
 
@@ -107,6 +113,16 @@ class CurlSource(sources.SourceService):
             if self.subscriptions is None:
                 self.subscriptions = Subscriptions.from_host_system()
             url["secrets"] = self.subscriptions.get_secrets(url.get("url"))
+        elif url.get("secrets", {}).get("name") == "org.osbuild.mtls":
+            key = os.getenv("OSBUILD_SOURCES_CURL_SSL_CLIENT_KEY")
+            cert = os.getenv("OSBUILD_SOURCES_CURL_SSL_CLIENT_CERT")
+            if not (key and cert):
+                raise RuntimeError(f"mtls secrets required but key ({key}) or cert ({cert}) not defined")
+            url["secrets"] = {
+                'ssl_ca_cert': os.getenv("OSBUILD_SOURCES_CURL_SSL_CA_CERT"),
+                'ssl_client_cert': cert,
+                'ssl_client_key': key,
+            }
 
         return checksum, url
 


Test patch:
diff --git a/sources/test/test_curl_source.py b/sources/test/test_curl_source.py
new file mode 100644
index 00000000..8ac4c8fd
--- /dev/null
+++ b/sources/test/test_curl_source.py
@@ -0,0 +1,82 @@
+#!/usr/bin/python3
+
+import contextlib
+import os
+import pathlib
+import socket
+import tempfile
+
+import pytest
+
+SOURCES_NAME = "org.osbuild.curl"
+
+
+def test_curl_source_not_exists(sources_module):
+    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    curl_source = sources_module.CurlSource.from_args(["--service-fd", str(sock.fileno())])
+    tmpdir = tempfile.TemporaryDirectory()
+    curl_source.cache = tmpdir.name
+    desc = {
+        "url": "http://localhost:80/a",
+    }
+    checksum = "sha256:1234567890123456789012345678901234567890909b14ffb032aa20fa23d9ad6"
+    assert not curl_source.exists(checksum, desc)
+
+
+def test_curl_source_exists(sources_module):
+    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    curl_source = sources_module.CurlSource.from_args(["--service-fd", str(sock.fileno())])
+    tmpdir = tempfile.TemporaryDirectory()
+    curl_source.cache = tmpdir.name
+    desc = {
+        "url": "http://localhost:80/a",
+    }
+    checksum = "sha256:1234567890123456789012345678901234567890909b14ffb032aa20fa23d9ad6"
+    pathlib.Path(os.path.join(tmpdir.name, checksum)).touch()
+    assert curl_source.exists(checksum, desc)
+
+
+def test_curl_source_transform(sources_module):
+    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    curl_source = sources_module.CurlSource.from_args(["--service-fd", str(sock.fileno())])
+    tmpdir = tempfile.TemporaryDirectory()
+    curl_source.cache = tmpdir.name
+    desc = {
+        "url": "http://localhost:80/a",
+        "secrets": {
+            "name": "org.osbuild.mtls",
+        },
+    }
+
+    with contextlib.ExitStack() as cm:
+        os.environ["OSBUILD_SOURCES_CURL_SSL_CLIENT_KEY"] = "key"
+        os.environ["OSBUILD_SOURCES_CURL_SSL_CLIENT_CERT"] = "cert"
+
+        def cb():
+            del os.environ["OSBUILD_SOURCES_CURL_SSL_CLIENT_KEY"]
+            del os.environ["OSBUILD_SOURCES_CURL_SSL_CLIENT_CERT"]
+        cm.callback(cb)
+        checksum = "sha256:1234567890123456789012345678901234567890909b14ffb032aa20fa23d9ad6"
+        pathlib.Path(os.path.join(tmpdir.name, checksum)).touch()
+        new_desc = curl_source.transform(checksum, desc)
+        assert new_desc[1]["secrets"]["ssl_client_key"] == "key"
+        assert new_desc[1]["secrets"]["ssl_client_cert"] == "cert"
+        assert new_desc[1]["secrets"]["ssl_ca_cert"] is None
+
+
+def test_curl_source_transform_fail(sources_module):
+    sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+    curl_source = sources_module.CurlSource.from_args(["--service-fd", str(sock.fileno())])
+    tmpdir = tempfile.TemporaryDirectory()
+    curl_source.cache = tmpdir.name
+    desc = {
+        "url": "http://localhost:80/a",
+        "secrets": {
+            "name": "org.osbuild.mtls",
+        },
+    }
+    checksum = "sha256:1234567890123456789012345678901234567890909b14ffb032aa20fa23d9ad6"
+    pathlib.Path(os.path.join(tmpdir.name, checksum)).touch()
+    with pytest.raises(RuntimeError) as exc:
+        curl_source.transform(checksum, desc)
+    assert "mtls secrets required" in str(exc)
diff --git a/stages/test/test_systemd_unit_create.py b/stages/test/test_systemd_unit_create.py
index 74e05f48..b0f251e0 100644
--- a/stages/test/test_systemd_unit_create.py
+++ b/stages/test/test_systemd_unit_create.py
@@ -46,9 +46,17 @@ def test_schema_validation(stage_schema, test_data, expected_err):
         testutil.assert_jsonschema_error_contains(res, expected_err, expected_num_errs=1)
 
 
-def test_systemd_unit_create_simple(tmp_path, stage_module):
+@pytest.mark.parametrize("unit_type,unit_path,expected_prefix", [
+    ("system", "usr", "usr/lib/systemd/system"),
+    ("system", "etc", "etc/systemd/system"),
+    ("global", "usr", "usr/lib/systemd/user"),
+    ("global", "etc", "etc/systemd/user"),
+])
+def test_systemd_unit_create(tmp_path, stage_module, unit_type, unit_path, expected_prefix):
     options = {
         "filename": "create-directory.service",
+        "unit-type": unit_type,
+        "unit-path": unit_path,
         "config": {
             "Unit": {
                 "Description": "Create directory",
@@ -75,7 +83,7 @@ def test_systemd_unit_create_simple(tmp_path, stage_module):
             }
         }
     }
-    expected_unit_path = tmp_path / "usr/lib/systemd/system/create-directory.service"
+    expected_unit_path = tmp_path / expected_prefix / "create-directory.service"
     # should the stage create the dir?
     expected_unit_path.parent.mkdir(parents=True)