Merge branch 'main' into net_iov_16K_fix

DEPRECATED.md

@@ -19,3 +19,5 @@ a future major Firecracker release, in accordance with our
 - \[[#4428](https://github.com/firecracker-microvm/firecracker/pull/4428)\]
   Booting microVMs using MPTable and command line parameters for VirtIO devices.
   The functionality is substituted with ACPI.
+- \[[#2628](https://github.com/firecracker-microvm/firecracker/pull/2628)\] The
+  `--basic` parameter of `seccompiler-bin`.

tests/framework/http_api.py

@@ -3,8 +3,6 @@
 
 """A simple HTTP client for the Firecracker API"""
 
-# pylint:disable=too-few-public-methods
-
 import urllib
 from http import HTTPStatus
 

tests/framework/microvm.py

@@ -247,6 +247,8 @@ def __init__(
         self.mem_size_bytes = None
         self.cpu_template_name = None
 
+        self._connections = []
+
         self._pre_cmd = []
         if numa_node:
             node_str = str(numa_node)
@@ -282,6 +284,10 @@ def kill(self):
         for monitor in self.monitors:
             monitor.stop()
 
+        # Kill all background SSH connections
+        for connection in self._connections:
+            connection.close()
+
         # We start with vhost-user backends,
         # because if we stop Firecracker first, the backend will want
         # to exit as well and this will cause a race condition.
@@ -1007,13 +1013,16 @@ def ssh_iface(self, iface_idx=0):
         """Return a cached SSH connection on a given interface id."""
         guest_ip = list(self.iface.values())[iface_idx]["iface"].guest_ip
         self.ssh_key = Path(self.ssh_key)
-        return net_tools.SSHConnection(
+        connection = net_tools.SSHConnection(
             netns=self.netns.id,
             ssh_key=self.ssh_key,
             user="root",
             host=guest_ip,
+            control_path=Path(self.chroot()) / f"ssh-{iface_idx}.sock",
             on_error=self._dump_debug_information,
         )
+        self._connections.append(connection)
+        return connection
 
     @property
     def ssh(self):

tests/framework/microvm_helpers.py

@@ -242,6 +242,6 @@ def trace_cmd_guest(self, fns, cmd, port=4321):
         print("guest> trace-cmd record")
         host_ip = self.vm.iface["eth0"]["iface"].host_ip
         _guest_ps = self.vm.ssh.run(
-            f"trace-cmd record -N {host_ip}:{port} -p function {" ".join(fns)} {cmd}"
+            f"trace-cmd record -N {host_ip}:{port} -p function {' '.join(fns)} {cmd}"
         )
         return list(Path(".").glob("trace.*.dat"))

tests/framework/properties.py

@@ -2,7 +2,6 @@
 # SPDX-License-Identifier: Apache-2.0
 
 # pylint:disable=broad-except
-# pylint:disable=too-few-public-methods
 
 """
 Metadata we want to attach to tests for further analysis and troubleshooting

tests/framework/state_machine.py

@@ -3,8 +3,6 @@
 """Defines a stream based string matcher and a generic state object."""
 
 
-# Too few public methods (1/2) (too-few-public-methods)
-# pylint: disable=R0903
 class MatchStaticString:
     """Match a static string versus input."""
 

tests/framework/utils.py

@@ -196,7 +196,6 @@ def __del__(self):
             self.proc.kill()
 
 
-# pylint: disable=too-few-public-methods
 class CpuMap:
     """Cpu map from real cpu cores to containers visible cores.
 
@@ -398,6 +397,12 @@ def run_cmd(cmd, check=False, shell=True, cwd=None, timeout=None) -> CommandRetu
         stdout, stderr = proc.communicate(timeout=timeout)
     except subprocess.TimeoutExpired:
         proc.kill()
+
+        # Sometimes stdout/stderr are passed on to children, in which case killing
+        # the parent won't close them and communicate will still hang.
+        proc.stdout.close()
+        proc.stderr.close()
+
         stdout, stderr = proc.communicate()
 
         # Log the message with one call so that multiple statuses

tests/host_tools/network.py

@@ -3,30 +3,35 @@
 """Utilities for test host microVM network setup."""
 
 import ipaddress
+import os
 import random
+import re
+import signal
 import string
-import subprocess
 from dataclasses import dataclass, field
 from pathlib import Path
 
-from tenacity import retry, retry_if_exception_type, stop_after_attempt, wait_fixed
+from tenacity import retry, stop_after_attempt, wait_fixed
 
 from framework import utils
+from framework.utils import Timeout
 
 
 class SSHConnection:
     """
     SSHConnection encapsulates functionality for microVM SSH interaction.
 
-    This class should be instantiated as part of the ssh fixture with the
+    This class should be instantiated as part of the ssh fixture with
     the hostname obtained from the MAC address, the username for logging into
     the image and the path of the ssh key.
 
-    This translates into an SSH connection as follows:
-    ssh -i ssh_key_path username@hostname
+    Establishes a ControlMaster upon construction, which is then re-used
+    for all subsequent SSH interactions.
     """
 
-    def __init__(self, netns, ssh_key: Path, host, user, *, on_error=None):
+    def __init__(
+        self, netns, ssh_key: Path, control_path: Path, host, user, *, on_error=None
+    ):
         """Instantiate a SSH client and connect to a microVM."""
         self.netns = netns
         self.ssh_key = ssh_key
@@ -37,22 +42,13 @@ def __init__(self, netns, ssh_key: Path, host, user, *, on_error=None):
         assert (ssh_key.stat().st_mode & 0o777) == 0o400
         self.host = host
         self.user = user
+        self._control_path = control_path
 
         self._on_error = None
 
         self.options = [
             "-o",
-            "LogLevel=ERROR",
-            "-o",
-            "ConnectTimeout=1",
-            "-o",
-            "StrictHostKeyChecking=no",
-            "-o",
-            "UserKnownHostsFile=/dev/null",
-            "-o",
-            "PreferredAuthentications=publickey",
-            "-i",
-            str(self.ssh_key),
+            f"ControlPath={self._control_path}",
         ]
 
         # _init_connection loops until it can connect to the guest
@@ -96,39 +92,103 @@ def scp_get(self, remote_path, local_path, recursive=False):
         self._scp(self.remote_path(remote_path), local_path, opts)
 
     @retry(
-        retry=retry_if_exception_type(ChildProcessError),
-        wait=wait_fixed(0.5),
+        wait=wait_fixed(1),
         stop=stop_after_attempt(20),
         reraise=True,
     )
     def _init_connection(self):
-        """Create an initial SSH client connection (retry until it works).
+        """Initialize the persistent background connection which will be used
+        to execute all commands sent via this `SSHConnection` object.
 
         Since we're connecting to a microVM we just started, we'll probably
         have to wait for it to boot up and start the SSH server.
         We'll keep trying to execute a remote command that can't fail
         (`/bin/true`), until we get a successful (0) exit code.
         """
-        self.check_output("true", timeout=100, debug=True)
+        assert not self._control_path.exists()
+
+        # Sadly, we cannot get debug output from this command (e.g. `-vvv`),
+        # because passing -vvv causes the daemonized ssh to hold on to stderr,
+        # and inside utils.run_cmd we're using subprocess.communicate, which
+        # only returns once stderr gets closed (which would thus result in an
+        # indefinite hang).
+        establish_cmd = [
+            "ssh",
+            # Only need to pass the ssh key here, as all multiplexed
+            # connections won't have to re-authenticate
+            "-i",
+            str(self.ssh_key),
+            "-o",
+            "StrictHostKeyChecking=no",
+            "-o",
+            "ConnectTimeout=2",
+            # Set up a persistent background connection
+            "-o",
+            "ControlMaster=auto",
+            "-o",
+            "ControlPersist=yes",
+            *self.options,
+            self.user_host,
+            "/usr/bin/true",
+        ]
+
+        # don't set a low timeout here, because otherwise we might get into a race condition
+        # where ssh already forked off the persisted connection daemon, but gets killed here
+        # before exiting itself. In that case, self._control_path will exist, and the retry
+        # will hit the assert at the start of this function.
+        self._exec(establish_cmd, check=True)
 
-    def run(self, cmd_string, timeout=None, *, check=False, debug=False):
+    def _check_liveness(self) -> int:
+        """Checks whether the ControlPersist connection is still alive"""
+        check_cmd = ["ssh", "-O", "check", *self.options, self.user_host]
+
+        _, _, stderr = self._exec(check_cmd, check=True)
+
+        pid_match = re.match(r"Master running \(pid=(\d+)\)", stderr)
+
+        assert pid_match, f"SSH ControlMaster connection not alive anymore: {stderr}"
+
+        return int(pid_match.group(1))
+
+    def close(self):
+        """Closes the ControlPersist connection"""
+        master_pid = self._check_liveness()
+
+        stop_cmd = ["ssh", "-O", "stop", *self.options, self.user_host]
+
+        _, _, stderr = self._exec(stop_cmd, check=True)
+
+        assert "Stop listening request sent" in stderr
+
+        try:
+            with Timeout(5):
+                utils.wait_process_termination(master_pid)
+        except TimeoutError:
+            # for some reason it won't exit, let's force it...
+            # if this also fails, when during teardown we'll get an error about
+            # "found a process with supposedly dead Firecracker's jailer ID"
+            os.kill(master_pid, signal.SIGKILL)
+
+    def run(self, cmd_string, timeout=100, *, check=False, debug=False):
         """
         Execute the command passed as a string in the ssh context.
 
         If `debug` is set, pass `-vvv` to `ssh`. Note that this will clobber stderr.
         """
+        self._check_liveness()
+
         command = ["ssh", *self.options, self.user_host, cmd_string]
 
         if debug:
             command.insert(1, "-vvv")
 
         return self._exec(command, timeout, check=check)
 
-    def check_output(self, cmd_string, timeout=None, *, debug=False):
+    def check_output(self, cmd_string, timeout=100, *, debug=False):
         """Same as `run`, but raises an exception on non-zero return code of remote command"""
         return self.run(cmd_string, timeout, check=True, debug=debug)
 
-    def _exec(self, cmd, timeout=None, check=False):
+    def _exec(self, cmd, timeout=100, check=False):
         """Private function that handles the ssh client invocation."""
         if self.netns is not None:
             cmd = ["ip", "netns", "exec", self.netns] + cmd
@@ -141,27 +201,6 @@ def _exec(self, cmd, timeout=None, check=False):
 
             raise
 
-    # pylint:disable=invalid-name
-    def Popen(
-        self,
-        cmd: str,
-        stdin=subprocess.DEVNULL,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        **kwargs,
-    ) -> subprocess.Popen:
-        """Execute the command in the guest and return a Popen object.
-
-        pop = uvm.ssh.Popen("while true; do echo $(date -Is) $RANDOM; sleep 1; done")
-        pop.stdout.read(16)
-        """
-        cmd = ["ssh", *self.options, self.user_host, cmd]
-        if self.netns is not None:
-            cmd = ["ip", "netns", "exec", self.netns] + cmd
-        return subprocess.Popen(
-            cmd, stdin=stdin, stdout=stdout, stderr=stderr, **kwargs
-        )
-
 
 def mac_from_ip(ip_address):
     """Create a MAC address based on the provided IP.

tests/host_tools/test_syscalls.c

@@ -0,0 +1,77 @@
+// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+// This is used by `test_seccomp_validate.py`
+
+#include <linux/types.h>
+#include <linux/filter.h>
+#include <linux/seccomp.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+
+
+void install_bpf_filter(char *bpf_file) {
+    int fd = open(bpf_file, O_RDONLY);
+    if (fd == -1) {
+        perror("open");
+        exit(EXIT_FAILURE);
+    }
+    struct stat sb;
+    if (fstat(fd, &sb) == -1) {
+        perror("stat");
+        exit(EXIT_FAILURE);
+    }
+    size_t size = sb.st_size;
+    size_t insn_len = size / sizeof(struct sock_filter);
+    struct sock_filter *filterbuf = (struct sock_filter*)malloc(size);
+    if (read(fd, filterbuf, size) == -1) {
+        perror("read");
+        exit(EXIT_FAILURE);
+    }
+
+    /* Install seccomp filter */
+    struct sock_fprog prog = {
+        .len = (unsigned short)(insn_len),
+        .filter = filterbuf,
+    };
+    if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
+        perror("prctl(NO_NEW_PRIVS)");
+        exit(EXIT_FAILURE);
+    }
+    if (prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
+        perror("prctl(SECCOMP)");
+        exit(EXIT_FAILURE);
+    }
+}
+
+
+int main(int argc, char **argv) {
+    /* parse arguments */
+    if (argc < 3) {
+        fprintf(stderr, "Usage: %s BPF_FILE ARG0..\n", argv[0]);
+        exit(EXIT_FAILURE);
+    }
+    char *bpf_file = argv[1];
+    long syscall_id = atoi(argv[2]);
+    long arg0, arg1, arg2, arg3;
+    arg0 = arg1 = arg2 = arg3 = 0;
+    if (argc > 3) arg0 = atoi(argv[3]);
+    if (argc > 4) arg1 = atoi(argv[4]);
+    if (argc > 5) arg2 = atoi(argv[5]);
+    if (argc > 6) arg3 = atoi(argv[6]);
+
+    /* read seccomp filter from file */
+    if (strcmp(bpf_file, "/dev/null") != 0) {
+        install_bpf_filter(bpf_file);
+    }
+
+    long res = syscall(syscall_id, arg0, arg1, arg2, arg3);
+    printf("%ld\n", res);
+    return EXIT_SUCCESS;
+}