diff --git a/agent/app/dto/firewall.go b/agent/app/dto/firewall.go index 5c46a9ceb..65d645fec 100644 --- a/agent/app/dto/firewall.go +++ b/agent/app/dto/firewall.go @@ -229,8 +229,9 @@ type DockerPortGuardOperation struct { } type FirewallRuleCheckItem struct { - UUID string `json:"uuid" validate:"omitempty,max=64"` - Rule filter.FirewallRule `json:"rule" validate:"required"` + AdoptLocator *filter.Locator `json:"adoptLocator,omitempty" validate:"excluded_with=UUID"` + UUID string `json:"uuid" validate:"omitempty,max=64"` + Rule filter.FirewallRule `json:"rule" validate:"required"` } type FirewallRuleCheck struct { @@ -342,8 +343,11 @@ type FirewallRuleDeleteFailure struct { } type FirewallRuleUpdate struct { - UUID string `json:"uuid" validate:"required,max=64"` - Rule filter.FirewallRule `json:"rule" validate:"required"` + UUID string `json:"uuid" validate:"required,max=64"` + Rule *filter.FirewallRule `json:"rule,omitempty" validate:"required_without_all=Description OrderIndex Priority,excluded_with=Description OrderIndex Priority"` + Description *string `json:"description,omitempty" validate:"excluded_with=Rule"` + OrderIndex *int64 `json:"orderIndex,omitempty" validate:"excluded_with=Rule Priority"` + Priority *int `json:"priority,omitempty" validate:"excluded_with=Rule OrderIndex"` } type FirewallRuleReorder struct { diff --git a/agent/app/service/firewall.go b/agent/app/service/firewall.go index 35d41555d..b66c90f6c 100644 --- a/agent/app/service/firewall.go +++ b/agent/app/service/firewall.go @@ -782,9 +782,6 @@ func (s *FirewallService) checkUpdate( if err != nil { return dto.FirewallRuleCheckResult{}, err } - if err := s.ensureFirewallRuleIdentityAvailable(ctx, semantic, prepared.Stored.UUID); err != nil { - return dto.FirewallRuleCheckResult{}, err - } return dto.FirewallRuleCheckResult{ Decision: filter.CheckDecisionReady, Classification: filter.CheckClassificationNone, @@ -885,17 +882,30 @@ func (s *FirewallService) Check( state = checkState{snapshot: snapshot, desired: desiredByScope[scopeKey], managedRevision: managedRevision} states[scopeKey] = state } - checked, checkErr := filter.CheckCreate(state.snapshot, rule, state.desired) + var checked filter.RuleCheckResult + var checkErr error + if item.AdoptLocator != nil { + checked, checkErr = filter.CheckAdopt(state.snapshot, rule, state.desired, *item.AdoptLocator) + } else { + checked, checkErr = filter.CheckCreate(state.snapshot, rule, state.desired) + } if checkErr != nil { return dto.FirewallRuleCheckResponse{}, checkErr } - if checked.Decision == filter.CheckDecisionReady { + adopting := checked.Decision == filter.CheckDecisionConfirmationRequired && checked.Classification == filter.CheckClassificationExactExternal + if checked.Decision == filter.CheckDecisionReady || adopting { for _, previous := range pending { if collision := filter.CheckRuleCollision(rule, previous); collision != nil { + if adopting && errors.Is(collision, filter.ErrRuleConflict) { + continue + } if errors.Is(collision, filter.ErrRuleConflict) { checked.Decision, checked.Classification, checked.Reason = filter.CheckDecisionBlocked, filter.CheckClassificationConflict, "exact_rule_conflict" } else if errors.Is(collision, filter.ErrRuleOperation) { checked.Decision, checked.Classification, checked.Reason = filter.CheckDecisionNoChange, filter.CheckClassificationExactManaged, "equivalent_batch_rule" + if adopting { + checked.Decision = filter.CheckDecisionBlocked + } } else { return dto.FirewallRuleCheckResponse{}, collision } @@ -903,7 +913,7 @@ func (s *FirewallService) Check( break } } - if checked.Decision == filter.CheckDecisionReady { + if checked.Decision == filter.CheckDecisionReady || checked.Decision == filter.CheckDecisionConfirmationRequired { pending = append(pending, rule) } } @@ -1132,7 +1142,16 @@ func (s *FirewallService) prepareCreate( sourceKind = constant.FirewallRuleSourceUser } for _, previous := range prepared { + if authorization.Operation == filter.ChangeAdopt || previous.authorization.Operation == filter.ChangeAdopt { + if authorization.Locator != nil && previous.authorization.Locator != nil && + filter.SameLocator(*authorization.Locator, *previous.authorization.Locator) { + return nil, index, filter.ErrRuleOperation + } + } if err := filter.CheckRuleCollision(rule, previous.request.Rule); err != nil { + if errors.Is(err, filter.ErrRuleConflict) && (authorization.Operation == filter.ChangeAdopt || previous.authorization.Operation == filter.ChangeAdopt) { + continue + } return nil, index, err } } @@ -1192,10 +1211,12 @@ func (s *FirewallService) createNativeRuleBatch(ctx context.Context, prepared [] if err != nil { return err } - identities := make(firewallRuleIdentityIndex, len(stored)+len(prepared)) + identities, err := firewallRuleCollisions(stored, runtime.Provider(), "") + if err != nil { + return err + } var maximumSequence int64 for _, record := range stored { - identities.add(record) if record.Sequence != nil && *record.Sequence > maximumSequence { maximumSequence = *record.Sequence } @@ -1213,7 +1234,7 @@ func (s *FirewallService) createNativeRuleBatch(ctx context.Context, prepared [] record.Sequence = &sequence nextSequence += model.FirewallRuleSequenceStep } - if err := identities.check(record); err != nil { + if err := identities.Check(domainRule); err != nil { return s.cleanupFirewallBatchRecords(ctx, created, err) } if err := filter.CheckObservedRuleCollisions(snapshot, domainRule, nil); err != nil { @@ -1222,7 +1243,9 @@ func (s *FirewallService) createNativeRuleBatch(ctx context.Context, prepared [] if recordErr = s.rules.Create(ctx, &record); recordErr != nil { return s.cleanupFirewallBatchRecords(ctx, created, recordErr) } - identities.add(record) + if err := identities.Add(domainRule); err != nil { + return s.cleanupFirewallBatchRecords(ctx, append(created, createdFirewallBatchRule{record: record, rule: domainRule}), err) + } domainRule.UUID = record.UUID created = append(created, createdFirewallBatchRule{record: record, rule: domainRule}) } @@ -1539,15 +1562,43 @@ func (s *FirewallService) restoreDeletedFirewallRecords( func (s *FirewallService) Update(ctx context.Context, clientIP string, request dto.FirewallRuleUpdate) error { firewallRuleMutationMu.Lock() defer firewallRuleMutationMu.Unlock() - rule := request.Rule - rule.UUID = request.UUID - return s.updateRule(ctx, clientIP, request.UUID, rule) + metadata := request.Description != nil || request.OrderIndex != nil || request.Priority != nil + if (request.Rule != nil) == metadata || request.OrderIndex != nil && request.Priority != nil { + return fmt.Errorf("%w: provide a rule or description/ordering fields", filter.ErrInvalidRule) + } + if request.Rule != nil { + rule := *request.Rule + rule.UUID = request.UUID + return s.updateRule(ctx, clientIP, request.UUID, rule) + } + if request.OrderIndex != nil || request.Priority != nil { + return s.updateRuleOrder(ctx, request.UUID, request.OrderIndex, request.Priority, request.Description) + } + return s.updateRuleDescription(ctx, request.UUID, *request.Description) +} + +func (s *FirewallService) updateRuleDescription(ctx context.Context, ruleUUID, description string) error { + stored, err := s.rules.GetByUUID(ctx, ruleUUID) + if err != nil { + return err + } + if isProtectedSystemFirewallRule(stored) { + return filter.ErrProtectedRule + } + if stored.Origin != constant.FirewallRuleOriginCreated && stored.Origin != constant.FirewallRuleOriginAdopted { + return fmt.Errorf("%w: only created or adopted rules can be changed", filter.ErrInvalidRule) + } + description = strings.TrimSpace(description) + if stored.Description == description { + return nil + } + return s.rules.UpdateWithRevision(ctx, stored.UUID, stored.Revision, map[string]interface{}{"description": description}) } func (s *FirewallService) Reorder(ctx context.Context, clientIP string, request dto.FirewallRuleReorder) error { firewallRuleMutationMu.Lock() defer firewallRuleMutationMu.Unlock() - return s.reorderRule(ctx, clientIP, request.UUID, request.TargetPosition, request.Priority) + return s.updateRuleOrder(ctx, request.UUID, request.TargetPosition, request.Priority, nil) } func (s *FirewallService) checkSelectedProvider(ctx context.Context, requested filter.Provider) error { @@ -1569,6 +1620,11 @@ func (s *FirewallService) createRule( authorization firewallRuleCreateAuthorization, ) error { domainRule := request.Rule + if authorization.Operation == filter.ChangeAdopt { + if err := filter.CheckAdoptDuplicates(snapshot, domainRule); err != nil { + return err + } + } if authorization.Operation == filter.ChangeCreate { if err := filter.CheckObservedRuleCollisions(snapshot, domainRule, nil); err != nil { return err @@ -1606,8 +1662,25 @@ func (s *FirewallService) createRule( if err != nil { return err } - if err := s.ensureFirewallRuleIdentityAvailable(ctx, ruleRecord, ""); err != nil { - return err + if authorization.Operation == filter.ChangeCreate { + if err := s.ensureFirewallRuleIdentityAvailable(ctx, domainRule, ""); err != nil { + return err + } + } else if authorization.Operation == filter.ChangeAdopt { + stored, err := s.rules.List(ctx) + if err != nil { + return err + } + identities, err := firewallRuleCollisions(stored, domainRule.Scope.Provider, "") + if err != nil { + return err + } + if err := identities.CheckDuplicate(domainRule); err != nil { + if errors.Is(err, filter.ErrRuleOperation) { + return filter.ErrDuplicateAdoption + } + return err + } } if domainRule.Scope.Provider != filter.ProviderFirewalld { sequence, sequenceErr := s.sequenceForCreatedFirewallRule(ctx, snapshot, domainRule) @@ -1746,11 +1819,43 @@ func managedFirewallRuleMissing(snapshot filter.Snapshot, desired filter.Desired } func (s *FirewallService) updateRule(ctx context.Context, clientIP, ruleUUID string, requestedRule filter.FirewallRule) error { + requestedRule, err := filter.NormalizeRule(requestedRule) + if err != nil { + return err + } + stored, err := s.rules.GetByUUID(ctx, ruleUUID) + if err != nil { + return err + } + previousRules, compileErr := stored.RulesForProvider(requestedRule.Scope.Provider) + if compileErr == nil && len(previousRules) == 1 { + sameContent, err := filter.SameRuleContent(previousRules[0], requestedRule) + if err != nil { + return err + } + if sameContent { + if requestedRule.Scope.Provider == filter.ProviderFirewalld { + beforePriority, afterPriority := 0, 0 + if stored.Priority != nil { + beforePriority = *stored.Priority + } + if requestedRule.Priority != nil { + afterPriority = *requestedRule.Priority + } + if beforePriority != afterPriority { + return s.updateRuleOrder(ctx, ruleUUID, nil, &afterPriority, &requestedRule.Description) + } + } else if requestedRule.OrderIndex != nil { + return s.updateRuleOrder(ctx, ruleUUID, requestedRule.OrderIndex, nil, &requestedRule.Description) + } + return s.updateRuleDescription(ctx, ruleUUID, requestedRule.Description) + } + } prepared, err := s.prepareManagedUpdate(ctx, clientIP, ruleUUID, requestedRule) if err != nil { return err } - metadataOnly, err := isUFWMetadataOnlyUpdate(prepared.Before.Rule, prepared.After, prepared.Observed.Locator) + metadataOnly, err := isFirewallMetadataOnlyUpdate(prepared.Before.Rule, prepared.After, prepared.Observed.Locator) if err != nil { return err } @@ -1769,10 +1874,7 @@ func (s *FirewallService) updateRule(ctx context.Context, clientIP, ruleUUID str }) } -func isUFWMetadataOnlyUpdate(before, after filter.FirewallRule, locator filter.Locator) (bool, error) { - if after.Scope.Provider != filter.ProviderUFW || locator.Position == nil || after.OrderIndex == nil { - return false, nil - } +func isFirewallMetadataOnlyUpdate(before, after filter.FirewallRule, locator filter.Locator) (bool, error) { beforeKey, err := filter.RuleKey(before) if err != nil { return false, err @@ -1781,22 +1883,22 @@ func isUFWMetadataOnlyUpdate(before, after filter.FirewallRule, locator filter.L if err != nil { return false, err } - return beforeKey == afterKey && *after.OrderIndex == int64(*locator.Position), nil + if beforeKey != afterKey { + return false, nil + } + if after.Scope.Provider == filter.ProviderFirewalld { + return true, nil + } + return locator.Position != nil && after.OrderIndex != nil && *after.OrderIndex == int64(*locator.Position), nil } -func (s *FirewallService) reorderRule(ctx context.Context, clientIP, ruleUUID string, targetPosition *int64, priority *int) error { +func (s *FirewallService) updateRuleOrder(ctx context.Context, ruleUUID string, targetPosition *int64, priority *int, description *string) error { + if (targetPosition == nil) == (priority == nil) { + return fmt.Errorf("%w: provide either position or priority", filter.ErrInvalidRule) + } if ruleUUID == "" { return fmt.Errorf("%w: rule UUID is required", repo.ErrFirewallPersistenceInvalid) } - if priority != nil { - stored, err := s.rules.GetByUUID(ctx, ruleUUID) - if err != nil { - return err - } - if stored.Priority == nil { - return fmt.Errorf("%w: rule has no explicit reorderable priority", filter.ErrUnsupportedScope) - } - } stored, before, snapshot, observed, runtime, err := s.loadManagedMutation(ctx, ruleUUID) if err != nil { return err @@ -1808,7 +1910,7 @@ func (s *FirewallService) reorderRule(ctx context.Context, clientIP, ruleUUID st after := before.Rule adapterOperation := filter.ChangeReorder switch { - case capabilities.OwnedChains: + case capabilities.ExplicitPosition || capabilities.OwnedChains: if targetPosition == nil || *targetPosition < 1 { return fmt.Errorf("%w: target position is required", filter.ErrInvalidRule) } @@ -1817,6 +1919,9 @@ func (s *FirewallService) reorderRule(ctx context.Context, clientIP, ruleUUID st } after.OrderIndex = targetPosition case capabilities.ExplicitPriority: + if before.Rule.NativeKind != filter.NativeKindRichRule { + return fmt.Errorf("%w: only rich rules support explicit priority", filter.ErrUnsupportedScope) + } if priority == nil { return fmt.Errorf("%w: priority is required", filter.ErrInvalidRule) } @@ -1825,10 +1930,23 @@ func (s *FirewallService) reorderRule(ctx context.Context, clientIP, ruleUUID st default: return fmt.Errorf("%w: provider does not support rule reordering", filter.ErrUnsupportedScope) } + if description != nil { + after.Description = strings.TrimSpace(*description) + } after, err = runtime.Prepare(after) if err != nil { return err } + if err := runtime.CheckRule(ctx, after); err != nil { + return err + } + metadataOnly, err := isFirewallMetadataOnlyUpdate(before.Rule, after, observed.Locator) + if err != nil { + return err + } + if metadataOnly { + return s.updateRuleDescription(ctx, stored.UUID, after.Description) + } if err := filter.GuardMutation(observed); err != nil { return err } @@ -1909,7 +2027,7 @@ func (s *FirewallService) prepareManagedUpdate( if err := filter.GuardMutation(observed); err != nil { return preparedManagedUpdate{}, err } - if err := filter.CheckObservedRuleCollisions(snapshot, after, &observed.Locator); err != nil { + if err := s.checkManagedMutationCollisions(ctx, before.Rule, after, snapshot, observed.Locator, stored.UUID); err != nil { return preparedManagedUpdate{}, err } return preparedManagedUpdate{ @@ -1990,18 +2108,8 @@ func (s *FirewallService) selectedProviderForStoredRule( func (s *FirewallService) executeManagedMutation(ctx context.Context, request managedMutationRequest) error { before, after := request.Before, request.After - if err := filter.CheckObservedRuleCollisions(request.Snapshot, after, &request.Locator); err != nil { - return err - } - semantic, err := model.FirewallRuleFromDomain(after) - if err != nil { - return err - } - if err := s.ensureFirewallRuleIdentityAvailable(ctx, semantic, request.Stored.UUID); err != nil { - return err - } appendRule, restoreAtEnd := false, false - if after.Scope.Provider == filter.ProviderUFW && request.AdapterOperation == filter.ChangeUpdate { + if after.Scope.Provider == filter.ProviderUFW && (request.AdapterOperation == filter.ChangeUpdate || request.AdapterOperation == filter.ChangeReorder) { maxPosition := maxObservedFirewallPosition(request.Snapshot) appendRule = after.OrderIndex != nil && *after.OrderIndex == maxPosition restoreAtEnd = request.Locator.Position != nil && int64(*request.Locator.Position) == maxPosition @@ -2059,50 +2167,55 @@ func maxObservedFirewallPosition(snapshot filter.Snapshot) int64 { return maximum } -func (s *FirewallService) ensureFirewallRuleIdentityAvailable( +func (s *FirewallService) checkManagedMutationCollisions( ctx context.Context, - requested model.FirewallRule, + before, after filter.FirewallRule, + snapshot filter.Snapshot, + locator filter.Locator, excludedUUID string, ) error { + sameContent, err := filter.SameRuleContent(before, after) + if err != nil { + return err + } + if sameContent { + return nil + } + if err := filter.CheckObservedRuleCollisions(snapshot, after, &locator); err != nil { + return err + } + return s.ensureFirewallRuleIdentityAvailable(ctx, after, excludedUUID) +} + +func (s *FirewallService) ensureFirewallRuleIdentityAvailable(ctx context.Context, requested filter.FirewallRule, excludedUUID string) error { stored, err := s.rules.List(ctx) if err != nil { return err } - identities := make(firewallRuleIdentityIndex, len(stored)) + identities, err := firewallRuleCollisions(stored, requested.Scope.Provider, excludedUUID) + if err != nil { + return err + } + return identities.Check(requested) +} + +func firewallRuleCollisions(stored []model.FirewallRule, provider filter.Provider, excludedUUID string) (filter.RuleCollisionIndex, error) { + identities := make(filter.RuleCollisionIndex, len(stored)) for _, candidate := range stored { - if candidate.UUID != excludedUUID { - identities.add(candidate) + if candidate.UUID == excludedUUID { + continue + } + rules, err := candidate.RulesForProvider(provider) + if err != nil { + continue + } + for _, rule := range rules { + if err := identities.Add(rule); err != nil { + return nil, err + } } } - return identities.check(requested) -} - -type firewallRuleIdentityIndex map[string][]filter.Action - -func firewallRuleComparisonKey(rule model.FirewallRule) string { - rule.Action = "" - priority := 0 - if rule.Priority != nil { - priority = *rule.Priority - } - return rule.PolicyKey() + "/" + strconv.Itoa(priority) -} - -func (index firewallRuleIdentityIndex) add(rule model.FirewallRule) { - key := firewallRuleComparisonKey(rule) - index[key] = append(index[key], filter.Action(rule.Action)) -} - -func (index firewallRuleIdentityIndex) check(rule model.FirewallRule) error { - for _, action := range index[firewallRuleComparisonKey(rule)] { - if action == filter.Action(rule.Action) { - return fmt.Errorf("%w: equivalent managed rule already exists", filter.ErrRuleOperation) - } - if filter.OppositeActions(action, filter.Action(rule.Action)) { - return filter.ErrRuleConflict - } - } - return nil + return identities, nil } func (s *FirewallService) resolveRuntime(ctx context.Context, provider filter.Provider) (*filterruntime.Engine, error) { @@ -2619,6 +2732,9 @@ func refreshCreateAuthorization( if authorization.Operation != filter.ChangeAdopt { return authorization, nil } + if err := filter.CheckAdoptDuplicates(snapshot, prepared.request.Rule); err != nil { + return firewallRuleCreateAuthorization{}, err + } if candidate, err := filter.FindCandidate(snapshot.Rules, prepared.request.AdoptInstanceKey); err == nil { locator := candidate.Locator authorization.Locator = &locator @@ -2644,16 +2760,6 @@ func refreshCreateAuthorization( authorization.Locator = &locator return authorization, nil } - if authorization.Locator != nil { - for _, candidate := range candidates { - if authorization.Locator.Canonical != "" && candidate.Locator.Canonical == authorization.Locator.Canonical || - authorization.Locator.NativeID != "" && candidate.Locator.NativeID == authorization.Locator.NativeID { - locator := candidate.Locator - authorization.Locator = &locator - return authorization, nil - } - } - } return firewallRuleCreateAuthorization{}, filter.ErrRuleStale } diff --git a/agent/app/service/firewall_sync.go b/agent/app/service/firewall_sync.go index e2c803db3..46cdbfa7f 100644 --- a/agent/app/service/firewall_sync.go +++ b/agent/app/service/firewall_sync.go @@ -371,17 +371,12 @@ func planFirewallManagedOrder( desiredMarkers = append(desiredMarkers, entry.desired.Marker) } } - drifted, feasible := firewallsync.ManagedOrderDrift(snapshot, desiredMarkers) + drifted := firewallsync.ManagedOrderDrift(snapshot, desiredMarkers) for _, marker := range desiredMarkers { if _, exists := drifted[marker]; !exists { continue } entry := byMarker[marker] - if !feasible { - entry.item.Status = firewallRuleSyncBlocked - entry.item.Reason = "managed rule order cannot cross external, opaque, or protected rules" - continue - } entry.reorder = true if entry.item.Status == firewallRuleSyncExisting { entry.item.Status = firewallRuleSyncReady diff --git a/agent/utils/firewall/filter/adopt.go b/agent/utils/firewall/filter/adopt.go new file mode 100644 index 000000000..8fe594683 --- /dev/null +++ b/agent/utils/firewall/filter/adopt.go @@ -0,0 +1,97 @@ +package filter + +import "fmt" + +var ErrDuplicateAdoption = fmt.Errorf("%w: duplicate firewall rules prevent adoption; manually delete duplicate rules and retry", ErrRuleOperation) + +func CheckAdoptDuplicates(snapshot Snapshot, requested FirewallRule) error { + count := 0 + for _, observed := range snapshot.Rules { + if observed.ParseStatus != ParseStatusSupported { + continue + } + same, err := SameRuleContent(observed.Rule, requested) + if err != nil { + return err + } + if same { + count++ + if count > 1 { + return ErrDuplicateAdoption + } + } + } + return nil +} + +func CheckAdopt(snapshot Snapshot, requested FirewallRule, desired []DesiredRule, locator Locator) (RuleCheckResult, error) { + normalized, err := NormalizeRule(requested) + if err != nil { + return RuleCheckResult{}, err + } + if normalized.Scope.Key() != snapshot.Scope.Key() { + return RuleCheckResult{}, ErrInvalidScope + } + key, err := RuleKey(normalized) + if err != nil { + return RuleCheckResult{}, err + } + result := RuleCheckResult{RequestedRule: normalized, RequestedRuleKey: key} + var selected *ObservedRule + for index := range snapshot.Rules { + candidate := &snapshot.Rules[index] + if SameLocator(candidate.Locator, locator) { + if selected != nil { + return RuleCheckResult{}, ErrRuleStale + } + selected = candidate + } + } + if selected == nil { + return RuleCheckResult{}, ErrRuleStale + } + selectedKey, err := RuleKey(selected.Rule) + if err != nil || selected.ParseStatus != ParseStatusSupported || selectedKey != key { + return RuleCheckResult{}, fmt.Errorf("%w: selected rule changed or cannot be adopted", ErrRuleStale) + } + if err := CheckAdoptDuplicates(snapshot, normalized); err != nil { + if err != ErrDuplicateAdoption { + return RuleCheckResult{}, err + } + result.Decision, result.Classification, result.Reason = CheckDecisionBlocked, CheckClassificationExactExternal, "duplicate_rules" + return finishCheck(result) + } + items, err := MergeInventory(InventoryMergeInput{Observed: snapshot.Rules, Desired: desired}) + if err != nil { + return RuleCheckResult{}, err + } + for _, item := range items { + if item.Desired != nil && item.Observed != nil && SameLocator(item.Observed.Locator, locator) { + result.Decision, result.Classification, result.Reason = CheckDecisionNoChange, CheckClassificationExactManaged, "equivalent_managed_rule" + result.ExistingRuleUUID = item.Desired.UUID + return result, nil + } + } + for _, owned := range desired { + same, err := SameRuleContent(owned.Rule, normalized) + if err != nil { + return RuleCheckResult{}, err + } + if same { + result.Decision, result.Classification, result.Reason = CheckDecisionBlocked, CheckClassificationExactManaged, "duplicate_rules" + result.ExistingRuleUUID = owned.UUID + return result, nil + } + } + result.Candidates = []ObservedRule{*selected} + switch { + case selected.Protected: + result.Decision, result.Classification, result.Reason = CheckDecisionBlocked, CheckClassificationProtected, "protected_rule" + case containsPersistenceDrift(result.Candidates): + result.Decision, result.Classification, result.Reason = CheckDecisionBlocked, CheckClassificationConflict, "runtime_permanent_mismatch" + default: + result.Decision, result.Classification, result.Reason = CheckDecisionConfirmationRequired, CheckClassificationExactExternal, "equivalent_external_rule" + result.AllowedActions = []CheckAction{CheckActionAdopt, CheckActionCancel} + } + return finishCheck(result) +} diff --git a/agent/utils/firewall/filter/check.go b/agent/utils/firewall/filter/check.go index 175e32d22..75064630f 100644 --- a/agent/utils/firewall/filter/check.go +++ b/agent/utils/firewall/filter/check.go @@ -154,6 +154,13 @@ func CheckCreate( } } + if err := CheckAdoptDuplicates(snapshot, normalized); err != nil { + if err != ErrDuplicateAdoption { + return RuleCheckResult{}, err + } + result.Decision, result.Classification, result.Reason = CheckDecisionBlocked, CheckClassificationExactExternal, "duplicate_rules" + return finishCheck(result) + } switch { case containsPersistenceDrift(exact): result.Decision = CheckDecisionBlocked @@ -171,12 +178,6 @@ func CheckCreate( result.Reason = "equivalent_external_rule" result.Candidates = exact result.AllowedActions = []CheckAction{CheckActionAdopt, CheckActionCancel} - case len(exact) > 1: - result.Decision = CheckDecisionConfirmationRequired - result.Classification = CheckClassificationExactExternal - result.Reason = "multiple_equivalent_external_rules" - result.Candidates = exact - result.AllowedActions = []CheckAction{CheckActionSelectAdopt, CheckActionCancel} case equivalentExternal: result.Decision = CheckDecisionNoChange result.Classification = CheckClassificationExactExternal diff --git a/agent/utils/firewall/filter/identity.go b/agent/utils/firewall/filter/identity.go index e86e55797..398dc8267 100644 --- a/agent/utils/firewall/filter/identity.go +++ b/agent/utils/firewall/filter/identity.go @@ -50,9 +50,7 @@ func RuleMatchKey(rule FirewallRule) (string, error) { return "", err } normalized.Action, normalized.NativeKind, normalized.OrderBucket = "", "", "" - if normalized.Priority != nil && *normalized.Priority == 0 { - normalized.Priority = nil - } + normalized.Priority = nil return normalizedRuleKey(normalized) } @@ -61,6 +59,60 @@ func OppositeActions(left, right Action) bool { right == ActionAccept && (left == ActionDrop || left == ActionReject) } +func SameRuleContent(before, after FirewallRule) (bool, error) { + before, err := NormalizeRule(before) + if err != nil { + return false, err + } + after, err = NormalizeRule(after) + if err != nil { + return false, err + } + previous, err := RuleMatchKey(before) + if err != nil { + return false, err + } + requested, err := RuleMatchKey(after) + return err == nil && previous == requested && before.Action == after.Action, err +} + +type RuleCollisionIndex map[string][]Action + +func (index RuleCollisionIndex) Add(rule FirewallRule) error { + key, err := RuleMatchKey(rule) + if err != nil { + return err + } + index[key] = append(index[key], rule.Action) + return nil +} + +func (index RuleCollisionIndex) CheckDuplicate(rule FirewallRule) error { + key, err := RuleMatchKey(rule) + if err != nil { + return err + } + for _, action := range index[key] { + if action == rule.Action { + return checkCollisionActions(rule.Action, action) + } + } + return nil +} + +func (index RuleCollisionIndex) Check(rule FirewallRule) error { + key, err := RuleMatchKey(rule) + if err != nil { + return err + } + for _, action := range index[key] { + if err := checkCollisionActions(rule.Action, action); err != nil { + return err + } + } + return nil +} + func CheckRuleCollision(requested, existing FirewallRule) error { wanted, err := RuleMatchKey(requested) if err != nil { @@ -73,10 +125,14 @@ func CheckRuleCollision(requested, existing FirewallRule) error { if wanted != actual { return nil } - if requested.Action == existing.Action { + return checkCollisionActions(requested.Action, existing.Action) +} + +func checkCollisionActions(requested, existing Action) error { + if requested == existing { return fmt.Errorf("%w: equivalent rule already exists", ErrRuleOperation) } - if OppositeActions(requested.Action, existing.Action) { + if OppositeActions(requested, existing) { return ErrRuleConflict } return nil diff --git a/agent/utils/firewall/filter/providers/firewalld/adapter.go b/agent/utils/firewall/filter/providers/firewalld/adapter.go index 382ba1e9d..cd8dc34bd 100644 --- a/agent/utils/firewall/filter/providers/firewalld/adapter.go +++ b/agent/utils/firewall/filter/providers/firewalld/adapter.go @@ -199,11 +199,6 @@ func (a *Adapter) Compile(snapshot filter.Snapshot, changes []filter.DesiredChan if len(changes) != 1 { return filter.BackendPlan{}, fmt.Errorf("%w: firewalld plans currently require exactly one change", filter.ErrInvalidRule) } - for _, observed := range snapshot.Rules { - if observed.Persistence != "" && observed.Persistence != filter.PersistenceStatusConverged { - return filter.BackendPlan{}, fmt.Errorf("%w: firewalld runtime and permanent state differ", filter.ErrRuleStale) - } - } rulePlan, err := a.compileChange(snapshot, changes[0]) if err != nil { return filter.BackendPlan{}, err @@ -315,7 +310,7 @@ func (a *Adapter) compileChange(snapshot filter.Snapshot, change filter.DesiredC plan := filter.NativeRulePlan{RuleUUID: normalized.UUID, Operation: change.Operation, Expected: expected} switch change.Operation { case filter.ChangeCreate: - plan.Commands, plan.RollbackCommands = pairedCommands(normalized, "add", "remove") + plan.Commands, plan.RollbackCommands = missingRuleCommands(snapshot, normalized) case filter.ChangeAdopt: target, targetErr := validateMutationTarget(snapshot, change, normalized, false) if targetErr != nil { @@ -330,8 +325,11 @@ func (a *Adapter) compileChange(snapshot filter.Snapshot, change filter.DesiredC return filter.NativeRulePlan{}, targetErr } plan.Previous = &target + if target.Locator.Canonical == expected.Locator.Canonical { + break + } removeCommands, restoreCommands := pairedCommands(target.Rule, "remove", "add") - addCommands, removeNewCommands := pairedCommands(normalized, "add", "remove") + addCommands, removeNewCommands := missingRuleCommands(snapshot, normalized) plan.Commands = append(removeCommands, addCommands...) plan.RollbackCommands = append(restoreCommands, removeNewCommands...) case filter.ChangeDelete: @@ -401,6 +399,26 @@ func nativeCanonical(rule filter.FirewallRule) string { return "rich:" + canonicalRichRule(rule) } +func missingRuleCommands(snapshot filter.Snapshot, rule filter.FirewallRule) ([]filter.NativeCommand, []filter.NativeCommand) { + commands, rollback := pairedCommands(rule, "add", "remove") + var runtimeExists, permanentExists bool + for _, observed := range snapshot.Rules { + if observed.Locator.Canonical != nativeCanonical(rule) { + continue + } + runtimeExists = runtimeExists || observed.Persistence == filter.PersistenceStatusConverged || observed.Persistence == filter.PersistenceStatusRuntimeOnly + permanentExists = permanentExists || observed.Persistence == filter.PersistenceStatusConverged || observed.Persistence == filter.PersistenceStatusPermanentOnly + } + var changes, inverses []filter.NativeCommand + for index, exists := range []bool{runtimeExists, permanentExists} { + if !exists { + changes = append(changes, commands[index]) + inverses = append(inverses, rollback[index]) + } + } + return changes, inverses +} + func pairedCommands(rule filter.FirewallRule, operation, inverse string) ([]filter.NativeCommand, []filter.NativeCommand) { option := nativeOption(rule, operation) rollback := nativeOption(rule, inverse) diff --git a/agent/utils/firewall/filter/providers/iptables/adapter.go b/agent/utils/firewall/filter/providers/iptables/adapter.go index 6564dea8b..ea18ad68b 100644 --- a/agent/utils/firewall/filter/providers/iptables/adapter.go +++ b/agent/utils/firewall/filter/providers/iptables/adapter.go @@ -457,9 +457,6 @@ func compileChange(snapshot filter.Snapshot, change filter.DesiredChange) (filte } targetPosition = int(*normalized.OrderIndex) } - if err := validateReorderPath(snapshot, position, targetPosition); err != nil { - return filter.NativeRulePlan{}, err - } if position != targetPosition { return positionalMutationPlan(snapshot, normalized, target, marker, position, targetPosition, change.Operation), nil } @@ -479,9 +476,6 @@ func compileChange(snapshot filter.Snapshot, change filter.DesiredChange) (filte return filter.NativeRulePlan{}, fmt.Errorf("%w: reorder target is out of range", filter.ErrInvalidRule) } targetPosition := int(*normalized.OrderIndex) - if err := validateReorderPath(snapshot, position, targetPosition); err != nil { - return filter.NativeRulePlan{}, err - } return positionalMutationPlan(snapshot, normalized, target, marker, position, targetPosition, change.Operation), nil default: return filter.NativeRulePlan{}, fmt.Errorf("%w: unsupported operation %s", filter.ErrInvalidRule, change.Operation) @@ -560,26 +554,6 @@ func pointerToObserved(rule filter.ObservedRule, include bool) *filter.ObservedR return &rule } -func validateReorderPath(snapshot filter.Snapshot, from, to int) error { - start, end := from, to - if start > end { - start, end = end, start - } - for position := start; position <= end; position++ { - if position == from { - continue - } - observed := snapshot.Rules[position-1] - if observed.Protected { - return filter.ErrProtectedRule - } - if observed.ParseStatus == filter.ParseStatusOpaque || observed.Marker == "" { - return fmt.Errorf("%w: reorder cannot cross external or opaque rules", filter.ErrUnsupportedScope) - } - } - return nil -} - func compileRuleArgs(rule filter.FirewallRule, marker string) []string { args := make([]string, 0, 24) if rule.Protocol != "all" { diff --git a/agent/utils/firewall/filter/providers/nftables/adapter.go b/agent/utils/firewall/filter/providers/nftables/adapter.go index 6de20649e..f6cd9d91d 100644 --- a/agent/utils/firewall/filter/providers/nftables/adapter.go +++ b/agent/utils/firewall/filter/providers/nftables/adapter.go @@ -273,18 +273,6 @@ func applyChange(snapshot filter.Snapshot, change filter.DesiredChange) ([]filte if target < 1 || target > len(rules)+1 { return nil, filter.ObservedRule{}, nil, fmt.Errorf("%w: target position is out of range", filter.ErrInvalidRule) } - if change.Operation == filter.ChangeReorder || change.Operation == filter.ChangeUpdate { - start, end := position, target - if start > end { - start, end = end, start - } - for index := start; index <= end && index <= len(snapshot.Rules); index++ { - candidate := snapshot.Rules[index-1] - if candidate.Protected || candidate.ParseStatus == filter.ParseStatusOpaque || candidate.Marker == "" { - return nil, filter.ObservedRule{}, nil, fmt.Errorf("%w: reorder cannot cross external or opaque rules", filter.ErrUnsupportedScope) - } - } - } expected := observedRule(normalized, marker, target, strings.Join(compileExpressionArgs(normalized, marker), " ")) rules = append(rules, filter.ObservedRule{}) copy(rules[target:], rules[target-1:]) diff --git a/agent/utils/firewall/filter/providers/ufw/adapter.go b/agent/utils/firewall/filter/providers/ufw/adapter.go index 14e81aea1..b5d499982 100644 --- a/agent/utils/firewall/filter/providers/ufw/adapter.go +++ b/agent/utils/firewall/filter/providers/ufw/adapter.go @@ -236,7 +236,7 @@ func (a *Adapter) failedCommandApplied(ctx context.Context, plan filter.NativeRu return plan.Previous == nil || !containsObservedRule(snapshot, *plan.Previous) } return markerCount > 0 - case filter.ChangeUpdate: + case filter.ChangeUpdate, filter.ChangeReorder: if commandIndex == 0 { return markerCount == 0 } @@ -355,7 +355,7 @@ func compileChange(snapshot filter.Snapshot, change filter.DesiredChange) (filte positionedCommand(position, target.Rule, observedComment(target), restoreAtEnd), deleteRuleCommand(normalized, marker), } - case filter.ChangeUpdate: + case filter.ChangeUpdate, filter.ChangeReorder: target, targetErr := validateMutationTarget(snapshot, change, normalized, marker, true) if targetErr != nil { return filter.NativeRulePlan{}, targetErr @@ -404,8 +404,6 @@ func compileChange(snapshot filter.Snapshot, change filter.DesiredChange) (filte plan.RollbackCommands = []filter.NativeCommand{ positionedCommand(position, target.Rule, observedComment(target), restoreAtEnd), } - case filter.ChangeReorder: - return filter.NativeRulePlan{}, fmt.Errorf("%w: ufw reorder is not supported", filter.ErrUnsupportedScope) default: return filter.NativeRulePlan{}, fmt.Errorf("%w: unsupported operation %s", filter.ErrInvalidRule, change.Operation) } diff --git a/agent/utils/firewall/filter/runtime/runtime.go b/agent/utils/firewall/filter/runtime/runtime.go index e719663f3..dba7adec6 100644 --- a/agent/utils/firewall/filter/runtime/runtime.go +++ b/agent/utils/firewall/filter/runtime/runtime.go @@ -165,6 +165,9 @@ func (e *Engine) ValidatePosition( rule filter.FirewallRule, target int64, ) error { + if target < 1 { + return fmt.Errorf("%w: target position must be positive", filter.ErrInvalidRule) + } if rule.Scope.Provider == filter.ProviderUFW { minimum, maximum := positionBounds(snapshot) if target < minimum || target > maximum { diff --git a/agent/utils/firewall/filter/safety.go b/agent/utils/firewall/filter/safety.go index 59826be3a..e9677f705 100644 --- a/agent/utils/firewall/filter/safety.go +++ b/agent/utils/firewall/filter/safety.go @@ -53,10 +53,19 @@ func SameLocator(left, right Locator) bool { if left.ScopeKey != right.ScopeKey { return false } - if left.Canonical != "" || right.Canonical != "" { - return left.Canonical != "" && left.Canonical == right.Canonical + if left.Provider != "" && right.Provider != "" && left.Provider != right.Provider { + return false } - return left.Position != nil && right.Position != nil && *left.Position == *right.Position + if left.Position != nil || right.Position != nil { + if left.Position == nil || right.Position == nil || *left.Position != *right.Position { + return false + } + if left.NativeID != "" && right.NativeID != "" && left.NativeID != right.NativeID { + return false + } + return left.Canonical == "" || right.Canonical == "" || left.Canonical == right.Canonical + } + return left.Canonical != "" && left.Canonical == right.Canonical } func MatchObservedByRuleKey(observed []ObservedRule, rule FirewallRule) ([]ObservedRule, error) { diff --git a/agent/utils/firewall/sync/order.go b/agent/utils/firewall/sync/order.go index 96e324755..e45b700e0 100644 --- a/agent/utils/firewall/sync/order.go +++ b/agent/utils/firewall/sync/order.go @@ -11,45 +11,30 @@ func SupportsManagedOrder(provider filter.Provider) bool { return provider == filter.ProviderIptables || provider == filter.ProviderNftables || provider == filter.ProviderUFW } -func ManagedOrderDrift(snapshot filter.Snapshot, desiredMarkers []string) (map[string]struct{}, bool) { +func ManagedOrderDrift(snapshot filter.Snapshot, desiredMarkers []string) map[string]struct{} { if !SupportsManagedOrder(snapshot.Scope.Provider) || len(desiredMarkers) < 2 { - return nil, true + return nil } expected := make(map[string]struct{}, len(desiredMarkers)) for _, marker := range desiredMarkers { expected[marker] = struct{}{} } actual := make([]string, 0, len(desiredMarkers)) - segments := make(map[string]int, len(desiredMarkers)) - segment := 0 + present := make(map[string]struct{}, len(desiredMarkers)) for _, observed := range snapshot.Rules { - _, wanted := expected[observed.Marker] - if wanted { + if _, wanted := expected[observed.Marker]; wanted { actual = append(actual, observed.Marker) - if observed.Protected || observed.ParseStatus == filter.ParseStatusOpaque { - segment++ - segments[observed.Marker] = segment - segment++ - } else { - segments[observed.Marker] = segment - } - continue + present[observed.Marker] = struct{}{} } - if strings.HasPrefix(observed.Marker, "1panel-rule:") && - !observed.Protected && observed.ParseStatus != filter.ParseStatusOpaque { - continue - } - segment++ } - desired := make([]string, 0, len(actual)) for _, marker := range desiredMarkers { - if _, exists := segments[marker]; exists { + if _, exists := present[marker]; exists { desired = append(desired, marker) } } if slices.Equal(actual, desired) { - return nil, true + return nil } drifted := make(map[string]struct{}, len(desired)) for index := range desired { @@ -58,15 +43,7 @@ func ManagedOrderDrift(snapshot filter.Snapshot, desiredMarkers []string) (map[s drifted[desired[index]] = struct{}{} } } - feasible, previousSegment := true, -1 - for _, marker := range desired { - if segments[marker] < previousSegment { - feasible = false - break - } - previousSegment = segments[marker] - } - return drifted, feasible + return drifted } func InsertionPosition(snapshot filter.Snapshot, desiredMarkers []string, targetMarker string) (int64, bool) { diff --git a/frontend/src/api/interface/firewall.ts b/frontend/src/api/interface/firewall.ts index 127e55f69..563df0ced 100644 --- a/frontend/src/api/interface/firewall.ts +++ b/frontend/src/api/interface/firewall.ts @@ -247,6 +247,7 @@ export namespace Firewall { export interface CheckItem { uuid?: string; rule: Rule; + adoptLocator?: Locator; } export interface CheckRequest { @@ -359,9 +360,11 @@ export namespace Firewall { error: string; } - export interface UpdateRequest { - rule: Rule; - } + export type UpdateRequest = + | { rule: Rule; description?: never; orderIndex?: never; priority?: never } + | { rule?: never; description: string; orderIndex?: never; priority?: never } + | { rule?: never; description?: string; orderIndex: number; priority?: never } + | { rule?: never; description?: string; orderIndex?: never; priority: number }; export interface DockerGuardBase { name: string; diff --git a/frontend/src/lang/modules/en.ts b/frontend/src/lang/modules/en.ts index 46d091cfa..e4467918e 100644 --- a/frontend/src/lang/modules/en.ts +++ b/frontend/src/lang/modules/en.ts @@ -4099,9 +4099,10 @@ const message = { ruleTargetRequired: 'Enter at least one IP address or port', batchRuleLimit: 'A maximum of {0} rules can be created at a time', resolution_adopt: 'Take over management', + plan_duplicate_rules: + 'Rules with identical conditions and actions cannot be adopted. Manually delete duplicate rules and retry.', adoptRuleConfirm: 'After takeover, 1Panel can maintain and delete this existing rule. Continue?', - plan_exact_rule_conflict: - 'A rule with identical matching conditions and priority has an opposing allow or deny action.', + plan_exact_rule_conflict: 'A rule with identical matching conditions has an opposing allow or deny action.', allRulesAlreadyExist: 'All {0} checked rules already exist. There are no new rules to create.', ruleCheckResult: 'Rule check results', ruleCheckStatus_creatable: 'Creatable', diff --git a/frontend/src/lang/modules/es-es.ts b/frontend/src/lang/modules/es-es.ts index f9e6e21a6..945379558 100644 --- a/frontend/src/lang/modules/es-es.ts +++ b/frontend/src/lang/modules/es-es.ts @@ -4145,9 +4145,11 @@ const message = { ruleTargetRequired: 'Introduce al menos una dirección IP o un puerto', batchRuleLimit: 'Se pueden crear como máximo {0} reglas a la vez', resolution_adopt: 'Asumir la gestión', + plan_duplicate_rules: + 'No se pueden gestionar reglas duplicadas con las mismas condiciones y acciones. Elimine manualmente los duplicados y vuelva a intentarlo.', adoptRuleConfirm: 'Después de asumirla, 1Panel podrá mantener y eliminar esta regla existente. ¿Continuar?', plan_exact_rule_conflict: - 'Ya existe una regla con las mismas condiciones y prioridad, pero con una acción opuesta de permitir o denegar.', + 'Ya existe una regla con las mismas condiciones, pero con una acción opuesta de permitir o denegar.', allRulesAlreadyExist: 'Las {0} reglas comprobadas ya existen. No hay reglas nuevas que crear.', ruleCheckResult: 'Resultados de la comprobación de reglas', ruleCheckStatus_creatable: 'Se puede crear', diff --git a/frontend/src/lang/modules/fa.ts b/frontend/src/lang/modules/fa.ts index 0ec0020f9..9bb49414b 100644 --- a/frontend/src/lang/modules/fa.ts +++ b/frontend/src/lang/modules/fa.ts @@ -4058,8 +4058,10 @@ const message = { ruleTargetRequired: 'حداقل یک نشانی IP یا درگاه وارد کنید', batchRuleLimit: 'در هر بار حداکثر می‌توان {0} قانون ایجاد کرد', resolution_adopt: 'واگذاری مدیریت', + plan_duplicate_rules: + 'قوانین تکراری با شرایط و اقدامات یکسان قابل پذیرش برای مدیریت نیستند. قوانین تکراری را به‌صورت دستی حذف کرده و دوباره تلاش کنید.', adoptRuleConfirm: 'پس از واگذاری، 1Panel می‌تواند این قانون موجود را نگهداری و حذف کند. ادامه می‌دهید؟', - plan_exact_rule_conflict: 'قانونی با شرایط تطبیق و اولویت یکسان، اما با اقدام مخالفِ اجازه یا رد وجود دارد.', + plan_exact_rule_conflict: 'قانونی با شرایط تطبیق یکسان، اما با اقدام مخالفِ اجازه یا رد وجود دارد.', allRulesAlreadyExist: 'هر {0} قانون بررسی‌شده از قبل وجود دارند. قانون جدیدی برای ایجاد وجود ندارد.', ruleCheckResult: 'نتایج بررسی قوانین', ruleCheckStatus_creatable: 'قابل ایجاد', diff --git a/frontend/src/lang/modules/ja.ts b/frontend/src/lang/modules/ja.ts index 402041ff9..9de9dd231 100644 --- a/frontend/src/lang/modules/ja.ts +++ b/frontend/src/lang/modules/ja.ts @@ -4083,8 +4083,10 @@ const message = { ruleTargetRequired: 'IP アドレスまたはポートを1つ以上入力してください', batchRuleLimit: '一度に作成できるルールは最大 {0} 件です', resolution_adopt: '管理対象にする', + plan_duplicate_rules: + '条件とアクションが同じルールが重複しているため、管理対象に追加できません。重複ルールを手動で削除してから再試行してください。', adoptRuleConfirm: '管理対象にすると、1Panel がこの既存ルールの保守と削除を行えるようになります。続行しますか？', - plan_exact_rule_conflict: '一致条件と優先度が同じで、許可・拒否の動作が逆のルールが存在します。', + plan_exact_rule_conflict: '一致条件が同じで、許可・拒否の動作が逆のルールが存在します。', allRulesAlreadyExist: '確認した {0} 件のルールはすべて既に存在します。新しく作成するルールはありません。', ruleCheckResult: 'ルール確認結果', ruleCheckStatus_creatable: '作成可能', diff --git a/frontend/src/lang/modules/ko.ts b/frontend/src/lang/modules/ko.ts index 5e5a0325e..766678921 100644 --- a/frontend/src/lang/modules/ko.ts +++ b/frontend/src/lang/modules/ko.ts @@ -4009,9 +4009,11 @@ const message = { ruleTargetRequired: 'IP 주소 또는 포트를 하나 이상 입력하세요', batchRuleLimit: '한 번에 최대 {0}개의 규칙을 생성할 수 있습니다', resolution_adopt: '관리 대상으로 전환', + plan_duplicate_rules: + '조건과 동작이 동일한 중복 규칙은 관리 대상으로 추가할 수 없습니다. 중복 규칙을 수동으로 삭제한 후 다시 시도하세요.', adoptRuleConfirm: '관리 대상으로 전환하면 1Panel이 이 기존 규칙을 유지하고 삭제할 수 있습니다. 계속하시겠습니까?', - plan_exact_rule_conflict: '일치 조건과 우선순위가 같지만 허용 또는 거부 동작이 반대인 규칙이 이미 있습니다.', + plan_exact_rule_conflict: '일치 조건이 같지만 허용 또는 거부 동작이 반대인 규칙이 이미 있습니다.', allRulesAlreadyExist: '확인한 규칙 {0}개가 모두 이미 존재합니다. 새로 생성할 규칙이 없습니다.', ruleCheckResult: '규칙 검사 결과', ruleCheckStatus_creatable: '생성 가능', diff --git a/frontend/src/lang/modules/lo.ts b/frontend/src/lang/modules/lo.ts index 025f1f44d..cb8bef952 100644 --- a/frontend/src/lang/modules/lo.ts +++ b/frontend/src/lang/modules/lo.ts @@ -3980,8 +3980,10 @@ const message = { ruleTargetRequired: 'ກະລຸນາປ້ອນ IP ຫຼື ພອດຢ່າງໜ້ອຍໜຶ່ງລາຍການ', batchRuleLimit: 'ສາມາດສ້າງກົດໄດ້ສູງສຸດ {0} ລາຍການຕໍ່ຄັ້ງ', resolution_adopt: 'ຮັບເຂົ້າຈັດການ', + plan_duplicate_rules: + 'ບໍ່ສາມາດນຳກົດທີ່ມີເງື່ອນໄຂ ແລະ ການກະທຳຄືກັນເຂົ້າມາຈັດການໄດ້. ກະລຸນາລຶບກົດທີ່ຊ້ຳກັນດ້ວຍຕົນເອງ ແລ້ວລອງອີກຄັ້ງ.', adoptRuleConfirm: 'ຫຼັງຈາກຮັບເຂົ້າແລ້ວ 1Panel ຈະສາມາດຮັກສາແລະລຶບກົດນີ້ໄດ້. ສືບຕໍ່ບໍ?', - plan_exact_rule_conflict: 'ມີກົດທີ່ມີເງື່ອນໄຂແລະລຳດັບຄວາມສຳຄັນຄືກັນ ແຕ່ການອະນຸຍາດຫຼືປະຕິເສດກົງກັນຂ້າມ.', + plan_exact_rule_conflict: 'ມີກົດທີ່ມີເງື່ອນໄຂຄືກັນ ແຕ່ການອະນຸຍາດຫຼືປະຕິເສດກົງກັນຂ້າມ.', allRulesAlreadyExist: 'ກົດທັງໝົດ {0} ລາຍການທີ່ກວດສອບມີຢູ່ແລ້ວ. ບໍ່ມີກົດໃໝ່ທີ່ຕ້ອງສ້າງ.', ruleCheckResult: 'ຜົນການກວດສອບກົດ', ruleCheckStatus_creatable: 'ສາມາດສ້າງໄດ້', diff --git a/frontend/src/lang/modules/ms.ts b/frontend/src/lang/modules/ms.ts index 62048e73a..6ee8bc1cf 100644 --- a/frontend/src/lang/modules/ms.ts +++ b/frontend/src/lang/modules/ms.ts @@ -4165,10 +4165,12 @@ const message = { ruleTargetRequired: 'Masukkan sekurang-kurangnya satu alamat IP atau port', batchRuleLimit: 'Maksimum {0} peraturan boleh dibuat pada satu masa', resolution_adopt: 'Ambil alih pengurusan', + plan_duplicate_rules: + 'Peraturan pendua dengan syarat dan tindakan yang sama tidak boleh diambil alih untuk diurus. Padam peraturan pendua secara manual dan cuba lagi.', adoptRuleConfirm: 'Selepas diambil alih, 1Panel boleh menyelenggara dan memadam peraturan sedia ada ini. Teruskan?', plan_exact_rule_conflict: - 'Peraturan dengan syarat padanan dan keutamaan yang sama mempunyai tindakan benarkan atau sekat yang bertentangan.', + 'Peraturan dengan syarat padanan yang sama mempunyai tindakan benarkan atau sekat yang bertentangan.', allRulesAlreadyExist: 'Kesemua {0} peraturan yang diperiksa sudah wujud. Tiada peraturan baharu untuk dicipta.', ruleCheckResult: 'Keputusan semakan peraturan', ruleCheckStatus_creatable: 'Boleh dicipta', diff --git a/frontend/src/lang/modules/pt-br.ts b/frontend/src/lang/modules/pt-br.ts index d4da71a8c..1de409d91 100644 --- a/frontend/src/lang/modules/pt-br.ts +++ b/frontend/src/lang/modules/pt-br.ts @@ -4184,9 +4184,11 @@ const message = { ruleTargetRequired: 'Informe pelo menos um endereço IP ou uma porta', batchRuleLimit: 'É possível criar no máximo {0} regras por vez', resolution_adopt: 'Assumir gerenciamento', + plan_duplicate_rules: + 'Regras duplicadas com condições e ações idênticas não podem ser gerenciadas. Exclua manualmente as regras duplicadas e tente novamente.', adoptRuleConfirm: 'Depois disso, o 1Panel poderá manter e excluir esta regra existente. Continuar?', plan_exact_rule_conflict: - 'Já existe uma regra com as mesmas condições e prioridade, mas com uma ação oposta de permitir ou negar.', + 'Já existe uma regra com as mesmas condições, mas com uma ação oposta de permitir ou negar.', allRulesAlreadyExist: 'Todas as {0} regras verificadas já existem. Não há novas regras para criar.', ruleCheckResult: 'Resultados da verificação de regras', ruleCheckStatus_creatable: 'Pode ser criada', diff --git a/frontend/src/lang/modules/ru.ts b/frontend/src/lang/modules/ru.ts index 08da62e82..28c45205f 100644 --- a/frontend/src/lang/modules/ru.ts +++ b/frontend/src/lang/modules/ru.ts @@ -4151,9 +4151,11 @@ const message = { ruleTargetRequired: 'Укажите хотя бы один IP-адрес или порт', batchRuleLimit: 'За один раз можно создать не более {0} правил', resolution_adopt: 'Взять под управление', + plan_duplicate_rules: + 'Нельзя принять под управление правила с одинаковыми условиями и действиями. Удалите дубликаты вручную и повторите попытку.', adoptRuleConfirm: 'После этого 1Panel сможет обслуживать и удалять существующее правило. Продолжить?', plan_exact_rule_conflict: - 'Уже существует правило с теми же условиями и приоритетом, но с противоположным действием разрешения или запрета.', + 'Уже существует правило с теми же условиями, но с противоположным действием разрешения или запрета.', allRulesAlreadyExist: 'Все проверенные правила ({0}) уже существуют. Новых правил для создания нет.', ruleCheckResult: 'Результаты проверки правил', ruleCheckStatus_creatable: 'Можно создать', diff --git a/frontend/src/lang/modules/tr.ts b/frontend/src/lang/modules/tr.ts index ac7b81091..85900b85f 100644 --- a/frontend/src/lang/modules/tr.ts +++ b/frontend/src/lang/modules/tr.ts @@ -4169,9 +4169,11 @@ const message = { ruleTargetRequired: 'En az bir IP adresi veya port girin', batchRuleLimit: 'Bir seferde en fazla {0} kural oluşturulabilir', resolution_adopt: 'Yönetimi devral', + plan_duplicate_rules: + 'Koşulları ve eylemleri aynı olan yinelenen kurallar yönetime alınamaz. Yinelenen kuralları elle silip yeniden deneyin.', adoptRuleConfirm: 'Devraldıktan sonra 1Panel bu mevcut kuralı yönetebilir ve silebilir. Devam edilsin mi?', plan_exact_rule_conflict: - 'Aynı eşleşme koşullarına ve önceliğe sahip, ancak izin verme veya reddetme eylemi zıt olan bir kural zaten var.', + 'Aynı eşleşme koşullarına sahip, ancak izin verme veya reddetme eylemi zıt olan bir kural zaten var.', allRulesAlreadyExist: 'Kontrol edilen {0} kuralın tümü zaten mevcut. Oluşturulacak yeni kural yok.', ruleCheckResult: 'Kural kontrol sonuçları', ruleCheckStatus_creatable: 'Oluşturulabilir', diff --git a/frontend/src/lang/modules/zh-Hant.ts b/frontend/src/lang/modules/zh-Hant.ts index b23687b15..a0239bc94 100644 --- a/frontend/src/lang/modules/zh-Hant.ts +++ b/frontend/src/lang/modules/zh-Hant.ts @@ -3829,8 +3829,9 @@ const message = { ruleTargetRequired: '請至少填寫一個 IP 位址或連接埠', batchRuleLimit: '單次最多可建立 {0} 條規則', resolution_adopt: '納管', + plan_duplicate_rules: '存在比對條件和動作相同的重複規則，無法納管。請手動刪除重複規則後重試。', adoptRuleConfirm: '納入管理後，1Panel 將負責維護及刪除這條現有規則，是否繼續？', - plan_exact_rule_conflict: '已存在比對條件和優先級相同、但允許與拒絕動作相反的規則。', + plan_exact_rule_conflict: '已存在比對條件相同、但允許與拒絕動作相反的規則。', allRulesAlreadyExist: '本次檢查的 {0} 條規則均已存在，沒有需要建立的新規則。', ruleCheckResult: '規則檢查結果', ruleCheckStatus_creatable: '可建立', diff --git a/frontend/src/lang/modules/zh.ts b/frontend/src/lang/modules/zh.ts index 965ca74e6..8e1dcc4c5 100644 --- a/frontend/src/lang/modules/zh.ts +++ b/frontend/src/lang/modules/zh.ts @@ -3875,8 +3875,9 @@ const message = { ruleTargetRequired: '请至少填写一个 IP 地址或端口', batchRuleLimit: '单次最多可创建 {0} 条规则', resolution_adopt: '纳管', + plan_duplicate_rules: '存在匹配条件和动作相同的重复规则，无法纳管。请手动删除重复规则后重试。', adoptRuleConfirm: '纳入管理后，1Panel 将负责维护和删除这条现有规则，是否继续？', - plan_exact_rule_conflict: '已存在匹配条件和优先级相同、但允许与拒绝动作相反的规则。', + plan_exact_rule_conflict: '已存在匹配条件相同、但允许与拒绝动作相反的规则。', allRulesAlreadyExist: '本次检查的 {0} 条规则均已存在，没有需要创建的新规则。', ruleCheckResult: '规则检查结果', ruleCheckStatus_creatable: '可创建', diff --git a/frontend/src/views/host/firewall/rule/index.vue b/frontend/src/views/host/firewall/rule/index.vue index 8f6080f5f..f1d8afcaa 100644 --- a/frontend/src/views/host/firewall/rule/index.vue +++ b/frontend/src/views/host/firewall/rule/index.vue @@ -1161,19 +1161,27 @@ const adoptRule = async (row: RuleRow) => { } loading.value = true; try { - const plan = (await checkFirewallRules({ items: [{ rule: row.rule }] })).data.items[0]; + const plan = (await checkFirewallRules({ items: [{ rule: row.rule, adoptLocator: row.observed.locator }] })) + .data.items[0]; + if (plan.decision === 'no_change' && plan.existingRuleUUID) { + MsgSuccess(i18n.global.t('commons.msg.operationSuccess')); + await search(); + return; + } + if (plan.reason === 'duplicate_rules') { + MsgError(i18n.global.t('firewall.plan_duplicate_rules')); + return; + } + if (plan.decision === 'blocked' && plan.classification === 'exact_managed') { + MsgError(i18n.global.t('commons.rule.duplicate')); + return; + } if (plan.decision !== 'confirmation_required' || plan.classification !== 'exact_external') { MsgError(i18n.global.t('firewall.plan_blocked')); return; } - const candidate = plan.candidates?.find( - (item) => - item.locator.position === row.observed?.locator.position && - item.locator.nativeId === row.observed?.locator.nativeId && - item.locator.canonical === row.observed?.locator.canonical, - ); - const resolution: Firewall.ApplicableCheckAction = plan.candidates?.length === 1 ? 'adopt' : 'select_adopt'; - if (resolution === 'select_adopt' && !candidate?.instanceKey) { + const candidate = plan.candidates?.[0]; + if (plan.candidates?.length !== 1 || !candidate?.instanceKey) { MsgError(i18n.global.t('firewall.plan_blocked')); return; } @@ -1182,9 +1190,8 @@ const adoptRule = async (row: RuleRow) => { items: [ { checkFlag: plan.checkFlag, - action: resolution, - adoptInstanceKey: - resolution === 'select_adopt' ? candidate?.instanceKey : plan.candidates?.[0]?.instanceKey, + action: 'adopt', + adoptInstanceKey: candidate.instanceKey, rule: plan.requestedRule, sourceKind: 'user', }, @@ -1204,6 +1211,11 @@ const adoptRule = async (row: RuleRow) => { const removeRule = (row: RuleRow) => removeRules([row]); +const canEditDescription = (row: Firewall.InventoryItem) => + Boolean(row.desired?.uuid) && + (row.desired?.origin === 'created' || row.desired?.origin === 'adopted') && + !row.desired?.protected; + const isEditableManagedRule = (row: Firewall.InventoryItem) => Boolean(row.desired?.uuid) && !row.desired?.expanded && @@ -1238,7 +1250,7 @@ const displayRulePriority = (row: Firewall.InventoryItem) => { }; const openEdit = (row: RuleRow) => { - if (!isEditableManagedRule(row)) return; + if (!isEditableManagedRule(row) && !canEditDescription(row)) return; const currentPosition = row.observed?.locator.position || row.rule.orderIndex || 1; const range = positionRanges.value[row.rule.scope.family] || { min: currentPosition, max: currentPosition }; ruleOperateRef.value?.acceptParams( @@ -1246,6 +1258,7 @@ const openEdit = (row: RuleRow) => { row, provider.value === 'firewalld' ? positionRanges.value : { [row.rule.scope.family]: range }, supportsFirewalldPriority.value, + !isEditableManagedRule(row), ); }; @@ -1268,7 +1281,7 @@ const operationButtons = [ label: i18n.global.t('commons.button.edit'), permission: true, nodeAdmin: true, - show: (row: RuleRow) => isEditableManagedRule(row), + show: (row: RuleRow) => isEditableManagedRule(row) || canEditDescription(row), click: openEdit, }, { diff --git a/frontend/src/views/host/firewall/rule/operate/index.vue b/frontend/src/views/host/firewall/rule/operate/index.vue index 31f5fdf95..53b36c996 100644 --- a/frontend/src/views/host/firewall/rule/operate/index.vue +++ b/frontend/src/views/host/firewall/rule/operate/index.vue @@ -15,7 +15,7 @@ :rules="rules" > - + {{ $t('firewall.accept') }} @@ -25,7 +25,7 @@ - + @@ -43,6 +43,7 @@ ref="sourceAddressRefs" v-model.trim="item.address" class="source-address-select" + :disabled="descriptionOnly" clearable :placeholder="$t('firewall.sourceAddressPlaceholder')" @keyup.enter.prevent="addSourceAddressOnEnter(index)" @@ -63,7 +64,7 @@ v-model.trim="form.destinationPorts[index]" class="destination-port-input" clearable - :disabled="!portProtocol" + :disabled="descriptionOnly || !portProtocol" :placeholder="$t('firewall.destinationPortPlaceholder')" @keyup.enter.prevent="addDestinationPortOnEnter(index)" > @@ -71,24 +72,30 @@ - + {{ $t('commons.button.add') }} - {{ priorityMin }} ~ {{ priorityMax }} + {{ priorityMin }} ~ {{ priorityMax }} @@ -172,7 +179,15 @@ :disabled="loading || (showingPreview && hasBlockingRules)" @click="onCheckOrSubmit" > - {{ $t(showingPreview ? 'commons.button.submit' : 'commons.button.check') }} + {{ + $t( + showingPreview + ? 'commons.button.submit' + : directEdit + ? 'commons.button.save' + : 'commons.button.check', + ) + }} @@ -200,7 +215,9 @@ import { const provider = ref('iptables'); const mode = ref<'create' | 'edit'>('create'); const editingUUID = ref(''); +const descriptionOnly = ref(false); const editingRule = ref(); +const originalFormRule = ref(); const drawerVisible = ref(false); const loading = ref(false); const formRef = ref(); @@ -307,6 +324,7 @@ const priorityMax = computed(() => Math.min(...selectedPositionRanges.value.map( const showingPreview = computed(() => previewVisible.value && previewRules.value.length > 0); const supportedPlanReasons = new Set([ + 'duplicate_rules', 'exact_rule_conflict', 'managed_rule_drifted', 'opaque_rule_in_target_scope', @@ -452,7 +470,9 @@ const resetForm = () => { form.priority = undefined; form.description = ''; editingUUID.value = ''; + descriptionOnly.value = false; editingRule.value = undefined; + originalFormRule.value = undefined; resetBatch(); formRef.value?.clearValidate(); }; @@ -462,14 +482,16 @@ const acceptParams = ( item?: Firewall.InventoryItem, ranges: Partial> = {}, supportsExplicitPriority = true, + onlyDescription = false, ) => { provider.value = value; firewalldPrioritySupported.value = supportsExplicitPriority; positionRanges.value = ranges; mode.value = item?.desired?.uuid ? 'edit' : 'create'; resetForm(); + descriptionOnly.value = mode.value === 'edit' && onlyDescription; if (mode.value === 'edit' && item?.desired?.uuid) { - const rule = item.rule; + const rule = descriptionOnly.value ? item.desired.rule : item.rule; const currentPosition = item.observed?.locator.position || rule.orderIndex; editingUUID.value = item.desired.uuid; editingRule.value = { @@ -492,6 +514,7 @@ const acceptParams = ( form.action = rule.action === 'reject' ? 'drop' : rule.action; form.priority = provider.value === 'firewalld' ? rule.priority : currentPosition || priorityMax.value; form.description = rule.description || ''; + originalFormRule.value = buildRule(); } drawerVisible.value = true; }; @@ -546,9 +569,11 @@ const buildRule = ( ? 'ipv6' : protocol === 'icmp' ? 'ipv4' - : provider.value === 'firewalld' - ? 'inet' - : source.family; + : mode.value === 'edit' + ? editingRule.value?.scope.family || source.family + : provider.value === 'firewalld' + ? 'inet' + : source.family; const action = mode.value === 'edit' && editingRule.value?.action === 'reject' && form.action === 'drop' ? 'reject' @@ -577,7 +602,7 @@ const buildRule = ( chain: 'incoming', direction: 'input', }, - protocol, + protocol: mode.value === 'edit' && provider.value === 'ufw' && protocol === 'tcp/udp' ? 'all' : protocol, sourceAddress: isWildcardAddress(source.family, source.address) ? '' : source.address, sourcePort: form.sourcePort, destinationAddress: form.destinationAddress, @@ -601,10 +626,20 @@ const editableFieldLabels: Array<[keyof Firewall.Rule, string]> = [ ['description', 'commons.table.description'], ]; -const changedFieldLabels = (before: Firewall.Rule, after: Firewall.Rule) => +const changedRuleFields = (before: Firewall.Rule, after: Firewall.Rule) => editableFieldLabels .filter(([field]) => JSON.stringify(before[field] ?? '') !== JSON.stringify(after[field] ?? '')) - .map(([, label]) => i18n.global.t(label)); + .map(([field]) => field); + +const canSaveWithoutCheck = (before: Firewall.Rule, after: Firewall.Rule) => + changedRuleFields(before, after).every((field) => ['description', 'orderIndex', 'priority'].includes(field)); + +const directEdit = computed( + () => + mode.value === 'edit' && + (descriptionOnly.value || + Boolean(originalFormRule.value && canSaveWithoutCheck(originalFormRule.value, buildRule()))), +); const availableResolutions = (result: Firewall.RuleCheckResult) => (result.allowedActions || []).filter((item): item is Firewall.ApplicableCheckAction => item !== 'cancel'); @@ -732,8 +767,38 @@ const prepareRulesFromForm = async () => { }; const checkRules = async () => { + if (descriptionOnly.value && editingUUID.value && originalFormRule.value) { + previewRules.value = [{ ...originalFormRule.value, description: form.description }]; + batchPlans.value = []; + await executeEdit(); + return; + } + if (directEdit.value && editingUUID.value && originalFormRule.value) { + const rule = buildRule(); + const changed = changedRuleFields(originalFormRule.value, rule); + if (changed.some((field) => field === 'priority' || field === 'orderIndex')) { + const value = previewRulePriority(rule); + if ( + typeof value !== 'number' || + !Number.isInteger(value) || + value < priorityMin.value || + value > priorityMax.value + ) { + MsgError(i18n.global.t('commons.rule.numberRange', [priorityMin.value, priorityMax.value])); + return; + } + } + previewRules.value = [rule]; + batchPlans.value = []; + await executeEdit(); + return; + } if (!(await prepareRulesFromForm())) return; if (mode.value === 'edit' && editingUUID.value) { + if (originalFormRule.value && canSaveWithoutCheck(originalFormRule.value, previewRules.value[0])) { + await executeEdit(); + return; + } const result = ( await checkFirewallRules({ items: [{ uuid: editingUUID.value, rule: previewRules.value[0] }], @@ -755,25 +820,42 @@ const executeEdit = async () => { return; } const updatedRule = previewRules.value[0]; - const changed = editingRule.value ? changedFieldLabels(editingRule.value, updatedRule) : []; + const before = originalFormRule.value || editingRule.value; + const changed = before ? changedRuleFields(before, updatedRule) : []; if (changed.length === 0) { drawerVisible.value = false; return; } - try { - await ElMessageBox.confirm( - i18n.global.t('firewall.editRuleConfirm', [changed.join(', ')]), - i18n.global.t('firewall.edit'), - { - confirmButtonText: i18n.global.t('commons.button.confirm'), - cancelButtonText: i18n.global.t('commons.button.cancel'), - type: 'warning', - }, - ); - } catch { - return; + if (!before || !canSaveWithoutCheck(before, updatedRule)) { + const labels = editableFieldLabels + .filter(([field]) => changed.includes(field)) + .map(([, label]) => i18n.global.t(label)); + try { + await ElMessageBox.confirm( + i18n.global.t('firewall.editRuleConfirm', [labels.join(', ')]), + i18n.global.t('firewall.edit'), + { + confirmButtonText: i18n.global.t('commons.button.confirm'), + cancelButtonText: i18n.global.t('commons.button.cancel'), + type: 'warning', + }, + ); + } catch { + return; + } } - await updateFirewallRule(editingUUID.value, { rule: updatedRule }); + let request: Firewall.UpdateRequest = { rule: updatedRule }; + if (before && canSaveWithoutCheck(before, updatedRule)) { + const description = changed.includes('description') ? updatedRule.description || '' : undefined; + if (changed.includes('priority')) { + request = { priority: updatedRule.priority!, description }; + } else if (changed.includes('orderIndex')) { + request = { orderIndex: updatedRule.orderIndex!, description }; + } else { + request = { description: description || '' }; + } + } + await updateFirewallRule(editingUUID.value, request); MsgSuccess(i18n.global.t('commons.msg.operationSuccess')); emit('search'); drawerVisible.value = false; diff --git a/frontend/src/views/host/firewall/sync/index.vue b/frontend/src/views/host/firewall/sync/index.vue index fe474fce1..ae0acfbc2 100644 --- a/frontend/src/views/host/firewall/sync/index.vue +++ b/frontend/src/views/host/firewall/sync/index.vue @@ -295,7 +295,6 @@ const syncReasonKeys: Record = { 'managed rule order differs from database sequence': 'managedOrderDiffers', 'managed rule exists only in target backend': 'managedOnlyInTarget', 'managed runtime rule cannot be safely removed': 'managedRuntimeCannotRemove', - 'managed rule order cannot cross external, opaque, or protected rules': 'managedOrderBlocked', 'rule is missing from target backend': 'missingFromTarget', 'target rule differs from database policy': 'targetDiffers', 'rule already exists in target backend': 'alreadyExistsInTarget',