Date: Sun, 24 Dec 2023 01:06:25 GMT From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: b9410313c66d - stable/14 - nfscl/kgssapi: Fix Kerberized NFS mounts to pNFS servers Message-ID: <202312240106.3BO16P9W050301@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202312240106.3BO16P9W050301>