Reset encryptor configs on query processing #628
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
clean metadata after testcases
…ptor-setting-for-queries
Zhaars
reviewed
Jan 28, 2023
tests/test.py
Outdated
| # insert data data | ||
| self.insert_via_1(default_client_id_table.insert(), data) | ||
|
|
||
| # expect that data was not encrypted with client_id which specified in ignore_client_id block |
There was a problem hiding this comment.
ignore_client_id? Just invalid comment?
There was a problem hiding this comment.
it's redundant copy-paste which was the source of bug reproducing but now redundant. already removed it.
Zhaars
reviewed
Jan 28, 2023
| be applied for the next query and will be cleared | ||
| """ | ||
| if not base.TEST_POSTGRESQL: | ||
| self.skipTest("MySQL doesn't support returning statement for insert") |
There was a problem hiding this comment.
MariaDB should support returning. Does it make sense to test for Maria too?
There was a problem hiding this comment.
you are right, added via raw sql because current version of sqlalchemy doesn't allow such expressions for mariadb and upgrading to 2.x requires changes and updating tests and should be done in separate PR, imho
Zhaars
reviewed
Jan 28, 2023
|
@Zhaars , please look one more time on added new changes for MySQL side of testing with returning expression. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Brief overview how Acra process transparent decryption. At first, it captures query from the application to the database, tries to match it to the encryptor_config and remembers which columns requested from db and which of them should be decrypted/detokenized. In case of simple decryption, Acra can decrypt even without configs, because parses everything that looks like AcraStruct/AcraBlock. In the case of tokenization, it should know token type.
Then, when database returns DataRow with data, Acra remembers the order of columns and which of them should be processed, and what rules to apply.
This PR fixes the situation, when Acra remembers the configuration for the decryption after processing the valid query and applies second time for the next query if it wasn't parsed correctly and previous configuration wasn't flushed.
So, here is added resetting
querySelectSettingon every OnQuery call that flushes the previously saved list of settings to not apply it on the wrong query. Before that, we expected that every OnQuery will execute parts at end ofonSelect/onReturningwith assigning new allocated slice to the variable but omitted cases with not parsed queries. So it allows cases when after correctly processed query with some transparent encryption, Acra will try to apply same config on the next query even because weren't executed parts with re-assigning.This fix is first part of whole fix that cover simple case with one query per TCP packet. But it doesn't cover case when one TCP packet will have several Parse + Bind + Execute db packets, when the second will override previously remembered setting and Acra will apply second config for the first Parse packet... It will be fixed in next PRs because required refactoring and some re-designing.
Checklist
with new changes