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

SNMP API not working from CLI #4113

Closed
simonemainardi opened this issue Jul 6, 2020 · 6 comments
Closed

SNMP API not working from CLI #4113

simonemainardi opened this issue Jul 6, 2020 · 6 comments
Assignees

Comments

@simonemainardi
Copy link
Contributor

simonemainardi commented Jul 6, 2020

Examples at https://www.ntop.org/guides/ntopng/api/rest/examples_v1.html#add-an-snmp-device used to work when called from the CLI. They no longer work as _POST doesn't contain parsed parameters, just a payload.

Expected behavior is to have _POST populated as now the _GET is populated, thing that happens when the rest API is called from the browser.

@simonemainardi simonemainardi changed the title SNMP API not woring from CLI SNMP API not working from CLI Jul 6, 2020
@lucaderi
Copy link
Member

lucaderi commented Jul 6, 2020

@simonemainardi Questions

  • Shall I leave _GET populated or not?
  • Shall _POST["payload"] be left populated or removed after JSON parsing?

@simonemainardi
Copy link
Contributor Author

Shall I leave _GET populated or not?

Yes. Leaving _GET populated will guarantee all REST endpoints to continue working either if you pass them data with the GET as well as if you pass them data with the POST.

Special care is only needed by REST endpoint authors to make sure contents of _POST are used when setting stuff. This is fundamental to protect from CSRF.

Shall _POST["payload"] be left populated or removed after JSON parsing?

With reference to this endpoint, payload can safely be removed. If you are planning to globally remove payload, which IMHO is a good idea, then the following scripts will need to be adjusted as well:

scripts/lua/google_assistant_utils.lua:  local info, pos, err = json.decode(_POST["payload"], 1, nil)--I assume only ONE outputContext
scripts/lua/rest/get/timeseries/ts.lua:if _POST["payload"] ~= nil then
scripts/lua/rest/v1/get/timeseries/ts.lua:if _POST["payload"] ~= nil then
scripts/lua/modules/google_assistant_utils.lua:  local info, pos, err = json.decode(_POST["payload"], 1, nil)--I assume only ONE outputContext
scripts/lua/admin/manage_pool_members.lua:local assn = _POST["payload"]

lucaderi added a commit that referenced this issue Jul 6, 2020
- Removed unused google assistant scripts
- Cleaned up existing REST calls using _POST["payload"]

The _GET["parameter"] element is still present and it will be removed soon
@simonemainardi
Copy link
Contributor Author

After internal review:

  • All /rest/v1 APIs must use POST. No GET. Consequently, all REST API code must be changed from _GET to _POST.
  • REST api v1 doc and examples must be updated accordingly
  • POST parameters must be validated and put into _POST both if they are submitted as application-json (see REST v1 API examples) and if they are submitted as application/x-www-form-urlencoded (as currently done from the ntopng GUI)
  • _POST must contain individual keys, i.e., not a single 'payload'
  • CSRF validation must be handled as well. No CSRF validation must be done for get-only endpoints (i.e., /get). CSRF validation must be done for set-related endpoints (i.e., add/ bind/ delete/ edit/ set/). Same for the pro part.
  • CSRF validation must be done only if the REST API is accessed using the session cookie (i.e., when accessed from the ntopng web GUI), i.e.. Must NOT be done when the REST API is accessed with direct credentials (e.g., curl -u admin:admin) as with direct credentials no CSRF attack can be done.

lucaderi added a commit that referenced this issue Aug 3, 2020
@lucaderi
Copy link
Member

lucaderi commented Aug 3, 2020

Shall I leave _GET populated or not?

Yes. Leaving _GET populated will guarantee all REST endpoints to continue working either if you pass them data with the GET as well as if you pass them data with the POST.

Special care is only needed by REST endpoint authors to make sure contents of _POST are used when setting stuff. This is fundamental to protect from CSRF.

Shall _POST["payload"] be left populated or removed after JSON parsing?

With reference to this endpoint, payload can safely be removed. If you are planning to globally remove payload, which IMHO is a good idea, then the following scripts will need to be adjusted as well:

scripts/lua/google_assistant_utils.lua:  local info, pos, err = json.decode(_POST["payload"], 1, nil)--I assume only ONE outputContext
scripts/lua/rest/get/timeseries/ts.lua:if _POST["payload"] ~= nil then
scripts/lua/rest/v1/get/timeseries/ts.lua:if _POST["payload"] ~= nil then
scripts/lua/modules/google_assistant_utils.lua:  local info, pos, err = json.decode(_POST["payload"], 1, nil)--I assume only ONE outputContext
scripts/lua/admin/manage_pool_members.lua:local assn = _POST["payload"]

Left _GET and removed payload. Now you can complete the rest of the work

@simonemainardi simonemainardi added this to the 4.2 milestone Aug 24, 2020
@simonemainardi
Copy link
Contributor Author

solved

@RicterZ
Copy link
Contributor

RicterZ commented Nov 7, 2020

Hi all, google_assistant_utils.lua was used by assistant_test.lua. It's useful to me.
Now I add this file manually on my ntopng server but could it be added again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants