[PATCH v2 0/4] Use correct NFSv4.0 callback credential

[PATCH v2 0/4] Use correct NFSv4.0 callback credential

[Chuck Lever]

Hi Bruce-

This is the same series as I posted in May, merged up to v4.18. I
tested it again this morning. Can it be included in v4.19 ?

The previous thread claims Simo Ack'd these patches, but I can't
find the actual Acked-by. I've cc'd him on this repost in case he
has additional comments.

---

Chuck Lever (4):
      sunrpc: Enable the kernel to specify the hostname part of service principals
      sunrpc: Extract target name into svc_cred
      nfsd: Use correct credential for NFSv4.0 callback with GSS
      nfsd: Remove callback_cred


 fs/nfsd/nfs4callback.c               |   29 ++++----------
 fs/nfsd/nfs4state.c                  |   17 +++-----
 fs/nfsd/state.h                      |    2 -
 include/linux/sunrpc/svcauth.h       |    3 +
 net/sunrpc/auth_gss/auth_gss.c       |   20 ++++++++--
 net/sunrpc/auth_gss/gss_rpc_upcall.c |   70 ++++++++++++++++++++++------------
 6 files changed, 80 insertions(+), 61 deletions(-)

--
Chuck Lever

[PATCH v2 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

[Chuck Lever]

A multi-homed NFS server may have more than one "nfs" key in its
keytab. Enable the kernel to pick the key it wants as a machine
credential when establishing a GSS context.

This is useful for GSS-protected NFSv4.0 callbacks, which are
required by RFC 7530 S3.3.3 to use the same principal as the service
principal the client used when establishing its lease.

A complementary modification to rpc.gssd is required to fully enable
this feature.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/auth_gss.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index be8f103..1943e11 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -284,7 +284,12 @@ struct gss_auth {
 	return p;
 }
 
-#define UPCALL_BUF_LEN 128
+/* XXX: Need some documentation about why UPCALL_BUF_LEN is so small.
+ *	Is user space expecting no more than UPCALL_BUF_LEN bytes?
+ *	Note that there are now _two_ NI_MAXHOST sized data items
+ *	being passed in this string.
+ */
+#define UPCALL_BUF_LEN	256
 
 struct gss_upcall_msg {
 	refcount_t count;
@@ -462,8 +467,17 @@ static int gss_encode_v1_msg(struct gss_upcall_msg *gss_msg,
 		p += len;
 		gss_msg->msg.len += len;
 	}
-	if (service_name != NULL) {
-		len = scnprintf(p, buflen, "service=%s ", service_name);
+	if (service_name) {
+		char *c = strchr(service_name, '@');
+
+		if (!c)
+			len = scnprintf(p, buflen, "service=%s ",
+					service_name);
+		else
+			len = scnprintf(p, buflen,
+					"service=%.*s srchost=%s ",
+					(int)(c - service_name),
+					service_name, c + 1);
 		buflen -= len;
 		p += len;
 		gss_msg->msg.len += len;

Re: [PATCH v2 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

[Trond Myklebust]

T24gVGh1LCAyMDE4LTA4LTE2IGF0IDEyOjA1IC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
QSBtdWx0aS1ob21lZCBORlMgc2VydmVyIG1heSBoYXZlIG1vcmUgdGhhbiBvbmUgIm5mcyIga2V5
IGluIGl0cw0KPiBrZXl0YWIuIEVuYWJsZSB0aGUga2VybmVsIHRvIHBpY2sgdGhlIGtleSBpdCB3
YW50cyBhcyBhIG1hY2hpbmUNCj4gY3JlZGVudGlhbCB3aGVuIGVzdGFibGlzaGluZyBhIEdTUyBj
b250ZXh0Lg0KPiANCj4gVGhpcyBpcyB1c2VmdWwgZm9yIEdTUy1wcm90ZWN0ZWQgTkZTdjQuMCBj
YWxsYmFja3MsIHdoaWNoIGFyZQ0KPiByZXF1aXJlZCBieSBSRkMgNzUzMCBTMy4zLjMgdG8gdXNl
IHRoZSBzYW1lIHByaW5jaXBhbCBhcyB0aGUgc2VydmljZQ0KPiBwcmluY2lwYWwgdGhlIGNsaWVu
dCB1c2VkIHdoZW4gZXN0YWJsaXNoaW5nIGl0cyBsZWFzZS4NCj4gDQo+IEEgY29tcGxlbWVudGFy
eSBtb2RpZmljYXRpb24gdG8gcnBjLmdzc2QgaXMgcmVxdWlyZWQgdG8gZnVsbHkgZW5hYmxlDQo+
IHRoaXMgZmVhdHVyZS4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IENodWNrIExldmVyIDxjaHVjay5s
ZXZlckBvcmFjbGUuY29tPg0KPiAtLS0NCj4gIG5ldC9zdW5ycGMvYXV0aF9nc3MvYXV0aF9nc3Mu
YyB8ICAgMjAgKysrKysrKysrKysrKysrKystLS0NCj4gIDEgZmlsZSBjaGFuZ2VkLCAxNyBpbnNl
cnRpb25zKCspLCAzIGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL25ldC9zdW5ycGMv
YXV0aF9nc3MvYXV0aF9nc3MuYw0KPiBiL25ldC9zdW5ycGMvYXV0aF9nc3MvYXV0aF9nc3MuYw0K
PiBpbmRleCBiZThmMTAzLi4xOTQzZTExIDEwMDY0NA0KPiAtLS0gYS9uZXQvc3VucnBjL2F1dGhf
Z3NzL2F1dGhfZ3NzLmMNCj4gKysrIGIvbmV0L3N1bnJwYy9hdXRoX2dzcy9hdXRoX2dzcy5jDQo+
IEBAIC0yODQsNyArMjg0LDEyIEBAIHN0cnVjdCBnc3NfYXV0aCB7DQo+ICAJcmV0dXJuIHA7DQo+
ICB9DQo+ICANCj4gLSNkZWZpbmUgVVBDQUxMX0JVRl9MRU4gMTI4DQo+ICsvKiBYWFg6IE5lZWQg
c29tZSBkb2N1bWVudGF0aW9uIGFib3V0IHdoeSBVUENBTExfQlVGX0xFTiBpcyBzbw0KPiBzbWFs
bC4NCj4gKyAqCUlzIHVzZXIgc3BhY2UgZXhwZWN0aW5nIG5vIG1vcmUgdGhhbiBVUENBTExfQlVG
X0xFTiBieXRlcz8NCj4gKyAqCU5vdGUgdGhhdCB0aGVyZSBhcmUgbm93IF90d29fIE5JX01BWEhP
U1Qgc2l6ZWQgZGF0YSBpdGVtcw0KPiArICoJYmVpbmcgcGFzc2VkIGluIHRoaXMgc3RyaW5nLg0K
PiArICovDQo+ICsjZGVmaW5lIFVQQ0FMTF9CVUZfTEVOCTI1Ng0KPiAgDQoNCldoeT8gVGhlIHNl
cnZpY2VzIGFyZSBjdXJyZW50bHkgIm5mcyIgb3IgIm5mc2QiLiBIb3N0bmFtZXMgYXJlIG5vcm1h
bGx5DQo8IDY0IGNoYXJhY3RlcnMuDQoNCj4gIHN0cnVjdCBnc3NfdXBjYWxsX21zZyB7DQo+ICAJ
cmVmY291bnRfdCBjb3VudDsNCj4gQEAgLTQ2Miw4ICs0NjcsMTcgQEAgc3RhdGljIGludCBnc3Nf
ZW5jb2RlX3YxX21zZyhzdHJ1Y3QNCj4gZ3NzX3VwY2FsbF9tc2cgKmdzc19tc2csDQo+ICAJCXAg
Kz0gbGVuOw0KPiAgCQlnc3NfbXNnLT5tc2cubGVuICs9IGxlbjsNCj4gIAl9DQo+IC0JaWYgKHNl
cnZpY2VfbmFtZSAhPSBOVUxMKSB7DQo+IC0JCWxlbiA9IHNjbnByaW50ZihwLCBidWZsZW4sICJz
ZXJ2aWNlPSVzICIsDQo+IHNlcnZpY2VfbmFtZSk7DQo+ICsJaWYgKHNlcnZpY2VfbmFtZSkgew0K
PiArCQljaGFyICpjID0gc3RyY2hyKHNlcnZpY2VfbmFtZSwgJ0AnKTsNCj4gKw0KPiArCQlpZiAo
IWMpDQo+ICsJCQlsZW4gPSBzY25wcmludGYocCwgYnVmbGVuLCAic2VydmljZT0lcyAiLA0KPiAr
CQkJCQlzZXJ2aWNlX25hbWUpOw0KPiArCQllbHNlDQo+ICsJCQlsZW4gPSBzY25wcmludGYocCwg
YnVmbGVuLA0KPiArCQkJCQkic2VydmljZT0lLipzIHNyY2hvc3Q9JXMgIiwNCj4gKwkJCQkJKGlu
dCkoYyAtIHNlcnZpY2VfbmFtZSksDQo+ICsJCQkJCXNlcnZpY2VfbmFtZSwgYyArIDEpOw0KPiAg
CQlidWZsZW4gLT0gbGVuOw0KPiAgCQlwICs9IGxlbjsNCj4gIAkJZ3NzX21zZy0+bXNnLmxlbiAr
PSBsZW47DQoNCklzbid0IHRoaXMganVzdCBkdXBsaWNhdGluZyB0aGUgZnVuY3Rpb25hbGl0eSBv
ZiB0aGUgJ3RhcmdldCcgYXJndW1lbnQ/DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpDVE8sIEhh
bW1lcnNwYWNlIEluYw0KNDMwMCBFbCBDYW1pbm8gUmVhbCwgU3VpdGUgMTA1DQpMb3MgQWx0b3Ms
IENBIDk0MDIyDQp3d3cuaGFtbWVyLnNwYWNlDQoNCg0K

Re: [PATCH v2 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

[Simo Sorce]

On Thu, 2018-08-16 at 16:27 +0000, Trond Myklebust wrote:
> On Thu, 2018-08-16 at 12:05 -0400, Chuck Lever wrote:
> > A multi-homed NFS server may have more than one "nfs" key in its
> > keytab. Enable the kernel to pick the key it wants as a machine
> > credential when establishing a GSS context.
> > 
> > This is useful for GSS-protected NFSv4.0 callbacks, which are
> > required by RFC 7530 S3.3.3 to use the same principal as the service
> > principal the client used when establishing its lease.
> > 
> > A complementary modification to rpc.gssd is required to fully enable
> > this feature.
> > 
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> >  net/sunrpc/auth_gss/auth_gss.c |   20 +++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/sunrpc/auth_gss/auth_gss.c
> > b/net/sunrpc/auth_gss/auth_gss.c
> > index be8f103..1943e11 100644
> > --- a/net/sunrpc/auth_gss/auth_gss.c
> > +++ b/net/sunrpc/auth_gss/auth_gss.c
> > @@ -284,7 +284,12 @@ struct gss_auth {
> >  	return p;
> >  }
> >  
> > -#define UPCALL_BUF_LEN 128
> > +/* XXX: Need some documentation about why UPCALL_BUF_LEN is so
> > small.
> > + *	Is user space expecting no more than UPCALL_BUF_LEN bytes?
> > + *	Note that there are now _two_ NI_MAXHOST sized data items
> > + *	being passed in this string.
> > + */
> > +#define UPCALL_BUF_LEN	256
> >  
> 
> Why? The services are currently "nfs" or "nfsd". Hostnames are normally
> < 64 characters.

For Kerberos hostnames are fully qualified DNS names, so easily longer
than 64 bytes.

> >  struct gss_upcall_msg {
> >  	refcount_t count;
> > @@ -462,8 +467,17 @@ static int gss_encode_v1_msg(struct
> > gss_upcall_msg *gss_msg,
> >  		p += len;
> >  		gss_msg->msg.len += len;
> >  	}
> > -	if (service_name != NULL) {
> > -		len = scnprintf(p, buflen, "service=%s ",
> > service_name);
> > +	if (service_name) {
> > +		char *c = strchr(service_name, '@');
> > +
> > +		if (!c)
> > +			len = scnprintf(p, buflen, "service=%s ",
> > +					service_name);
> > +		else
> > +			len = scnprintf(p, buflen,
> > +					"service=%.*s srchost=%s ",
> > +					(int)(c - service_name),
> > +					service_name, c + 1);
> >  		buflen -= len;
> >  		p += len;
> >  		gss_msg->msg.len += len;
> 
> Isn't this just duplicating the functionality of the 'target' argument?

No, but I'll let Chuck re-explain.

Chuck, people are often confused about this, perhaps we need a
clarifying comment here to avoid some "optimization" to the code to
happen later.

HTH,
Simo.

> 
> -- 
> Trond Myklebust
> CTO, Hammerspace Inc
> 4300 El Camino Real, Suite 105
> Los Altos, CA 94022
> www.hammer.space
> 
> 

Re: [PATCH v2 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

[Chuck Lever]

> On Aug 16, 2018, at 12:27 PM, Trond Myklebust =
<trondmy@hammerspace.com> wrote:
>=20
> On Thu, 2018-08-16 at 12:05 -0400, Chuck Lever wrote:
>> A multi-homed NFS server may have more than one "nfs" key in its
>> keytab. Enable the kernel to pick the key it wants as a machine
>> credential when establishing a GSS context.
>>=20
>> This is useful for GSS-protected NFSv4.0 callbacks, which are
>> required by RFC 7530 S3.3.3 to use the same principal as the service
>> principal the client used when establishing its lease.
>>=20
>> A complementary modification to rpc.gssd is required to fully enable
>> this feature.
>>=20
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> net/sunrpc/auth_gss/auth_gss.c |   20 +++++++++++++++++---
>> 1 file changed, 17 insertions(+), 3 deletions(-)
>>=20
>> diff --git a/net/sunrpc/auth_gss/auth_gss.c
>> b/net/sunrpc/auth_gss/auth_gss.c
>> index be8f103..1943e11 100644
>> --- a/net/sunrpc/auth_gss/auth_gss.c
>> +++ b/net/sunrpc/auth_gss/auth_gss.c
>> @@ -284,7 +284,12 @@ struct gss_auth {
>> 	return p;
>> }
>>=20
>> -#define UPCALL_BUF_LEN 128
>> +/* XXX: Need some documentation about why UPCALL_BUF_LEN is so
>> small.
>> + *	Is user space expecting no more than UPCALL_BUF_LEN bytes?
>> + *	Note that there are now _two_ NI_MAXHOST sized data items
>> + *	being passed in this string.
>> + */
>> +#define UPCALL_BUF_LEN	256
>>=20
>=20
> Why? The services are currently "nfs" or "nfsd". Hostnames are =
normally
> < 64 characters.

True, but that's an average. We actually should be accommodating the
largest possible hostname here: /usr/include/netdb.h defines NI_MAXHOST
as 1024. Thus even 256 is too small, but as you point out it will work
fine in most real world cases.


>> struct gss_upcall_msg {
>> 	refcount_t count;
>> @@ -462,8 +467,17 @@ static int gss_encode_v1_msg(struct
>> gss_upcall_msg *gss_msg,
>> 		p +=3D len;
>> 		gss_msg->msg.len +=3D len;
>> 	}
>> -	if (service_name !=3D NULL) {
>> -		len =3D scnprintf(p, buflen, "service=3D%s ",
>> service_name);
>> +	if (service_name) {
>> +		char *c =3D strchr(service_name, '@');
>> +
>> +		if (!c)
>> +			len =3D scnprintf(p, buflen, "service=3D%s ",
>> +					service_name);
>> +		else
>> +			len =3D scnprintf(p, buflen,
>> +					"service=3D%.*s srchost=3D%s ",
>> +					(int)(c - service_name),
>> +					service_name, c + 1);
>> 		buflen -=3D len;
>> 		p +=3D len;
>> 		gss_msg->msg.len +=3D len;
>=20
> Isn't this just duplicating the functionality of the 'target' =
argument?

I might not understand your question, but I believe the answer is no.

I have a multi-homed client and server. The primary hostname of the
client is "manet.example.net" and the server is "klimt.example.net"
Both have IB interfaces, respectively "manet.ib.example.net" and
"klimt.ib.example.net".

Now I do this:

manet# mount -o vers=3D4.0,sec=3Dsys klimt.ib.example.net:/export /mnt

The mount is AUTH_SYS, but lease management is done using krb5i because
both systems have a keytab. For the callback channel, RFC 7530 requires
that the server use the client's lease management auth flavor. Moreover,
the server is required to use the same acceptor that the client
authenticated to (in this case, "klimt.ib.example.net").

So now the server establishes a GSS context for it's callback channel:

Aug 16 11:00:17 klimt rpc.gssd[1186]: handle_gssd_upcall: 'mech=3Dkrb5 =
uid=3D0 target=3Dhost@manet.example.net service=3Dnfs =
srchost=3Dklimt.ib.example.net enctypes=3D18,17,16,23,3,1,2 ' =
(nfsd4_cb/clnt4)

The "target" is the principal of the remote service the server is
authenticating to. In full, the target is:

   host/manet.example.net@EXAMPLE.NET

The "srchost" is the hostname part of the server's service principal. =
This
has to match the acceptor string provided by the client, thus the full
source principal has to be:

   nfs/klimt.ib.example.net@EXAMPLE.NET

gssd uses the service=3D and srchost=3D to select the correct key in its
keytab when it establishes a GSS context for the callback channel. =
Without
srchost it would select the key for klimt.example.net, which is the
wrong key in this case. Since this commit:

commit f11b2a1cfbf5dd783eb55cb470509d06e20d1c78
Author:     Jeff Layton <jlayton@poochiereds.net>
AuthorDate: Sat Jun 21 20:52:17 2014 -0400
Commit:     Trond Myklebust <trond.myklebust@primarydata.com>
CommitDate: Sat Jul 12 18:41:25 2014 -0400

    nfs4: copy acceptor name from context to nfs_client

the client rejects NFSv4.0 callbacks using krb5 with the wrong acceptor
string with "NFSv4 callback contains invalid cred". This occurs _after_
the server has established a GSS context and a connection to the client,
so it has already granted delegations which it now cannot recall.


--
Chuck Lever

Re: [PATCH v2 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

[Trond Myklebust]

T24gVGh1LCAyMDE4LTA4LTE2IGF0IDE1OjAzIC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
PiBPbiBBdWcgMTYsIDIwMTgsIGF0IDEyOjI3IFBNLCBUcm9uZCBNeWtsZWJ1c3QgPA0KPiA+IHRy
b25kbXlAaGFtbWVyc3BhY2UuY29tPiB3cm90ZToNCj4gPiANCj4gPiBPbiBUaHUsIDIwMTgtMDgt
MTYgYXQgMTI6MDUgLTA0MDAsIENodWNrIExldmVyIHdyb3RlOg0KPiA+ID4gQSBtdWx0aS1ob21l
ZCBORlMgc2VydmVyIG1heSBoYXZlIG1vcmUgdGhhbiBvbmUgIm5mcyIga2V5IGluIGl0cw0KPiA+
ID4ga2V5dGFiLiBFbmFibGUgdGhlIGtlcm5lbCB0byBwaWNrIHRoZSBrZXkgaXQgd2FudHMgYXMg
YSBtYWNoaW5lDQo+ID4gPiBjcmVkZW50aWFsIHdoZW4gZXN0YWJsaXNoaW5nIGEgR1NTIGNvbnRl
eHQuDQo+ID4gPiANCj4gPiA+IFRoaXMgaXMgdXNlZnVsIGZvciBHU1MtcHJvdGVjdGVkIE5GU3Y0
LjAgY2FsbGJhY2tzLCB3aGljaCBhcmUNCj4gPiA+IHJlcXVpcmVkIGJ5IFJGQyA3NTMwIFMzLjMu
MyB0byB1c2UgdGhlIHNhbWUgcHJpbmNpcGFsIGFzIHRoZQ0KPiA+ID4gc2VydmljZQ0KPiA+ID4g
cHJpbmNpcGFsIHRoZSBjbGllbnQgdXNlZCB3aGVuIGVzdGFibGlzaGluZyBpdHMgbGVhc2UuDQo+
ID4gPiANCj4gPiA+IEEgY29tcGxlbWVudGFyeSBtb2RpZmljYXRpb24gdG8gcnBjLmdzc2QgaXMg
cmVxdWlyZWQgdG8gZnVsbHkNCj4gPiA+IGVuYWJsZQ0KPiA+ID4gdGhpcyBmZWF0dXJlLg0KPiA+
ID4gDQo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBDaHVjayBMZXZlciA8Y2h1Y2subGV2ZXJAb3JhY2xl
LmNvbT4NCj4gPiA+IC0tLQ0KPiA+ID4gbmV0L3N1bnJwYy9hdXRoX2dzcy9hdXRoX2dzcy5jIHwg
ICAyMCArKysrKysrKysrKysrKysrKy0tLQ0KPiA+ID4gMSBmaWxlIGNoYW5nZWQsIDE3IGluc2Vy
dGlvbnMoKyksIDMgZGVsZXRpb25zKC0pDQo+ID4gPiANCj4gPiA+IGRpZmYgLS1naXQgYS9uZXQv
c3VucnBjL2F1dGhfZ3NzL2F1dGhfZ3NzLmMNCj4gPiA+IGIvbmV0L3N1bnJwYy9hdXRoX2dzcy9h
dXRoX2dzcy5jDQo+ID4gPiBpbmRleCBiZThmMTAzLi4xOTQzZTExIDEwMDY0NA0KPiA+ID4gLS0t
IGEvbmV0L3N1bnJwYy9hdXRoX2dzcy9hdXRoX2dzcy5jDQo+ID4gPiArKysgYi9uZXQvc3VucnBj
L2F1dGhfZ3NzL2F1dGhfZ3NzLmMNCj4gPiA+IEBAIC0yODQsNyArMjg0LDEyIEBAIHN0cnVjdCBn
c3NfYXV0aCB7DQo+ID4gPiAJcmV0dXJuIHA7DQo+ID4gPiB9DQo+ID4gPiANCj4gPiA+IC0jZGVm
aW5lIFVQQ0FMTF9CVUZfTEVOIDEyOA0KPiA+ID4gKy8qIFhYWDogTmVlZCBzb21lIGRvY3VtZW50
YXRpb24gYWJvdXQgd2h5IFVQQ0FMTF9CVUZfTEVOIGlzIHNvDQo+ID4gPiBzbWFsbC4NCj4gPiA+
ICsgKglJcyB1c2VyIHNwYWNlIGV4cGVjdGluZyBubyBtb3JlIHRoYW4gVVBDQUxMX0JVRl9MRU4N
Cj4gPiA+IGJ5dGVzPw0KPiA+ID4gKyAqCU5vdGUgdGhhdCB0aGVyZSBhcmUgbm93IF90d29fIE5J
X01BWEhPU1Qgc2l6ZWQgZGF0YQ0KPiA+ID4gaXRlbXMNCj4gPiA+ICsgKgliZWluZyBwYXNzZWQg
aW4gdGhpcyBzdHJpbmcuDQo+ID4gPiArICovDQo+ID4gPiArI2RlZmluZSBVUENBTExfQlVGX0xF
TgkyNTYNCj4gPiA+IA0KPiA+IA0KPiA+IFdoeT8gVGhlIHNlcnZpY2VzIGFyZSBjdXJyZW50bHkg
Im5mcyIgb3IgIm5mc2QiLiBIb3N0bmFtZXMgYXJlDQo+ID4gbm9ybWFsbHkNCj4gPiA8IDY0IGNo
YXJhY3RlcnMuDQo+IA0KPiBUcnVlLCBidXQgdGhhdCdzIGFuIGF2ZXJhZ2UuIFdlIGFjdHVhbGx5
IHNob3VsZCBiZSBhY2NvbW1vZGF0aW5nIHRoZQ0KPiBsYXJnZXN0IHBvc3NpYmxlIGhvc3RuYW1l
IGhlcmU6IC91c3IvaW5jbHVkZS9uZXRkYi5oIGRlZmluZXMNCj4gTklfTUFYSE9TVA0KPiBhcyAx
MDI0LiBUaHVzIGV2ZW4gMjU2IGlzIHRvbyBzbWFsbCwgYnV0IGFzIHlvdSBwb2ludCBvdXQgaXQg
d2lsbA0KPiB3b3JrDQo+IGZpbmUgaW4gbW9zdCByZWFsIHdvcmxkIGNhc2VzLg0KPiANCj4gDQo+
ID4gPiBzdHJ1Y3QgZ3NzX3VwY2FsbF9tc2cgew0KPiA+ID4gCXJlZmNvdW50X3QgY291bnQ7DQo+
ID4gPiBAQCAtNDYyLDggKzQ2NywxNyBAQCBzdGF0aWMgaW50IGdzc19lbmNvZGVfdjFfbXNnKHN0
cnVjdA0KPiA+ID4gZ3NzX3VwY2FsbF9tc2cgKmdzc19tc2csDQo+ID4gPiAJCXAgKz0gbGVuOw0K
PiA+ID4gCQlnc3NfbXNnLT5tc2cubGVuICs9IGxlbjsNCj4gPiA+IAl9DQo+ID4gPiAtCWlmIChz
ZXJ2aWNlX25hbWUgIT0gTlVMTCkgew0KPiA+ID4gLQkJbGVuID0gc2NucHJpbnRmKHAsIGJ1Zmxl
biwgInNlcnZpY2U9JXMgIiwNCj4gPiA+IHNlcnZpY2VfbmFtZSk7DQo+ID4gPiArCWlmIChzZXJ2
aWNlX25hbWUpIHsNCj4gPiA+ICsJCWNoYXIgKmMgPSBzdHJjaHIoc2VydmljZV9uYW1lLCAnQCcp
Ow0KPiA+ID4gKw0KPiA+ID4gKwkJaWYgKCFjKQ0KPiA+ID4gKwkJCWxlbiA9IHNjbnByaW50Zihw
LCBidWZsZW4sICJzZXJ2aWNlPSVzICIsDQo+ID4gPiArCQkJCQlzZXJ2aWNlX25hbWUpOw0KPiA+
ID4gKwkJZWxzZQ0KPiA+ID4gKwkJCWxlbiA9IHNjbnByaW50ZihwLCBidWZsZW4sDQo+ID4gPiAr
CQkJCQkic2VydmljZT0lLipzIHNyY2hvc3Q9JXMgIiwNCj4gPiA+ICsJCQkJCShpbnQpKGMgLSBz
ZXJ2aWNlX25hbWUpLA0KPiA+ID4gKwkJCQkJc2VydmljZV9uYW1lLCBjICsgMSk7DQo+ID4gPiAJ
CWJ1ZmxlbiAtPSBsZW47DQo+ID4gPiAJCXAgKz0gbGVuOw0KPiA+ID4gCQlnc3NfbXNnLT5tc2cu
bGVuICs9IGxlbjsNCj4gPiANCj4gPiBJc24ndCB0aGlzIGp1c3QgZHVwbGljYXRpbmcgdGhlIGZ1
bmN0aW9uYWxpdHkgb2YgdGhlICd0YXJnZXQnDQo+ID4gYXJndW1lbnQ/DQo+IA0KPiBJIG1pZ2h0
IG5vdCB1bmRlcnN0YW5kIHlvdXIgcXVlc3Rpb24sIGJ1dCBJIGJlbGlldmUgdGhlIGFuc3dlciBp
cyBuby4NCj4gDQo+IEkgaGF2ZSBhIG11bHRpLWhvbWVkIGNsaWVudCBhbmQgc2VydmVyLiBUaGUg
cHJpbWFyeSBob3N0bmFtZSBvZiB0aGUNCj4gY2xpZW50IGlzICJtYW5ldC5leGFtcGxlLm5ldCIg
YW5kIHRoZSBzZXJ2ZXIgaXMgImtsaW10LmV4YW1wbGUubmV0Ig0KPiBCb3RoIGhhdmUgSUIgaW50
ZXJmYWNlcywgcmVzcGVjdGl2ZWx5ICJtYW5ldC5pYi5leGFtcGxlLm5ldCIgYW5kDQo+ICJrbGlt
dC5pYi5leGFtcGxlLm5ldCIuDQo+IA0KPiBOb3cgSSBkbyB0aGlzOg0KPiANCj4gbWFuZXQjIG1v
dW50IC1vIHZlcnM9NC4wLHNlYz1zeXMga2xpbXQuaWIuZXhhbXBsZS5uZXQ6L2V4cG9ydCAvbW50
DQo+IA0KPiBUaGUgbW91bnQgaXMgQVVUSF9TWVMsIGJ1dCBsZWFzZSBtYW5hZ2VtZW50IGlzIGRv
bmUgdXNpbmcga3JiNWkNCj4gYmVjYXVzZQ0KPiBib3RoIHN5c3RlbXMgaGF2ZSBhIGtleXRhYi4g
Rm9yIHRoZSBjYWxsYmFjayBjaGFubmVsLCBSRkMgNzUzMA0KPiByZXF1aXJlcw0KPiB0aGF0IHRo
ZSBzZXJ2ZXIgdXNlIHRoZSBjbGllbnQncyBsZWFzZSBtYW5hZ2VtZW50IGF1dGggZmxhdm9yLg0K
PiBNb3Jlb3ZlciwNCj4gdGhlIHNlcnZlciBpcyByZXF1aXJlZCB0byB1c2UgdGhlIHNhbWUgYWNj
ZXB0b3IgdGhhdCB0aGUgY2xpZW50DQo+IGF1dGhlbnRpY2F0ZWQgdG8gKGluIHRoaXMgY2FzZSwg
ImtsaW10LmliLmV4YW1wbGUubmV0IikuDQo+IA0KPiBTbyBub3cgdGhlIHNlcnZlciBlc3RhYmxp
c2hlcyBhIEdTUyBjb250ZXh0IGZvciBpdCdzIGNhbGxiYWNrDQo+IGNoYW5uZWw6DQo+IA0KPiBB
dWcgMTYgMTE6MDA6MTcga2xpbXQgcnBjLmdzc2RbMTE4Nl06IGhhbmRsZV9nc3NkX3VwY2FsbDog
J21lY2g9a3JiNQ0KPiB1aWQ9MCB0YXJnZXQ9aG9zdEBtYW5ldC5leGFtcGxlLm5ldCBzZXJ2aWNl
PW5mcw0KPiBzcmNob3N0PWtsaW10LmliLmV4YW1wbGUubmV0IGVuY3R5cGVzPTE4LDE3LDE2LDIz
LDMsMSwyICcNCj4gKG5mc2Q0X2NiL2NsbnQ0KQ0KPiANCj4gVGhlICJ0YXJnZXQiIGlzIHRoZSBw
cmluY2lwYWwgb2YgdGhlIHJlbW90ZSBzZXJ2aWNlIHRoZSBzZXJ2ZXIgaXMNCj4gYXV0aGVudGlj
YXRpbmcgdG8uIEluIGZ1bGwsIHRoZSB0YXJnZXQgaXM6DQo+IA0KPiAgICBob3N0L21hbmV0LmV4
YW1wbGUubmV0QEVYQU1QTEUuTkVUDQo+IA0KPiBUaGUgInNyY2hvc3QiIGlzIHRoZSBob3N0bmFt
ZSBwYXJ0IG9mIHRoZSBzZXJ2ZXIncyBzZXJ2aWNlIHByaW5jaXBhbC4NCj4gVGhpcw0KPiBoYXMg
dG8gbWF0Y2ggdGhlIGFjY2VwdG9yIHN0cmluZyBwcm92aWRlZCBieSB0aGUgY2xpZW50LCB0aHVz
IHRoZQ0KPiBmdWxsDQo+IHNvdXJjZSBwcmluY2lwYWwgaGFzIHRvIGJlOg0KPiANCj4gICAgbmZz
L2tsaW10LmliLmV4YW1wbGUubmV0QEVYQU1QTEUuTkVUDQo+IA0KPiBnc3NkIHVzZXMgdGhlIHNl
cnZpY2U9IGFuZCBzcmNob3N0PSB0byBzZWxlY3QgdGhlIGNvcnJlY3Qga2V5IGluIGl0cw0KPiBr
ZXl0YWIgd2hlbiBpdCBlc3RhYmxpc2hlcyBhIEdTUyBjb250ZXh0IGZvciB0aGUgY2FsbGJhY2sg
Y2hhbm5lbC4NCj4gV2l0aG91dA0KPiBzcmNob3N0IGl0IHdvdWxkIHNlbGVjdCB0aGUga2V5IGZv
ciBrbGltdC5leGFtcGxlLm5ldCwgd2hpY2ggaXMgdGhlDQo+IHdyb25nIGtleSBpbiB0aGlzIGNh
c2UuIFNpbmNlIHRoaXMgY29tbWl0Og0KPiANCj4gY29tbWl0IGYxMWIyYTFjZmJmNWRkNzgzZWI1
NWNiNDcwNTA5ZDA2ZTIwZDFjNzgNCj4gQXV0aG9yOiAgICAgSmVmZiBMYXl0b24gPGpsYXl0b25A
cG9vY2hpZXJlZHMubmV0Pg0KPiBBdXRob3JEYXRlOiBTYXQgSnVuIDIxIDIwOjUyOjE3IDIwMTQg
LTA0MDANCj4gQ29tbWl0OiAgICAgVHJvbmQgTXlrbGVidXN0IDx0cm9uZC5teWtsZWJ1c3RAcHJp
bWFyeWRhdGEuY29tPg0KPiBDb21taXREYXRlOiBTYXQgSnVsIDEyIDE4OjQxOjI1IDIwMTQgLTA0
MDANCj4gDQo+ICAgICBuZnM0OiBjb3B5IGFjY2VwdG9yIG5hbWUgZnJvbSBjb250ZXh0IHRvIG5m
c19jbGllbnQNCj4gDQo+IHRoZSBjbGllbnQgcmVqZWN0cyBORlN2NC4wIGNhbGxiYWNrcyB1c2lu
ZyBrcmI1IHdpdGggdGhlIHdyb25nDQo+IGFjY2VwdG9yDQo+IHN0cmluZyB3aXRoICJORlN2NCBj
YWxsYmFjayBjb250YWlucyBpbnZhbGlkIGNyZWQiLiBUaGlzIG9jY3Vycw0KPiBfYWZ0ZXJfDQo+
IHRoZSBzZXJ2ZXIgaGFzIGVzdGFibGlzaGVkIGEgR1NTIGNvbnRleHQgYW5kIGEgY29ubmVjdGlv
biB0byB0aGUNCj4gY2xpZW50LA0KPiBzbyBpdCBoYXMgYWxyZWFkeSBncmFudGVkIGRlbGVnYXRp
b25zIHdoaWNoIGl0IG5vdyBjYW5ub3QgcmVjYWxsLg0KPiANCg0KVGhlIGFib3ZlIHN0b3J5IHJl
YWxseSBkb2VzIGRlc2VydmUgYW4gZXhwbGFuYXRpb24gaW4gdGhlIGNvZGUgZm9yIHRoZQ0KdXBj
YWxsLg0KUmlnaHQgbm93LCB5b3UgaGF2ZSB0byBodW50IHRocm91Z2ggbmVzdGVkIGxheWVycyBv
ZiBzdHJ1Y3QgY29waWVzIHRvDQpmaWd1cmUgb3V0IHRoYXQgJ3RhcmdldCcgcmVmZXJzIHRvIHRo
ZSBzZXJ2aWNlIG9uIHRoZSBzZXJ2ZXIgc2lkZQ0KY2FsbGJhY2sgY2hhbm5lbCBmb3IgTkZTdjQu
MCBvbmx5LiBBIHNlY29uZCBob3N0bmFtZSBmaWVsZCBmb3IgdGhlIHNhbWUNCnVzZSBjYXNlIGp1
c3QgYWRkcyB0byB0aGF0IGNvbmZ1c2lvbi4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4
IE5GUyBjbGllbnQgbWFpbnRhaW5lciwgSGFtbWVyc3BhY2UNCnRyb25kLm15a2xlYnVzdEBoYW1t
ZXJzcGFjZS5jb20NCg0KDQo=

Re: [PATCH v2 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

[Chuck Lever]

> On Aug 17, 2018, at 12:54 AM, Trond Myklebust =
<trondmy@hammerspace.com> wrote:
>=20
> On Thu, 2018-08-16 at 15:03 -0400, Chuck Lever wrote:
>>> On Aug 16, 2018, at 12:27 PM, Trond Myklebust <
>>> trondmy@hammerspace.com> wrote:
>>>=20
>>> On Thu, 2018-08-16 at 12:05 -0400, Chuck Lever wrote:
>>>> A multi-homed NFS server may have more than one "nfs" key in its
>>>> keytab. Enable the kernel to pick the key it wants as a machine
>>>> credential when establishing a GSS context.
>>>>=20
>>>> This is useful for GSS-protected NFSv4.0 callbacks, which are
>>>> required by RFC 7530 S3.3.3 to use the same principal as the
>>>> service
>>>> principal the client used when establishing its lease.
>>>>=20
>>>> A complementary modification to rpc.gssd is required to fully
>>>> enable
>>>> this feature.
>>>>=20
>>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>>> ---
>>>> net/sunrpc/auth_gss/auth_gss.c |   20 +++++++++++++++++---
>>>> 1 file changed, 17 insertions(+), 3 deletions(-)
>>>>=20
>>>> diff --git a/net/sunrpc/auth_gss/auth_gss.c
>>>> b/net/sunrpc/auth_gss/auth_gss.c
>>>> index be8f103..1943e11 100644
>>>> --- a/net/sunrpc/auth_gss/auth_gss.c
>>>> +++ b/net/sunrpc/auth_gss/auth_gss.c
>>>> @@ -284,7 +284,12 @@ struct gss_auth {
>>>> 	return p;
>>>> }
>>>>=20
>>>> -#define UPCALL_BUF_LEN 128
>>>> +/* XXX: Need some documentation about why UPCALL_BUF_LEN is so
>>>> small.
>>>> + *	Is user space expecting no more than UPCALL_BUF_LEN
>>>> bytes?
>>>> + *	Note that there are now _two_ NI_MAXHOST sized data
>>>> items
>>>> + *	being passed in this string.
>>>> + */
>>>> +#define UPCALL_BUF_LEN	256
>>>>=20
>>>=20
>>> Why? The services are currently "nfs" or "nfsd". Hostnames are
>>> normally
>>> < 64 characters.
>>=20
>> True, but that's an average. We actually should be accommodating the
>> largest possible hostname here: /usr/include/netdb.h defines
>> NI_MAXHOST
>> as 1024. Thus even 256 is too small, but as you point out it will
>> work
>> fine in most real world cases.
>>=20
>>=20
>>>> struct gss_upcall_msg {
>>>> 	refcount_t count;
>>>> @@ -462,8 +467,17 @@ static int gss_encode_v1_msg(struct
>>>> gss_upcall_msg *gss_msg,
>>>> 		p +=3D len;
>>>> 		gss_msg->msg.len +=3D len;
>>>> 	}
>>>> -	if (service_name !=3D NULL) {
>>>> -		len =3D scnprintf(p, buflen, "service=3D%s ",
>>>> service_name);
>>>> +	if (service_name) {
>>>> +		char *c =3D strchr(service_name, '@');
>>>> +
>>>> +		if (!c)
>>>> +			len =3D scnprintf(p, buflen, "service=3D%s ",
>>>> +					service_name);
>>>> +		else
>>>> +			len =3D scnprintf(p, buflen,
>>>> +					"service=3D%.*s srchost=3D%s ",
>>>> +					(int)(c - service_name),
>>>> +					service_name, c + 1);
>>>> 		buflen -=3D len;
>>>> 		p +=3D len;
>>>> 		gss_msg->msg.len +=3D len;
>>>=20
>>> Isn't this just duplicating the functionality of the 'target'
>>> argument?
>>=20
>> I might not understand your question, but I believe the answer is no.
>>=20
>> I have a multi-homed client and server. The primary hostname of the
>> client is "manet.example.net" and the server is "klimt.example.net"
>> Both have IB interfaces, respectively "manet.ib.example.net" and
>> "klimt.ib.example.net".
>>=20
>> Now I do this:
>>=20
>> manet# mount -o vers=3D4.0,sec=3Dsys klimt.ib.example.net:/export =
/mnt
>>=20
>> The mount is AUTH_SYS, but lease management is done using krb5i
>> because
>> both systems have a keytab. For the callback channel, RFC 7530
>> requires
>> that the server use the client's lease management auth flavor.
>> Moreover,
>> the server is required to use the same acceptor that the client
>> authenticated to (in this case, "klimt.ib.example.net").
>>=20
>> So now the server establishes a GSS context for it's callback
>> channel:
>>=20
>> Aug 16 11:00:17 klimt rpc.gssd[1186]: handle_gssd_upcall: 'mech=3Dkrb5
>> uid=3D0 target=3Dhost@manet.example.net service=3Dnfs
>> srchost=3Dklimt.ib.example.net enctypes=3D18,17,16,23,3,1,2 '
>> (nfsd4_cb/clnt4)
>>=20
>> The "target" is the principal of the remote service the server is
>> authenticating to. In full, the target is:
>>=20
>>   host/manet.example.net@EXAMPLE.NET
>>=20
>> The "srchost" is the hostname part of the server's service principal.
>> This
>> has to match the acceptor string provided by the client, thus the
>> full
>> source principal has to be:
>>=20
>>   nfs/klimt.ib.example.net@EXAMPLE.NET
>>=20
>> gssd uses the service=3D and srchost=3D to select the correct key in =
its
>> keytab when it establishes a GSS context for the callback channel.
>> Without
>> srchost it would select the key for klimt.example.net, which is the
>> wrong key in this case. Since this commit:
>>=20
>> commit f11b2a1cfbf5dd783eb55cb470509d06e20d1c78
>> Author:     Jeff Layton <jlayton@poochiereds.net>
>> AuthorDate: Sat Jun 21 20:52:17 2014 -0400
>> Commit:     Trond Myklebust <trond.myklebust@primarydata.com>
>> CommitDate: Sat Jul 12 18:41:25 2014 -0400
>>=20
>>    nfs4: copy acceptor name from context to nfs_client
>>=20
>> the client rejects NFSv4.0 callbacks using krb5 with the wrong
>> acceptor
>> string with "NFSv4 callback contains invalid cred". This occurs
>> _after_
>> the server has established a GSS context and a connection to the
>> client,
>> so it has already granted delegations which it now cannot recall.
>>=20
>=20
> The above story really does deserve an explanation in the code for the
> upcall.
> Right now, you have to hunt through nested layers of struct copies to
> figure out that 'target' refers to the service on the server side
> callback channel for NFSv4.0 only. A second hostname field for the =
same
> use case just adds to that confusion.

I can send an additional patch that adds one or more comments =
documenting
the function of target=3D, service=3D, and srchost=3D.


--
Chuck Lever

Re: [PATCH v2 1/4] sunrpc: Enable the kernel to specify the hostname part of service principals

[Trond Myklebust]

T24gRnJpLCAyMDE4LTA4LTE3IGF0IDA5OjQzIC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
SSBjYW4gc2VuZCBhbiBhZGRpdGlvbmFsIHBhdGNoIHRoYXQgYWRkcyBvbmUgb3IgbW9yZSBjb21t
ZW50cw0KPiBkb2N1bWVudGluZw0KPiB0aGUgZnVuY3Rpb24gb2YgdGFyZ2V0PSwgc2VydmljZT0s
IGFuZCBzcmNob3N0PS4NCg0KUGxlYXNlIGRvLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGlu
dXggTkZTIGNsaWVudCBtYWludGFpbmVyLCBIYW1tZXJzcGFjZQ0KdHJvbmQubXlrbGVidXN0QGhh
bW1lcnNwYWNlLmNvbQ0KDQoNCg==

[PATCH v2 2/4] sunrpc: Extract target name into svc_cred

[Chuck Lever]

NFSv4.0 callback needs to know the GSS target name the client used
when it established its lease. That information is available from
the GSS context created by gssproxy. Make it available in each
svc_cred.

Note this will also give us access to the real target service
principal name (which is typically "nfs", but spec does not require
that).

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4state.c                  |    7 ++-
 include/linux/sunrpc/svcauth.h       |    3 +
 net/sunrpc/auth_gss/gss_rpc_upcall.c |   70 ++++++++++++++++++++++------------
 3 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8571414..9d9bc4c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1979,8 +1979,10 @@ static int copy_cred(struct svc_cred *target, struct svc_cred *source)
 	target->cr_principal = kstrdup(source->cr_principal, GFP_KERNEL);
 	target->cr_raw_principal = kstrdup(source->cr_raw_principal,
 								GFP_KERNEL);
-	if ((source->cr_principal && ! target->cr_principal) ||
-	    (source->cr_raw_principal && ! target->cr_raw_principal))
+	target->cr_targ_princ = kstrdup(source->cr_targ_princ, GFP_KERNEL);
+	if ((source->cr_principal && !target->cr_principal) ||
+	    (source->cr_raw_principal && !target->cr_raw_principal) ||
+	    (source->cr_targ_princ && !target->cr_targ_princ))
 		return -ENOMEM;
 
 	target->cr_flavor = source->cr_flavor;
@@ -2057,6 +2059,7 @@ static bool is_gss_cred(struct svc_cred *cr)
 		|| (!gid_eq(cr1->cr_gid, cr2->cr_gid))
 		|| !groups_equal(cr1->cr_group_info, cr2->cr_group_info))
 		return false;
+	/* XXX: check that cr_targ_princ fields match ? */
 	if (cr1->cr_principal == cr2->cr_principal)
 		return true;
 	if (!cr1->cr_principal || !cr2->cr_principal)
diff --git a/include/linux/sunrpc/svcauth.h b/include/linux/sunrpc/svcauth.h
index 7c36565..04e404a 100644
--- a/include/linux/sunrpc/svcauth.h
+++ b/include/linux/sunrpc/svcauth.h
@@ -31,6 +31,7 @@ struct svc_cred {
 	/* name of form servicetype@hostname, passed down by
 	 * rpc.svcgssd, or computed from the above: */
 	char			*cr_principal;
+	char			*cr_targ_princ;
 	struct gss_api_mech	*cr_gss_mech;
 };
 
@@ -39,6 +40,7 @@ static inline void init_svc_cred(struct svc_cred *cred)
 	cred->cr_group_info = NULL;
 	cred->cr_raw_principal = NULL;
 	cred->cr_principal = NULL;
+	cred->cr_targ_princ = NULL;
 	cred->cr_gss_mech = NULL;
 }
 
@@ -48,6 +50,7 @@ static inline void free_svc_cred(struct svc_cred *cred)
 		put_group_info(cred->cr_group_info);
 	kfree(cred->cr_raw_principal);
 	kfree(cred->cr_principal);
+	kfree(cred->cr_targ_princ);
 	gss_mech_put(cred->cr_gss_mech);
 	init_svc_cred(cred);
 }
diff --git a/net/sunrpc/auth_gss/gss_rpc_upcall.c b/net/sunrpc/auth_gss/gss_rpc_upcall.c
index 1c7c49d..73dcda0 100644
--- a/net/sunrpc/auth_gss/gss_rpc_upcall.c
+++ b/net/sunrpc/auth_gss/gss_rpc_upcall.c
@@ -234,6 +234,35 @@ static int gssp_alloc_receive_pages(struct gssx_arg_accept_sec_context *arg)
 	return 0;
 }
 
+static char *gssp_stringify(struct xdr_netobj *netobj)
+{
+	return kstrndup(netobj->data, netobj->len, GFP_KERNEL);
+}
+
+static void gssp_hostbased_service(char **principal)
+{
+	char *c;
+
+	if (!*principal)
+		return;
+
+	/* terminate and remove realm part */
+	c = strchr(*principal, '@');
+	if (c) {
+		*c = '\0';
+
+		/* change service-hostname delimiter */
+		c = strchr(*principal, '/');
+		if (c)
+			*c = '@';
+	}
+	if (!c) {
+		/* not a service principal */
+		kfree(*principal);
+		*principal = NULL;
+	}
+}
+
 /*
  * Public functions
  */
@@ -262,6 +291,7 @@ int gssp_accept_sec_context_upcall(struct net *net,
 		 */
 		.exported_context_token.len = GSSX_max_output_handle_sz,
 		.mech.len = GSS_OID_MAX_LEN,
+		.targ_name.display_name.len = GSSX_max_princ_sz,
 		.src_name.display_name.len = GSSX_max_princ_sz
 	};
 	struct gssx_res_accept_sec_context res = {
@@ -275,6 +305,7 @@ int gssp_accept_sec_context_upcall(struct net *net,
 		.rpc_cred = NULL, /* FIXME ? */
 	};
 	struct xdr_netobj client_name = { 0 , NULL };
+	struct xdr_netobj target_name = { 0, NULL };
 	int ret;
 
 	if (data->in_handle.len != 0)
@@ -285,8 +316,6 @@ int gssp_accept_sec_context_upcall(struct net *net,
 	if (ret)
 		return ret;
 
-	/* use nfs/ for targ_name ? */
-
 	ret = gssp_call(net, &msg);
 
 	gssp_free_receive_pages(&arg);
@@ -304,6 +333,7 @@ int gssp_accept_sec_context_upcall(struct net *net,
 			kfree(rctxh.mech.data);
 		}
 		client_name = rctxh.src_name.display_name;
+		target_name = rctxh.targ_name.display_name;
 	}
 
 	if (res.options.count == 1) {
@@ -325,32 +355,22 @@ int gssp_accept_sec_context_upcall(struct net *net,
 	}
 
 	/* convert to GSS_NT_HOSTBASED_SERVICE form and set into creds */
-	if (data->found_creds && client_name.data != NULL) {
-		char *c;
-
-		data->creds.cr_raw_principal = kstrndup(client_name.data,
-						client_name.len, GFP_KERNEL);
-
-		data->creds.cr_principal = kstrndup(client_name.data,
-						client_name.len, GFP_KERNEL);
-		if (data->creds.cr_principal) {
-			/* terminate and remove realm part */
-			c = strchr(data->creds.cr_principal, '@');
-			if (c) {
-				*c = '\0';
-
-				/* change service-hostname delimiter */
-				c = strchr(data->creds.cr_principal, '/');
-				if (c) *c = '@';
-			}
-			if (!c) {
-				/* not a service principal */
-				kfree(data->creds.cr_principal);
-				data->creds.cr_principal = NULL;
-			}
+	if (data->found_creds) {
+		if (client_name.data) {
+			data->creds.cr_raw_principal =
+					gssp_stringify(&client_name);
+			data->creds.cr_principal =
+					gssp_stringify(&client_name);
+			gssp_hostbased_service(&data->creds.cr_principal);
+		}
+		if (target_name.data) {
+			data->creds.cr_targ_princ =
+					gssp_stringify(&target_name);
+			gssp_hostbased_service(&data->creds.cr_targ_princ);
 		}
 	}
 	kfree(client_name.data);
+	kfree(target_name.data);
 
 	return ret;
 }

[PATCH v2 3/4] nfsd: Use correct credential for NFSv4.0 callback with GSS

[Chuck Lever]

I've had trouble when operating a multi-homed Linux NFS server with
Kerberos using NFSv4.0. Lately, I've seen my clients reporting
this (and then hanging):

May  9 11:43:26 manet kernel: NFS: NFSv4 callback contains invalid cred

The client-side commit f11b2a1cfbf5 ("nfs4: copy acceptor name from
context to nfs_client") appears to be related, but I suspect this
problem has been going on for some time before that.

RFC 7530 Section 3.3.3 says:
> For Kerberos V5, nfs/hostname would be a server principal in the
> Kerberos Key Distribution Center database.  This is the same
> principal the client acquired a GSS-API context for when it issued
> the SETCLIENTID operation ...

In other words, an NFSv4.0 client expects that the server will use
the same GSS principal for callback that the client used to
establish its lease. For example, if the client used the service
principal "nfs@server.domain" to establish its lease, the server
is required to use "nfs@server.domain" when performing NFSv4.0
callback operations.

The Linux NFS server currently does not. It uses a common service
principal for all callback connections. Sometimes this works as
expected, and other times -- for example, when the server is
accessible via multiple hostnames -- it won't work at all.

This patch scrapes the target name from the client credential,
and uses that for the NFSv4.0 callback credential. That should
be correct much more often.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4callback.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 1f04d2a..0e9ff86 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -769,7 +769,14 @@ void cleanup_callback_cred(void)
 static struct rpc_cred *get_backchannel_cred(struct nfs4_client *clp, struct rpc_clnt *client, struct nfsd4_session *ses)
 {
 	if (clp->cl_minorversion == 0) {
-		return get_rpccred(callback_cred);
+		char *principal = clp->cl_cred.cr_targ_princ ?
+					clp->cl_cred.cr_targ_princ : "nfs";
+		struct rpc_cred *cred;
+
+		cred = rpc_lookup_machine_cred(principal);
+		if (!IS_ERR(cred))
+			get_rpccred(cred);
+		return cred;
 	} else {
 		struct rpc_auth *auth = client->cl_auth;
 		struct auth_cred acred = {};

[PATCH v2 4/4] nfsd: Remove callback_cred

[Chuck Lever]

Clean up: The global callback_cred is no longer used, so it can be
removed.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4callback.c |   20 --------------------
 fs/nfsd/nfs4state.c    |   10 ++--------
 fs/nfsd/state.h        |    2 --
 3 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 0e9ff86..c088f97 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -746,26 +746,6 @@ static int max_cb_time(struct net *net)
 	return max(nn->nfsd4_lease/10, (time_t)1) * HZ;
 }
 
-static struct rpc_cred *callback_cred;
-
-int set_callback_cred(void)
-{
-	if (callback_cred)
-		return 0;
-	callback_cred = rpc_lookup_machine_cred("nfs");
-	if (!callback_cred)
-		return -ENOMEM;
-	return 0;
-}
-
-void cleanup_callback_cred(void)
-{
-	if (callback_cred) {
-		put_rpccred(callback_cred);
-		callback_cred = NULL;
-	}
-}
-
 static struct rpc_cred *get_backchannel_cred(struct nfs4_client *clp, struct rpc_clnt *client, struct nfsd4_session *ses)
 {
 	if (clp->cl_minorversion == 0) {
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9d9bc4c..b7e68fd 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7202,14 +7202,10 @@ static int nfs4_state_create_net(struct net *net)
 {
 	int ret;
 
-	ret = set_callback_cred();
-	if (ret)
-		return ret;
-
 	laundry_wq = alloc_workqueue("%s", WQ_UNBOUND, 0, "nfsd4");
 	if (laundry_wq == NULL) {
 		ret = -ENOMEM;
-		goto out_cleanup_cred;
+		goto out;
 	}
 	ret = nfsd4_create_callback_queue();
 	if (ret)
@@ -7220,8 +7216,7 @@ static int nfs4_state_create_net(struct net *net)
 
 out_free_laundry:
 	destroy_workqueue(laundry_wq);
-out_cleanup_cred:
-	cleanup_callback_cred();
+out:
 	return ret;
 }
 
@@ -7258,7 +7253,6 @@ static int nfs4_state_create_net(struct net *net)
 {
 	destroy_workqueue(laundry_wq);
 	nfsd4_destroy_callback_queue();
-	cleanup_callback_cred();
 }
 
 static void
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index f3772ea..0b15dac 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -617,8 +617,6 @@ extern struct nfs4_client_reclaim *nfsd4_find_reclaim_client(const char *recdir,
 							struct nfsd_net *nn);
 extern __be32 nfs4_check_open_reclaim(clientid_t *clid,
 		struct nfsd4_compound_state *cstate, struct nfsd_net *nn);
-extern int set_callback_cred(void);
-extern void cleanup_callback_cred(void);
 extern void nfsd4_probe_callback(struct nfs4_client *clp);
 extern void nfsd4_probe_callback_sync(struct nfs4_client *clp);
 extern void nfsd4_change_callback(struct nfs4_client *clp, struct nfs4_cb_conn *);