From: Jeff Layton <hidden> Date: 2017-08-11 14:22:24
On Fri, 2017-08-11 at 09:49 -0400, Olga Kornievskaia wrote:
quoted
On Aug 11, 2017, at 7:18 AM, Jeff Layton [off-list ref] wrote:
On Fri, 2017-08-11 at 17:17 +1000, NeilBrown wrote:
quoted
On Wed, Aug 09 2017, Jeff Layton wrote:
....
quoted
Thanks, that helps a bit. I'm less clear on what the higher-level
vision is here though:
Are we all going to be running scripts on logout that scrape
/proc/mounts and run fslogout on each? Will this be added to kdestroy?
Or are you aiming to have KCM do this on some trigger? (see:
https://fedoraproject.org/wiki/Changes/KerberosKCMCache)
Also, doing this per-mount seems wrong to me. Shouldn't this be done on
a per-net-namespace basis or maybe even globally?
Having looked at the code, I think this is invalidating cached
credentials globally -- or at least, globally for all filesystems that
use sunrpc.
I actually question the premise for wanting to do this. Tickets have a
timeout and will expire. Any code that is allowed to get a ticket, can
hold on to it as long as it likes - but it will cease to work after the
expiry time. Hunting out all the places that a key might be cached, and
invalidating them, seems to deviate from the model. If you are concerned
about leaving credentials around where they can theoretically be
misused, then set a smaller expiry time.
What is the threat-model that this change is supposed to guard against?
Looking that the syscall itself:
1/ why restrict the call to directories only?
2/ Every new syscall should have a 'flags' argument, because you never
know when you'll need one.
I have some of the same concerns. For instance, we don't kill off ssh
sessions that were established with krb5 just because the credcache was
destroyed. RPC is a bit different since we authenticate every call, but
is this fundamentally different from keeping an ssh session around that
was established before the credcache was destroyed?
Probably because fundamentally, it’s the same user that keeps using it.
If the same ssh connection was shared by multiple users that were inserting
and deleting their credentials then it would be as problematic.
quoted
Are we just getting tickets with too long a lifetime here? Maybe we just
need to be more cavalier about destroying cached creds on some event or
on a more timely basis?
Also, the whole gssapi credcache in the kernel is showing its age a bit.
struct auth_cred has had this over it for about as long as I've been
doing kernel work:
/* Work around the lack of a VFS credential */
We've had struct cred for ages now.
David and I were chatting about this the other day and were wondering if
we could change the RPC gssapi code to cache credentials in one of the
keyrings in struct cred. Then, once the struct cred goes away, the key
would go away as well. It wouldn't be destroyed on kdestroy, but once
the last process with those creds exits, they would go away.
One argument against it: Kerberos has changed their storage location
over the years (FILES … to keyring). What if they change again? Then NFS
would have to change their implementation as well.
Having said that: outside of the fs-mailing list, I have asked Trond that
if VFS decides to reject the syscall idea, what would be an alternative
and one of the choices is the keyring. Of course there are variations of
how the keyring would be used. One option would be to totally switch to
storing credentials in the keyring. To what what Andy had originally
proposed of introducing a gss key type and storing the gss context in
the keyring.
I think I wasn't clear here. I'm not proposing that you move everyone to
KEYRING: credcaches. This would not be a visible change to userland.
We'd still use rpc.gssd to upcall for creds.
What I'm saying is that instead of storing the creds in a hashtable like
we do today, we'd just stash them in one of the keyrings hanging off of
struct cred.
Change all of the authgss_ops operations to do query/store from the
appropriate keyring directly. With that, the effective lifetime of
GSSAPI creds would be bounded by the lifetime of the keyrings that hold
references to it.
We'd probably need a new key_type for this to ensure that this couldn't
be manipulated directly from userland. Or...maybe you'd still want to
allow userland to destroy the creds? No need for a new syscall with that
-- they can just do a "keyctl unlink". There are a lot of options here.
It's a non-trivial amount of work though (rpcauth_lookupcred() on down
would probably need to be reworked) and I haven't looked at it detail.
Still, it seems like it could be a more modern and cleaner design than
what we have today.
--
Jeff Layton [off-list ref]
On Fri, 2017-08-11 at 10:22 -0400, Jeff Layton wrote:
I think I wasn't clear here. I'm not proposing that you move everyone
to
KEYRING: credcaches. This would not be a visible change to userland.
We'd still use rpc.gssd to upcall for creds.
What I'm saying is that instead of storing the creds in a hashtable
like
we do today, we'd just stash them in one of the keyrings hanging off
of
struct cred.
Change all of the authgss_ops operations to do query/store from the
appropriate keyring directly. With that, the effective lifetime of
GSSAPI creds would be bounded by the lifetime of the keyrings that
hold
references to it.
We'd probably need a new key_type for this to ensure that this
couldn't
be manipulated directly from userland. Or...maybe you'd still want to
allow userland to destroy the creds? No need for a new syscall with
that
-- they can just do a "keyctl unlink". There are a lot of options
here.
It's a non-trivial amount of work though (rpcauth_lookupcred() on
down
would probably need to be reworked) and I haven't looked at it
detail.
Still, it seems like it could be a more modern and cleaner design
than
what we have today.
The main annoyance with going from a global to a local cache such as
the keyrings is that it makes comparing credentials a lot more work.
Today, because the credentials are essentially unique per server, we
just do pointer comparisons. Once we have non-global caches, we would
need to do more elaborate comparisons to ensure that the uid, gid, and
list of groups match.
That's also why we never made the leap to using 'struct cred', btw...
--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com
From: Jeff Layton <hidden> Date: 2017-08-13 11:39:02
On Fri, 2017-08-11 at 15:12 +0000, Trond Myklebust wrote:
On Fri, 2017-08-11 at 10:22 -0400, Jeff Layton wrote:
quoted
I think I wasn't clear here. I'm not proposing that you move everyone
to
KEYRING: credcaches. This would not be a visible change to userland.
We'd still use rpc.gssd to upcall for creds.
What I'm saying is that instead of storing the creds in a hashtable
like
we do today, we'd just stash them in one of the keyrings hanging off
of
struct cred.
Change all of the authgss_ops operations to do query/store from the
appropriate keyring directly. With that, the effective lifetime of
GSSAPI creds would be bounded by the lifetime of the keyrings that
hold
references to it.
We'd probably need a new key_type for this to ensure that this
couldn't
be manipulated directly from userland. Or...maybe you'd still want to
allow userland to destroy the creds? No need for a new syscall with
that
-- they can just do a "keyctl unlink". There are a lot of options
here.
It's a non-trivial amount of work though (rpcauth_lookupcred() on
down
would probably need to be reworked) and I haven't looked at it
detail.
Still, it seems like it could be a more modern and cleaner design
than
what we have today.
The main annoyance with going from a global to a local cache such as
the keyrings is that it makes comparing credentials a lot more work.
Today, because the credentials are essentially unique per server, we
just do pointer comparisons. Once we have non-global caches, we would
need to do more elaborate comparisons to ensure that the uid, gid, and
list of groups match.
That's also why we never made the leap to using 'struct cred', btw...
Ok, it does seem better to have a global cache from that standpoint.
Still, a new syscall for this doesn't seem very elegant. I also worry a
bit about writeback here too (like David and Neil have pointed out).
What about changing how we hold references on these objects instead?
After we look up an auth token in e.g. rpcauth_lookupcred, take a
reference to it and stash a pointer to it somewhere in the cred.
Possibly in the thread or process keyrings, but it may work better
elsewhere.
When we go to look up creds from that thread in the future, we can get
to it directly (which is a nice bonus). When the cred is destroyed
(usually on process destruction), we'd drop the reference to the object,
which would drop the reference to the global cache object.
The global cache could then be changed to have a pretty short timeout (a
few seconds?) and reap the object soon afterward when there are no more
active processes that have used it.
It's a bit more work and we might need to grow struct cred to handle it
(maybe give it its own keyring?), but it seems like that might be a
cleaner solution than giving userland knobs to manage the kernel's
caches.
--
Jeff Layton [off-list ref]
From: Olga Kornievskaia <hidden> Date: 2017-08-14 15:43:39
On Sun, Aug 13, 2017 at 7:38 AM, Jeff Layton [off-list ref] wrote:
On Fri, 2017-08-11 at 15:12 +0000, Trond Myklebust wrote:
quoted
On Fri, 2017-08-11 at 10:22 -0400, Jeff Layton wrote:
quoted
I think I wasn't clear here. I'm not proposing that you move everyone
to
KEYRING: credcaches. This would not be a visible change to userland.
We'd still use rpc.gssd to upcall for creds.
What I'm saying is that instead of storing the creds in a hashtable
like
we do today, we'd just stash them in one of the keyrings hanging off
of
struct cred.
Change all of the authgss_ops operations to do query/store from the
appropriate keyring directly. With that, the effective lifetime of
GSSAPI creds would be bounded by the lifetime of the keyrings that
hold
references to it.
We'd probably need a new key_type for this to ensure that this
couldn't
be manipulated directly from userland. Or...maybe you'd still want to
allow userland to destroy the creds? No need for a new syscall with
that
-- they can just do a "keyctl unlink". There are a lot of options
here.
It's a non-trivial amount of work though (rpcauth_lookupcred() on
down
would probably need to be reworked) and I haven't looked at it
detail.
Still, it seems like it could be a more modern and cleaner design
than
what we have today.
The main annoyance with going from a global to a local cache such as
the keyrings is that it makes comparing credentials a lot more work.
Today, because the credentials are essentially unique per server, we
just do pointer comparisons. Once we have non-global caches, we would
need to do more elaborate comparisons to ensure that the uid, gid, and
list of groups match.
That's also why we never made the leap to using 'struct cred', btw...
Ok, it does seem better to have a global cache from that standpoint.
Still, a new syscall for this doesn't seem very elegant. I also worry a
bit about writeback here too (like David and Neil have pointed out).
What about changing how we hold references on these objects instead?
After we look up an auth token in e.g. rpcauth_lookupcred, take a
reference to it and stash a pointer to it somewhere in the cred.
Possibly in the thread or process keyrings, but it may work better
elsewhere.
When we go to look up creds from that thread in the future, we can get
to it directly (which is a nice bonus). When the cred is destroyed
(usually on process destruction), we'd drop the reference to the object,
which would drop the reference to the global cache object.
The global cache could then be changed to have a pretty short timeout (a
few seconds?) and reap the object soon afterward when there are no more
active processes that have used it.
Wouldn’t that produce a lot of unnecessary context re-establishments.
It's a bit more work and we might need to grow struct cred to handle it
(maybe give it its own keyring?), but it seems like that might be a
cleaner solution than giving userland knobs to manage the kernel's
caches.
Userland is the only place that know that kdestroy ran and is the best
place to tell the kernel to remove its cache. Everything else is guessing.
--
Jeff Layton [off-list ref]
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html