diff mbox

Do not bind to reserved ports registered in /etc/services

Message ID 20180308212600.GB16485@fieldses.org (mailing list archive)
State New, archived
Headers show

Commit Message

J. Bruce Fields March 8, 2018, 9:26 p.m. UTC
On Thu, Mar 08, 2018 at 03:24:23PM -0500, bfields wrote:
> Looks like knfsd's not helpful here, though: the export option
> ("secure"/"insecure") defaults to "secure", which always requires a low
> port.  It should be easy to modify "secure" to mean "require low ports
> only for auth_sys/auth_null", and that's probably the right thing to do.

Disclaimer: totally untested.

--b.

commit ddc2a5f5ce98
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Thu Mar 8 15:49:48 2018 -0500

    nfsd: don't require low ports for gss requests
    
    In a traditional NFS deployment using auth_unix, the clients are trusted
    to correctly report the credentials of their logged-in users.  The
    server assumes that only root on client machines is allowed to send
    requests from low-numbered ports, so it can use the originating port
    number to distinguish "real" NFS clients from NFS clients run by
    ordinary users, to prevent ordinary users from spoofing credentials.
    
    The originating port number on a gss-authenticated request is less
    important.  The authentication ties the request to a user, and we take
    it as proof that that user authorized the request.  The low port number
    check no longer adds much.
    
    So, don't enforce low port numbers in the auth_gss case.
    
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Chuck Lever March 8, 2018, 9:28 p.m. UTC | #1
> On Mar 8, 2018, at 4:26 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> 
> On Thu, Mar 08, 2018 at 03:24:23PM -0500, bfields wrote:
>> Looks like knfsd's not helpful here, though: the export option
>> ("secure"/"insecure") defaults to "secure", which always requires a low
>> port.  It should be easy to modify "secure" to mean "require low ports
>> only for auth_sys/auth_null", and that's probably the right thing to do.
> 
> Disclaimer: totally untested.
> 
> --b.
> 
> commit ddc2a5f5ce98
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Thu Mar 8 15:49:48 2018 -0500
> 
>    nfsd: don't require low ports for gss requests
> 
>    In a traditional NFS deployment using auth_unix, the clients are trusted
>    to correctly report the credentials of their logged-in users.  The
>    server assumes that only root on client machines is allowed to send
>    requests from low-numbered ports, so it can use the originating port
>    number to distinguish "real" NFS clients from NFS clients run by
>    ordinary users, to prevent ordinary users from spoofing credentials.
> 
>    The originating port number on a gss-authenticated request is less
>    important.  The authentication ties the request to a user, and we take
>    it as proof that that user authorized the request.  The low port number
>    check no longer adds much.
> 
>    So, don't enforce low port numbers in the auth_gss case.
> 
>    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

Looks plausible to me, and I like the approach.

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>


> diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
> index 8aa011820c4a..764e6cae6533 100644
> --- a/fs/nfsd/nfsfh.c
> +++ b/fs/nfsd/nfsfh.c
> @@ -87,13 +87,23 @@ nfsd_mode_check(struct svc_rqst *rqstp, struct dentry *dentry,
> 	return nfserr_inval;
> }
> 
> +static bool nfsd_originating_port_ok(struct svc_rqst *rqstp, int flags)
> +{
> +	if (flags & NFSEXP_INSECURE_PORT)
> +		return true;
> +	/* We don't require gss requests to use low ports: */
> +	if (rqstp->rq_cred.cr_flavor >= RPC_AUTH_GSS)
> +		return true;
> +	return test_bit(RQ_SECURE, &rqstp->rq_flags);
> +}
> +
> static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
> 					  struct svc_export *exp)
> {
> 	int flags = nfsexp_flags(rqstp, exp);
> 
> 	/* Check if the request originated from a secure port. */
> -	if (!test_bit(RQ_SECURE, &rqstp->rq_flags) && !(flags & NFSEXP_INSECURE_PORT)) {
> +	if (!nfsd_originating_port_ok(rqstp, flags)) {
> 		RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
> 		dprintk("nfsd: request from insecure port %s!\n",
> 		        svc_print_addr(rqstp, buf, sizeof(buf)));
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Libtirpc-devel mailing list
> Libtirpc-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/libtirpc-devel

--
Chuck Lever
chucklever@gmail.com



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 8aa011820c4a..764e6cae6533 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -87,13 +87,23 @@  nfsd_mode_check(struct svc_rqst *rqstp, struct dentry *dentry,
 	return nfserr_inval;
 }
 
+static bool nfsd_originating_port_ok(struct svc_rqst *rqstp, int flags)
+{
+	if (flags & NFSEXP_INSECURE_PORT)
+		return true;
+	/* We don't require gss requests to use low ports: */
+	if (rqstp->rq_cred.cr_flavor >= RPC_AUTH_GSS)
+		return true;
+	return test_bit(RQ_SECURE, &rqstp->rq_flags);
+}
+
 static __be32 nfsd_setuser_and_check_port(struct svc_rqst *rqstp,
 					  struct svc_export *exp)
 {
 	int flags = nfsexp_flags(rqstp, exp);
 
 	/* Check if the request originated from a secure port. */
-	if (!test_bit(RQ_SECURE, &rqstp->rq_flags) && !(flags & NFSEXP_INSECURE_PORT)) {
+	if (!nfsd_originating_port_ok(rqstp, flags)) {
 		RPC_IFDEBUG(char buf[RPC_MAX_ADDRBUFLEN]);
 		dprintk("nfsd: request from insecure port %s!\n",
 		        svc_print_addr(rqstp, buf, sizeof(buf)));