45Drives / cockpit-file-sharing

A Cockpit plugin to easily manage samba and NFS file sharing.
GNU General Public License v3.0
556 stars 30 forks source link

Client Name field value containing a comma breaks access to NFS share #88

Closed Phx01 closed 5 months ago

Phx01 commented 1 year ago

Bug Info

Describe the bug Adding/editing an NFS share and filling (one or more of) the Client Name field(s) with a value containing a comma breaks access to the NFS shares from inside Cockpit.

To Reproduce Steps to reproduce the behavior:

  1. Create a share using a "Client Name"-value containing a comma (",")
  2. The share is created and all looks normal (one can even go back in and continue edit/remove the entries or add new ones)
  3. Reload the page using your Browser's reload function (e.g. F5)
  4. An error reading "Could not list /etc/exports.d/cockpit-file-sharing.exports. Is there a syntax error?" will overlay the configuration making it inaccessible.
  5. Manually opening the configuration file from the command line and removing the "extra-comma" from the client name(s) resolves the issue after reloading the page thereafter.

Expected behavior Either the "Client Name"-value should be stored inside the config file in a format allowing all feasible characters[*] for a common single-line field (e.g. one per line instead of comma-separated) or the field should disallow the usage of the field separator in use, either by auto-removing it when typed (or pasted) or alternatively highlighting it to the user (e.g. red-mark the field disallowing "Add"/"Apply" until resolved).

[*] "feasible characters" means any character a non-malicious person could perceivably use to ordinarily describe the access (potentially in their local language).

Client Side

Desktop (please complete the following information):

Server Side

Additional context In my specific case I used "IoT, Printers & handhelds" since part of that share is allowing devices in that DHCP section to reach it and the Client IP field uses a CIDR notation respectively.

joshuaboud commented 5 months ago

Cockpit File Sharing version 2.4.5 has been outdated since May 2022. Any issues with the Fedora version should be brought to the Fedora packaging team to have them update their packaged version.
However, I believe your issue stems from a misunderstanding (and poor naming on our part) of the "Client Name" field. This field is actually for the host names, IP addresses, or netgroups that are allowed to connect to the share. See https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/5/html/deployment_guide/s1-nfs-server-config-exports for what the host name field can/should be. And FYI the input field label was changed to "Host" in current File Sharing versions (3.*) for better clarity.

Phx01 commented 5 months ago

@joshuaboud

Cockpit File Sharing version 2.4.5 has been outdated since May 2022. Any issues with the Fedora version should be brought to the Fedora packaging team to have them update their packaged version.

Sure can do.

However, I believe your issue stems from a misunderstanding (and poor naming on our part) of the "Client Name" field. This field is actually for the host names, IP addresses, or netgroups that are allowed to connect to the share.

Since I have "Client Name", "Client IP" and "Options" within Cockpit where the "Client Name" is actually a comment inside the "/etc/exports.d/cockpit-file-sharing.exports" file, I believe that this is not a misunderstanding on my side. The field for the host names and IP addresses is the "Client IP" field, which is used as such and works as outlined. Just to clarify: Even though Cockpit shows the error overlay, the configuration continues to work since the content of the "Client Name" field is simply a comment inside the exports file and as such not regarded by the bare configuration. Only Cockpit appears to be disturbed by it.

Understanding that v2.4.5 is outdated, if this field still exists in the current installment one way or another, it may need to be checked as outlined in my initial post.