luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

auto-config: relax node name validation for JWT authorization (#15370) 

* auto-config: relax node name validation for JWT authorization

This changes the JWT authorization logic to allow all non-whitespace,
non-quote characters when validating node names. Consul had previously
allowed these characters in node names, until this validation was added
to fix a security vulnerability with whitespace/quotes being passed to
the `bexpr` library. This unintentionally broke node names with
characters like `.` which aren't related to this vulnerability.

* Update website/content/docs/agent/config/cli-flags.mdx

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>

Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com>
This commit is contained in:
Kyle Havlovitz 2022-11-14 16:24:40 -08:00 committed by GitHub
parent a0c4ccd1b0
commit f5c5d2f5c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 10 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/15370.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
auto-config: Relax the validation on auto-config JWT authorization to allow non-whitespace, non-quote characters in node names.
```

4
agent/config/builder.go
View File

@ -1258,6 +1258,10 @@ func (b *builder) validate(rt RuntimeConfig) error {
b.warn("Node name %q will not be discoverable "+
"via DNS due to invalid characters. Valid characters include "+
"all alpha-numerics and dashes.", rt.NodeName)
case consul.InvalidNodeName.MatchString(rt.NodeName):
// todo(kyhavlov): Add stronger validation here for node names.
b.warn("Found invalid characters in node name %q - whitespace and quotes "+
"(', \", `) cannot be used with auto-config.", rt.NodeName)
case len(rt.NodeName) > dns.MaxLabelLength:
b.warn("Node name %q will not be discoverable "+
"via DNS due to it being too long. Valid lengths are between "+

3
agent/consul/auto_config_endpoint.go
View File

@ -57,6 +57,7 @@ type jwtAuthorizer struct {
// This includes an extra single-quote character not specified in the grammar for safety in case it is later added.
// https://github.com/hashicorp/go-bexpr/blob/v0.1.11/grammar/grammar.peg#L188-L191
var invalidSegmentName = regexp.MustCompile("[`'\"\\s]+")
var InvalidNodeName = invalidSegmentName
func (a *jwtAuthorizer) Authorize(req *pbautoconf.AutoConfigRequest) (AutoConfigOptions, error) {
// perform basic JWT Authorization
@ -70,7 +71,7 @@ func (a *jwtAuthorizer) Authorize(req *pbautoconf.AutoConfigRequest) (AutoConfig
// This is not the cleanest way to prevent this behavior. Ideally, the bexpr would allow us to
// inject a variable on the RHS for comparison as well, but it would be a complex change to implement
// that would likely break backwards-compatibility in certain circumstances.
if dns.InvalidNameRe.MatchString(req.Node) {
if InvalidNodeName.MatchString(req.Node) {
return AutoConfigOptions{}, fmt.Errorf("Invalid request field. %v = `%v`", "node", req.Node)
}
if invalidSegmentName.MatchString(req.Segment) {

1
website/content/docs/agent/config/cli-flags.mdx
View File

@ -482,6 +482,7 @@ information.
- `-node` ((#\_node)) - The name of this node in the cluster. This must
be unique within the cluster. By default this is the hostname of the machine.
The node name cannot contain whitespace or quotation marks. To query the node from DNS, the name must only contain alphanumeric characters and hyphens (`-`).
- `-node-id` ((#\_node_id)) - Available in Consul 0.7.3 and later, this
is a unique identifier for this node across all time, even if the name of the node