hba: stricter parsing and better pretty-printing#43811
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Jan 8, 2020
Merged
hba: stricter parsing and better pretty-printing#43811craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
35dc6f8 to
8bfc6fe
Compare
The following defects were found by @mjibson using `go-fuzz` and are fixed by this patch: - the code would parse then fail to pretty-print an empty connection type and auth method. - the code would parse then pretty-print incorrectly a connection type or auth method containing commas or other special characters. - the code would accept an empty string as address. - the code would accept then fail to pretty-print columns containing special (control) characters. Note that the code previously supported tabs (\t) and carriage returns (\r) inside quoted strings, and this is now rejected. For most fields, this does not matter, as these special characters were not valid semantically anyway. However, for the method options in the last column it is theoretically possible for a method to wish to use such special characters. This is not currently the case for the CCL `gss` method already implemented, so this change is not introducing a regression. A comment is left in the code to indicate what needs to be changed if a future method needs this support. Release note (security): CockroachDB now properly rejects control characters in the value of the cluster setting `server.host_based_authentication.configuration`. These characters were previously accepted and would silently yield rules matching differently from intended. Deployments careful to strip control characters from their HBA configuration are not affected.
8bfc6fe to
98231d0
Compare
madelynnblue
approved these changes
Jan 8, 2020
Contributor
Author
|
thanks! bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Jan 8, 2020
43792: batcheval: clean up remaning variable names of `{Reader,ReadWriter}` types r=irfansharif a=irfansharif
Ignore first commit, reviewed in #43764.
We should be using {reader,readWriter} for
{Reader,ReadWriter} respectively instead. I'd skipped everything under
batcheval when working on #43265, back when I thought it'd be a good
idea to plumb in an engine.Batch throughout replica read codepaths. Due
to perf penalty as seen on #43388, I'm no longer going to be doing that.
This is the remaining work.
Release note: None
43811: hba: stricter parsing and better pretty-printing r=knz a=knz
The following defects were found by @mjibson using
`go-fuzz` (#43805) and are fixed by this patch:
- the code would parse then fail to pretty-print an empty connection
type and auth method.
- the code would parse then pretty-print incorrectly a connection
type or auth method containing commas or other special characters.
- the code would accept an empty string as address.
- the code would accept then fail to pretty-print columns containing
special (control) characters.
Note that the code previously supported tabs (\t) and carriage
returns (\r) inside quoted strings, and this is now rejected. For most
fields, this does not matter, as these special characters were not
valid semantically anyway.
However, for the method options in the last column it is theoretically
possible for a method to wish to use such special characters. This is
not currently the case for the CCL `gss` method already implemented,
so this change is not introducing a regression. A comment is left in
the code to indicate what needs to be changed if a future method needs
this support.
Release note (security): CockroachDB now properly rejects control
characters in the value of the cluster setting
`server.host_based_authentication.configuration`. These characters
were previously accepted and would silently yield rules matching
differently from intended. Deployments careful to strip control
characters from their HBA configuration are not affected.
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Contributor
Build succeeded |
Contributor
Author
|
(found while working on #31113) |
This file contains hidden or 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
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.
The following defects were found by @mjibson using
go-fuzz(#43805) and are fixed by this patch:type and auth method.
type or auth method containing commas or other special characters.
special (control) characters.
Note that the code previously supported tabs (\t) and carriage
returns (\r) inside quoted strings, and this is now rejected. For most
fields, this does not matter, as these special characters were not
valid semantically anyway.
However, for the method options in the last column it is theoretically
possible for a method to wish to use such special characters. This is
not currently the case for the CCL
gssmethod already implemented,so this change is not introducing a regression. A comment is left in
the code to indicate what needs to be changed if a future method needs
this support.
Release note (security): CockroachDB now properly rejects control
characters in the value of the cluster setting
server.host_based_authentication.configuration. These characterswere previously accepted and would silently yield rules matching
differently from intended. Deployments careful to strip control
characters from their HBA configuration are not affected.