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

DbCrypt/KeyHolder plugins key changing issues on running server #7415

Closed
AlexeyMochalov opened this issue Dec 8, 2022 · 1 comment
Closed

Comments

@AlexeyMochalov
Copy link
Contributor

Hi, colleagues!

I'm implementing two DbCrypt plugins with KeyHolder plugin built in functionality right now and got into an issue when trying to change keys on the fly on running server with multiple attachments.
I've tested it on Firebird 3.0 but 4 and 5 seems vulnerable too.

1) Resetting encryption key on Classic server

If there are multiple attachments to encrypted db and we'll change (decrypt and encrypt again) the encryption key, it won't be reloaded for any attachment on Classic server except for attachment which change it. And these attachments with old key will die after next select or something similar.

It could be reproduced on example KeyHolder and DbCrypt plugins.

  1. Add keyholder config:
Plugin = CryptKeyHolder_example {
	Module = $(dir_plugins)/CryptKeyHolder_example
	Config = CryptKeyHolder_example_config
}

Config = CryptKeyHolder_example_config {
	Auto = true
	Key111 = 111
	Key222 = 222
}
  1. create an attachment to employee.fdb on classic
  2. encrypt it with key "111"
  3. create second attachment to employee.fdb on classic
  4. decrypt db in first attachment and after that encrypt it again with key "222"
  5. try to select from "country" table in second attachment, it will crash

2) Accidental key damaging/deleting

If we connected to encrypted database and delete or change key which was used for encryption, attachments will hang on exit due to CryptThread can't load or loads wrong key for decryption process on Super server.
On Classic it'll exit but db will be in "crypt process" state until someone restores original key and connects to it. That is not a big issue compared to Super server, but I'd prefer more strict behavior and do not allow change crypt state if we lost original key.

To demonstrate this issue you can use this patch.txt for CryptKeyHolder.cpp example. It'll use "/tmp/somekeyfile" on linux to read key from. Create "/tmp/somekeyfile" file and add any value to it, then encrypt employee.fdb with key described in example plugin config. After encryption just delete or change value inside the file and then try to decrypt. You won't get any error message but when you'll try to close conneciton it will hang. It's not happening with initial CryptKeyHolder_example plugin because key it use stored in plugin configuration and cached by server.

Conclusion:

these issues related to CryptoManager.cpp logic and CryptThreads starting algorithm. I fixed it and I'll create a PR soon. Would highly appreciate review on it.

@AlexPeshkoff
Copy link
Member

  1. Agreed, check for key correctness (like during attach) is needed after crypt state change. Specially dangerous if it will need not to read but write data to DB file and use wrong key for it.
  2. CryptKeyHolder is expected to cache keys in RAM (no matter where were they received from - plugin's name is "HOLDER"). I.e. I see this as sooner bug in plugin but if such hang can be avoided in main code - why not?

AlexeyMochalov added a commit to red-soft-ru/firebird that referenced this issue Dec 13, 2022
This should add key correctness check in case of resetting encryption key on Classic server with multiple attachments, and hanging Super server in case of accidental key damaging/deleting
AlexeyMochalov added a commit to red-soft-ru/firebird that referenced this issue Dec 21, 2022
…SQL#7415)

This should add key correctness check in case of resetting encryption key on Classic server with multiple attachments, and fix hanging Super server in case of accidental key damaging/deleting
AlexeyMochalov added a commit to red-soft-ru/firebird that referenced this issue Dec 21, 2022
…SQL#7415)

This should add key correctness check in case of resetting encryption key on Classic server with multiple attachments, and fix hanging Super server in case of accidental key damaging/deleting
AlexPeshkoff pushed a commit that referenced this issue Dec 21, 2022
This should add key correctness check in case of resetting encryption key on Classic server with multiple attachments, and fix hanging Super server in case of accidental key damaging/deleting
AlexPeshkoff pushed a commit that referenced this issue Jan 7, 2023
This should add key correctness check in case of resetting encryption key on Classic server with multiple attachments, and fix hanging Super server in case of accidental key damaging/deleting
AlexPeshkoff pushed a commit that referenced this issue Jan 10, 2023
Fix resetting and accidental key damaging/deleting (#7415)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment