Bug 201073 - [nfsclient] RPCSEC_GSS principal includes inappropriate directory path
Summary: [nfsclient] RPCSEC_GSS principal includes inappropriate directory path
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.1-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Rick Macklem
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-06-23 11:55 UTC by KAKIUCHI Masatoshi
Modified: 2015-12-03 12:37 UTC (History)
1 user (show)

See Also:
rmacklem: mfc-stable10+
rmacklem: mfc-stable9+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description KAKIUCHI Masatoshi 2015-06-23 11:55:33 UTC
NFS client requests invalid principal name for RPCSEC_GSS security.
The man page mount_nfs(8) explains that default principal name is nfs@<server-fqdn>, but NFS client uses nfs@<rhost>:<path> which includes inappropriate directory path.

Workaround:
Explicitly use principal option for mount_nfs command.

Fix:
===================================================================
--- sys/fs/nfsclient/nfs_clvfsops.c	(revision 284717)
+++ sys/fs/nfsclient/nfs_clvfsops.c	(working copy)
@@ -774,7 +774,7 @@
 	struct thread *td;
 	char hst[MNAMELEN];
 	u_char nfh[NFSX_FHMAX], krbname[100], dirpath[100], srvkrbname[100];
-	char *opt, *name, *secname;
+	char *opt, *name, *secname, *cp;
 	int nametimeo = NFS_DEFAULT_NAMETIMEO;
 	int negnametimeo = NFS_DEFAULT_NEGNAMETIMEO;
 	int minvers = 0;
@@ -1153,8 +1153,13 @@
 
 	if (vfs_getopt(mp->mnt_optnew, "principal", (void **)&name, NULL) == 0)
 		strlcpy(srvkrbname, name, sizeof (srvkrbname));
-	else
+	else {
 		snprintf(srvkrbname, sizeof (srvkrbname), "nfs@%s", hst);
+		cp = strchr(srvkrbname, ':');
+		if (cp != NULL) {
+			*cp = '\0';
+		}
+	}
 	srvkrbnamelen = strlen(srvkrbname);
 
 	if (vfs_getopt(mp->mnt_optnew, "gssname", (void **)&name, NULL) == 0)
Comment 1 Rick Macklem freebsd_committer freebsd_triage 2015-07-03 20:48:38 UTC
I'll take this one.
Btw, normally mount_nfs creates a "principal" argument even if the caller
hasn't specified one. (See line#590-600 of mount_nfs.c.)
The only way I can see that this would fail is if getaddrinfo() fails to
acquire a canonical hostname for the server.
--> As such, I suspect this bug only affects sites with unusual hostname
    configurations, such as not using DNS or ???
Comment 2 commit-hook freebsd_committer freebsd_triage 2015-07-03 22:11:43 UTC
A commit references this bug:

Author: rmacklem
Date: Fri Jul  3 22:11:08 UTC 2015
New revision: 285113
URL: https://svnweb.freebsd.org/changeset/base/285113

Log:
  If a "principal" argument isn't provided for a Kerberized NFS mount,
  the kernel would generate a bogus one with a ":/<path>" suffix.
  This would only occur for the case where there was no explicit
  "principal" argument and the getaddrinfo() call in mount_nfs.c failed to a
  return a cannonical name for the server.
  This patch fixes this unusual case.

  PR:		201073
  Submitted by:	masato@itc.naist.jp
  MFC after:	2 weeks

Changes:
  head/sys/fs/nfsclient/nfs_clvfsops.c
Comment 3 KAKIUCHI Masatoshi 2015-07-04 04:23:22 UTC
Thank you Rick,

I read line#590-600 of mount_nfs.c.
This code generates principal name from canonical hostname for the server
only if mount_nfs is executed with "sec=krb5/krb5i/krb5p" arguments.
If mount_nfs is executed without any of "sec" arguments,
mount_nfs negotiates which sec flavor to use with the NFS server (line #316-330, #830-865)
and creates a "sec" argument for nmount(2) from the results of the negotiation (line #882-886).

As a result, bad principal name is used
if we execute mount_nfs command without "sec" and "principal" arguments
for NFS servers which support krb5/krb5i/krb5p security flavors.
I met this situation.

I think it is better to remove checks for secflavor from line#590-600 of mount_nfs.c.

diff with stable/10:
Index: sbin/mount_nfs/mount_nfs.c
===================================================================
--- sbin/mount_nfs/mount_nfs.c	(revision 285123)
+++ sbin/mount_nfs/mount_nfs.c	(working copy)
@@ -591,8 +591,7 @@
 		 * For a Kerberized nfs mount where the "principal"
 		 * argument has not been set, add it here.
 		 */
-		if (got_principal == 0 && secflavor >= 0 &&
-		    secflavor != AUTH_SYS && ai_nfs->ai_canonname != NULL) {
+		if (got_principal == 0 && ai_nfs->ai_canonname != NULL) {
 			snprintf(pname, sizeof (pname), "nfs@%s",
 			    ai_nfs->ai_canonname);
 			build_iovec(iov, iovlen, "principal", pname,
Comment 4 Rick Macklem freebsd_committer freebsd_triage 2015-07-04 20:57:45 UTC
Well, for the case of secflavor == AUTH_SYS, you don't need to do it.
The case you're thinking of is when secflavor < 0, which means it hasn't
been set by a "sec" option yet.

I'll admit I don't think I even realized this "auto-negotiation" existed
for NFSv3 (I know it doesn't happen for NFSv4).

I did take a look in nfs_clvfsops.c and don't see any harm in setting a
"principal" argument for the AUTH_SYS case, although it won't be used.
(The code you changed for the first patch ends up filling in srvkrbname
 even when the "principal" argument doesn't exist.)

I think there might be cases where getaddrinfo() fails, so leaving the
first patch seems fine.

I'll think about adding one similar to your second one, which would allow
the secflavor < 0 case as well as AUTH_SYS to generate the "principal"
argument.

Have you tested the second patch to see if it works for you?
(I'll admit I don't have a Kerberized NFS setup handy at the moment to
 test with.)
Comment 5 KAKIUCHI Masatoshi 2015-07-06 14:29:21 UTC
Just like you said, when secflavor == AUTH_SYS, I does not need to take care principal.
But when secflavor < 0, necessity of principal depends on the result of negotiation.

When mount_nfs executed with "sec=krb5" argument and succeeded getaddrinfo(), mount_nfs generates principal argument with canonicalized server name.
When mount_nfs executed without any "sec" arguments and negotiated sec=krb5 with NFSv3 server, mount_nfs generates "sec=krb5" argument, then nfs_clvfsops.c generates principal argument with non-canonicalized server name.

I think it is better to canonicalize the server name by getaddrinfo() in both cases sec=krb5 is clearly specified and sec=krb5 is negotiated.
If this idea is not acceptable, I will withdraw my second patch.

I tested both patches.
The client and server are FreeBSD 10.1 and Solaris 11.2 respectively.
The first patch is good and the second patch is better than the first one on canonicalization.
But I admit this is not exhaustive tests.


====
NFS Client: FreeBSD 10.1R without patches
NFS Server: Solaris 11.2 exports with sec=krb5:krb5p:krb5i:sys

# mount_nfs -o gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=1692370,timeout=120,retrans=2

another shell:
# truss -s 2000 -p $(pgrep gssd)
(snip)
21.091720463 read(4,"\M^@\0\0\\h\M-v-e\0\0\0\0\0\0\0\^B@gss\0\0\0\^A\0\0\0\^E\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\^Znfs@<server-name>:/export/test\0\0\0\0\0\0\0\0\0\n*\M^FH\M^F\M-w\^R\^A\^B\^A\^D\0\0",9000) = 96 (0x60)
(snip)

gssd is requested wrong principal: "nfs@<server-name>:/export/test"

KDC log:

KDC is requested wrong principal: logged "<unknown server>"


# mount_nfs -o sec=krb5,gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=1692370,timeout=120,retrans=2

gssd is requested correct FQDN principal: "nfs@<server-fqdn>"
KDC is requested correct principal: "nfs/<server-fqdn>@<REALM>"


====
NFS Client: FreeBSD 10.1R + stable kernel with the first patch
NFS Server: Solaris 11.2 exports with sec=krb5:krb5p:krb5i:sys

# mount_nfs -o gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=4194304,timeout=120,retrans=2

gssd is requested non-FQDN principal: "nfs@<server-name>"
KDC is requested two unknowns principals and then correct principal: <unknown server> and "nfs/<server-fqdn>@<REALM>"


# mount_nfs -o sec=krb5,gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=4194304,timeout=120,retrans=2

gssd is requested correct FQDN principal: "nfs@<server-fqdn>"
KDC is requested correct principal: "nfs/<server-fqdn>@<REALM>"


====
NFS Client: FreeBSD 10.1R with the second patch
NFS Server: Solaris 11.2 exports with sec=krb5:krb5p:krb5i:sys

# mount_nfs -o gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=1692370,timeout=120,retrans=2

gssd is requested correct FQDN principal: "nfs@<server-fqdn>"
KDC is requested correct principal: "nfs/<server-fqdn>@<REALM>"


# mount_nfs -o sec=krb5,gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=1692370,timeout=120,retrans=2

gssd is requested correct FQDN principal: "nfs@<server-fqdn>"
KDC is requested correct principal: "nfs/<server-fqdn>@<REALM>"


====
NFS Client: FreeBSD 10.1R with the second patch + stable kernel with the first patch
NFS Server: Solaris 11.2 exports with sec=krb5:krb5p:krb5i:sys

# mount_nfs -o gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=4194304,timeout=120,retrans=2

gssd is requested correct FQDN principal: "nfs@<server-fqdn>"
KDC is requested correct principal: "nfs/<server-fqdn>@<REALM>"


# mount_nfs -o sec=krb5,gssname=root <server-name>:/export/test /mnt
# nfsstat -m
<server-name>:/export/test on /mnt
nfsv3,tcp,resvport,hard,cto,lockd,sec=krb5,acdirmin=3,acdirmax=60,acregmin=5,acregmax=60,nametimeo=60,negnametimeo=60,rsize=65536,wsize=65536,readdirsize=8192,readahead=1,wcommitsize=4194304,timeout=120,retrans=2

gssd is requested correct FQDN principal: "nfs@<server-fqdn>"
KDC is requested correct principal: "nfs/<server-fqdn>@<REALM>"
Comment 6 commit-hook freebsd_committer freebsd_triage 2015-07-07 23:42:04 UTC
A commit references this bug:

Author: rmacklem
Date: Tue Jul  7 23:41:26 UTC 2015
New revision: 285260
URL: https://svnweb.freebsd.org/changeset/base/285260

Log:
  Since the case where secflavor < 0 indicates the security flavor is
  to be negotiated, it could be a Kerberized mount. As such, filling
  in the "principal" argument using the canonized host name makes sense.
  If it is negotiated as AUTH_SYS, the "principal" argument is meaningless
  but harmless.

  Requested by:	masato@itc.naist.jp
  Tested by:	masato@itc.naist.jp
  PR:		201073
  MFC after:	1 month

Changes:
  head/sbin/mount_nfs/mount_nfs.c
Comment 7 Rick Macklem freebsd_committer freebsd_triage 2015-12-02 23:20:00 UTC
The patch that fixes this has now been MFC'd to stable/10 and stable/9.