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

How to check the relationship between requested object with session token object? #1420

Open
cthulhu-rider opened this issue May 23, 2022 · 4 comments
Labels
bug Something isn't working I4 No visible changes neofs-storage Storage node application issues S2 Regular significance U3 Regular

Comments

@cthulhu-rider
Copy link
Contributor

In current implementation ACL service uses object ID from session token context for access control. As I remember, this was done in order to grant access to parties which assemble the object (request generation). However, storage node does not check the relationship between the object from the token and the explicitly requested object. This gives rise to potential access control violations in the system when a token issued to another object is used to obtain an object.

I propose to discuss possible approaches to fix this problem. Here are some solutions from my mind:

  1. Let it be as it is
  2. Explicit checking of object association (this generally requires network requests, which can lead to new access questions)
  3. Gather split chains and write them into session token (removes the problem completely, at the same time complicates the formation of a token)
@cthulhu-rider cthulhu-rider added bug Something isn't working question Further information is requested discussion Open discussion of some problem triage neofs-storage Storage node application issues labels May 23, 2022
@cthulhu-rider
Copy link
Contributor Author

cthulhu-rider commented May 26, 2022

Discussed with @realloc @fyrchik @carpawell

2nd looks attractive. We need to make object list in object session context repeated (backward compatible change). We also need to provide iteration over object split-chain similar to the assembly of an object.

With this approach we will tighten authorization and access control of generated requests.

Cc: @anatoly-bogatyrev @alexvanin

@alexvanin
Copy link
Contributor

From the service side, the cost of 2nd option is to have one/two extra requests. Is it possible to take advantage of nspcc-dev/neofs-api#220 in trusted environment and avoid setting object.IDs?

To issue session token we need to find all children, if they exist. Probably the fastest way is to find linking object and get info from there. It will be nice to have search filter to that.

Another option is to get SplitInfoError. In this case, if I remember correctly, app should send object.Head request to the container node. Don't think that it is a good idea to work with placements and network map manually.

cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 16, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 16, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.
Disclosed problem of object context inheritance will be solved within
nspcc-dev#1420.

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Sep 16, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.

Disclosed problem of object context inheritance will be solved within
nspcc-dev#1420.

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Oct 4, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.

Disclosed problem of object context inheritance will be solved within

Signed-off-by: Leonard Lyubich <[email protected]>
Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Oct 4, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.

Disclosed problem of object context inheritance will be solved within

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Oct 5, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.

Disclosed problem of object context inheritance will be solved within

Signed-off-by: Leonard Lyubich <[email protected]>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Oct 6, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.

Disclosed problem of object context inheritance will be solved within

Signed-off-by: Leonard Lyubich <[email protected]>
aprasolova pushed a commit to aprasolova/neofs-node that referenced this issue Oct 19, 2022
…uest

In previous implementation of `neofs-node` app object session was not
checked for substitution of the object related to it. Also, for access
checks, the session object was substituted instead of the one from the
request. This, on the one hand, made it possible to inherit the session
from the parent object for authorization for certain actions. On the
other hand, it covered the mentioned object substitution, which is a
critical vulnerability.

Next changes are applied to processing of all Object service requests:
 - check if object session relates to the requested object
 - use requested object in access checks.

Disclosed problem of object context inheritance will be solved within

Signed-off-by: Leonard Lyubich <[email protected]>
@roman-khimov roman-khimov added S2 Regular significance I4 No visible changes and removed triage question Further information is requested discussion Open discussion of some problem labels Dec 21, 2023
@roman-khimov
Copy link
Member

Related to #2667.

@roman-khimov
Copy link
Member

I think this is solved now. @cthulhu-rider?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working I4 No visible changes neofs-storage Storage node application issues S2 Regular significance U3 Regular
Projects
None yet
Development

No branches or pull requests

4 participants