Merge pull request #2416 from openziti/fix-policy-type-change

fix policy type change

CHANGELOG.md

@@ -3,6 +3,56 @@
 ## What's New
 
 * Bug fixes, enhancements and continuing progress on controller HA
+* Data corruption Fix
+
+## Data Corruption Fix
+
+Previous to version 1.1.12, the controller would not handle changes to the policy type of a service policy.
+Specifically if the type was changed from Bind -> Dial, or Dial -> Bind, a set of denormalized data would
+be left behind, leaving the permissions with the old policy type. 
+
+Example:
+
+1. Identity A has Bind access to service B via Bind service policy C. 
+2. The policy type of service policy C is changed from Bind to Dial.
+3. The service list would now likely show that Identity A has Dial and Bind access to service B, instead of
+  just Dial access.
+
+### Mitigation/Fixing Bad Data
+
+If you encounter this problem, the easiest and safest way to solve the problem is to to delete and recreate
+the affected service policy.
+
+If changing policy types is something you do on a regular basis, and can't upgrade to a version with the fix,
+you can work around the issue by deleting and recreating policies, instead of updating them. 
+
+If you're not sure if you have ever changed a policy type, there is a database integrity check tool which can
+ be run which looks for data integrity errors. It is run against a running system. 
+
+Start the check using:
+
+```
+ziti fabric db start-check-integrity
+```
+
+This kicks off the operation in the background. The status of the check can be seen using:
+
+```
+ziti fabric db check-integrity-status 
+```
+
+By default this is a read-only operation. If the read-only run reports errors, it can be run 
+with the `-f` flag, which will have it try to fix errors. The data integrity errors caused
+by this bug should all be fixable by the integrity checker.
+
+```
+ziti fabric db start-check-integrity -f
+```
+
+**WARNINGS**: 
+* Always make a database snapshot before running the integrity checker: `ziti db fabric snapshot <optional path`
+* The integrity checker can be very resource intensive, depending on the size of your data model. 
+  It is recommended that you run the integrity checker when the system is otherwise not busy.
 
 ## Component Updates and Bug Fixes
 
@@ -14,18 +64,30 @@
 * github.com/openziti/edge-api: [v0.26.29 -> v0.26.30](https://github.com/openziti/edge-api/compare/v0.26.29...v0.26.30)
 * github.com/openziti/foundation/v2: [v2.0.48 -> v2.0.49](https://github.com/openziti/foundation/compare/v2.0.48...v2.0.49)
 * github.com/openziti/identity: [v1.0.84 -> v1.0.85](https://github.com/openziti/identity/compare/v1.0.84...v1.0.85)
+* github.com/openziti/jwks: [v1.0.4 -> v1.0.5](https://github.com/openziti/jwks/compare/v1.0.4...v1.0.5)
+    * [Issue #9](https://github.com/openziti/jwks/issues/9) - Using NewKey w/ RSA key results in nil pointer exception
+
 * github.com/openziti/metrics: [v1.2.57 -> v1.2.58](https://github.com/openziti/metrics/compare/v1.2.57...v1.2.58)
 * github.com/openziti/runzmd: [v1.0.50 -> v1.0.51](https://github.com/openziti/runzmd/compare/v1.0.50...v1.0.51)
-* github.com/openziti/sdk-golang: [v0.23.40 -> v0.23.41](https://github.com/openziti/sdk-golang/compare/v0.23.40...v0.23.41)
+* github.com/openziti/sdk-golang: [v0.23.40 -> v0.23.42](https://github.com/openziti/sdk-golang/compare/v0.23.40...v0.23.42)
+    * [Issue #625](https://github.com/openziti/sdk-golang/issues/625) - traffic optimization: implement support for receiving multi-part edge payloads
+
 * github.com/openziti/secretstream: [v0.1.21 -> v0.1.24](https://github.com/openziti/secretstream/compare/v0.1.21...v0.1.24)
-* github.com/openziti/storage: [v0.3.0 -> v0.3.1](https://github.com/openziti/storage/compare/v0.3.0...v0.3.1)
+* github.com/openziti/storage: [v0.3.0 -> v0.3.2](https://github.com/openziti/storage/compare/v0.3.0...v0.3.2)
 * github.com/openziti/transport/v2: [v2.0.143 -> v2.0.146](https://github.com/openziti/transport/compare/v2.0.143...v2.0.146)
     * [Issue #92](https://github.com/openziti/transport/issues/92) - Implement simple traffic shaper
 
+* github.com/openziti/xweb/v2: [v2.1.1 -> v2.1.2](https://github.com/openziti/xweb/compare/v2.1.1...v2.1.2)
 * github.com/openziti-incubator/cf: v0.0.3 (new)
 * github.com/openziti/dilithium: [v0.3.3 -> v0.3.5](https://github.com/openziti/dilithium/compare/v0.3.3...v0.3.5)
 * github.com/openziti/ziti: [v1.1.11 -> v1.1.12](https://github.com/openziti/ziti/compare/v1.1.11...v1.1.12)
+    * [Issue #2415](https://github.com/openziti/ziti/issues/2415) - Fix policy denormalization when service policy type is changed
+    * [Issue #2406](https://github.com/openziti/ziti/issues/2406) - ziti agent controller snapshot-db exit code is always successful
+    * [Issue #2405](https://github.com/openziti/ziti/issues/2405) - Investigate Older SDKs Not Enrolling Not Connecting in HA
+    * [Issue #2403](https://github.com/openziti/ziti/issues/2403) - Fix terminator costing concurrency issue
+    * [Issue #2397](https://github.com/openziti/ziti/issues/2397) - JWKS endpoints w/ new keys do not get refreshed
     * [Issue #2390](https://github.com/openziti/ziti/issues/2390) - Update to github.com/openziti/channel/v3
+    * [Issue #2388](https://github.com/openziti/ziti/issues/2388) - Remove use of ziti fabric add-identity commands in 004-controller-pki.md
 
 # Release 1.1.11
 

controller/db/service_policy_store.go

@@ -158,15 +158,19 @@ func (store *servicePolicyStoreImpl) FillEntity(entity *ServicePolicy, bucket *b
 }
 
 func (store *servicePolicyStoreImpl) PersistEntity(entity *ServicePolicy, ctx *boltz.PersistContext) {
+	policyTypeChanged := false
+
+	currentPolicyType := GetPolicyTypeForId(ctx.Bucket.GetInt32WithDefault(FieldServicePolicyType, PolicyTypeDial.Id()))
 	if ctx.ProceedWithSet(FieldServicePolicyType) {
 		if entity.PolicyType != PolicyTypeBind && entity.PolicyType != PolicyTypeDial {
 			ctx.Bucket.SetError(errorz.NewFieldError("invalid policy type", FieldServicePolicyType, entity.PolicyType))
 			return
 		}
+		policyTypeChanged = entity.PolicyType != currentPolicyType
 	} else {
 		// PolicyType needs to be correct in the entity as we use it later
 		// TODO: Add test for this
-		entity.PolicyType = GetPolicyTypeForId(ctx.Bucket.GetInt32WithDefault(FieldServicePolicyType, PolicyTypeDial.Id()))
+		entity.PolicyType = currentPolicyType
 	}
 
 	if err := validateRolesAndIds(FieldIdentityRoles, entity.IdentityRoles); err != nil {
@@ -196,19 +200,78 @@ func (store *servicePolicyStoreImpl) PersistEntity(entity *ServicePolicy, ctx *b
 	sort.Strings(entity.IdentityRoles)
 	sort.Strings(entity.PostureCheckRoles)
 
-	oldIdentityRoles, valueSet := ctx.GetAndSetStringList(FieldIdentityRoles, entity.IdentityRoles)
-	if valueSet && !stringz.EqualSlices(oldIdentityRoles, entity.IdentityRoles) {
-		servicePolicyStore.identityRolesUpdated(ctx, entity)
-	}
+	if policyTypeChanged && !ctx.IsCreate {
+		// if the policy type has changed, we need to remove all roles for the old policy type and then all the roles
+		// for the new policy type
+
+		updatedFields := ctx.FieldChecker
+		if updatedFields == nil {
+			updatedFields = FieldCheckerF(func(s string) bool {
+				return true
+			})
+		}
+
+		ctx.FieldChecker = FieldCheckerF(func(s string) bool {
+			return s == FieldIdentityRoles || s == FieldServiceRoles || s == FieldPostureCheckRoles || updatedFields.IsUpdated(s)
+		})
+
+		newIdentityRoles := entity.IdentityRoles
+		newServiceRoles := entity.ServiceRoles
+		newPostureCheckRoles := entity.PostureCheckRoles
+
+		entity.IdentityRoles = nil
+		entity.ServiceRoles = nil
+		entity.PostureCheckRoles = nil
+
+		currentIdentityRoles, _ := ctx.GetAndSetStringList(FieldIdentityRoles, entity.IdentityRoles)
+		currentServiceRoles, _ := ctx.GetAndSetStringList(FieldServiceRoles, entity.ServiceRoles)
+		currentPostureCheckRoles, _ := ctx.GetAndSetStringList(FieldPostureCheckRoles, entity.PostureCheckRoles)
+
+		if !updatedFields.IsUpdated(FieldIdentityRoles) {
+			newIdentityRoles = currentIdentityRoles
+		}
+
+		if !updatedFields.IsUpdated(FieldServiceRoles) {
+			newServiceRoles = currentServiceRoles
+		}
+
+		if !updatedFields.IsUpdated(FieldPostureCheckRoles) {
+			newPostureCheckRoles = currentPostureCheckRoles
+		}
+
+		newPolicyType := entity.PolicyType
+		entity.PolicyType = currentPolicyType
 
-	oldServiceRoles, valueSet := ctx.GetAndSetStringList(FieldServiceRoles, entity.ServiceRoles)
-	if valueSet && !stringz.EqualSlices(oldServiceRoles, entity.ServiceRoles) {
+		servicePolicyStore.identityRolesUpdated(ctx, entity)
 		servicePolicyStore.serviceRolesUpdated(ctx, entity)
-	}
+		servicePolicyStore.postureCheckRolesUpdated(ctx, entity)
+
+		entity.PolicyType = newPolicyType
+		entity.IdentityRoles = newIdentityRoles
+		entity.ServiceRoles = newServiceRoles
+		entity.PostureCheckRoles = newPostureCheckRoles
 
-	oldPostureCheckRoles, valueSet := ctx.GetAndSetStringList(FieldPostureCheckRoles, entity.PostureCheckRoles)
-	if valueSet && !stringz.EqualSlices(oldPostureCheckRoles, entity.PostureCheckRoles) {
+		_, _ = ctx.GetAndSetStringList(FieldIdentityRoles, entity.IdentityRoles)
+		_, _ = ctx.GetAndSetStringList(FieldServiceRoles, entity.ServiceRoles)
+
+		servicePolicyStore.identityRolesUpdated(ctx, entity)
+		servicePolicyStore.serviceRolesUpdated(ctx, entity)
 		servicePolicyStore.postureCheckRolesUpdated(ctx, entity)
+	} else {
+		currentIdentityRoles, identityRolesSet := ctx.GetAndSetStringList(FieldIdentityRoles, entity.IdentityRoles)
+		currentServiceRoles, serviceRolesSet := ctx.GetAndSetStringList(FieldServiceRoles, entity.ServiceRoles)
+		currentPostureCheckRoles, postureCheckRolesSet := ctx.GetAndSetStringList(FieldPostureCheckRoles, entity.PostureCheckRoles)
+
+		if identityRolesSet && !stringz.EqualSlices(currentIdentityRoles, entity.IdentityRoles) {
+			servicePolicyStore.identityRolesUpdated(ctx, entity)
+		}
+
+		if serviceRolesSet && !stringz.EqualSlices(currentServiceRoles, entity.ServiceRoles) {
+			servicePolicyStore.serviceRolesUpdated(ctx, entity)
+		}
+		if postureCheckRolesSet && !stringz.EqualSlices(currentPostureCheckRoles, entity.PostureCheckRoles) {
+			servicePolicyStore.postureCheckRolesUpdated(ctx, entity)
+		}
 	}
 }
 
@@ -366,3 +429,9 @@ func (store *servicePolicyStoreImpl) CheckIntegrity(mutateCtx boltz.MutateContex
 
 	return store.BaseStore.CheckIntegrity(mutateCtx, fix, errorSink)
 }
+
+type FieldCheckerF func(string) bool
+
+func (f FieldCheckerF) IsUpdated(s string) bool {
+	return f(s)
+}

controller/db/service_policy_store_test.go

@@ -190,6 +190,7 @@ func (ctx *TestContext) testServicePolicyRoleEvaluation(_ *testing.T) {
 	// add 5 new policies, check
 	// modify polices, add roles, check
 	// modify policies, remove roles, check
+	// change policy types, check
 
 	identityTypeId := ctx.getIdentityTypeId()
 
@@ -341,6 +342,16 @@ func (ctx *TestContext) testServicePolicyRoleEvaluation(_ *testing.T) {
 	}
 
 	ctx.validateServicePolicies(identities, services, policies)
+
+	for _, policy := range policies {
+		if policy.PolicyType == PolicyTypeDial {
+			policy.PolicyType = PolicyTypeBind
+		} else {
+			policy.PolicyType = PolicyTypeDial
+		}
+		boltztest.RequireUpdate(ctx, policy)
+	}
+	ctx.validateServicePolicies(identities, services, policies)
 }
 
 func (ctx *TestContext) createServicePolicies(identityRoles, serviceRoles []string, identities []*Identity, services []*EdgeService, oncreate bool) []*ServicePolicy {

go.mod

@@ -27,7 +27,7 @@ require (
 	github.com/go-openapi/strfmt v0.23.0
 	github.com/go-openapi/swag v0.23.0
 	github.com/go-openapi/validate v0.24.0
-	github.com/go-resty/resty/v2 v2.14.0
+	github.com/go-resty/resty/v2 v2.15.0
 	github.com/golang-jwt/jwt/v5 v5.2.1
 	github.com/google/go-cmp v0.6.0
 	github.com/google/gopacket v1.1.19
@@ -58,9 +58,9 @@ require (
 	github.com/openziti/jwks v1.0.5
 	github.com/openziti/metrics v1.2.58
 	github.com/openziti/runzmd v1.0.51
-	github.com/openziti/sdk-golang v0.23.41
+	github.com/openziti/sdk-golang v0.23.42
 	github.com/openziti/secretstream v0.1.24
-	github.com/openziti/storage v0.3.1
+	github.com/openziti/storage v0.3.2
 	github.com/openziti/transport/v2 v2.0.146
 	github.com/openziti/x509-claims v1.0.3
 	github.com/openziti/xweb/v2 v2.1.2