Refactor chain router #83

New issue

Open

opened 2024-07-08 10:58:43 +00:00 by aarifullin · 0 comments

aarifullin commented

2024-07-08 10:58:43 +00:00

Member

The current default chain router implementation has problems and this issue is to discuss them and suggest solutions.

Local overrides allows to brutely override chains in Policy contract. That means if a user intended to restrict the access to his container, an owner of a storage node may use control service and override such policy. It's okay if the storage node denies any requests for this container, but the user can experience issues as soon as the owner sets allowing overrides for the container - is that ok or policy-engine must ignore non-deny local overrides? If it's so, then IsAllowed should be fixed like that:

func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r resource.Request) (chain.Status, bool, error) {
    localStatus, localRuleFound, err := dr.checkLocal(name, rt, r)
    // Deny statuses are returned immediately
    if err != nil || (localRuleFound && localStatus != chain.NoRuleFound && localStatus != chain.Allow) {
        return localStatus, localRuleFound, err
    }

    morphStatus, morphRuleFound, err := dr.checkMorph(name, rt, r)
    // Deny statuses are returned immediately.
    if err != nil || (morphRuleFound && morphStatus != chain.NoRuleFound && morphStatus != chain.Allow) {
        return morphStatus, morphRuleFound, err
    }
   
    // ignore other local override statuses
    return morphStatus, morphRuleFound, nil
}

The current [default chain router](https://git.frostfs.info/TrueCloudLab/policy-engine/src/commit/ac965e8d176a74cc501a1d20cd89b6ee7feaaca5/pkg/engine/chain_router.go#L8-L12) implementation has problems and this issue is to discuss them and suggest solutions. Local overrides allows to *brutely* override chains in `Policy` contract. That means if a user intended to restrict the access to his container, an owner of a storage node may use *control service* and override such policy. It's okay if the storage node **denies** any requests for this container, but the user can experience issues as soon as the owner sets allowing overrides for the container - is that ok or policy-engine must ignore non-deny local overrides? If it's so, then `IsAllowed` should be fixed like that: ```go func (dr *defaultChainRouter) IsAllowed(name chain.Name, rt RequestTarget, r resource.Request) (chain.Status, bool, error) { localStatus, localRuleFound, err := dr.checkLocal(name, rt, r) // Deny statuses are returned immediately if err != nil || (localRuleFound && localStatus != chain.NoRuleFound && localStatus != chain.Allow) { return localStatus, localRuleFound, err } morphStatus, morphRuleFound, err := dr.checkMorph(name, rt, r) // Deny statuses are returned immediately. if err != nil || (morphRuleFound && morphStatus != chain.NoRuleFound && morphStatus != chain.Allow) { return morphStatus, morphRuleFound, err } // ignore other local override statuses return morphStatus, morphRuleFound, nil } ```