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

CLI commands should not echo responses to the gateway log at INFO level #421

Closed
pcuzner opened this issue Feb 5, 2024 · 4 comments · Fixed by #569
Closed

CLI commands should not echo responses to the gateway log at INFO level #421

pcuzner opened this issue Feb 5, 2024 · 4 comments · Fixed by #569
Assignees

Comments

@pcuzner
Copy link
Contributor

pcuzner commented Feb 5, 2024

On version 1.0.0, I noticed that the gw log file contains the output of the command, from the grpc.py module

As an example, the here's the connections list references

for the command being received

self.logger.info(f"Received request to list connections for {request.subsystem}, context: {context}")

And the output returned

self.logger.info(f"list_connections get_qpairs: {qpair_ret}")

Here's an example using subsystem list

INFO:nvmeof:Received request to list the subsystem, context: <grpc._server._Context object at 0x7f331696b220>
INFO:nvmeof:list_subsystems: [{'nqn': 'nqn.2016-06.io.spdk:cnode1', 'subtype': 'NVMe', 'listen_addresses': [{'transport': 'TCP', 'trtype': 'TCP', 'adrfam': 'IPv4', 'traddr': '192.168.122.68', 'trsvcid': '4420'}], 'allow_any_host': False,'hosts': [{'nqn': 'nqn.2014-08.org.nvmexpress:uuid:d7a44e1c-d712-4016-bde1-a484ada4a668'}], 'serial_number': 'SPDK16261751890748', 'model_number': 'SPDK bdev Controller', 'max_namespaces': 256, 'min_cntlid': 1, 'max_cntlid': 65519, 'namespaces': [{'nsid': 1, 'bdev_name': 'bdev_0f756283-0210-4469-b628-fd3aa57d2916', 'name': 'bdev_0f756283-0210-4469-b628-fd3aa57d2916', 'nguid': '0F75628302104469B628FD3AA57D2916', 'uuid': '0f756283-0210-4469-b628-fd3aa57d2916'}]}]
INFO:nvmeof:Subsystem nqn.2016-06.io.spdk:cnode1 enable_ha: False
@caroav
Copy link
Collaborator

caroav commented Feb 5, 2024

OK we need to print the output of the CLIs to the log only in debug level. By default we should not make the log so noisy.

@pcuzner
Copy link
Contributor Author

pcuzner commented Feb 6, 2024

Depending on content, it's probably also something that would get flagged in a security audit.

@gbregman gbregman self-assigned this Apr 4, 2024
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Apr 4, 2024
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Apr 4, 2024
@caroav
Copy link
Collaborator

caroav commented Apr 4, 2024

@pcuzner here is what I think we need to do:

  1. We do need to see the "requests" in INFO, e.g. received request to list the subsystem.
  2. We don't need to see the "response" in INFO => make it DBEUG.
  3. Internal logic should stay as is in terms of logging levels. If need to change anything, it will be in the context of other issues.

@pcuzner
Copy link
Contributor Author

pcuzner commented Apr 4, 2024

@caroav Sounds good. Its would be good if the log entry generated to record a request has been received also included the from IP - this would help if there are any bad actors at work spamming the endpoint.

gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Apr 8, 2024
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Apr 8, 2024
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in NVMe-oF Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
3 participants