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

Gateway can deny bearer token accepted by NeoFS itself #192

Open
cthulhu-rider opened this issue Apr 1, 2024 · 2 comments
Open

Gateway can deny bearer token accepted by NeoFS itself #192

cthulhu-rider opened this issue Apr 1, 2024 · 2 comments
Labels
blocked Can't be done because of something bug Something isn't working I4 No visible changes S2 Regular significance U3 Regular

Comments

@cthulhu-rider
Copy link
Contributor

cthulhu-rider commented Apr 1, 2024

NeoFS bearer token was recently extended with new field (nspcc-dev/neofs-api#266). Now it has one more field that could be signed and transferred. I tried to use the latest token version w/o REST gateway upgrade (being done in #176) and see how it goes

Current Behavior

REST denies bearer token:

{"message":"get bearer token: invalid signature","type":"GW"}

while NeoFS accepts it.

Expected Behavior

REST accepts valid bearer token and op proceeds in the same way as when contacting directly to NeoFS

Possible Solution

this is a function that handles passed bearer token. As we can see, it:

  1. decodes token structure defined in SDK
  2. verifies the signature again via SDK

currently, signature mismatch is expected cuz:

  1. SDK unmarshaler "loses" all token fields that havent been added yet (issuer in this case)
  2. verification checks only decoded fields, i.e. known ones

next can be done to solve the problem:

  1. dont encode bearer token, just forward received token binary to the NeoFS. This is worth tbd regardless of token verification to reduce resource consumption
  2. support unknown fields in signature verification. If the fields are encoded correctly, the gate can order them and this will allow it to check the signature even without interpretation (which is obviously impossible cuz the fields are unknown)

Describe alternatives you've considered

  1. do nothing: keep gateways as much fresh as possible, indirectly this will hide the problem
  2. do not validate tokens by gateway at all. They are checked by NeoFS anyway. For "good" tokens this will increase system performance (no duplicated actions). At the same time, "bad" tokens will reach NeoFS more often in general. This could also be a gateway option in config

Steps to Reproduce

  1. run NeoFS Dev Env with REST gateway (default)
  2. create public container with extendable access rules. Policy can be any (valid) for this scenario
$ neofs-cli -r s01.neofs.devenv:8080 -w ../devenv/services/chain/node-wallet.json container create --policy 'REP 2 CBF 1' --basic-acl eacl-public-read-write --await
Enter password > 
container creation request accepted for processing (the operation may not be completed yet)
container ID: wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ
awaiting...
container has been persisted on sidechain
  1. add access rule to deny write ops for OTHERS
$ neofs-cli acl extended create --cid wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ --out eacl.json --rule 'deny put others'
$ neofs-cli -r s01.neofs.devenv:8080 -w ../devenv/services/chain/node-wallet.json container set-eacl --cid wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ --table eacl.json --await
Enter password > 
Checking the ability to modify access rights in the container...
ACL extension is enabled in the container, continue processing.
eACL modification request accepted for processing (the operation may not be completed yet)
awaiting...
EACL has been persisted on sidechain
  1. assert rule action with REST account which is "other"
$ neofs-cli -r s01.neofs.devenv:8080 -w ../devenv/services/rest_gate/wallet.json object put --cid wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ  --file Makefile
Enter password > 
 4218 / 4218 [============================================================================================================================================================================================================================================] 100.00% 0s
rpc error: finish object stream: status: code = 2048 message = access to object operation denied: access to operation OBJECT_PUT is denied by extended ACL check: denied by rule
  1. issue bearer token granting the access to the REST account on behalf of the container creator
$ neofs-cli acl extended create --cid wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ --rule 'allow put others' --out bearer_eacl.json
$ neofs-cli -r s01.neofs.devenv:8080 bearer create -i 0 -n 0 -x 102400 -e bearer_eacl.json -o NbUgTSFvPmsRxmGeWpuuGeJUoRoi6PErcM --out bearer_blank.json
$ neofs-cli util sign bearer-token --from bearer_blank.json --to bearer_signed -w ../devenv/services/chain/node-wallet.json 
Enter password > 
signed bearer token was successfully dumped to bearer.json
  1. try the token through NeoFS CLI
$ neofs-cli -r s01.neofs.devenv:8080 -w ../devenv/services/rest_gate/wallet.json object put --cid wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ  --file Makefile --bearer bearer_signed
Enter password > 
 4218 / 4218 [============================================================================================================================================================================================================================================] 100.00% 0s
[Makefile] Object successfully stored
  OID: ECqtkxzY6Kq8dvpanoaWgQebqS6odCwcZvqm22WLfL7b
  CID: wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ
  1. try the token through REST gateway
$ curl http://rest.neofs.devenv:8090/v1/upload/wLb8QEdU2oi8CE3aBCYNPpiS6fgLr7JP1ssgwAZwQRQ \
-F 'file=@Makefile;filename=Makefile' \
-b 'Bearer=CnYKNAoECAIQDRIiCiAN64Mq1FT0ytPc3Sz+e/uFfSVCFuR8bUgHERnb053N0xoICAMQASICCAMSGwoZNSSLrQB6gIku+V7oMAHXinR47Juhw3qlfhoECICgBiIbChk17p6iLCfjS9AUj8QQjgj3To9QSLLxlcpzEmcKIQKzYiv0AXvf4xfFiu1fTHU/IGt9uJYEb6fXdLvEv3+NwhJA50e/wldmNiqr2t9KSk4qbSdIFRKoTAOpljz/oeQVhYl+CHsS2A7ZNrLxlnhZCDZqty4nfjEZ91XdCiHt+6+XUBgB'
{"message":"get bearer token: invalid signature","type":"GW"}

Context

btw storage nodes are not forward compatible too. We can do same steps with upgraded REST and "old" nodes. Then output is:

{"code":1024,"message":"writer close: status: code = 1024 message = wrong signature","type":"API"}

but it consistent with NeoFS (CLI) itself:

rpc error: finish object stream: status: code = 1024 message = invalid signature

same problem may be encountered by any app which inherit NeoFS binary models

Regression

no: this worked (and still), but iff SDK revisions are synced

Your Environment

@cthulhu-rider cthulhu-rider added U3 Regular bug Something isn't working S3 Minimally significant I3 Minimal impact labels Apr 1, 2024
@roman-khimov
Copy link
Member

The desired behavior is likely:

  • for ordinary requests pass token as is, don't mess with it on the gateway
  • have additional API for token checks at the gateway

This is strongly related to nspcc-dev/neofs-api#241, I'd like to have a token that can be created on the user side without reliance on the REST gateway (which is technically possible, but not trivial at the moment, so REST users like Panel never do this). This would mean that REST should be more transparent, but it can provide some additional API specifically to check tokens as they're known to the gateway.

@roman-khimov roman-khimov added S2 Regular significance I4 No visible changes and removed S3 Minimally significant I3 Minimal impact labels Apr 1, 2024
@roman-khimov roman-khimov added this to the v0.8.4 milestone Apr 2, 2024
@smallhive smallhive self-assigned this Apr 15, 2024
@smallhive
Copy link
Contributor

There are some hidden things. The problem is deeper than it looks on the first sight. In fact the problem could be mirrored to node, if the gate would be updated first for the new version of SDK. In this case the gate approves the token, but node will say "invalid signature" error.

In the node we have the same place, where we have to unmarshal token to struct from binary or json representation. It is required to be able to pass the token to commands via parameters.

We can try to make this thing more transparent for the gate in case the gate will not interfere inside token. But this require new command API like WithBearerTokenBinary(token []byte) instead of WithBearerToken(t bearer.Token).

@smallhive smallhive removed their assignment Apr 15, 2024
@roman-khimov roman-khimov removed this from the v0.8.4 milestone Apr 15, 2024
@roman-khimov roman-khimov added the blocked Can't be done because of something label Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Can't be done because of something bug Something isn't working I4 No visible changes S2 Regular significance U3 Regular
Projects
None yet
Development

No branches or pull requests

3 participants