git: b9410313c66d - stable/14 - nfscl/kgssapi: Fix Kerberized NFS mounts to pNFS servers

From: Rick Macklem <rmacklem_at_FreeBSD.org>
Date: Sun, 24 Dec 2023 01:06:25 UTC
The branch stable/14 has been updated by rmacklem:

URL: https://cgit.FreeBSD.org/src/commit/?id=b9410313c66d8a23e51a5d2c9730cee7e1586317

commit b9410313c66d8a23e51a5d2c9730cee7e1586317
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-10-23 20:21:14 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-12-24 01:03:58 +0000

    nfscl/kgssapi: Fix Kerberized NFS mounts to pNFS servers
    
    During recent testing related to the IETF NFSv4 Bakeathon, it was
    discovered that Kerberized NFSv4.1/4.2 mounts to pNFS servers
    (sec=krb5[ip],pnfs mount options) was broken.
    The FreeBSD client was using the "service principal" for
    the MDS to try and establish a rpcsec_gss credential for a DS,
    which is incorrect. (A "service principal" looks like
    "nfs@<fqdn-of-server>" and the <fqdn-of-server> for the DS is not
    the same as the MDS for most pNFS servers.)
    
    To fix this, the rpcsec_gss code needs to be able to do a
    reverse DNS lookup of the DS's IP address.  A new kgssapi upcall
    to the gssd(8) daemon is added by this patch to do the reverse DNS
    along with a new rpcsec_gss function to generate the "service
    principal".
    
    A separate patch to the gssd(8) will be committed, so that this
    patch will fix the problem.  Without the gssd(8) patch, the new
    upcall fails and current/incorrect behaviour remains.
    
    This bug only affects the rare case of a Kerberized (sec=krb5[ip],pnfs)
    mount using pNFS.
    
    This patch changes the internal KAPI between the kgssapi and
    nfscl modules, but since I did a version bump a few days ago,
    I will not do one this time.
    
    (cherry picked from commit dd7d42a1fae5a4879b62689a165238082421f343)
---
 sys/conf/files                      |  1 +
 sys/fs/nfs/nfs.h                    |  1 +
 sys/fs/nfs/nfs_commonkrpc.c         | 13 ++++++++---
 sys/fs/nfsclient/nfs_clrpcops.c     | 44 +++++++++++++++++++++++++++++++++++--
 sys/kgssapi/gss_impl.c              |  2 ++
 sys/kgssapi/gssapi.h                | 18 +++++++++++++++
 sys/kgssapi/gssd.x                  | 14 ++++++++++++
 sys/modules/kgssapi/Makefile        |  1 +
 sys/rpc/rpcsec_gss.h                | 17 ++++++++++++++
 sys/rpc/rpcsec_gss/svc_rpcsec_gss.c | 31 ++++++++++++++++++++++++++
 10 files changed, 137 insertions(+), 5 deletions(-)

diff --git a/sys/conf/files b/sys/conf/files
index f737f6ca6352..a8c1893b1341 100644
--- a/sys/conf/files
+++ b/sys/conf/files
@@ -3983,6 +3983,7 @@ kgssapi/gss_get_mic.c		optional kgssapi
 kgssapi/gss_init_sec_context.c	optional kgssapi
 kgssapi/gss_impl.c		optional kgssapi
 kgssapi/gss_import_name.c	optional kgssapi
+kgssapi/gss_ip_to_dns.c		optional kgssapi
 kgssapi/gss_names.c		optional kgssapi
 kgssapi/gss_pname_to_uid.c	optional kgssapi
 kgssapi/gss_release_buffer.c	optional kgssapi
diff --git a/sys/fs/nfs/nfs.h b/sys/fs/nfs/nfs.h
index 0ed96fe43c0a..9b09520b3257 100644
--- a/sys/fs/nfs/nfs.h
+++ b/sys/fs/nfs/nfs.h
@@ -636,6 +636,7 @@ struct nfssockreq {
 	u_int32_t	nr_vers;
 	struct __rpc_client *nr_client;
 	AUTH		*nr_auth;
+	char		nr_srvprinc[1];
 };
 
 /*
diff --git a/sys/fs/nfs/nfs_commonkrpc.c b/sys/fs/nfs/nfs_commonkrpc.c
index 29fbb8dc4351..7ca150d4f54c 100644
--- a/sys/fs/nfs/nfs_commonkrpc.c
+++ b/sys/fs/nfs/nfs_commonkrpc.c
@@ -599,10 +599,14 @@ nfs_getauth(struct nfssockreq *nrp, int secflavour, char *clnt_principal,
 		else
 			svc = rpc_gss_svc_privacy;
 
-		if (clnt_principal == NULL)
+		if (clnt_principal == NULL) {
+			NFSCL_DEBUG(1, "nfs_getauth: clnt princ=NULL, "
+			    "srv princ=%s\n", srv_principal);
 			auth = rpc_gss_secfind_call(nrp->nr_client, cred,
 			    srv_principal, mech_oid, svc);
-		else {
+		} else {
+			NFSCL_DEBUG(1, "nfs_getauth: clnt princ=%s "
+			    "srv princ=%s\n", clnt_principal, srv_principal);
 			auth = rpc_gss_seccreate_call(nrp->nr_client, cred,
 			    clnt_principal, srv_principal, "kerberosv5",
 			    svc, NULL, NULL, NULL);
@@ -799,7 +803,10 @@ newnfs_request(struct nfsrv_descript *nd, struct nfsmount *nmp,
 			secflavour = RPCSEC_GSS_KRB5P;
 		else
 			secflavour = RPCSEC_GSS_KRB5;
-		srv_principal = NFSMNT_SRVKRBNAME(nmp);
+		if (nrp->nr_srvprinc[0] == '\0')
+			srv_principal = NFSMNT_SRVKRBNAME(nmp);
+		else
+			srv_principal = nrp->nr_srvprinc;
 	} else if (nmp != NULL && (!NFSHASKERB(nmp) || NFSHASSYSKRB5(nmp)) &&
 	    nd->nd_procnum != NFSPROC_NULL &&
 	    (nd->nd_flag & ND_USEGSSNAME) != 0) {
diff --git a/sys/fs/nfsclient/nfs_clrpcops.c b/sys/fs/nfsclient/nfs_clrpcops.c
index 12ce568dbedd..9d56d06c1c84 100644
--- a/sys/fs/nfsclient/nfs_clrpcops.c
+++ b/sys/fs/nfsclient/nfs_clrpcops.c
@@ -6019,7 +6019,27 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_in *sin,
 		sad->sin_family = AF_INET;
 		sad->sin_port = sin->sin_port;
 		sad->sin_addr.s_addr = sin->sin_addr.s_addr;
-		nrp = malloc(sizeof(*nrp), M_NFSSOCKREQ, M_WAITOK | M_ZERO);
+		if (NFSHASPNFS(nmp) && NFSHASKERB(nmp)) {
+			/* For pNFS, a separate server principal is needed. */
+			nrp = malloc(sizeof(*nrp) + NI_MAXSERV + NI_MAXHOST,
+			    M_NFSSOCKREQ, M_WAITOK | M_ZERO);
+			/*
+			 * Use the latter part of nr_srvprinc as a temporary
+			 * buffer for the IP address.
+			 */
+			inet_ntoa_r(sad->sin_addr,
+			    &nrp->nr_srvprinc[NI_MAXSERV]);
+			NFSCL_DEBUG(1, "nfsrpc_fillsa: DS IP=%s\n",
+			    &nrp->nr_srvprinc[NI_MAXSERV]);
+			if (!rpc_gss_ip_to_srv_principal_call(
+			    &nrp->nr_srvprinc[NI_MAXSERV], "nfs",
+			    nrp->nr_srvprinc))
+				nrp->nr_srvprinc[0] = '\0';
+			NFSCL_DEBUG(1, "nfsrpc_fillsa: srv principal=%s\n",
+			    nrp->nr_srvprinc);
+		} else
+			nrp = malloc(sizeof(*nrp), M_NFSSOCKREQ,
+			    M_WAITOK | M_ZERO);
 		nrp->nr_nam = (struct sockaddr *)sad;
 	} else if (af == AF_INET6) {
 		NFSLOCKMNT(nmp);
@@ -6058,7 +6078,27 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_in *sin,
 		sad6->sin6_port = sin6->sin6_port;
 		NFSBCOPY(&sin6->sin6_addr, &sad6->sin6_addr,
 		    sizeof(struct in6_addr));
-		nrp = malloc(sizeof(*nrp), M_NFSSOCKREQ, M_WAITOK | M_ZERO);
+		if (NFSHASPNFS(nmp) && NFSHASKERB(nmp)) {
+			/* For pNFS, a separate server principal is needed. */
+			nrp = malloc(sizeof(*nrp) + NI_MAXSERV + NI_MAXHOST,
+			    M_NFSSOCKREQ, M_WAITOK | M_ZERO);
+			/*
+			 * Use the latter part of nr_srvprinc as a temporary
+			 * buffer for the IP address.
+			 */
+			inet_ntop(AF_INET6, &sad6->sin6_addr,
+			    &nrp->nr_srvprinc[NI_MAXSERV], NI_MAXHOST);
+			NFSCL_DEBUG(1, "nfsrpc_fillsa: DS IP=%s\n",
+			    &nrp->nr_srvprinc[NI_MAXSERV]);
+			if (!rpc_gss_ip_to_srv_principal_call(
+			    &nrp->nr_srvprinc[NI_MAXSERV], "nfs",
+			    nrp->nr_srvprinc))
+				nrp->nr_srvprinc[0] = '\0';
+			NFSCL_DEBUG(1, "nfsrpc_fillsa: srv principal=%s\n",
+			    nrp->nr_srvprinc);
+		} else
+			nrp = malloc(sizeof(*nrp), M_NFSSOCKREQ,
+			    M_WAITOK | M_ZERO);
 		nrp->nr_nam = (struct sockaddr *)sad6;
 	} else
 		return (EPERM);
diff --git a/sys/kgssapi/gss_impl.c b/sys/kgssapi/gss_impl.c
index 2bb785b63345..ae37cb646f1f 100644
--- a/sys/kgssapi/gss_impl.c
+++ b/sys/kgssapi/gss_impl.c
@@ -338,6 +338,8 @@ kgssapi_modevent(module_t mod, int type, void *data)
 		    rpc_gss_get_principal_name;
 		rpc_gss_entries.rpc_gss_svc_max_data_length =
 		    rpc_gss_svc_max_data_length;
+		rpc_gss_entries.rpc_gss_ip_to_srv_principal =
+		    rpc_gss_ip_to_srv_principal;
 		mtx_init(&kgss_gssd_lock, "kgss_gssd_lock", NULL, MTX_DEF);
 		error = kgss_load();
 		break;
diff --git a/sys/kgssapi/gssapi.h b/sys/kgssapi/gssapi.h
index ef9181bf1251..37cc8a1a5a09 100644
--- a/sys/kgssapi/gssapi.h
+++ b/sys/kgssapi/gssapi.h
@@ -372,6 +372,18 @@ extern gss_OID GSS_KRB5_NT_STRING_UID_NAME;
 #define GSS_S_GAP_TOKEN \
 	 (1ul << (GSS_C_SUPPLEMENTARY_OFFSET + 4))
 
+/*
+ * NI_MAXSERV and NI_MAXHOST.  The srv_principal argument for
+ * rpc_gss_ip_to_srv_principal should point to at least
+ * NI_MAXSERV + NI_MAXHOST + 1 bytes of storage. The "+ 1" is for the '@'.
+ * The NI_MAXHOST limit is checked for gss_ip_to_dns().
+ * These should be set to the same value as they are in <netdb.h>.
+ */
+#ifndef NI_MAXHOST
+#define	NI_MAXSERV	32
+#define	NI_MAXHOST	1025
+#endif
+
 __BEGIN_DECLS
 
 /*
@@ -568,6 +580,12 @@ OM_uint32 gss_pname_to_unix_cred
 	       gid_t *groups		/* pointer to group list */
 	      );
 
+OM_uint32 gss_ip_to_dns
+	      (OM_uint32 *,		/* minor status */
+	       char *ip_addr,	/* IP host address string */
+	       char *dns_name		/* pointer to dns_name for result */
+	      );
+
 /*
  * Mbuf oriented message signing and encryption.
  *
diff --git a/sys/kgssapi/gssd.x b/sys/kgssapi/gssd.x
index 29503ac8599f..b50f39b33554 100644
--- a/sys/kgssapi/gssd.x
+++ b/sys/kgssapi/gssd.x
@@ -32,6 +32,7 @@
 %#include <kgssapi/gssapi.h>
 %#else
 %#include <gssapi/gssapi.h>
+%#include <netdb.h>
 %#endif
 
 %extern bool_t xdr_gss_buffer_desc(XDR *xdrs, gss_buffer_desc *buf);
@@ -218,6 +219,16 @@ struct display_status_args {
        uint32_t		message_context;
 };
 
+struct ip_to_dns_res {
+	uint32_t	major_status;
+	uint32_t	minor_status;
+	char		dns_name<NI_MAXHOST>;
+};
+
+struct ip_to_dns_args {
+	char		ip_addr<NI_MAXHOST>;
+};
+
 program GSSD {
 	version GSSDVERS {
 		void GSSD_NULL(void) = 0;
@@ -260,5 +271,8 @@ program GSSD {
 
 		display_status_res
 		GSSD_DISPLAY_STATUS(display_status_args) = 13;
+
+		ip_to_dns_res
+		GSSD_IP_TO_DNS(ip_to_dns_args) = 14;
 	} = 1;
 } = 0x40677373;
diff --git a/sys/modules/kgssapi/Makefile b/sys/modules/kgssapi/Makefile
index 2cf36246137c..a3eb812fd6e6 100644
--- a/sys/modules/kgssapi/Makefile
+++ b/sys/modules/kgssapi/Makefile
@@ -14,6 +14,7 @@ SRCS=	gss_accept_sec_context.c \
 	gss_init_sec_context.c \
 	gss_impl.c \
 	gss_import_name.c \
+	gss_ip_to_dns.c \
 	gss_names.c \
 	gss_pname_to_uid.c \
 	gss_release_buffer.c \
diff --git a/sys/rpc/rpcsec_gss.h b/sys/rpc/rpcsec_gss.h
index 51381762fe29..2475f61a057e 100644
--- a/sys/rpc/rpcsec_gss.h
+++ b/sys/rpc/rpcsec_gss.h
@@ -184,6 +184,8 @@ typedef bool_t	rpc_gss_get_principal_name_ftype(rpc_gss_principal_t *principal,
 typedef int	rpc_gss_svc_max_data_length_ftype(struct svc_req *req,
 		    int max_tp_unit_len);
 typedef void	rpc_gss_refresh_auth_ftype(AUTH *auth);
+typedef bool_t	rpc_gss_ip_to_srv_principal_ftype(char *ip_addr,
+		    const char *srv_name, char *dns_name);
 
 struct rpc_gss_entries {
 	rpc_gss_secfind_ftype		*rpc_gss_secfind;
@@ -206,6 +208,7 @@ struct rpc_gss_entries {
 	rpc_gss_get_principal_name_ftype *rpc_gss_get_principal_name;
 	rpc_gss_svc_max_data_length_ftype *rpc_gss_svc_max_data_length;
 	rpc_gss_refresh_auth_ftype	*rpc_gss_refresh_auth;
+	rpc_gss_ip_to_srv_principal_ftype *rpc_gss_ip_to_srv_principal;
 };
 extern struct rpc_gss_entries	rpc_gss_entries;
 
@@ -417,6 +420,18 @@ rpc_gss_refresh_auth_call(AUTH *auth)
 		(*rpc_gss_entries.rpc_gss_refresh_auth)(auth);
 }
 
+static __inline bool_t
+rpc_gss_ip_to_srv_principal_call(char *ip_addr, const char *srv_name,
+    char *dns_name)
+{
+	bool_t ret = FALSE;
+
+	if (rpc_gss_entries.rpc_gss_ip_to_srv_principal != NULL)
+		ret = (*rpc_gss_entries.rpc_gss_ip_to_srv_principal)(ip_addr,
+		    srv_name, dns_name);
+	return (ret);
+}
+
 AUTH	*rpc_gss_secfind(CLIENT *clnt, struct ucred *cred,
     const char *principal, gss_OID mech_oid, rpc_gss_service_t service);
 void	rpc_gss_secpurge(CLIENT *clnt);
@@ -455,6 +470,8 @@ void rpc_gss_clear_callback(rpc_gss_callback_t *cb);
 bool_t	rpc_gss_get_principal_name(rpc_gss_principal_t *principal,
     const char *mech, const char *name, const char *node, const char *domain);
 int	rpc_gss_svc_max_data_length(struct svc_req *req, int max_tp_unit_len);
+bool_t	rpc_gss_ip_to_srv_principal(char *ip_addr, const char *srv_name,
+    char *dns_name);
 
 /*
  * Internal interface from the RPC implementation.
diff --git a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
index 2707c5ed0582..90aa9e0d7d4f 100644
--- a/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
+++ b/sys/rpc/rpcsec_gss/svc_rpcsec_gss.c
@@ -457,6 +457,37 @@ rpc_gss_get_principal_name(rpc_gss_principal_t *principal,
 	return (TRUE);
 }
 
+/*
+ * Note that the ip_addr and srv_principal pointers can point to the same
+ * buffer, so long as ip_addr is at least strlen(srv_name) + 1 > srv_principal.
+ */
+bool_t
+rpc_gss_ip_to_srv_principal(char *ip_addr, const char *srv_name,
+    char *srv_principal)
+{
+	OM_uint32		maj_stat, min_stat;
+	size_t			len;
+
+	/*
+	 * First fill in the service name and '@'.
+	 */
+	len = strlen(srv_name);
+	if (len > NI_MAXSERV)
+		return (FALSE);
+	memcpy(srv_principal, srv_name, len);
+	srv_principal[len] = '@';
+
+	/*
+	 * Do reverse DNS to get the DNS name for the ip_addr.
+	 */
+	maj_stat = gss_ip_to_dns(&min_stat, ip_addr, &srv_principal[len + 1]);
+	if (maj_stat != GSS_S_COMPLETE) {
+		rpc_gss_log_status("gss_ip_to_dns", NULL, maj_stat, min_stat);
+		return (FALSE);
+	}
+	return (TRUE);
+}
+
 bool_t
 rpc_gss_getcred(struct svc_req *req, rpc_gss_rawcred_t **rcred,
     rpc_gss_ucred_t **ucred, void **cookie)