Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: fix nondeterministic policy generation #1053

Merged
merged 3 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
platform, err := platforms.FromString(platformStr)
require := require.New(t)
elchead marked this conversation as resolved.
Show resolved Hide resolved
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()
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