Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: Threads created by RocksDB are not impersonated #2036

Open
soerendd opened this issue Mar 24, 2017 · 7 comments
Open

Windows: Threads created by RocksDB are not impersonated #2036

soerendd opened this issue Mar 24, 2017 · 7 comments
Labels

Comments

@soerendd
Copy link

soerendd commented Mar 24, 2017

presumptions:

  • the thread which opens a rocksdb is impersonating a user - the process user is a different user
  • the process user does not have sufficient access rights for the database directory

rocksdb opens the database with success. Some functions (compacting?) are failing because they try to open files or try to move (rename) files because they run in a background thread. On windows newly created threads are running as the process user (and not as a impersonated user)

This is somewhat inconsistent, because there is no error when opening the database and also when inserting values (sometimes inserts fail - because files needs to be created/moved)

Solution could be that the database instance stores the impersonation (DuplicateTokenEx). Every tasks that starts for that specific database also gets that token and impersonates at the very beginning of execution of the task and reverts after the work is done.

PS: Thank you all for this wonderful piece of software.

@yiwu-arbug
Copy link
Contributor

cc @yuslepukhin for window question.

@gfosco
Copy link
Contributor

gfosco commented Jan 10, 2018

Closing this via automation due to lack of activity. If discussion is still needed here, please re-open or create a new/updated issue.

@soerendd
Copy link
Author

I think for windows users which are using rocksdb within services and so on it is very important.

@ajkr ajkr reopened this Jan 11, 2018
@ajkr
Copy link
Contributor

ajkr commented Jan 11, 2018

We can keep it open but we don't have much windows expertise within our team. We mostly rely on @yuslepukhin for supporting RocksDB on windows.

@yuslepukhin
Copy link
Contributor

@soerendd I have done something similar to support memory isolation with rocksdb internally. Basically, we wanted to have that every db instance within the process has its own memory pool where the allocation would come from. However, as you know there are global things such as environment and the threadpool which can do things on behalf of different instances.

Here is the approach that I used and you can use too.

  1. Before calling into the database set thread local storage with regards to which memory pool this thread is using (in your case impersonation token)
  2. override global new/delete and consult a thread-local storage as to which instance (which memory pool) to use. In your case, this is not needed.
  3. I have modified threadpool_impl, thread_local so they are implemented with a pimpl idiom. I replace .cc implementation at home with my pimpl.
  4. I have replaced std::thread with port::Thread -> WindowsThread object with a pimpl idiom in the public version. So when building at home I replace the .cc file with my impl. When a new thread is created it remembers the ID for my memory pool of the submitting thread. So when the thread starts executing it immediately switches to that memory arena. You can do the same and impersonate the thread at the beginning.
  5. I replace the default Windows environment in the github with a customized version. In order to do this I organized the GH version in IO and thread parts. One can be customized by overriding things and another by containment. In particular, I replaced the standard thread pools for different priorities with a VistaThread pool which as you know is implemented in the OS. It works fast and provides also IO completion, timers etc.
  6. In that customized env from step 5) I have wrapped VistThreadpool to make sure that it carries the memory pool token when a task is submitted. Thus, just like in 4) for standalone threads, in the threadpool the thread consults the memory pool ID and switched to it. You can do the same for thread impersonation.

I have striven to avoid modification of the main source since mine is , and your's as well, not a common case though. It seems to me that mine is more common than yours since you care about security. :)

@soerendd
Copy link
Author

soerendd commented Feb 13, 2020

So in the end I get the impression that rocksdb is not really targeting the windows platform. Otherwise this issue would be left active. Shall I create a pr for the implementation?

@adamretter
Copy link
Collaborator

@soerendd PRs are always welcome if they have tests and improve the situation :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants