Remove the standard JSON enforcer and the JSON policy parsing in the Rego enforcer (#2539)

* securitypolicyenforcer: Remove the standard JSON enforcer

This commit removes the long deprecated standard JSON enforcer - all
confidential containers now has to either use Rego (the default), or the
open_door enforcer if provided with an empty policy or a policy that is
`{"allow_all": true}` (both case checked against host data).

Note that the host can still choose either rego or open_door, and this is not
measured into host data, but the policy check in createOpenDoorEnforcer ensures
that if the policy is a rego policy, trying to use an open_door enforcer will
error, leaving the enforcer at the default (which for confidential is a
deny-everything ClosedDoorSecurityPolicyEnforcer).

Signed-off-by: Tingmao Wang <[email protected]>

* rego enforcer: Remove support for JSON policies using Rego enforcer

This removes the ability to send in a standard JSON policy to the Rego enforcer.
Previously it would translate it into the equivalent Rego policy (the format is
different from the `containers := [...]` definition in Rego), but we need to
stop supporting this.

Signed-off-by: Tingmao Wang <[email protected]>

* Do not allow {"allow_all": true} as a valid "open_door" policy.

For the open door enforcer to be used, policy must be empty (i.e. the case in
non-confidential containers).

Signed-off-by: Tingmao Wang <[email protected]>

* Remove the custom "JSON policy is not supported." error message

Suggested-by: Ken Gordon <[email protected]>
Signed-off-by: Tingmao Wang <[email protected]>

* Remove unused function and add nolint where it's wrong

Signed-off-by: Tingmao Wang <[email protected]>

* Move substituteUVMPath to test specific go files

This function is now only used by tests, and having it in non _test files
causses unused function lints.

Signed-off-by: Tingmao Wang <[email protected]>

* Fix function name typo in comment

Signed-off-by: Tingmao Wang <[email protected]>

---------

Signed-off-by: Tingmao Wang <[email protected]>

Author: micromaomao

pkg/securitypolicy/rego_utils_test.go

@@ -2218,40 +2218,6 @@ func (*generatedConstraints) Generate(r *rand.Rand, _ int) reflect.Value {
 	return reflect.ValueOf(c)
 }
 
-type testConfig struct {
-	container   *securityPolicyContainer
-	layers      []string
-	containerID string
-	policy      *StandardSecurityPolicyEnforcer
-}
-
-func setupContainerWithOverlay(gc *generatedConstraints, valid bool) (tc *testConfig, err error) {
-	sp := NewStandardSecurityPolicyEnforcer(gc.containers, ignoredEncodedPolicyString)
-
-	containerID := testDataGenerator.uniqueContainerID()
-	c := selectContainerFromContainerList(gc.containers, testRand)
-
-	var layerPaths []string
-	if valid {
-		layerPaths, err = testDataGenerator.createValidOverlayForContainer(sp, c)
-		if err != nil {
-			return nil, fmt.Errorf("error creating valid overlay: %w", err)
-		}
-	} else {
-		layerPaths, err = testDataGenerator.createInvalidOverlayForContainer(sp, c)
-		if err != nil {
-			return nil, fmt.Errorf("error creating invalid overlay: %w", err)
-		}
-	}
-
-	return &testConfig{
-		container:   c,
-		layers:      layerPaths,
-		containerID: containerID,
-		policy:      sp,
-	}, nil
-}
-
 func generateConstraints(r *rand.Rand, maxContainers int32) *generatedConstraints {
 	var containers []*securityPolicyContainer
 

pkg/securitypolicy/regopolicy_linux_test.go

@@ -17,6 +17,8 @@ import (
 	"testing"
 	"testing/quick"
 
+	specInternal "github.com/Microsoft/hcsshim/internal/guest/spec"
+	"github.com/Microsoft/hcsshim/internal/guestpath"
 	rpi "github.com/Microsoft/hcsshim/internal/regopolicyinterpreter"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 )
@@ -6324,3 +6326,15 @@ func testGetUserInfo(t *testing.T, tc getUserInfoTestCase, userStr string, regoE
 		}
 	})
 }
+
+// substituteUVMPath substitutes mount prefix to an appropriate path inside
+// UVM. At policy generation time, it's impossible to tell what the sandboxID
+// will be, so the prefix substitution needs to happen during runtime.
+func substituteUVMPath(sandboxID string, m mountInternal) mountInternal {
+	if strings.HasPrefix(m.Source, guestpath.SandboxMountPrefix) {
+		m.Source = specInternal.SandboxMountSource(sandboxID, m.Source)
+	} else if strings.HasPrefix(m.Source, guestpath.HugePagesMountPrefix) {
+		m.Source = specInternal.HugePagesMountSource(sandboxID, m.Source)
+	}
+	return m
+}

pkg/securitypolicy/regopolicy_windows_test.go

@@ -1329,3 +1329,13 @@ func Test_Rego_DumpStacksPolicy_Off(t *testing.T) {
 		t.Errorf("Test_Rego_DumpStacksPolicy_Off: %v", err)
 	}
 }
+
+// This is a no-op for windows.
+// substituteUVMPath substitutes mount prefix to an appropriate path inside
+// UVM. At policy generation time, it's impossible to tell what the sandboxID
+// will be, so the prefix substitution needs to happen during runtime.
+func substituteUVMPath(sandboxID string, m mountInternal) mountInternal {
+	//no-op for windows
+	_ = sandboxID
+	return m
+}

pkg/securitypolicy/securitypolicy.go

@@ -26,7 +26,7 @@ var apiCodeTemplate string
 var APICode = strings.Replace(apiCodeTemplate, "@@API_VERSION@@", apiVersion, 1)
 var FrameworkCode = strings.Replace(frameworkCodeTemplate, "@@FRAMEWORK_VERSION@@", frameworkVersion, 1)
 
-var ErrInvalidOpenDoorPolicy = errors.New("allow_all cannot be set to 'true' when Containers are non-empty")
+var ErrInvalidOpenDoorPolicy = errors.New("Invalid policy for open-door enforcer")
 
 type EnvVarRule string
 

pkg/securitypolicy/securitypolicy_linux.go

@@ -8,10 +8,8 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
-	"strings"
 
 	specInternal "github.com/Microsoft/hcsshim/internal/guest/spec"
-	"github.com/Microsoft/hcsshim/internal/guestpath"
 	"github.com/moby/sys/user"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
@@ -20,19 +18,6 @@ import (
 //nolint:unused
 const osType = "linux"
 
-// This is being used by StandEnforcer.
-// substituteUVMPath substitutes mount prefix to an appropriate path inside
-// UVM. At policy generation time, it's impossible to tell what the sandboxID
-// will be, so the prefix substitution needs to happen during runtime.
-func substituteUVMPath(sandboxID string, m mountInternal) mountInternal {
-	if strings.HasPrefix(m.Source, guestpath.SandboxMountPrefix) {
-		m.Source = specInternal.SandboxMountSource(sandboxID, m.Source)
-	} else if strings.HasPrefix(m.Source, guestpath.HugePagesMountPrefix) {
-		m.Source = specInternal.HugePagesMountSource(sandboxID, m.Source)
-	}
-	return m
-}
-
 // SandboxMountsDir returns sandbox mounts directory inside UVM/host.
 func SandboxMountsDir(sandboxID string) string {
 	return specInternal.SandboxMountsDir((sandboxID))

pkg/securitypolicy/securitypolicy_windows.go

@@ -8,16 +8,6 @@ import oci "github.com/opencontainers/runtime-spec/specs-go"
 //nolint:unused
 const osType = "windows"
 
-// This is being used by StandEnforcer and is a no-op for windows.
-// substituteUVMPath substitutes mount prefix to an appropriate path inside
-// UVM. At policy generation time, it's impossible to tell what the sandboxID
-// will be, so the prefix substitution needs to happen during runtime.
-func substituteUVMPath(sandboxID string, m mountInternal) mountInternal {
-	//no-op for windows
-	_ = sandboxID
-	return m
-}
-
 // SandboxMountsDir returns sandbox mounts directory inside UVM/host.
 func SandboxMountsDir(sandboxID string) string {
 	return ""