[PATCHES] smbcontrol disconnect-client

Post by Christof Schmitt
Here are a few patches that Christian had developed some time ago. They
implement a 'smbcontrol disconnect-client <ip address>' call that
disconnects all clients from a certain IP address. That is useful when a
config file for a specific client has been changed and that change has
to be enforced.
Comments?

Oh, I really like this - looks like a cool new feature
we should add to 4.1.x at least !

Let me do a proper review and get into master first.

Cheers,

Jeremy.

Stefan (metze) Metzmacher

2013-12-07 12:05:58 UTC

I think this is too dangerous, if the client still has files with
batch/exclusive oplocks
open and changes in the local buffer, we may trigger data corruption. At
least without
durable opens.

I think the command (maybe a 2nd one) should first trigger a break to
level2 to all opens
and don't grant more for the rest of the connection lifetime. And the
disconnect
should only work if the connection is in good shape.

metze

David Collier-Brown

2013-12-07 20:59:02 UTC

I think this is too dangerous, if the client still has files with
batch/exclusive oplocks
open and changes in the local buffer, we may trigger data corruption. At
least without
durable opens.
I think the command (maybe a 2nd one) should first trigger a break to
level2 to all opens
and don't grant more for the rest of the connection lifetime. And the
disconnect
should only work if the connection is in good shape.
metze

If you're strictly thinking of security, you may want to separate this
out into one or more specific operations.

One might be a "quiesce client", so that the client will stop using a
samba service and disconnect safely. It can then reconnect to the same
or a different service, under different ground-rules. That will work for
misconfigurations, brain-dead clients and the like.

A second one might be "kick quiesced client entirely off", for
recovering from wedged clients, bugs and the like. It would only apply
after one did what one could to quiesce the user.

In the scurity space, a possible third is "apply new restriction", which
would succeed if the client was not currently using the thing
restricted, and fail if not. On failure, you might then need to quiesce
them so that they disconnected and thereafter could only connect with
the restriction in place. That's less dangerous that an unconditional
disconnect.

Or, you could simply say "restrictions come into play on open/connect",
and not have to do anything with the client.
The latter won't pass orange-book requirements, but it's not
unreasonable so long as one does have a "kill" in one's back pocket to
deal with really evil cases.

--dave
[Multics AIM under the Orange Book had a "apply MAC change right now"
functionality that could utterly mess up a process that was losing
privilege. It was one of the few cases I know of where a sysadmin could
really do something harmful]

--
David Collier-Brown, | Always do right. This will gratify
System Programmer and Author | some people and astonish the rest
davecb at spamcop.net | -- Mark Twain
(416) 223-8968

Jeremy Allison

2013-12-11 01:24:57 UTC

I think this is too dangerous, if the client still has files with
batch/exclusive oplocks
open and changes in the local buffer, we may trigger data corruption. At
least without
durable opens.
I think the command (maybe a 2nd one) should first trigger a break to
level2 to all opens
and don't grant more for the rest of the connection lifetime. And the
disconnect
should only work if the connection is in good shape.

That's a good enhancement (make the command trigger a
break first). However this is a "be careful what you
wish for" command - people have been asking for a
way to forcibly disconnect a specific client for
quite a long time.

Maybe it should be renamed "smbcontrol force-disconnect-client"
or "smbcontrol disconnect-client-with-extreme-prejudice"
command ?

:-).

Jeremy.

Stefan (metze) Metzmacher

2013-12-11 17:48:00 UTC

I think this is too dangerous, if the client still has files with
batch/exclusive oplocks
open and changes in the local buffer, we may trigger data corruption. At
least without
durable opens.
I think the command (maybe a 2nd one) should first trigger a break to
level2 to all opens
and don't grant more for the rest of the connection lifetime. And the
disconnect
should only work if the connection is in good shape.

That's a good enhancement (make the command trigger a
break first). However this is a "be careful what you
wish for" command - people have been asking for a
way to forcibly disconnect a specific client for
quite a long time.
Maybe it should be renamed "smbcontrol force-disconnect-client"
or "smbcontrol disconnect-client-with-extreme-prejudice"
command ?

Maybe "smbcontrol kill-client-connections" ?

metze

Christof Schmitt

2013-12-11 18:15:10 UTC

I think this is too dangerous, if the client still has files with
batch/exclusive oplocks
open and changes in the local buffer, we may trigger data corruption. At
least without
durable opens.
I think the command (maybe a 2nd one) should first trigger a break to
level2 to all opens
and don't grant more for the rest of the connection lifetime. And the
disconnect
should only work if the connection is in good shape.

That's a good enhancement (make the command trigger a
break first). However this is a "be careful what you
wish for" command - people have been asking for a
way to forcibly disconnect a specific client for
quite a long time.
Maybe it should be renamed "smbcontrol force-disconnect-client"
or "smbcontrol disconnect-client-with-extreme-prejudice"
command ?

Maybe "smbcontrol kill-client-connections" ?

Changing the name is easy. :-)

I took a look at the suggestions and it should not be too difficult to
implement a call that revokes all oplocks. So we could have two calls to
disconnect a client:

1) smbcontrol kill-client-connections:
- Just exit the server.

2) smbcontrol drop-client-connections:
- Set a flag to no longer grant exclusive oplocks
- Send a break for all exclusive oplocks
- When oplocks.exclusive_open is zero, then exit.
This should always work, since oplocks are considered broken after
the OPLOCK_BREAK_TIMEOUT.

What is the best way forward here? Rename the control to
kill-client-connections and push the patches? And then start working on
the second command?

Christof

Stefan (metze) Metzmacher

2013-12-11 18:22:42 UTC

Hi Christof,

Post by Christof Schmitt

Post by Stefan (metze) Metzmacher
Maybe "smbcontrol kill-client-connections" ?

Changing the name is easy. :-)
I took a look at the suggestions and it should not be too difficult to
implement a call that revokes all oplocks. So we could have two calls to
- Just exit the server.
- Set a flag to no longer grant exclusive oplocks
- Send a break for all exclusive oplocks
- When oplocks.exclusive_open is zero, then exit.
This should always work, since oplocks are considered broken after
the OPLOCK_BREAK_TIMEOUT.
What is the best way forward here? Rename the control to
kill-client-connections and push the patches? And then start working on
the second command?

Sounds like a plan :-)

metze

Jeremy Allison

2013-12-12 00:29:03 UTC

Post by Christof Schmitt
Changing the name is easy. :-)
I took a look at the suggestions and it should not be too difficult to
implement a call that revokes all oplocks. So we could have two calls to
- Just exit the server.
- Set a flag to no longer grant exclusive oplocks
- Send a break for all exclusive oplocks
- When oplocks.exclusive_open is zero, then exit.
This should always work, since oplocks are considered broken after
the OPLOCK_BREAK_TIMEOUT.
What is the best way forward here? Rename the control to
kill-client-connections and push the patches? And then start working on
the second command?

How about 'kill-client-ip' - more specific ?

Here is the modified patchset that implements
the rename. Let me know if you're ok with it
(and feel free to push if happy).

Cheers,

Jeremy.
-------------- next part --------------

Christof Schmitt

2013-12-12 17:55:41 UTC

How about 'kill-client-ip' - more specific ?
Here is the modified patchset that implements
the rename. Let me know if you're ok with it
(and feel free to push if happy).

Looks good. I fixed the signed-off-by on the doc patch and pushed the
patches to autobuild.

Thanks,

Christof

Jeremy Allison

2013-12-12 17:57:38 UTC

Post by Christof Schmitt