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

[Rust] API reports no error when scan lookup fails #1744

Open
mryellow opened this issue Oct 28, 2024 · 10 comments
Open

[Rust] API reports no error when scan lookup fails #1744

mryellow opened this issue Oct 28, 2024 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@mryellow
Copy link

mryellow commented Oct 28, 2024

Expected behavior

When a scan is not found, or contains invalid data the backend warns in logs, however the API fails to pass on this warning and instead reports everything being fine. This gives clients no opportunity to stop checking for updates or retry.

  • When backend can not find a scan; response code should be 404
  • When backend chokes on invalid data; response code should be 4xx
  • When backend is in an failed state; response code should be 5xx

Actual behavior

/results and /status API continue to respond with 200 saying that the scan is running. i.e. status": "running"

Errors and warnings on the backend then seem to cascade into greater issues which result in server being inaccessible.

Steps to reproduce

Start scans until one breaks.

GVM versions

gsa: (gsad --version)

Greenbone Security Assistant 22.04.1

gvm: (gvmd --version)

Greenbone Vulnerability Manager 22.4.2
Manager DB revision 250
Copyright (C) 2009-2021 Greenbone Networks GmbH
License: AGPL-3.0-or-later
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

openvas: (openvas --version)

OpenVAS 22.4.1
gvm-libs 22.4.4
Most new code since 2005: (C) 2022 Greenbone Networks GmbH
Nessus origin: (C) 2004 Renaud Deraison <[email protected]>
License GPLv2: GNU GPL version 2
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

gvm-libs:

openvas-smb:

ospd-openvas: (ospd-openvas --version)

Environment

Operating system:

Linux XXXXX 5.15.0-107-generic #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04.4 LTS"

Installation method / source: (packages, source installation)

Source.

Logfiles

2024-10-28T22:51:28.533478Z DEBUG openvasd::controller::entry: process call method=GET path="/scans/e66432cd-181c-43f6-97b6-1728e756b400/results"
2024-10-28T22:51:28.533723Z DEBUG from_path{path="/scans/e66432cd-181c-43f6-97b6-1728e756b400/status" mode=Service}: openvasd::controller::entry: Scan endpoint enabled mode=Service path="/scans/e66432cd-181c-43f6-97b6-1728e756b400/status"
2024-10-28T22:51:28.533810Z DEBUG openvasd::controller::entry: process call method=GET path="/scans/e66432cd-181c-43f6-97b6-1728e756b400/status"
2024-10-28T22:51:28.534136Z DEBUG ok_byte_stream: openvasd::response: starting to send values
2024-10-28T22:51:28.534305Z DEBUG ok_byte_stream: openvasd::response: end send values
2024-10-28T22:51:28.926480Z  WARN openvasd::scheduling: unable to fetch results scan_id=e66432cd-181c-43f6-97b6-1728e756b400 e=Unexpected issue: ReadXML("invalid number")
2024-10-28T22:51:29.429674Z  WARN openvasd::scheduling: unable to fetch results scan_id=e66432cd-181c-43f6-97b6-1728e756b400 e=Unexpected issue: ReadXML("invalid number")

Possibly then causes the socket to fail:

2024-10-28T22:51:56.923084Z  WARN openvasd::scheduling: unable to fetch results scan_id=e66432cd-181c-43f6-97b6-1728e756b400 e=Unexpected issue: OSPD socket /run/ospd/ospd-openvas.sock does not exist.

Which then ends up in a loop repeating the same log over and over:

2024-10-28T22:56:08.424993Z  WARN openvasd::scheduling: unable to fetch results scan_id=e66432cd-181c-43f6-97b6-1728e756b400 e=Unexpected issue: InvalidResponse(Status { text: "Failed to find scan 'e66432cd-181c-43f6-97b6-1728e756b400'", code: StringU32(404) })
@mryellow mryellow added the bug Something isn't working label Oct 28, 2024
@mryellow mryellow changed the title Rust API reports no error when scan lookup fails [Rust] API reports no error when scan lookup fails Oct 29, 2024
@nichtsfrei
Copy link
Member

Thank you for filing this bug, we highly appreciate it.

When the OSPD socket is unreachable, the loop of warnings persists. However, in this case, scans should not remain in the 'OK' state. It’s unclear if an immediate abort is appropriate, as there could still be results left in Redis if ospd-openvas has restarted. Maybe we should introduce a new state.

Aside from the incorrect behavior in openvasd, could you retrieve the result from ospd-openvas and include the XML that triggered the ReadXML("invalid number") error?

For mitigation did you try to run openvasd in 'openvas' mode rather than ospd mode?

@mryellow
Copy link
Author

could you retrieve the result from ospd-openvas and include the XML that triggered the ReadXML("invalid number") error?

I'm using the default memory store.

Agree that need to see what is causing the error, what is in the XML. Just haven't put the time in that direction yet.

What would be a good setup where I can access those interim XML files?

@mryellow
Copy link
Author

For mitigation did you try to run openvasd in 'openvas' mode rather than ospd mode?

Toyed around with it near the beginning. Will get back to you tomorrow.

@nichtsfrei
Copy link
Member

nichtsfrei commented Oct 29, 2024

when you're using ospd mode, openvasd delegates each call to ospd-openvas. To get the XML reply of ospd you need to send the following XML to the configured ospd socket (/run/ospd/ospd-openvas.sock):

 <get_scans scan_id="f14747d3-a4d7-4e79-99bb-a0a1276cb78c"
                details="1"
                pop_results="0"/>

Replace scan_id with the actual scan_id.

@nichtsfrei
Copy link
Member

nichtsfrei commented Oct 30, 2024

This pull request modifies the error handling by setting the scan status to 'failed' when an occurs while fetching results. I decided against your proposal to return a 500 error and opted for a 206 status code instead. Although this is a backend issue and, in your case, no results are possible due to a deserialization bug in the OSP response, partial results may exist from previous successful fetch attempts. To indicate incomplete data, we will return a 206 status when the scan status is 'failed' on /scans/id/results.

That's open for negotiation.

@mryellow
Copy link
Author

I did notice that it would be a status of 'failed' at the end of the day.

Easy to argue that is in fact a 2xx response.

There is the case of "Scan not found" more broadly though, do still wonder if when there is no status to return then a 404 might work.

@nichtsfrei
Copy link
Member

There is the case of "Scan not found" more broadly though, do still wonder if when there is no status to return then a 404 might work.

That's a fair point. For now we decided that we don't want to change the API definition and will return a 200 until a product decision was made.

Did you manage to get the xml response that triggered that invalid integer exception to begin with?

@mryellow
Copy link
Author

mryellow commented Nov 4, 2024

Did you manage to get the xml response that triggered that invalid integer exception to begin with?

Haven't got there yet. Some priorities ahead of it but will get there next few days.

@nichtsfrei
Copy link
Member

After testing a bit more ScanResult struct of ospd has been changed, can you verify if it resolves the issue?

#1754

@mryellow
Copy link
Author

can you verify

Project priorities switched around for a week or two. Will get back to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants