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

Possible incorrect error when a container is deleted by a static session token without the right permissions #2466

Closed
vvarg229 opened this issue Aug 1, 2023 · 5 comments · Fixed by #2727
Assignees
Labels
bug Something isn't working I4 No visible changes neofs-cli NeoFS CLI application issues S4 Routine security Affects security U1 Critically important to resolve quickly
Milestone

Comments

@vvarg229
Copy link
Contributor

vvarg229 commented Aug 1, 2023

I write tests on the issue nspcc-dev/neofs-testcases#579
and I found that neofs-cli container delete handle differently in the negative case when a non-valid wallet is specified.
The difference is which static session token I use - the one created on the container owner's wallet, or the one created on another wallet.

Expected Behavior

I'm not sure it's exactly a bug, I'd like to discuss what behavior we expect in which case.

Steps to Reproduce (for bugs)

  1. Create container
  2. Create static session token using container owner wallet
  3. Create static session token using another wallet
  4. Try to delete container using static session token from step 2 - you'll get a "timed out after 90 seconds" error.
  5. Try to delete container using static session token from step 3 - you'll get a "session issuer differs with the container owner" error.

See allure reports:

report.zip
report_2.zip

Test code:

    @allure.title("Not owner and not trusted party can NOT delete container")
    def test_static_session_token_container_delete_only_trusted_party_proved_by_the_container_owner(
            self,
            owner_wallet: WalletFile,
            user_wallet: WalletFile,
            stranger_wallet: WalletFile,
            scammer_wallet: WalletFile,
            static_sessions: dict[ContainerVerb, str],
            temp_directory: str,
            not_owner_wallet,
    ):
        with allure.step("Create container"):
            cid = create_container(
                owner_wallet.path,
                shell=self.shell,
                endpoint=self.cluster.default_rpc_endpoint,
            )

        user_token = self.static_session_token(owner_wallet, user_wallet, self.shell, temp_directory)
        stranger_token = self.static_session_token(user_wallet, stranger_wallet, self.shell, temp_directory)

        with allure.step("Try to delete container using scammer token"):
            with pytest.raises(RuntimeError, match=NOT_SESSION_CONTAINER_OWNER):
                delete_container(
                    wallet=scammer_wallet.path,
                    cid=cid,
                    session_token=stranger_token[ContainerVerb.DELETE],
                    shell=self.shell,
                    endpoint=self.cluster.default_rpc_endpoint,
                    await_mode=True,
                )

        with allure.step("Try to delete container using scammer token"):
            with pytest.raises(RuntimeError, match=NOT_SESSION_CONTAINER_OWNER):
                delete_container(
                    wallet=scammer_wallet.path,
                    cid=cid,
                    session_token=user_token[ContainerVerb.DELETE],
                    shell=self.shell,
                    endpoint=self.cluster.default_rpc_endpoint,
                    await_mode=True,
                )

Your Environment

neo-go
0.101.1
neofs-s3-authmate
0.27.1
neofs-cli
0.36.1-125-g8cda197e
neofs-adm
0.36.1-125-g8cda197e
AWS
aws-cli/2.9.19 Python/3.11.4 Linux/6.2.0-25-generic source/x86_64.ubuntu.23 prompt/off

@cthulhu-rider
Copy link
Contributor

I'd like to discuss what behavior we expect in which case

expected behavior is described in nspcc-dev/neofs-testcases#579, so any other behaviors are incorrect (bug)

@vvarg229
Copy link
Contributor Author

vvarg229 commented Aug 2, 2023

I'd like to discuss what behavior we expect in which case

expected behavior is described in nspcc-dev/neofs-testcases#579, so any other behaviors are incorrect (bug)

Yes, but there's no description of expected errors

@cthulhu-rider
Copy link
Contributor

I'd like to discuss what behavior we expect in which case

expected behavior is described in nspcc-dev/neofs-testcases#579, so any other behaviors are incorrect (bug)

Yes, but there's no description of expected errors

added nspcc-dev/neofs-testcases#579 (comment)

@vvarg229
Copy link
Contributor Author

@cthulhu-rider What is this new error message?
Why is it not timed out after 90 seconds?

COMMAND: neofs-cli --config /home/runner/work/neofs-node/neofs-node/neofs-testcases/wallet_config.yml container delete --rpc-endpoint 's01.neofs.devenv:8080' --wallet '/home/runner/work/neofs-node/neofs-node/neofs-testcases/TemporaryDir/f08b207a-0989-4410-a928-e20b2a50d761.json' --cid '2qg15aNao5nFsVn6ewRAoAC2DoxcYWniNuamqMmGapmS' --await --session '/home/runner/work/neofs-node/neofs-node/neofs-testcases/TemporaryDir/TestFilesDir/96426527-7d23-4eab-b397-acc85d73eccc' --force
RETCODE: 1

STDOUT:
container removal request accepted for processing (the operation may not be completed yet)
awaiting...
container removal was requested, but timeout has happened while waiting for the outcome

https://http.t5.fs.neo.org/86C4P6uJC7gb5n3KkwEGpXRfdczubXyRNW5N9KeJRW73/315-1692200357/index.html#suites/bd4768b761544a5d33cf712e8068510b/de594002d37709e2/

@roman-khimov
Copy link
Member

It's the same thing (see #2501 as well).

@roman-khimov roman-khimov added bug Something isn't working security Affects security U1 Critically important to resolve quickly S4 Routine I4 No visible changes and removed triage labels Dec 21, 2023
@roman-khimov roman-khimov added this to the v0.40.0 milestone Jan 15, 2024
@carpawell carpawell self-assigned this Jan 22, 2024
carpawell added a commit that referenced this issue Jan 22, 2024
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this issue Jan 22, 2024
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this issue Jan 22, 2024
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this issue Jan 22, 2024
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this issue Jan 23, 2024
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this issue Jan 23, 2024
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
carpawell added a commit that referenced this issue Jan 25, 2024
If container session token is incorrect, there is no need in Alphabet's check,
the error can be returned immediately to a client; otherwise he has to wait for
the TX to persist infinitely with not feedback. Closes #2466.

Signed-off-by: Pavel Karpy <carpawell@nspcc.ru>
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-cli NeoFS CLI application issues S4 Routine security Affects security U1 Critically important to resolve quickly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants