| Message ID | 1378232772-4066-1-git-send-email-dros@netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sep 3, 2013, at 2:26 PM, Weston Andros Adamson <dros@netapp.com> wrote: > SECINFO and SECINFO_NONAME should be able to use integrity protected auth > flavors even if the export wasn't explicitly configured to use them. > > An example is a sec=sys export - upstream linux clients will attempt to use > krb5i for EXCHANGE_ID, CREATE_SESSION, DESTROY_SESSION, etc. If this is > successful, the client will try to use the same auth flavor for SECINFO > as described in the Security Considerations sections of rfc3530 and rfc5661. The reason krb5i can work for lease management operations is because those operations are not connected to any particular export, so typically they are not controlled directly by export security settings. But I'm not sure all server implementations will allow a SECINFO on an FSID using a flavor not allowed by the FSID's export options. SECINFO is definitely connected to a particular FSID, unlike lease management operations. Given FreeBSD's implementation choices wrt lease management security flavors, that would be the first server I'd check. > This patch adds a nfsd4_op_flag to describe operations that may use these > auth flavors to get around nfsd_access() checks. This should be useful in > future implementations of SP4_MACH_CRED (nfsd_permission still needs to be > handled). > > This patch also stops SECINFO* from returning NFS4ERR_WRONGSEC which is > not allowed by either rfc3530 (not in list of allowed errors) or rfc6551 5661 > (section 2.6.3.1.1.5 says it MUST NOT). That sounds inconsistent with the Security Considerations recommendation. Recommending that a client use integrity protection means servers MUST support it, or the client has to be prepared to retry it with some other flavor, and the server MUST have a way to ask for a retry (ie in that case, wouldn't WRONGSEC be the correct way to make such a request?). Is there anything more in these RFCs that would require a server to support SECINFO via krb5i? Thus I wonder if clients can depend on a krb5i SECINFO on a sec=sys FSID working everywhere. > > Signed-off-by: Weston Andros Adamson <dros@netapp.com> > --- > fs/nfsd/export.c | 10 ++++++++++ > fs/nfsd/nfs4proc.c | 27 ++++++++++++++++++++++++--- > fs/nfsd/xdr4.h | 2 ++ > 3 files changed, 36 insertions(+), 3 deletions(-) > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > index 5f38ea3..e2e5a13 100644 > --- a/fs/nfsd/export.c > +++ b/fs/nfsd/export.c > @@ -22,6 +22,7 @@ > #include "nfsd.h" > #include "nfsfh.h" > #include "netns.h" > +#include "xdr4.h" > > #define NFSDDBG_FACILITY NFSDDBG_EXPORT > > @@ -909,6 +910,15 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp) > rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX) > return 0; > } > + > + /* some operations allow use of integrity even though the mount > + * may not be */ > + if (nfsd4_allow_integrity(rqstp)) { > + if (rqstp->rq_cred.cr_flavor == RPC_AUTH_GSS_KRB5I || > + rqstp->rq_cred.cr_flavor == RPC_AUTH_GSS_KRB5P) > + return 0; > + } > + > return nfserr_wrongsec; > } > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index 0d4c410..ce79483 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -1158,6 +1158,11 @@ enum nfsd4_op_flags { > * These are ops which clear current state id. > */ > OP_CLEAR_STATEID = 1 << 7, > + /* > + * Procedures that can use auth flavors other than flavors > + * specified in the export as long as they are integrity protected. > + */ > + OP_ALLOW_INTEGRITY = 1 << 8, > }; > > struct nfsd4_operation { > @@ -1249,7 +1254,23 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp) > * errors themselves as necessary; others should check for them > * now: > */ > - return !(nextd->op_flags & OP_HANDLES_WRONGSEC); > + return !(nextd->op_flags & (OP_HANDLES_WRONGSEC | OP_ALLOW_INTEGRITY)); > +} > + > +bool > +nfsd4_allow_integrity(struct svc_rqst *rqstp) > +{ > + struct nfsd4_compoundres *resp = rqstp->rq_resp; > + struct nfsd4_compoundargs *argp = rqstp->rq_argp; > + struct nfsd4_op *this = &argp->ops[resp->opcnt - 1]; > + struct nfsd4_operation *thisd; > + > + thisd = OPDESC(this); > + > + if (thisd->op_flags & OP_ALLOW_INTEGRITY) > + return true; > + > + return false; > } > > /* > @@ -1727,7 +1748,7 @@ static struct nfsd4_operation nfsd4_ops[] = { > }, > [OP_SECINFO] = { > .op_func = (nfsd4op_func)nfsd4_secinfo, > - .op_flags = OP_HANDLES_WRONGSEC, > + .op_flags = OP_ALLOW_INTEGRITY, > .op_name = "OP_SECINFO", > }, > [OP_SETATTR] = { > @@ -1825,7 +1846,7 @@ static struct nfsd4_operation nfsd4_ops[] = { > }, > [OP_SECINFO_NO_NAME] = { > .op_func = (nfsd4op_func)nfsd4_secinfo_no_name, > - .op_flags = OP_HANDLES_WRONGSEC, > + .op_flags = OP_ALLOW_INTEGRITY, > .op_name = "OP_SECINFO_NO_NAME", > }, > [OP_TEST_STATEID] = { > diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h > index b3ed644..ed79b6d 100644 > --- a/fs/nfsd/xdr4.h > +++ b/fs/nfsd/xdr4.h > @@ -481,6 +481,8 @@ struct nfsd4_op { > > bool nfsd4_cache_this_op(struct nfsd4_op *); > > +bool nfsd4_allow_integrity(struct svc_rqst *); > + > struct nfsd4_compoundargs { > /* scratch variables for XDR decode */ > __be32 * p; > -- > 1.7.12.4 (Apple Git-37) > > -- > 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
On Sep 3, 2013, at 2:48 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Sep 3, 2013, at 2:26 PM, Weston Andros Adamson <dros@netapp.com> wrote: > >> SECINFO and SECINFO_NONAME should be able to use integrity protected auth >> flavors even if the export wasn't explicitly configured to use them. >> >> An example is a sec=sys export - upstream linux clients will attempt to use >> krb5i for EXCHANGE_ID, CREATE_SESSION, DESTROY_SESSION, etc. If this is >> successful, the client will try to use the same auth flavor for SECINFO >> as described in the Security Considerations sections of rfc3530 and rfc5661. > > The reason krb5i can work for lease management operations is because those operations are not connected to any particular export, so typically they are not controlled directly by export security settings. > > But I'm not sure all server implementations will allow a SECINFO on an FSID using a flavor not allowed by the FSID's export options. SECINFO is definitely connected to a particular FSID, unlike lease management operations. I understand how these operations are different from SECINFO, but the spec is pretty clear that krb5i/p should be used if at all possible. > > Given FreeBSD's implementation choices wrt lease management security flavors, that would be the first server I'd check. > >> This patch adds a nfsd4_op_flag to describe operations that may use these >> auth flavors to get around nfsd_access() checks. This should be useful in >> future implementations of SP4_MACH_CRED (nfsd_permission still needs to be >> handled). >> >> This patch also stops SECINFO* from returning NFS4ERR_WRONGSEC which is >> not allowed by either rfc3530 (not in list of allowed errors) or rfc6551 > > 5661 Heh, good catch! > >> (section 2.6.3.1.1.5 says it MUST NOT). > > That sounds inconsistent with the Security Considerations recommendation. How so? Just because we don't know what error to return if the server doesn't support the recommendation to use integrity protection for SECINFO? > > Recommending that a client use integrity protection means servers MUST support it, or the client has to be prepared to retry it with some other flavor, and the server MUST have a way to ask for a retry (ie in that case, wouldn't WRONGSEC be the correct way to make such a request?). So, I'm preparing to post a client-side patch that retries (with filesystem's rpc_client) if the server returns NFS4ERR_WRONGSEC as Trond doesn't want the client to be broken against deployed linux servers, but this is a good question - what error should a server return in this case? > > Is there anything more in these RFCs that would require a server to support SECINFO via krb5i? > > Thus I wonder if clients can depend on a krb5i SECINFO on a sec=sys FSID working everywhere. Sections 17 of rfc3530bis and 21 of rfc 5661 clearly state that it's RECOMMENDED to use integrity protection for SECINFO and SECINFO_NONAME, so I think we should try, but your point remains - what error should a server return if the server doesn't support this? -dros > >> >> Signed-off-by: Weston Andros Adamson <dros@netapp.com> >> --- >> fs/nfsd/export.c | 10 ++++++++++ >> fs/nfsd/nfs4proc.c | 27 ++++++++++++++++++++++++--- >> fs/nfsd/xdr4.h | 2 ++ >> 3 files changed, 36 insertions(+), 3 deletions(-) >> >> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c >> index 5f38ea3..e2e5a13 100644 >> --- a/fs/nfsd/export.c >> +++ b/fs/nfsd/export.c >> @@ -22,6 +22,7 @@ >> #include "nfsd.h" >> #include "nfsfh.h" >> #include "netns.h" >> +#include "xdr4.h" >> >> #define NFSDDBG_FACILITY NFSDDBG_EXPORT >> >> @@ -909,6 +910,15 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp) >> rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX) >> return 0; >> } >> + >> + /* some operations allow use of integrity even though the mount >> + * may not be */ >> + if (nfsd4_allow_integrity(rqstp)) { >> + if (rqstp->rq_cred.cr_flavor == RPC_AUTH_GSS_KRB5I || >> + rqstp->rq_cred.cr_flavor == RPC_AUTH_GSS_KRB5P) >> + return 0; >> + } >> + >> return nfserr_wrongsec; >> } >> >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c >> index 0d4c410..ce79483 100644 >> --- a/fs/nfsd/nfs4proc.c >> +++ b/fs/nfsd/nfs4proc.c >> @@ -1158,6 +1158,11 @@ enum nfsd4_op_flags { >> * These are ops which clear current state id. >> */ >> OP_CLEAR_STATEID = 1 << 7, >> + /* >> + * Procedures that can use auth flavors other than flavors >> + * specified in the export as long as they are integrity protected. >> + */ >> + OP_ALLOW_INTEGRITY = 1 << 8, >> }; >> >> struct nfsd4_operation { >> @@ -1249,7 +1254,23 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp) >> * errors themselves as necessary; others should check for them >> * now: >> */ >> - return !(nextd->op_flags & OP_HANDLES_WRONGSEC); >> + return !(nextd->op_flags & (OP_HANDLES_WRONGSEC | OP_ALLOW_INTEGRITY)); >> +} >> + >> +bool >> +nfsd4_allow_integrity(struct svc_rqst *rqstp) >> +{ >> + struct nfsd4_compoundres *resp = rqstp->rq_resp; >> + struct nfsd4_compoundargs *argp = rqstp->rq_argp; >> + struct nfsd4_op *this = &argp->ops[resp->opcnt - 1]; >> + struct nfsd4_operation *thisd; >> + >> + thisd = OPDESC(this); >> + >> + if (thisd->op_flags & OP_ALLOW_INTEGRITY) >> + return true; >> + >> + return false; >> } >> >> /* >> @@ -1727,7 +1748,7 @@ static struct nfsd4_operation nfsd4_ops[] = { >> }, >> [OP_SECINFO] = { >> .op_func = (nfsd4op_func)nfsd4_secinfo, >> - .op_flags = OP_HANDLES_WRONGSEC, >> + .op_flags = OP_ALLOW_INTEGRITY, >> .op_name = "OP_SECINFO", >> }, >> [OP_SETATTR] = { >> @@ -1825,7 +1846,7 @@ static struct nfsd4_operation nfsd4_ops[] = { >> }, >> [OP_SECINFO_NO_NAME] = { >> .op_func = (nfsd4op_func)nfsd4_secinfo_no_name, >> - .op_flags = OP_HANDLES_WRONGSEC, >> + .op_flags = OP_ALLOW_INTEGRITY, >> .op_name = "OP_SECINFO_NO_NAME", >> }, >> [OP_TEST_STATEID] = { >> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h >> index b3ed644..ed79b6d 100644 >> --- a/fs/nfsd/xdr4.h >> +++ b/fs/nfsd/xdr4.h >> @@ -481,6 +481,8 @@ struct nfsd4_op { >> >> bool nfsd4_cache_this_op(struct nfsd4_op *); >> >> +bool nfsd4_allow_integrity(struct svc_rqst *); >> + >> struct nfsd4_compoundargs { >> /* scratch variables for XDR decode */ >> __be32 * p; >> -- >> 1.7.12.4 (Apple Git-37) >> >> -- >> 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 > > -- > Chuck Lever > chuck[dot]lever[at]oracle[dot]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 -- 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
On Tue, Sep 03, 2013 at 07:11:57PM +0000, Adamson, Dros wrote: > > On Sep 3, 2013, at 2:48 PM, Chuck Lever <chuck.lever@oracle.com> > wrote: > > > > > On Sep 3, 2013, at 2:26 PM, Weston Andros Adamson <dros@netapp.com> wrote: > > > >> SECINFO and SECINFO_NONAME should be able to use integrity protected auth > >> flavors even if the export wasn't explicitly configured to use them. > >> > >> An example is a sec=sys export - upstream linux clients will attempt to use > >> krb5i for EXCHANGE_ID, CREATE_SESSION, DESTROY_SESSION, etc. If this is > >> successful, the client will try to use the same auth flavor for SECINFO > >> as described in the Security Considerations sections of rfc3530 and rfc5661. > > > > The reason krb5i can work for lease management operations is because those operations are not connected to any particular export, so typically they are not controlled directly by export security settings. > > > > But I'm not sure all server implementations will allow a SECINFO on an FSID using a flavor not allowed by the FSID's export options. SECINFO is definitely connected to a particular FSID, unlike lease management operations. > > I understand how these operations are different from SECINFO, but the spec is pretty clear that krb5i/p should be used if at all possible. > > > > > Given FreeBSD's implementation choices wrt lease management security flavors, that would be the first server I'd check. > > > >> This patch adds a nfsd4_op_flag to describe operations that may use these > >> auth flavors to get around nfsd_access() checks. This should be useful in > >> future implementations of SP4_MACH_CRED (nfsd_permission still needs to be > >> handled). > >> > >> This patch also stops SECINFO* from returning NFS4ERR_WRONGSEC which is > >> not allowed by either rfc3530 (not in list of allowed errors) or rfc6551 > > > > 5661 > > Heh, good catch! > > > > >> (section 2.6.3.1.1.5 says it MUST NOT). > > > > That sounds inconsistent with the Security Considerations recommendation. > > How so? Just because we don't know what error to return if the server doesn't support the recommendation to use integrity protection for SECINFO? > > > > > Recommending that a client use integrity protection means servers MUST support it, or the client has to be prepared to retry it with some other flavor, and the server MUST have a way to ask for a retry (ie in that case, wouldn't WRONGSEC be the correct way to make such a request?). > > So, I'm preparing to post a client-side patch that retries (with filesystem's rpc_client) if the server returns NFS4ERR_WRONGSEC as Trond doesn't want the client to be broken against deployed linux servers, but this is a good question - what error should a server return in this case? > > > > > Is there anything more in these RFCs that would require a server to support SECINFO via krb5i? > > > > Thus I wonder if clients can depend on a krb5i SECINFO on a sec=sys FSID working everywhere. > > Sections 17 of rfc3530bis and 21 of rfc 5661 clearly state that it's RECOMMENDED to use integrity protection for SECINFO and SECINFO_NONAME, so I think we should try, but your point remains - what error should a server return if the server doesn't support this? I'm confused--we should just allow SECINFO/SECINFO_NO_NAME in these cases and then we don't have to decide, right? --b. -- 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
On Sep 4, 2013, at 6:18 PM, bfields@fieldses.org wrote: > On Tue, Sep 03, 2013 at 07:11:57PM +0000, Adamson, Dros wrote: >> >> On Sep 3, 2013, at 2:48 PM, Chuck Lever <chuck.lever@oracle.com> >> wrote: >> >>> >>> On Sep 3, 2013, at 2:26 PM, Weston Andros Adamson <dros@netapp.com> wrote: >>> >>>> SECINFO and SECINFO_NONAME should be able to use integrity protected auth >>>> flavors even if the export wasn't explicitly configured to use them. >>>> >>>> An example is a sec=sys export - upstream linux clients will attempt to use >>>> krb5i for EXCHANGE_ID, CREATE_SESSION, DESTROY_SESSION, etc. If this is >>>> successful, the client will try to use the same auth flavor for SECINFO >>>> as described in the Security Considerations sections of rfc3530 and rfc5661. >>> >>> The reason krb5i can work for lease management operations is because those operations are not connected to any particular export, so typically they are not controlled directly by export security settings. >>> >>> But I'm not sure all server implementations will allow a SECINFO on an FSID using a flavor not allowed by the FSID's export options. SECINFO is definitely connected to a particular FSID, unlike lease management operations. >> >> I understand how these operations are different from SECINFO, but the spec is pretty clear that krb5i/p should be used if at all possible. >> >>> >>> Given FreeBSD's implementation choices wrt lease management security flavors, that would be the first server I'd check. >>> >>>> This patch adds a nfsd4_op_flag to describe operations that may use these >>>> auth flavors to get around nfsd_access() checks. This should be useful in >>>> future implementations of SP4_MACH_CRED (nfsd_permission still needs to be >>>> handled). >>>> >>>> This patch also stops SECINFO* from returning NFS4ERR_WRONGSEC which is >>>> not allowed by either rfc3530 (not in list of allowed errors) or rfc6551 >>> >>> 5661 >> >> Heh, good catch! >> >>> >>>> (section 2.6.3.1.1.5 says it MUST NOT). >>> >>> That sounds inconsistent with the Security Considerations recommendation. >> >> How so? Just because we don't know what error to return if the server doesn't support the recommendation to use integrity protection for SECINFO? >> >>> >>> Recommending that a client use integrity protection means servers MUST support it, or the client has to be prepared to retry it with some other flavor, and the server MUST have a way to ask for a retry (ie in that case, wouldn't WRONGSEC be the correct way to make such a request?). >> >> So, I'm preparing to post a client-side patch that retries (with filesystem's rpc_client) if the server returns NFS4ERR_WRONGSEC as Trond doesn't want the client to be broken against deployed linux servers, but this is a good question - what error should a server return in this case? >> >>> >>> Is there anything more in these RFCs that would require a server to support SECINFO via krb5i? >>> >>> Thus I wonder if clients can depend on a krb5i SECINFO on a sec=sys FSID working everywhere. >> >> Sections 17 of rfc3530bis and 21 of rfc 5661 clearly state that it's RECOMMENDED to use integrity protection for SECINFO and SECINFO_NONAME, so I think we should try, but your point remains - what error should a server return if the server doesn't support this? > > I'm confused--we should just allow SECINFO/SECINFO_NO_NAME in these > cases and then we don't have to decide, right? I've finally read the relevant sections of the RFCs (doi!). RFC 5661 section 2.6.3.1.1.4 and following discuss SECINFO/SECINFO_NO_NAME and NFS4ERR_WRONGSEC in some detail. The way I read this, a server MUST NOT return NFS4ERR_WRONGSEC on any SECINFO or SECINFO_NO_NAME request, and MUST NOT return NFS4ERR_WRONGSEC on a PUTFH if the subsequent operation is SECINFO or SECINFO_NO_NAME. Section 2.6.3.1.1.5 explicitly says "In theory, there is no connection between the security flavor used by SECINFO or SECINFO_NO_NAME and those supported by the security policy." Despite the fact that both of these operations take a FH, and thus would be otherwise subject to the security policy in place for the containing FSID, they should be permitted under all security flavors a server supports. If the server truly does not support the specified security flavor, an RPC-level error occurs, as Trond points out, and as the first paragraph of section 2.6.3.1.1.5 says. This is an indication to the client that it should retry the SECINFO with another flavor. RFC 3530bis does not state a similar strong requirement. However, NFS4ERR_WRONGSEC is not listed in the valid status codes for the SECINFO operation, so we might assume that a client should expect similar behavior for NFSv4.0. For the server, "we should just allow SECINFO/SECINFO_NO_NAME in these cases". Perhaps there should be no security check at all -- the RPC layer should already cover the totally unsupported flavor case. The only detail is ignoring security policy conflicts in a preceding PUTFH in the same compound. I think the main difficulty now is that some versions of the Linux server do not behave this way.
On Sep 5, 2013, at 10:07 AM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Sep 4, 2013, at 6:18 PM, bfields@fieldses.org wrote: > >> On Tue, Sep 03, 2013 at 07:11:57PM +0000, Adamson, Dros wrote: >>> >>> On Sep 3, 2013, at 2:48 PM, Chuck Lever <chuck.lever@oracle.com> >>> wrote: >>> >>>> >>>> On Sep 3, 2013, at 2:26 PM, Weston Andros Adamson <dros@netapp.com> wrote: >>>> >>>>> SECINFO and SECINFO_NONAME should be able to use integrity protected auth >>>>> flavors even if the export wasn't explicitly configured to use them. >>>>> >>>>> An example is a sec=sys export - upstream linux clients will attempt to use >>>>> krb5i for EXCHANGE_ID, CREATE_SESSION, DESTROY_SESSION, etc. If this is >>>>> successful, the client will try to use the same auth flavor for SECINFO >>>>> as described in the Security Considerations sections of rfc3530 and rfc5661. >>>> >>>> The reason krb5i can work for lease management operations is because those operations are not connected to any particular export, so typically they are not controlled directly by export security settings. >>>> >>>> But I'm not sure all server implementations will allow a SECINFO on an FSID using a flavor not allowed by the FSID's export options. SECINFO is definitely connected to a particular FSID, unlike lease management operations. >>> >>> I understand how these operations are different from SECINFO, but the spec is pretty clear that krb5i/p should be used if at all possible. >>> >>>> >>>> Given FreeBSD's implementation choices wrt lease management security flavors, that would be the first server I'd check. >>>> >>>>> This patch adds a nfsd4_op_flag to describe operations that may use these >>>>> auth flavors to get around nfsd_access() checks. This should be useful in >>>>> future implementations of SP4_MACH_CRED (nfsd_permission still needs to be >>>>> handled). >>>>> >>>>> This patch also stops SECINFO* from returning NFS4ERR_WRONGSEC which is >>>>> not allowed by either rfc3530 (not in list of allowed errors) or rfc6551 >>>> >>>> 5661 >>> >>> Heh, good catch! >>> >>>> >>>>> (section 2.6.3.1.1.5 says it MUST NOT). >>>> >>>> That sounds inconsistent with the Security Considerations recommendation. >>> >>> How so? Just because we don't know what error to return if the server doesn't support the recommendation to use integrity protection for SECINFO? >>> >>>> >>>> Recommending that a client use integrity protection means servers MUST support it, or the client has to be prepared to retry it with some other flavor, and the server MUST have a way to ask for a retry (ie in that case, wouldn't WRONGSEC be the correct way to make such a request?). >>> >>> So, I'm preparing to post a client-side patch that retries (with filesystem's rpc_client) if the server returns NFS4ERR_WRONGSEC as Trond doesn't want the client to be broken against deployed linux servers, but this is a good question - what error should a server return in this case? >>> >>>> >>>> Is there anything more in these RFCs that would require a server to support SECINFO via krb5i? >>>> >>>> Thus I wonder if clients can depend on a krb5i SECINFO on a sec=sys FSID working everywhere. >>> >>> Sections 17 of rfc3530bis and 21 of rfc 5661 clearly state that it's RECOMMENDED to use integrity protection for SECINFO and SECINFO_NONAME, so I think we should try, but your point remains - what error should a server return if the server doesn't support this? >> >> I'm confused--we should just allow SECINFO/SECINFO_NO_NAME in these >> cases and then we don't have to decide, right? > > I've finally read the relevant sections of the RFCs (doi!). RFC 5661 section 2.6.3.1.1.4 and following discuss SECINFO/SECINFO_NO_NAME and NFS4ERR_WRONGSEC in some detail. > > The way I read this, a server MUST NOT return NFS4ERR_WRONGSEC on any SECINFO or SECINFO_NO_NAME request, and MUST NOT return NFS4ERR_WRONGSEC on a PUTFH if the subsequent operation is SECINFO or SECINFO_NO_NAME. > > Section 2.6.3.1.1.5 explicitly says "In theory, there is no connection between the security flavor used by SECINFO or SECINFO_NO_NAME and those supported by the security policy." Despite the fact that both of these operations take a FH, and thus would be otherwise subject to the security policy in place for the containing FSID, they should be permitted under all security flavors a server supports. > > If the server truly does not support the specified security flavor, an RPC-level error occurs, as Trond points out, and as the first paragraph of section 2.6.3.1.1.5 says. This is an indication to the client that it should retry the SECINFO with another flavor. > > RFC 3530bis does not state a similar strong requirement. However, NFS4ERR_WRONGSEC is not listed in the valid status codes for the SECINFO operation, so we might assume that a client should expect similar behavior for NFSv4.0. Well put - thanks Chuck! > > For the server, "we should just allow SECINFO/SECINFO_NO_NAME in these cases". Perhaps there should be no security check at all -- the RPC layer should already cover the totally unsupported flavor case. The only detail is ignoring security policy conflicts in a preceding PUTFH in the same compound. > So should I refactor this patch so that it doesn't "allow integrity" for flagged operations and instead simply skips access checks? > I think the main difficulty now is that some versions of the Linux server do not behave this way. Indeed - thats why my recent client patches will handle WRONGSEC on SECINFO* - the SECINFO patch has been accepted by Trond, but the SECINFO_NO_NAME patch is still under discussion. -dros > > -- > Chuck Lever > chuck[dot]lever[at]oracle[dot]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 --git a/fs/nfsd/export.c b/fs/nfsd/export.c index 5f38ea3..e2e5a13 100644 --- a/fs/nfsd/export.c +++ b/fs/nfsd/export.c @@ -22,6 +22,7 @@ #include "nfsd.h" #include "nfsfh.h" #include "netns.h" +#include "xdr4.h" #define NFSDDBG_FACILITY NFSDDBG_EXPORT @@ -909,6 +910,15 @@ __be32 check_nfsd_access(struct svc_export *exp, struct svc_rqst *rqstp) rqstp->rq_cred.cr_flavor == RPC_AUTH_UNIX) return 0; } + + /* some operations allow use of integrity even though the mount + * may not be */ + if (nfsd4_allow_integrity(rqstp)) { + if (rqstp->rq_cred.cr_flavor == RPC_AUTH_GSS_KRB5I || + rqstp->rq_cred.cr_flavor == RPC_AUTH_GSS_KRB5P) + return 0; + } + return nfserr_wrongsec; } diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 0d4c410..ce79483 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1158,6 +1158,11 @@ enum nfsd4_op_flags { * These are ops which clear current state id. */ OP_CLEAR_STATEID = 1 << 7, + /* + * Procedures that can use auth flavors other than flavors + * specified in the export as long as they are integrity protected. + */ + OP_ALLOW_INTEGRITY = 1 << 8, }; struct nfsd4_operation { @@ -1249,7 +1254,23 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp) * errors themselves as necessary; others should check for them * now: */ - return !(nextd->op_flags & OP_HANDLES_WRONGSEC); + return !(nextd->op_flags & (OP_HANDLES_WRONGSEC | OP_ALLOW_INTEGRITY)); +} + +bool +nfsd4_allow_integrity(struct svc_rqst *rqstp) +{ + struct nfsd4_compoundres *resp = rqstp->rq_resp; + struct nfsd4_compoundargs *argp = rqstp->rq_argp; + struct nfsd4_op *this = &argp->ops[resp->opcnt - 1]; + struct nfsd4_operation *thisd; + + thisd = OPDESC(this); + + if (thisd->op_flags & OP_ALLOW_INTEGRITY) + return true; + + return false; } /* @@ -1727,7 +1748,7 @@ static struct nfsd4_operation nfsd4_ops[] = { }, [OP_SECINFO] = { .op_func = (nfsd4op_func)nfsd4_secinfo, - .op_flags = OP_HANDLES_WRONGSEC, + .op_flags = OP_ALLOW_INTEGRITY, .op_name = "OP_SECINFO", }, [OP_SETATTR] = { @@ -1825,7 +1846,7 @@ static struct nfsd4_operation nfsd4_ops[] = { }, [OP_SECINFO_NO_NAME] = { .op_func = (nfsd4op_func)nfsd4_secinfo_no_name, - .op_flags = OP_HANDLES_WRONGSEC, + .op_flags = OP_ALLOW_INTEGRITY, .op_name = "OP_SECINFO_NO_NAME", }, [OP_TEST_STATEID] = { diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h index b3ed644..ed79b6d 100644 --- a/fs/nfsd/xdr4.h +++ b/fs/nfsd/xdr4.h @@ -481,6 +481,8 @@ struct nfsd4_op { bool nfsd4_cache_this_op(struct nfsd4_op *); +bool nfsd4_allow_integrity(struct svc_rqst *); + struct nfsd4_compoundargs { /* scratch variables for XDR decode */ __be32 * p;
SECINFO and SECINFO_NONAME should be able to use integrity protected auth flavors even if the export wasn't explicitly configured to use them. An example is a sec=sys export - upstream linux clients will attempt to use krb5i for EXCHANGE_ID, CREATE_SESSION, DESTROY_SESSION, etc. If this is successful, the client will try to use the same auth flavor for SECINFO as described in the Security Considerations sections of rfc3530 and rfc5661. This patch adds a nfsd4_op_flag to describe operations that may use these auth flavors to get around nfsd_access() checks. This should be useful in future implementations of SP4_MACH_CRED (nfsd_permission still needs to be handled). This patch also stops SECINFO* from returning NFS4ERR_WRONGSEC which is not allowed by either rfc3530 (not in list of allowed errors) or rfc6551 (section 2.6.3.1.1.5 says it MUST NOT). Signed-off-by: Weston Andros Adamson <dros@netapp.com> --- fs/nfsd/export.c | 10 ++++++++++ fs/nfsd/nfs4proc.c | 27 ++++++++++++++++++++++++--- fs/nfsd/xdr4.h | 2 ++ 3 files changed, 36 insertions(+), 3 deletions(-)