Skip to content

Commit

Permalink
acl: fix parsing of policies with blocks w/o label
Browse files Browse the repository at this point in the history
An ACL policy with a block without label generates unexpected results.
For example, a policy such as this:

```
namespace {
  policy = "read"
}
```

Is applied to a namespace called `policy` instead of the documented
behaviour of applying it to the `default` namespace.

This happens because of the way HCL1 decodes blocks. Since it doesn't
know if a block is expected to have a label it applies the `key` tag to
the content of the block and, in the example above, the first key is
`policy`, so it sets that as the `namespace` block label.

Since this happens internally in the HCL decoder it's not possible to
detect the problem externally.

Fixing the problem inside the decoder is challenging because the JSON
and HCL parsers generate different ASTs that makes impossible to
differentiate between a JSON tree from an invalid HCL tree within the
decoder.

The fix in this commit consists of manually parsing the policy after
decoding to clear labels that were not set in the file. This allows the
validation rules to consistently catch and return any errors, no matter
if the policy is an invalid HCL or JSON.
  • Loading branch information
lgfa29 authored and jrasell committed Jul 26, 2023
1 parent 0a0a63e commit b55d770
Show file tree
Hide file tree
Showing 4 changed files with 506 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/17908.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:security
acl: Fixed a bug where a namespace ACL policy without label was applied to an unexpected namespace. [CVE-2023-3072](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-3072)
```
72 changes: 71 additions & 1 deletion acl/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
package acl

import (
"errors"
"fmt"
"regexp"

"github.com/hashicorp/hcl"
"github.com/hashicorp/hcl/hcl/ast"
)

const (
Expand Down Expand Up @@ -477,5 +479,73 @@ func hclDecode(p *Policy, rules string) (err error) {
}
}()

return hcl.Decode(p, rules)
if err = hcl.Decode(p, rules); err != nil {
return err
}

// Manually parse the policy to fix blocks without labels.
//
// Due to a bug in the way HCL decodes files, a block without a label may
// return an incorrect key value and make it impossible to determine if the
// key was set by the user or incorrectly set by the decoder.
//
// By manually parsing the file we are able to determine if the label is
// missing in the file and set them to an empty string so the policy
// validation can return the appropriate errors.
root, err := hcl.Parse(rules)
if err != nil {
return fmt.Errorf("failed to parse policy: %w", err)
}

list, ok := root.Node.(*ast.ObjectList)
if !ok {
return errors.New("error parsing: root should be an object")
}

nsList := list.Filter("namespace")
for i, nsObj := range nsList.Items {
// Fix missing namespace key.
if len(nsObj.Keys) == 0 {
p.Namespaces[i].Name = ""
}

// Fix missing variable paths.
nsOT, ok := nsObj.Val.(*ast.ObjectType)
if !ok {
continue
}
varsList := nsOT.List.Filter("variables")
if varsList == nil || len(varsList.Items) == 0 {
continue
}

varsObj, ok := varsList.Items[0].Val.(*ast.ObjectType)
if !ok {
continue
}
paths := varsObj.List.Filter("path")
for j, path := range paths.Items {
if len(path.Keys) == 0 {
p.Namespaces[i].Variables.Paths[j].PathSpec = ""
}
}
}

npList := list.Filter("node_pool")
for i, npObj := range npList.Items {
// Fix missing node pool key.
if len(npObj.Keys) == 0 {
p.NodePools[i].Name = ""
}
}

hvList := list.Filter("host_volume")
for i, hvObj := range hvList.Items {
// Fix missing host volume key.
if len(hvObj.Keys) == 0 {
p.HostVolumes[i].Name = ""
}
}

return nil
}
Loading

0 comments on commit b55d770

Please sign in to comment.