Skip to content

Commit

Permalink
cli: fix nondeterministic policy generation (#1053)
Browse files Browse the repository at this point in the history
  • Loading branch information
elchead authored Dec 5, 2024
1 parent c75797e commit 5b3a3f8
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 24 deletions.
59 changes: 59 additions & 0 deletions e2e/policy/deterministic_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright 2024 Edgeless Systems GmbH
// SPDX-License-Identifier: AGPL-3.0-only

//go:build e2e

package policy

import (
"encoding/json"
"os"
"path"
"testing"

"github.com/edgelesssys/contrast/e2e/internal/contrasttest"
"github.com/edgelesssys/contrast/internal/kuberesource"
"github.com/edgelesssys/contrast/internal/manifest"
"github.com/edgelesssys/contrast/internal/platforms"
"github.com/stretchr/testify/require"
)

func TestDeterminsticPolicyGeneration(t *testing.T) {
require := require.New(t)
platform, err := platforms.FromString(platformStr)
require.NoError(err)
skipUndeploy := true // doesn't matter, because we don't deploy
ct := contrasttest.New(t, imageReplacementsFile, namespaceFile, platform, skipUndeploy)

// create K8s resources
runtimeHandler, err := manifest.RuntimeHandler(platform)
require.NoError(err)
resources := kuberesource.OpenSSL()
coordinatorBundle := kuberesource.CoordinatorBundle() // only required because ct.Generate requires the coordinator hash file to be present
resources = append(resources, coordinatorBundle...)
resources = kuberesource.PatchRuntimeHandlers(resources, runtimeHandler)
unstructuredResources, err := kuberesource.ResourcesToUnstructured(resources)
require.NoError(err)
buf, err := kuberesource.EncodeUnstructured(unstructuredResources)
require.NoError(err)

// generate policy 5 times and check if the policy hash is the same
var expectedPolicies map[manifest.HexString]manifest.PolicyEntry
for i := range 5 {
t.Log("Generate run", i)
require.NoError(os.WriteFile(path.Join(ct.WorkDir, "resources.yml"), buf, 0o644)) // reset file for each run
require.True(t.Run("generate", ct.Generate), "contrast generate needs to succeed for subsequent tests")
manifestBytes, err := os.ReadFile(ct.WorkDir + "/manifest.json")
require.NoError(err)

// verify that policies are deterministic
var m manifest.Manifest
require.NoError(json.Unmarshal(manifestBytes, &m))
if expectedPolicies != nil {
require.Equal(expectedPolicies, m.Policies, "expected deterministic policy generation")
} else {
expectedPolicies = m.Policies // only set policies on the first run
}
}
t.Log("Policies are deterministic")
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ environment-dependent sysctls in the settings file.
src/tools/genpolicy/rules.rego | 19 ++++++++++++++++++-
src/tools/genpolicy/src/containerd.rs | 4 ++++
src/tools/genpolicy/src/pod.rs | 20 ++++++++++++++++++++
src/tools/genpolicy/src/policy.rs | 10 ++++++++++
5 files changed, 62 insertions(+), 1 deletion(-)
src/tools/genpolicy/src/policy.rs | 9 +++++++++
5 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/src/tools/genpolicy/genpolicy-settings.json b/src/tools/genpolicy/genpolicy-settings.json
index fe1625bac119b59ce2094b2220e2a87c486e670a..e50d5e545e3fe42db486771345310d4c2157be2f 100644
Expand Down Expand Up @@ -88,14 +88,14 @@ index 1d95bfe699bb5082f8bbfb2cc4d89c8bde3a08ec..a89b13ed158ad8524e11ffbdad8ccb1c
# and io.kubernetes.cri.sandbox-id" values with other fields.
allow_by_bundle_or_sandbox_id(p_oci, i_oci, p_storages, i_storages) {
diff --git a/src/tools/genpolicy/src/containerd.rs b/src/tools/genpolicy/src/containerd.rs
index 075fced5bfec11b27e529f0b1d2dba5e6271ba82..2922ea0ab54671269c8eedab3890ba35529db05a 100644
index 075fced5bfec11b27e529f0b1d2dba5e6271ba82..df7e4d110802c0a856dd4197acdd1325acdb5060 100644
--- a/src/tools/genpolicy/src/containerd.rs
+++ b/src/tools/genpolicy/src/containerd.rs
@@ -3,6 +3,8 @@
// SPDX-License-Identifier: Apache-2.0
//

+use std::collections::HashMap;
+use std::collections::BTreeMap;
+
use crate::policy;

Expand All @@ -104,20 +104,20 @@ index 075fced5bfec11b27e529f0b1d2dba5e6271ba82..2922ea0ab54671269c8eedab3890ba35
"/proc/sysrq-trigger".to_string(),
],
Devices: vec![],
+ Sysctl: HashMap::new(),
+ Sysctl: BTreeMap::new(),
}
} else {
policy::KataLinux {
@@ -160,6 +163,7 @@ pub fn get_linux(privileged_container: bool) -> policy::KataLinux {
MaskedPaths: vec![],
ReadonlyPaths: vec![],
Devices: vec![],
+ Sysctl: HashMap::new(),
+ Sysctl: BTreeMap::new(),
}
}
}
diff --git a/src/tools/genpolicy/src/pod.rs b/src/tools/genpolicy/src/pod.rs
index 19f8822395ca225961bcf77bc3e5ae25e3c31119..5030144c6364cd929c53d18a24459748c1ce20aa 100644
index 19f8822395ca225961bcf77bc3e5ae25e3c31119..1769ca02e1e159e977d22e216b540e299003e29b 100644
--- a/src/tools/genpolicy/src/pod.rs
+++ b/src/tools/genpolicy/src/pod.rs
@@ -21,6 +21,7 @@ use log::{debug, warn};
Expand Down Expand Up @@ -157,7 +157,7 @@ index 19f8822395ca225961bcf77bc3e5ae25e3c31119..5030144c6364cd929c53d18a24459748
commands
}
+
+ pub fn apply_sysctls(&self, sysctls: &mut HashMap<String, String>) {
+ pub fn apply_sysctls(&self, sysctls: &mut BTreeMap<String, String>) {
+ if let Some(securityContext) = &self.securityContext {
+ if let Some(container_sysctls) = &securityContext.sysctls {
+ sysctls.extend(container_sysctls.iter().map(|el| (el.name.clone(), el.value.clone())));
Expand All @@ -176,18 +176,10 @@ index 19f8822395ca225961bcf77bc3e5ae25e3c31119..5030144c6364cd929c53d18a24459748
..Default::default()
};
diff --git a/src/tools/genpolicy/src/policy.rs b/src/tools/genpolicy/src/policy.rs
index 973643e1f270b589e30e0b2e9235dbfa70df0f20..adbdf97f33c449e905cbf9044a118da4598c69cd 100644
index 973643e1f270b589e30e0b2e9235dbfa70df0f20..c2753c0f89f28638f955903db412407cb6b90ef9 100644
--- a/src/tools/genpolicy/src/policy.rs
+++ b/src/tools/genpolicy/src/policy.rs
@@ -27,6 +27,7 @@ use serde_yaml::Value;
use sha2::{Digest, Sha256};
use std::boxed;
use std::collections::BTreeMap;
+use std::collections::HashMap;
use std::fs::read_to_string;
use std::io::Write;

@@ -180,14 +181,20 @@ pub struct KataLinux {
@@ -180,14 +180,20 @@ pub struct KataLinux {
pub Namespaces: Vec<KataLinuxNamespace>,

/// MaskedPaths masks over the provided paths inside the container.
Expand All @@ -203,12 +195,12 @@ index 973643e1f270b589e30e0b2e9235dbfa70df0f20..adbdf97f33c449e905cbf9044a118da4
pub Devices: Vec<KataLinuxDevice>,
+
+ /// Sysctls contains sysctls to be applied inside the container.
+ #[serde(default, skip_serializing_if = "HashMap::is_empty")]
+ pub Sysctl: HashMap<String, String>,
+ #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
+ pub Sysctl: BTreeMap<String, String>,
}

/// OCI container LinuxNamespace struct. This struct is similar to the LinuxNamespace
@@ -616,6 +623,9 @@ impl AgentPolicy {
@@ -616,6 +622,9 @@ impl AgentPolicy {
linux.Devices.push(default_device.clone())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,10 @@ index 6ddcd18cd1334dfabeadd1b0e7a54c723c7cae4d..44af45437f550877652c33019f42b0b2

allow_storage(p_storages, i_storage, bundle_id, sandbox_id, layer_ids, root_hashes) {
diff --git a/src/tools/genpolicy/src/policy.rs b/src/tools/genpolicy/src/policy.rs
index adbdf97f33c449e905cbf9044a118da4598c69cd..c4dc4ac3c2a10211909ae8ee8d77050add0e5cc1 100644
index c2753c0f89f28638f955903db412407cb6b90ef9..9e69126d9008f361e77086018414abc75a8cc092 100644
--- a/src/tools/genpolicy/src/policy.rs
+++ b/src/tools/genpolicy/src/policy.rs
@@ -270,6 +270,9 @@ pub struct ContainerPolicy {
@@ -269,6 +269,9 @@ pub struct ContainerPolicy {
/// Data compared with req.OCI for CreateContainerRequest calls.
pub OCI: KataSpec,

Expand All @@ -262,7 +262,7 @@ index adbdf97f33c449e905cbf9044a118da4598c69cd..c4dc4ac3c2a10211909ae8ee8d77050a
/// Data compared with req.storages for CreateContainerRequest calls.
storages: Vec<agent::Storage>,

@@ -636,6 +639,7 @@ impl AgentPolicy {
@@ -635,6 +638,7 @@ impl AgentPolicy {
Annotations: annotations,
Linux: linux,
},
Expand Down

0 comments on commit 5b3a3f8

Please sign in to comment.