-
Notifications
You must be signed in to change notification settings - Fork 0
SET command to disable mapping #267
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
SET command to disable mapping #267
Conversation
25c7ef7 to
5d3569a
Compare
When the proxy sees the following command from a client: ```sql SET LOCAL UNSAFE_SKIP_MAPPING_NEXT_STATEMENT = t; ``` Parsing & mapping of the next statement ONLY will be skipped. This is useful for working around `sqlparser` SQL grammar coverage issues for database migrations in particular. This command is not yet documented. It's possible that its full implications may not be fully understood re: security and therefore it's probably best to reveal its existence to customers on an as-needed basis. We should also consider a Proxy configuration option for completely disabling this command. NOT YET IMPLEMENTED: - Setting this variable should also disable decryption of any results returned from the query, closing off a potential exploit. - Integration test
d13b1f9 to
031cc69
Compare
031cc69 to
b5a6b5f
Compare
auxesis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid work @tobyhede.
I have included some small docs changes for clarity, and have requested one section at the end of the README be done differently.
Co-authored-by: Lindsay Holmwood <lindsay@cipherstash.com> Signed-off-by: Toby Hede <tobyhede@info-architects.net>
Co-authored-by: Lindsay Holmwood <lindsay@cipherstash.com> Signed-off-by: Toby Hede <tobyhede@info-architects.net>
Co-authored-by: Lindsay Holmwood <lindsay@cipherstash.com> Signed-off-by: Toby Hede <tobyhede@info-architects.net>
Co-authored-by: Lindsay Holmwood <lindsay@cipherstash.com> Signed-off-by: Toby Hede <tobyhede@info-architects.net>
Co-authored-by: Lindsay Holmwood <lindsay@cipherstash.com> Signed-off-by: Toby Hede <tobyhede@info-architects.net>
auxesis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for these updates @tobyhede!
I added some more suggestions, will commit them and merge the PR.
- make title active - explain context of encrypted mapping - remove definite article, to remove diminuitive - line per sentence, to improve maintainability - explain that the section refers to mapping behaviour
a9550cc to
1c96542
Compare
Adds ability to disable and re-enable mapping for a connection
Disable mapping
Enable mapping
The
SETcommand is always scoped to the connectionSESSION- mapping is only ever disabled for the client connection theSETcommand was issued on.This also adds a prometheus metric called
STATEMENTS_PASSTHROUGH_MAPPING_DISABLED_TOTALthat tracks the count of statements passed through while mapping was disabled using this command.