Don't change impersonation for the whole process when using threads

[chevah-robot]

T2842 story was created by adiroiban on 2015-05-12 17:49:44Z.
Last changed on 2015-11-23 18:05:29Z.

• #346 - process_capabilities._elevatePrivileges

Using executeAsUser:

• #345 - system_user.getHomeFolder on Windows - calls executeAsUser

• #343 - authenticateWithUsernameAndPassword on Unix - calls executeAsUser

• #342 - filesystem operations on Windows in separate thread - calls executeAsUser

• #341 - filesystem operations on Unix in separate process - calls executeAsUser

• #344 - system_user.userExists on Unix

The current implementation is naive and insecure in multi threaded environment... and we do use multi threads.

The fact that we use Twisted and we don't rely on thread for normal operations mitigate the risk... but the risk is still there.

Here is an example.

App user1 logs in after a successful authentication and the server will use a thread to send them message to the DB.
In the same time OS user2 does an auth request.

To auth user2 the whole SFTPPlus Server process is switched to root mode in order to read the password db. User2 authentication is executed in parallel with the DB tread. At any time after process is switched as root the OS might choose to allow the DB thread to continue and it will continue as root.

This is also valid on Windows, not for authentication but for opening a file as an user. On Windows the other threads will not get root/admin but will run as the OS account rather than the system account.

The problem is in compat system_users.executeAsUser

The problem affect SFTPPlus Server when operating with OS accounts. All is fine as long as only app accounts are used.

---

This should be split into multiple tickets as the refactoring will be ugly. We will need to change the whole security design

1. Fix shadow and PAM authentication to use a separate root process . Only affecting Unix.
2. Fix atomic filesystem operations move file / delete file / delete folder / list folder
3. Fix file description opening.

The only way to fix this is to have a separate process doing the privilege changes/user impersonations.

For 1 and 2 we might use a separate process (or process pool) which is launched together with the main process and which expose a public API which will include Unix shadow and PAM authentication or atomic filesystem access. These process are guarded by the public API and have threads disabled... they will use impersonation but will make sure no other thread is executed in the process.

For 3 things are complicated as we need to pass file descriptors.

On Unix you can pass file descriptors between process over a Unix Socket using send_msg recv_msg ... and there is a helper http://www.normalesup.org/~george/comp/libancillary/

On Windows things might be simple for 2 and 3. As far as I know Windows API support impersonating a single thread so we don't need to worry about spawning a new process and communicating with that process just make sure that the executeAsUser impersonation context is executed in separate impersonated thread

Since for both Windows and Unix the executeAsUser code will be exected outside of the main thread we will need to refactor the API based on deferred as we will have deferToProcess and deferToThread to execute the contained code.

On Windows some filesytem API might provide the option to pass an impersonated token... but I think that is easier to just use an impersonated thread.

There is one good part in refactoring the compat API to use thread/process and hence deferred: we will make sure that it will not block.

Under exceptional conditions Unix or Windows might take a long time to delete or move a file. With the current API the whole process is blocked.
With a deferred based API other operation can continue in main thread while the OS is handling the filesysystem access.

On Unix is easy to block the filesystem access with a NFS filesytem is used. For Windows I assume that same thing is valid for network shares.

This is huge and needs to be broken in multiple tickets.