agent: update the processes hashmap to use exec_id as primary key #358

Ankita13-code · 2025-05-22T08:16:25Z

Merge Checklist

Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
- Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
Aware about the PR to be merged using "create a merge commit" rather than "squash and merge" (or similar)
The upstream/missing label (or upstream/not-needed) has been set on the PR.

Summary

Test Methodology

Specify --release when BUILD_TYPE was not specified, or when BUILD_TYPE=release. The default "cargo build" behavior is to build in debug mode. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

- post-process to remove box_pointers annotation for generated files - while this unblocks the build, should look for better solution: - request/teach codegen to not add certain linter annotation - update ttrpc-codegen or other dependencies Signed-off-by: Manuel Huber <mahuber@microsoft.com>

- counterpart to upstream a131eec - unblocks build with rust v1.84 Signed-off-by: Manuel Huber <mahuber@microsoft.com>

- rust linter detects a potential future name collision - see rustc lint static 'UNSTABLE_NAME_COLLISIONS' - we can revisit this at a later point Signed-off-by: Manuel Huber <mahuber@microsoft.com>

Use protobuf = "=3.7.1" for both agent and genpolicy, to fix version mismatches with the protobuf crate. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Fix rebase merge in genpolicy-settings.json. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Enable allow_storages after rule.rego rebase merge. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

$ git rev-parse cc-msft-prototypes 328e440 $ git checkout cc-msft-prototypes src/utarfs $ git checkout cc-msft-prototypes src/tarfs $ git checkout cc-msft-prototypes src/overlay $ git checkout cc-msft-prototypes src/tardev-snapshotter $ git checkout cc-msft-prototypes Makefile

Add option to use an IGVM image in the UVM Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

Add configuration file configuration-clh-snp.toml with SNP related settings enabled Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

* Remove unused param from CLH SNP config * Remove extra package definition from cbl-mariner rootfs config

* Enable SNP=on with CH conf-guest enabled and add dummy host_data value * Add IGVM, HostData, Snp to config markdown doc * sanitize clh-snp.toml.in and clh.toml.in * Further changes required for SEV SNP enablement * Remove unnecessary debug output * Update outdated comment in config

Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

* Set PCI segments in all cases * Clean-up

This makes it so that any container has access to /dev/sev-guest out of the box with no privileges required. Since /dev/sev-guest isn't available yet, I've validated this change using /dev/cpu_dma_latency (original chmod 600) by: 1. Verifying that the device is present in the container. 2. Verifying that reading from the device from a container yields the same result as from the VM context. Signed-off-by: Aurélien Bombo <abombo@microsoft.com>

Add image build macro to change partition format for kernel's "dm-mod.create" command, and allow for igvm + image usecase in kata shim Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

This is a workaround for kata-containers#7993. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

* rootfs: delete some of the mariner packages Delete some of the mariner packages from the Guest image, for faster TEE memory measurement. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

The shell is useful for debugging. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Useful for debugging. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Make sure the hash of an incoming Policy matches the value of the SNP Host Data field. The value of Host Data will be validated through Remote Attestation, outside of this patch. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

There are 10 segments in the ACPI tables, and CLH works better when it uses all of them. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

This is needed when enabling dm-verity. `udevd` reads kernel uevents that announce the creation of `/dev/dm-XXX` devices, and then creates devices with the actual names under `/dev/mapper/`. Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

This allows us to avoid repeating paths when they're the same. Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

This replicates Wedson's changes in 0935263 in a way that is aligned with the upstream implementation introduced in kata-containers#7200. NOTE: This will require compiling the runtime with DEFSHAREDFS_CLH_SNP_VIRTIOFS=none.

Upstream now uses the new DEFSTATICRESOURCEMGMT_TEE variable to set static resource management for TEEs so we align on that. It's true by default so we don't have to update our build script for this. NOTE: For non-tee CH, upstream now uses DEFSTATICRESOURCEMGMT_CLH (already in our codebase) instead of DEFSTATICRESOURCEMGMT. It's still false by default so we WILL have to update our build script for this one.

Adjust configuration-clh-snp.toml.in to be more consistent with configuration-qemu-snp.toml.in. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

The layer string is now base64-encoded, so decode it before inspecting the fields. Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

The policy module augments the policy generated with genpolicy by keeping and providing state to each invocation. Therefore, it is not sufficient anymore to test the passing of requests in the genpolicy crate. Since in Rust, integration tests cannot call functions that are not exposed publicly, this commit factors out the policy module of the agent into its own crate and exposes the necessary functions to be consumed by the agent and an integration tests. The integration test itself is implemented in the following commits. Signed-off-by: Leonard Cohnen <lc@edgeless.systems>

The generated rego policies for `CreateContainerRequest` are stateful and that state is handled in the policy crate. We use this policy crate in the genpolicy integration test to be able to test if those state changes are handled correctly without spinning up an agent or even a cluster. This also allows to easily test on a e.g., CreateContainerRequest level instead of relying on changing the yaml that is applied to a cluster. Signed-off-by: Leonard Cohnen <lc@edgeless.systems>

Move PolicyCopyFile request to shared policy crate so we can test it Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Useful for go dependency upgrades, so that we actually commit changes in the vendor folder Signed-off-by: Manuel Huber <mahuber@microsoft.com>

Introduce rule to block routes from source addresses which are the loopback. Block routes added to the lo device. Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>

Introduce rules for UpdateInterfaceRequest and genpolicy tests for them. Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>

Add test cases for basic and legacy requests to create pause container Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Validate sandbox name using a regex. If the YAML specifies metadata.name, use a regex that exact matches. If the YAML specifies metadata.generateName, use a regex that matches the prefix of the generated name. Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Copy the sample yaml files from the msft-main branch after rebasing to upstream. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Fix the scripts after rebase merge. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Re-generate Cargo.lock for protocols, agent, and genpolicy. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

Update samples with the latest policy after rebasing to recent upstream code. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

This keeps the repo changes clean while running update script and removes the need to remove the test settings at the end of the script Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

Replicate some samples from mcr to marinerconfpodstest since they fail to pull sometimes from mcr

src/agent/rustjail/src/container.rs

Sumynwa

We also need to handle

kata-containers/src/agent/src/sandbox.rs

Line 291 in f635a99

.get_mut(&ctr.init_process_pid)

Ankita13-code · 2025-05-22T18:30:55Z

We also need to handle

kata-containers/src/agent/src/sandbox.rs

Line 291 in f635a99

.get_mut(&ctr.init_process_pid)

Handled this case as well!

danmihai1 · 2025-05-22T20:46:51Z

Please explain in the commit message that multiple Execs using a single exec_id can be problematic especially for use cases where the Host is trusted less than the Guest (e.g., CoCo use cases).

danmihai1 · 2025-05-22T23:45:21Z

src/agent/src/rpc.rs

@@ -584,7 +584,7 @@ impl AgentService {
            let p = sandbox.find_container_process(cid.as_str(), eid.as_str())?;

            p.exit_watchers.push(exit_send);
-            pid = p.pid;
+            // pid = p.pid;


Looks like this line should be deleted.

danmihai1 · 2025-05-23T00:31:29Z

src/agent/rustjail/src/container.rs

@@ -1258,7 +1254,7 @@ impl BaseContainer for LinuxContainer {
            let spec = self.config.spec.as_mut().unwrap();
            update_namespaces(&self.logger, spec, p.pid)?;
        }
-        self.processes.insert(p.pid, p);
+        self.processes.insert(p.exec_id.clone(), p);


I believe we need to check at the beginning of start() if the exec_id has been used already and return failure instead of executing if we detect such collision.

Thanks @danmihai1 for pointing this out. Yes, we need to handle collision & return failure.
This was the expectation in previous implementation as well, but it allowed misconfiguration and, hence allowed collision.

Nice catch @danmihai1! Added a check for this scenario at the beginning of start().

This patch changes the container process HashMap to use exec_id as the primary key instead of PID, preventing exec_id collisions that could be exploited in Confidential Computing scenarios where the host is less trusted than the guest. Key changes: - Changed `processes: HashMap<pid_t, Process>` to `HashMap<String, Process>` - Added exec_id collision detection in `start()` method - Updated process lookup operations to use exec_id directly - Simplified `get_process()` with direct HashMap access This prevents multiple exec operations from reusing the same exec_id, which could be problematic in CoCo use cases where process isolation and unique identification are critical for security. Signed-off-by: Ankita Pareek <ankitapareek@microsoft.com>

Ankita13-code · 2025-05-23T11:15:06Z

Please explain in the commit message that multiple Execs using a single exec_id can be problematic especially for use cases where the Host is trusted less than the Guest (e.g., CoCo use cases).

Done!

danmihai1 · 2025-05-23T15:36:01Z

src/agent/src/rpc.rs

@@ -584,8 +584,7 @@ impl AgentService {
            let p = sandbox.find_container_process(cid.as_str(), eid.as_str())?;

            p.exit_watchers.push(exit_send);
-            pid = p.pid;
-
+


Unnecessary empty line.

danmihai1 and others added 30 commits March 26, 2025 17:02

genpolicy: add support for BUILD_TYPE=debug

13d36e0

Specify --release when BUILD_TYPE was not specified, or when BUILD_TYPE=release. The default "cargo build" behavior is to build in debug mode. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

agent: config: Remove supports_seccomp

0f964ea

- counterpart to upstream a131eec - unblocks build with rust v1.84 Signed-off-by: Manuel Huber <mahuber@microsoft.com>

genpolicy: Fix rust v1.84 linter issue

bb21d50

- rust linter detects a potential future name collision - see rustc lint static 'UNSTABLE_NAME_COLLISIONS' - we can revisit this at a later point Signed-off-by: Manuel Huber <mahuber@microsoft.com>

protocols: use protobuf = "=3.7.1"

4819d3a

Use protobuf = "=3.7.1" for both agent and genpolicy, to fix version mismatches with the protobuf crate. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

genpolicy: fix settings file after rebase merge

a40dcd0

Fix rebase merge in genpolicy-settings.json. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

genpolicy: re-enable allow_storages

f1f017d

Enable allow_storages after rule.rego rebase merge. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

runtime: enable configuring IGVM images

51d1eff

Add option to use an IGVM image in the UVM Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

runtime: add SNP related changes for CLH (#33)

d9c229c

Add configuration file configuration-clh-snp.toml with SNP related settings enabled Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

Update clh-snp config and remove extra rootfs package definition (#38)

19cb39b

* Remove unused param from CLH SNP config * Remove extra package definition from cbl-mariner rootfs config

Re-enable seccomp in clh-snp config (#46)

95881e1

Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

Always assign 10 PCI segments and modify IGVM config validation (#92)

76909be

* Set PCI segments in all cases * Clean-up

osbuilder: add support to build enable dm-verity protected images (#55)

f6adb43

Add image build macro to change partition format for kernel's "dm-mod.create" command, and allow for igvm + image usecase in kata shim Signed-off-by: Dallas Delaney <dadelan@microsoft.com>

image: don't insert the DAX header (#56)

6b6c8b4

This is a workaround for kata-containers#7993. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

rootfs: delete some of the mariner packages (#45)

92e27ef

* rootfs: delete some of the mariner packages Delete some of the mariner packages from the Guest image, for faster TEE memory measurement. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

rootfs: add back bash to mariner's rootfs (#47)

62302af

The shell is useful for debugging. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

rootfs: add back coreutils to mariner's image (#48)

762d0de

Useful for debugging. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

rootfs: add back ps to mariner's rootfs (#49)

fa1b140

Useful for debugging. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

runtime: agent: SNP HOST_DATA set-up (#43)

a359e6d

runtime: agent: validate policy contents (#50)

ee1e0ec

Make sure the hash of an incoming Policy matches the value of the SNP Host Data field. The value of Host Data will be validated through Remote Attestation, outside of this patch. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

runtime: use 10 PCI segments for the Guest (#59)

f520afb

There are 10 segments in the ACPI tables, and CLH works better when it uses all of them. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

runtime: add support for layer-src-prefix option

c9f1d5d

This allows us to avoid repeating paths when they're the same. Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

runtime: Allow disabling FS sharing for CLH/SNP

8f1d98f

This replicates Wedson's changes in 0935263 in a way that is aligned with the upstream implementation introduced in kata-containers#7200. NOTE: This will require compiling the runtime with DEFSHAREDFS_CLH_SNP_VIRTIOFS=none.

runtime: configuration-clh-snp.toml.in adjustments

7be7c09

Adjust configuration-clh-snp.toml.in to be more consistent with configuration-qemu-snp.toml.in. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

runtime: decode layer string

b2644a7

The layer string is now base64-encoded, so decode it before inspecting the fields. Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>

3u13r and others added 14 commits March 31, 2025 19:18

policy: enable unit tests for copyfile

15bc059

Move PolicyCopyFile request to shared policy crate so we can test it Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

gitignore: Remove vendor folder

3ec6100

Useful for go dependency upgrades, so that we actually commit changes in the vendor folder Signed-off-by: Manuel Huber <mahuber@microsoft.com>

genpolicy: Introduce UpdateRoutesRequest rules in genpolicy-settings

684ef38

Introduce rule to block routes from source addresses which are the loopback. Block routes added to the lo device. Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>

genpolicy: Introduce UpdateInterfaceRequest rules in genpolicy-settings

03a1b09

Introduce rules for UpdateInterfaceRequest and genpolicy tests for them. Signed-off-by: Cameron Baird <cameronbaird@microsoft.com>

policy: add tests for createContainerRequest

1a99895

Add test cases for basic and legacy requests to create pause container Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

samples: copy samples from msft-main

c378a2d

Copy the sample yaml files from the msft-main branch after rebasing to upstream. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

genpolicy: fix the sample/test scripts

403c5e9

Fix the scripts after rebase merge. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

protocols: re-generate Cargo.lock

6445e01

Re-generate Cargo.lock for protocols, agent, and genpolicy. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

samples: update sample yaml files after rebase

b900faf

Update samples with the latest policy after rebasing to recent upstream code. Signed-off-by: Dan Mihai <dmihai@microsoft.com>

samples: write test settings to /tmp

9d9c386

This keeps the repo changes clean while running update script and removes the need to remove the test settings at the end of the script Signed-off-by: Saul Paredes <saulparedes@microsoft.com>

samples: update samples

7025072

Replicate some samples from mcr to marinerconfpodstest since they fail to pull sometimes from mcr

Sumynwa reviewed May 22, 2025

View reviewed changes

src/agent/rustjail/src/container.rs Outdated Show resolved Hide resolved

Sumynwa reviewed May 22, 2025

View reviewed changes

src/agent/rustjail/src/container.rs Show resolved Hide resolved

Sumynwa reviewed May 22, 2025

View reviewed changes

Ankita13-code force-pushed the ankitapareek/exec-id-agent-fix branch from 497cb9d to 25c2185 Compare May 22, 2025 13:42

danmihai1 approved these changes May 22, 2025

View reviewed changes

danmihai1 requested changes May 23, 2025

View reviewed changes

Ankita13-code force-pushed the ankitapareek/exec-id-agent-fix branch from d1865ca to c317634 Compare May 23, 2025 10:43

danmihai1 approved these changes May 23, 2025

View reviewed changes

Redent0r force-pushed the msft-main branch from 7954edb to 9746966 Compare June 5, 2025 17:21

Redent0r force-pushed the msft-main branch from f4b3c21 to 57499f2 Compare June 17, 2025 17:08

ms-mahuber force-pushed the msft-main branch from 57499f2 to c04bfdc Compare June 25, 2025 22:33

ms-mahuber force-pushed the msft-main branch from c04bfdc to 5586d27 Compare July 25, 2025 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

agent: update the processes hashmap to use exec_id as primary key #358

agent: update the processes hashmap to use exec_id as primary key #358

Uh oh!

Ankita13-code commented May 22, 2025

Uh oh!

Uh oh!

Uh oh!

Sumynwa left a comment

Uh oh!

Ankita13-code commented May 22, 2025

Uh oh!

danmihai1 commented May 22, 2025

Uh oh!

danmihai1 May 22, 2025

Uh oh!

danmihai1 May 23, 2025

Uh oh!

Sumynwa May 23, 2025 •

edited

Loading

Uh oh!

Ankita13-code May 23, 2025

Uh oh!

Ankita13-code commented May 23, 2025

Uh oh!

danmihai1 May 23, 2025

Uh oh!

Uh oh!

agent: update the processes hashmap to use exec_id as primary key #358

Are you sure you want to change the base?

agent: update the processes hashmap to use exec_id as primary key #358

Uh oh!

Conversation

Ankita13-code commented May 22, 2025

Merge Checklist

Summary

Test Methodology

Uh oh!

Uh oh!

Uh oh!

Sumynwa left a comment

Choose a reason for hiding this comment

Uh oh!

Ankita13-code commented May 22, 2025

Uh oh!

danmihai1 commented May 22, 2025

Uh oh!

danmihai1 May 22, 2025

Choose a reason for hiding this comment

Uh oh!

danmihai1 May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Sumynwa May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ankita13-code May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Ankita13-code commented May 23, 2025

Uh oh!

danmihai1 May 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Sumynwa May 23, 2025 •

edited

Loading