Add a field "enable_annotations" to the runtime configuration that can
be used to whitelist annotations using a list of regular expressions,
which are used to match any part of the base annotation name, i.e. the
part after "io.katacontainers.config.hypervisor."
For example, the following configuraiton will match "virtio_fs_daemon",
"initrd" and "jailer_path", but not "path" nor "firmware":
enable_annotations = [ "virtio.*", "initrd", "_path" ]
The default is an empty list of enabled annotations, which disables
annotations entirely.
If an anontation is rejected, the message is something like:
annotation io.katacontainers.config.hypervisor.virtio_fs_daemon is not enabled
Fixes: #901
Suggested-by: Peng Tao <tao.peng@linux.alibaba.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
When filtering annotations that correspond to paths,
e.g. hypervisor.path, it is better to use a glob syntax than a regexp
syntax, as it is more usual for paths, and prevents classes of matches
that are undesirable in our case, such as matching .. against .*
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
A comment talking about runtime related annotations describes them as
being related to the agent. A similar comment for the agent
annotations is missing.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Add variables to override defaults at build time for the various lists
used to control path annotations.
Fixes: #901
Suggested-by: Fabiano Fidencio <fidencio@redhat.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
This one could theoretically be used to overwrite data on the host.
It seems somewhat less risky than the earlier ones for a number
of reasons, but worth protecting a little anyway.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Add the following text explaining the risk of using regular
expressions in path lists:
Each member of the list can be a regular expression, but prefer names.
Otherwise, please read and understand the following carefully.
SECURITY WARNING: If you use regular expressions, be mindful that
an attacker could craft an annotation that uses .. to escape the paths
you gave. For example, if your regexp is /bin/qemu.* then if there is
a directory named /bin/qemu.d/, then an attacker can pass an annotation
containing /bin/qemu.d/../put-any-binary-name-here and attack your host.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
This also adds annotation for ctlpath which were not present
before. It's better to implement the code consistenly right now to make
sure that we don't end up with a leaky implementation tacked on later.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
The jailer_path annotation can be used to execute arbitrary code on
the host. Add a jailer_path_list configuration entry providing a list
of regular expressions that can be used to filter annotations that
represent valid file names.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
The path_list configuration gives a series of regular expressions that
limit which values are acceptable through annotations in order to
avoid kata launching arbitrary binaries on the host when receiving an
annotation.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
The annotation is provided, so it should be respected.
Furthermore, it is important to implement it with the appropriate
protetions similar to what was done for virtiofsd.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Sending the virtio_fs_daemon annotation can be used to execute
arbitrary code on the host. In order to prevent this, restrict the
values of the annotation to a list provided by the configuration
file.
Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Paths mentioned in the hypervisor configuration can be overriden
using annotations, which is potentially dangerous. For each path,
add a 'List' variant that specifies the list of acceptable values
from annotations.
Bug: https://bugs.launchpad.net/katacontainers.io/+bug/1878234Fixes: #901
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
To update device resource entries from host to guest, we search for
the right entry by host major:minor numbers, then later update it.
However block and character devices exist in separate major:minor
namespaces so we could have one block and one character device with
matching major:minor and thus incorrectly update both with the details
for whichever device is processed second.
Add a check on device type to prevent this.
Port from the Kata 1 Go agent
https://github.com/kata-containers/agent/commit/27ebdc9d2761Fixes: #703
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The agent needs to update device entries in the OCI spec so that it
has the correct major:minor numbers for the guest, which may differ
from the host.
Entries in the main device list are looked up by device path, but
entries in the device resources list are looked up by (host)
major:minor. This is done one device at a time, updating as we go in
update_spec_device_list().
But since the host and guest have different namespaces, one device
might have the same major:minor as a different device on the host. In
that case we could update one resource entry to the correct guest
values, then mistakenly update it again because it now matches a
different host device.
To avoid this, rather than looking up and updating one by one, we make
all the lookups in advance, creating a map from (host) device path to
the indices in the spec where the device and resource entries can be
found.
Port from the Go agent in Kata 1,
https://github.com/kata-containers/agent/commit/d88d46849130Fixes: #703
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The Kata 1 Go agent included a unit test for updateSpecDeviceList, but no
such unit test exists for the Rust agent's equivalent
update_spec_device_list(). Port the Kata1 test to Rust.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
If update_spec_device_list() is given a device that can't be found in the
OCI spec, it currently does nothing, and returns Ok(()). That doesn't
seem like what we'd expect and is not what the Go agent in Kata 1 does.
Change it to return an error in that case, like Kata 1.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
We were assuming base 10 string before, when the block size from sysfs
is actually a hex string. Let's fix that.
Fixes: #908
Signed-off-by: Eric Ernst <eric.g.ernst@gmail.com>
Sometimes `Option.or_or` and `Result.map_err` may be simpler
than match statement. Especially in rpc.rs, there are
many `ctr.get_process` and `sandbox.get_container` which
are using `match`.
Signed-off-by: bin liu <bin@hyper.sh>
There are some uses/codes/struct fields are commented out, and
may not turn into un-comment these codes, so delete these comments.
Signed-off-by: bin liu <bin@hyper.sh>
Use rust `Result`'s `or_else`/`and_then` can write clean codes.
And can avoid early return by check wether the `Result`
is `Ok` or `Err`.
Signed-off-by: bin liu <bin@hyper.sh>
This commit includes:
- update comments that not matched the function name
- file path with doubled slash
Fixes: #922
Signed-off-by: bin liu <bin@hyper.sh>
In function parse_cmdline there are some similar codes, if we want
to add more commandline arguments, the code will grow too long.
Use macro can reduce some codes with the same logic/processing.
Fixes: #914
Signed-off-by: bin liu <bin@hyper.sh>
Attackers might use it to explore other containers in the same pod.
While it is still safe to allow it, we can just close the race window
like runc does.
Fixes: #885
Signed-off-by: Peng Tao <bergwolf@hyper.sh>
On old kernels (like v4.9), kernel applies CLOECEC in wrong order w.r.t.
dumpable task flags. As a result, we might leak guest file descriptor to
containers. This is a former runc CVE-2016-9962 and still applies to
kata agent. Although Kata container is still valid at protecting the
host, we should not leak extra resources to user containers.
This sets the init processes that join and setup the container's
namespaces as non-dumpable before they setns to the container's pid (or
any other ) namespace.
This settings is automatically reset to the default after the Exec in
the container so that it does not change functionality for the
applications that are running inside, just our init processes.
This prevents parent processes, the pid 1 of the container, to ptrace
the init process before it drops caps and other sets LSMs.
The order during the exec syscall is that the process is set back to
dumpable before O_CLOEXEC are processed.
Refs:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=613cc2b6f272c1a8ad33aefa21cad77af23139f7https://github.com/torvalds/linux/blob/v4.9/fs/exec.c#L1290-L1318opencontainers/runc@50a19c6https://nvd.nist.gov/vuln/detail/CVE-2016-9962Fixes: #890
Signed-off-by: Peng Tao <bergwolf@hyper.sh>
virtiofs DAX support is not stable today, there are
a few corner cases to make it default.
Fixes: #862Fixes: #875
Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Assign unused results to _ in order to silence warnings.
This addresses the following warnings:
warning: unused `std::result::Result` that must be used
--> rustjail/src/mount.rs:1182:16
|
1182 | defer!(unistd::chdir(&olddir););
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/mount.rs:1183:9
|
1183 | unistd::chdir(tempdir.path());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
While in regular code, we want to log possible errors, in test code
it's OK to simply ignore the returned value.
Fixes: #750
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
In a number of cases, we have functions that return a Result<...>
and where the possible error case is simply ignored. This is a bit
unhealthy.
Add a `check!` macro that allows us to not ignore error values
that we want to log, while not interrupting the flow by returning
them. This is useful for low-level functions such as `signal::kill` or
`unistd::close` where an error is probably significant, but should not
necessarily interrupt the flow of the program (i.e. using `call()?` is
not the right answer.
The check! macro is then used on low-level calls. This addresses the
following warnings from #750:
This addresses the following warning:
warning: unused `std::result::Result` that must be used
--> /home/ddd/go/src/github.com/kata-containers-2.0/src/agent/rustjail/src/container.rs:903:17
|
903 | signal::kill(Pid::from_raw(p.pid), Some(Signal::SIGKILL));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> /home/ddd/go/src/github.com/kata-containers-2.0/src/agent/rustjail/src/container.rs:916:17
|
916 | signal::kill(Pid::from_raw(child.id() as i32), Some(Signal::SIGKILL));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:340:13
|
340 | write_sync(cwfd, SYNC_FAILED, format!("{:?}", e).as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:554:13
|
554 | / write_sync(
555 | | cwfd,
556 | | SYNC_FAILED,
557 | | format!("setgroups failed: {:?}", e).as_str(),
558 | | );
| |______________^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:340:13
|
340 | write_sync(cwfd, SYNC_FAILED, format!("{:?}", e).as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:340:13
|
340 | write_sync(cwfd, SYNC_FAILED, format!("{:?}", e).as_str());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:554:13
|
554 | / write_sync(
555 | | cwfd,
556 | | SYNC_FAILED,
557 | | format!("setgroups failed: {:?}", e).as_str(),
558 | | );
| |______________^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:626:5
|
626 | unistd::close(cfd_log);
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:627:5
|
627 | unistd::close(crfd);
| ^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:628:5
|
628 | unistd::close(cwfd);
| ^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:770:9
|
770 | fcntl::fcntl(pfd_log, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:799:9
|
799 | fcntl::fcntl(prfd, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:800:9
|
800 | fcntl::fcntl(pwfd, FcntlArg::F_SETFD(FdFlag::FD_CLOEXEC));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:803:13
|
803 | unistd::close(prfd);
| ^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:930:9
|
930 | log_handler.join();
| ^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:803:13
|
803 | unistd::close(prfd);
| ^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:804:13
|
804 | unistd::close(pwfd);
| ^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:842:13
|
842 | sched::setns(old_pid_ns, CloneFlags::CLONE_NEWPID);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/container.rs:843:13
|
843 | unistd::close(old_pid_ns);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
Fixes: #844Fixes: #750
Suggested-by: Tim Zhang <tim@hyper.sh>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Various recently added error-causing calls
This addresses the following warning:
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:93:9
|
93 | cg.add_task(CgroupPid::from(pid as u64));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unused_must_use)]` on by default
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:196:17
|
196 | freezer_controller.thaw();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:199:17
|
199 | freezer_controller.freeze();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:365:9
|
365 | cpuset_controller.set_cpus(&cpu.cpus);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:369:9
|
369 | cpuset_controller.set_mems(&cpu.mems);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:381:13
|
381 | cpu_controller.set_shares(shares);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:385:5
|
385 | cpu_controller.set_cfs_quota_and_period(cpu.quota, cpu.period);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
warning: unused `std::result::Result` that must be used
--> rustjail/src/cgroups/fs/mod.rs:1061:13
|
1061 | cpuset_controller.set_cpus(cpuset_cpus);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: this `Result` may be an `Err` variant, which should be handled
The specific case of cpu_controller.set_cfs_quota_and_period is
addressed in a way that changes the logic following a suggestion by
Liu Bin, who had just added the code.
Fixes: #750
Suggested-by: Liu Bin <bin@hyper.sh>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
When we are writing to the logs and there is an error doing so, there
is not much we can do. Chances are that a panic would make things
worse. So let it go through.
warning: unused `std::result::Result` that must be used
--> rustjail/src/sync.rs:26:9
|
26 | write_count(lfd, log_str.as_bytes(), log_str.len());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
::: rustjail/src/container.rs:339:13
|
339 | log_child!(cfd_log, "child exit: {:?}", e);
| ------------------------------------------- in this macro invocation
|
= note: this `Result` may be an `Err` variant, which should be handled
= note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
Fixes: #750
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Some functions have undefined behavior and are not actually used.
This addresses the following warning:
warning: the type `oci::User` does not permit zero-initialization
--> rustjail/src/lib.rs:99:18
|
99 | unsafe { MaybeUninit::zeroed().assume_init() }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
|
= note: `#[warn(invalid_value)]` on by default
note: `std::ptr::Unique<u32>` must be non-null (in this struct field)
warning: the type `protocols::oci::Process` does not permit zero-initialization
--> rustjail/src/lib.rs:146:14
|
146 | unsafe { MaybeUninit::zeroed().assume_init() }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| this code causes undefined behavior when executed
| help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
|
note: `std::ptr::Unique<std::string::String>` must be non-null (in this struct field)
Fixes: #750
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
