I wonder if it's ordinary thing or not for you. But I felt needs to report. I found the issue following: I set sharenfs dataset like this: #+begin_src zfs set sharenfs="192.168.1.55,ro=192.168.1.56" zroot/usr/src #+end src Of course I can open this on 192.168.1.55 with NFS. And then I set like this: #+begin_src zfs set sharenfs="ro=192.168.1.56" zroot/usr/src #+end_src I can open zroot/usr/src on 192.168.1.55 I thought this can be a security issue for many people. Thanks.
This appears to be a bug in mountd. -ro=192.168.1.56 is bogus, but mountd does not spot it as an error. Btw, both of your sharenfs settings are bogus, since -ro is not supposed to have "=192.168.1.56" after it. All the sharenfs property does is autogenerate an exports line. It can be found in /etc/zfs/exports and should obey the same syntax as "man 5 exports" describes. (Althougn the sharenfs parser in OpenZFS should be improved, the bogus case should not get past mountd.) I'll take a look at mountd.c and try to come up with a patch. I am not sure what you were trying to do by specifying one IP address followed by ro=anther-ip, but you cannot mix read/write and read-only exports on the same line. --> Until you have the very recent patch in PR#147881, you cannot generate multiple lines via sharenfs. --> To do multiple lines, you need to create them manually in /etc/exports and not use the sharenfs property. Thanks for reporting it, rick ps: And never put a "=XXX" after "ro" for an exports option.
I git-pulled the 14-STABLE code AM 6:26 JST. And I make-install-ed for the all machines that FreeBSD are installed. I checked the same operations that I had yesterday: mount 192.168.1.55:/usr/src /usr/src on 192.168.1.52 zfs set sharenfs="192.168.1.51,192.168.1.52,192.168.1.56" zroot/usr/src -> mounted zfs set sharenfs="192.168.1.51,ro=192.168.1.56" zroot/usr/src -> couldn't mount zfs set sharenfs="192.168.1.52,ro=192.168.1.56" zroot/usr/src -> mounted zfs set sharenfs="ro=192.168.1.56" zroot/usr/src -> error occured [tcp] 192.168.1.55:/usr/src: RPCPROG_NFS: RPC: Remote system error - Connection refused This error wasn't seen yesterday's operation. So I thought probably I had something like a mistake. Thanks.
Well, the "-ro" exports option is not supposed to have "=XXX" after it. There are only certain exports options that do have "=XXX". -network, -mask and -sec are the ones that come to mind. mountd should spot this error, generate a syslog error message for it and fail the exports line. I will take a look and come up with a patch. (mountd.c is actually code that was first written in 1987, believe it or not.) As I noted, all the ZFS sharenfs property does is generate an exports file in /etc/zfs/exports that is concatenated with /etc/exports by mountd.
(In reply to Toshio Kagiwada from comment #2) Just to make sure it is clear to you (and others reading this PR): zfs set sharenfs="192.168.1.51,192.168.1.52,192.168.1.56" zroot/usr/src This is valid syntax and exports 192.168.1.51, 192.168.1.52 and 192.168.1.56 read-write. zfs set sharenfs="192.168.1.51,ro=192.168.1.56" zroot/usr/src This in NOT valid syntax (there should never be a =192.168.1.56 after "ro") and mountd should fail this entry with an error. (The bug is that mountd does not fail the entry. I suspect it just ignores the =192.168.1.56, which is incorrect.) zfs set sharenfs="192.168.1.52,ro=192.168.1.56" zroot/usr/src Again, this is NOT valid syntax and mountd should fail it with an error. If you wish to export 192.168.1.56 read-only, the correct syntax is: zfs set sharenfs="-ro,192.168.1.56" zroot/usr/src
I found I had a mistake about NFS and ZFS settings. And I'd like to cancel this report in Bugzilla. Thank you for advice, Rick. I'll change the status to "closed" and "works as intended". Please fix this if I made a wrong choice.
(In reply to Toshio Kagiwada from comment #5) Nope, there is a real bug, as I have noted. (mountd does not fail the bogus exports line.) I will come up with a patch. Please leave the bug report open until the system is patched. (I will close it then.) rick
Created attachment 253366 [details] Check for a '=arg' after exports options This patch adds a check for an '=arg' after exports options that do not take a 'arg' and generates an error for these bogus entries in the exports file(s). This will avoid at least this case, where an improperly formed exports file (/etc/zfs/exports, generated by the ZFS sharenfs property) would be allowed by mountd. I will be committing this patch to main and MFC'ng it soon. In general, it is a good idea to look at the /etc/zfs/exports file generated by ZFS's sharenfs property, to ensure it is of the format you had intended it to be. (With reference to "man 5 exports".)
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=3df987c99d1194a0e43a84853e934aa0c0ab09db commit 3df987c99d1194a0e43a84853e934aa0c0ab09db Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2024-09-06 23:41:12 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2024-09-06 23:41:12 +0000 mountd: Add check for "=" after exports(5) options Some exports(5) options take a "=arg" component that provides an argument value for the option. Others do not. Without this patch, if "=arg" was provided for an option that did not take an argument value, the "=arg" was simply ignored. This could result in confusion w.r.t. what was being exported, as noted by the Problem Report. This patch adds a check for "=arg" for the options that do not take an argument value and fails the exports line if one is found. PR: 281003 MFC after: 2 weeks usr.sbin/mountd/mountd.c | 44 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-)
A commit in branch stable/14 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=c09ca8f43de12fecf701920675b793cbafba58c5 commit c09ca8f43de12fecf701920675b793cbafba58c5 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2024-09-06 23:41:12 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2024-09-27 22:13:49 +0000 mountd: Add check for "=" after exports(5) options Some exports(5) options take a "=arg" component that provides an argument value for the option. Others do not. Without this patch, if "=arg" was provided for an option that did not take an argument value, the "=arg" was simply ignored. This could result in confusion w.r.t. what was being exported, as noted by the Problem Report. This patch adds a check for "=arg" for the options that do not take an argument value and fails the exports line if one is found. PR: 281003 (cherry picked from commit 3df987c99d1194a0e43a84853e934aa0c0ab09db) usr.sbin/mountd/mountd.c | 44 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=132f5d03d358d89d4030de9173cd6ca5b4b48d68 commit 132f5d03d358d89d4030de9173cd6ca5b4b48d68 Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2024-09-06 23:41:12 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2024-09-27 22:18:46 +0000 mountd: Add check for "=" after exports(5) options Some exports(5) options take a "=arg" component that provides an argument value for the option. Others do not. Without this patch, if "=arg" was provided for an option that did not take an argument value, the "=arg" was simply ignored. This could result in confusion w.r.t. what was being exported, as noted by the Problem Report. This patch adds a check for "=arg" for the options that do not take an argument value and fails the exports line if one is found. PR: 281003 (cherry picked from commit 3df987c99d1194a0e43a84853e934aa0c0ab09db) usr.sbin/mountd/mountd.c | 44 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-)
The patch has now been commmitted and MFC'd.