Using smb safely in multiuser environments

[bernt-matthias]

I was considering to implement access to smb for the Galaxy project where we use fsspec a lot. Galaxy is a multi user environment where we would safely store the information needed to access a smb share (server, port, username, password, path ...). Then we connect via fsspec.implementations.smb.SMBFileSystem.

By chance I noticed that the underlying smbprotocol uses a connection cache which is shared between all connections in a python process. The cache seems to work on server + port as key (maybe username .. but I'm unsure). Hence to me it seems unsafe to use this in a multi user environment, since users might hijack other users sessions (involuntary or deliberately) since they only need to know the server+port+username.

Would you agree with this? Are there possibilities to fix this, e.g. by disabling the global connection cache (one can pass a dict that would be used) as outlined here. Are there maybe other fsspec implementations using different libraries?

Edit: Meanwhile I found the tests and verified my theory (I think) #2007

[martindurant]

Interesting problem! I only know about the smb connection cache now from having followed your links.

I suppose you can experiment by providing your on cache dict and seeing what gets set there. If your model is correct, then the fsspec implementation certainly could (optionally) supply a cache of its own and provide ways to interact with it. Then, you can modify SMBFileSystem until #2007 does the right thing. (for the sake of style, I would add pytest.raises around the critical lines - the behaviour we want - so that the test fails; this will prevent accidentally merging the PR)

You should also be aware, that fsspec also maintains a cache of filesystem instances, keyed on the kwargs supplied. I don't think this will apply to you, since each instantiation contains the username/password, so there's no way to guess that.

[bernt-matthias]

Interesting problem! I only know about the smb connection cache now from having followed your links.

I accidentally stumbled over some exception message. Was wondering if other libraries used in the project do similar stuff.

I suppose you can experiment by providing your on cache dict and seeing what gets set there. If your model is correct, then the fsspec implementation certainly could (optionally) supply a cache of its own and provide ways to interact with it. Then, you can modify SMBFileSystem until #2007 does the right thing.

True. But even if I create a ton of tests I would have a hard time to convince myself, the maintainers of the Galaxy project, or admins to use/deploy this. The filesystems may contain sensitive data and I would prefer them not to get leaked :)

But I might be wrong. Maybe its sufficient to supply a fresh dict with each operation in fsspec that interacts with the smbclient/protocol and destroy it afterwards?

(for the sake of style, I would add pytest.raises around the critical lines - the behaviour we want - so that the test fails; this will prevent accidentally merging the PR)

Good point.

You should also be aware, that fsspec also maintains a cache of filesystem instances, keyed on the kwargs supplied.

Thanks for the note. Meanwhile I also discovered this and discuss with other devs of the Galaxy project if this might be a problem. Maybe shared credentials might be a problem.

I don't think this will apply to you, since each instantiation contains the username/password, so there's no way to guess that.

Could be a problem if username + keyfilename would be possible.

[martindurant]

No, I have not heard of this kind of problem before.

even if I create a ton of tests I would have a hard time to convince myself, the maintainers of the Galaxy project, or admins to use/deploy this

I meant experiments in code versus the test case you already submitted. We should be able to introspect all the instances created to check for credentials.

Maybe its sufficient to supply a fresh dict with each operation in fsspec that interacts with the smbclient/protocol and destroy it afterwards?

The linked issue suggests this. I assume it comes with some extra per-connection overhead, but that's OK for the sake of security. I don't really know how the runtime within Galaxy works, I'm afraid.