Skip to content

Commit

Permalink
add disable-config-keywords command-line options
Browse files Browse the repository at this point in the history
This option allows to disable annotation based configuration snippets,
partially or completely. Configuration snippets can be the source of
failures in the HAProxy reload, and also vulnerable configurations.
  • Loading branch information
jcmoraisjr committed Jul 9, 2021
1 parent d1eff38 commit 3380f04
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 1 deletion.
13 changes: 13 additions & 0 deletions docs/content/en/docs/configuration/command-line.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ The following command-line options are supported:
| [`--default-backend-service`](#default-backend-service) | namespace/servicename | haproxy's 404 page | |
| [`--default-ssl-certificate`](#default-ssl-certificate) | namespace/secretname | fake, auto generated | |
| [`--disable-api-warnings`](#disable-api-warnings) | [true\|false] | `false` | v0.12 |
| [`--disable-config-keywords`](#disable-config-keywords) | comma-separated list of keywords | `""` | v0.10 |
| [`--disable-external-name`](#disable-external-name) | [true\|false] | `false` | v0.10 |
| [`--disable-pod-list`](#disable-pod-list) | [true\|false] | `false` | v0.11 |
| [`--election-id`](#election-id) | identifier | `ingress-controller-leader` | |
Expand Down Expand Up @@ -181,6 +182,18 @@ deprecation. The default behavior is to log all API server warnings.

---

## --disable-config-keywords

Since v0.10.9

Defines a comma-separated list of HAProxy keywords that should not be used on annotation based configuration snippets. Configuration snippets added as a global config does not follow this option. Use an asterisk `*` to disable configuration snippets using annotations.

Every keyword in the configuration will be compared with the first token of every configuration line, ignoring tabs and spaces. If a match occur, all the configuration snippet will be ignored and a warning is logged.

The default value is an empty string, enabling the configuration and accepting any HAProxy keyword.

---

## --disable-external-name

Since v0.10.9
Expand Down
1 change: 1 addition & 0 deletions pkg/common/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ type Configuration struct {
DisableNodeList bool
DisablePodList bool
DisableExternalName bool
DisableConfigKeywords string
AnnPrefix []string

AcmeServer bool
Expand Down
7 changes: 7 additions & 0 deletions pkg/common/ingress/controller/launch.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,12 @@ one, applying every new configuration changes made between this interval`)
disableExternalName = flags.Bool("disable-external-name", false,
`Disables services of type ExternalName`)

disableConfigKeywords = flags.String("disable-config-keywords", "",
`Defines a comma-separated list of HAProxy keywords that should not be used on
annotation based configuration snippets. Configuration snippets added as a
global config does not follow this option. Use an asterisk * to disable
configuration snippets using annotations.`)

updateStatusOnShutdown = flags.Bool("update-status-on-shutdown", true, `Indicates if the
ingress controller should update the Ingress status IP/hostname when the controller
is being stopped. Default is true`)
Expand Down Expand Up @@ -428,6 +434,7 @@ one, applying every new configuration changes made between this interval`)
DisableNodeList: *disableNodeList,
DisablePodList: *disablePodList,
DisableExternalName: *disableExternalName,
DisableConfigKeywords: *disableConfigKeywords,
UpdateStatusOnShutdown: *updateStatusOnShutdown,
BackendShards: *backendShards,
SortEndpointsBy: sortEndpoints,
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"net/http"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -154,6 +155,7 @@ func (hc *HAProxyController) configController() {
DefaultCrtSecret: hc.cfg.DefaultSSLCertificate,
FakeCrtFile: hc.createFakeCrtFile(),
FakeCAFile: hc.createFakeCAFile(),
DisableKeywords: strings.Split(hc.cfg.DisableConfigKeywords, ","),
AcmeTrackTLSAnn: hc.cfg.AcmeTrackTLSAnn,
HasGateway: hc.cache.hasGateway(),
}
Expand Down
44 changes: 44 additions & 0 deletions pkg/converters/ingress/annotations/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,50 @@ func (c *updater) buildBackendCors(d *backData) {
}
}

func (c *updater) buildBackendCustomConfig(d *backData) {
config := d.mapper.Get(ingtypes.BackConfigBackend)
if config.Source == nil {
return
}
lines := utils.LineToSlice(config.Value)
if len(lines) == 0 {
return
}
for _, keyword := range c.options.DisableKeywords {
if keyword == "*" {
c.logger.Warn("skipping configuration snippet on %s: custom configuration is disabled", config.Source)
return
}
for _, line := range lines {
if firstToken(line) == keyword {
c.logger.Warn("skipping configuration snippet on %s: keyword '%s' not allowed", config.Source, keyword)
return
}
}
}
d.backend.CustomConfig = lines
}

// kindly provided by strings/strings.go
var asciiSpace = [256]uint8{'\t': 1, '\n': 1, '\v': 1, '\f': 1, '\r': 1, ' ': 1}

func firstToken(s string) string {
// only ascii spaces supported, code<128
start := 0
for ; len(s) > start; start++ {
if asciiSpace[s[start]] == 0 {
break
}
}
end := start
for ; len(s) > end; end++ {
if asciiSpace[s[end]] == 1 {
break
}
}
return s[start:end]
}

func (c *updater) buildBackendDNS(d *backData) {
resolverName := d.mapper.Get(ingtypes.BackUseResolver).Value
if resolverName == "" {
Expand Down
120 changes: 120 additions & 0 deletions pkg/converters/ingress/annotations/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1308,6 +1308,126 @@ func TestCors(t *testing.T) {
}
}

func TestCustomConfig(t *testing.T) {
testCases := []struct {
disabled []string
config string
expected []string
logging string
}{
// 0
{
config: " server srv001 127.0.0.1:8080",
expected: []string{" server srv001 127.0.0.1:8080"},
},
// 1
{
disabled: []string{"server"},
config: " server srv001 127.0.0.1:8080",
logging: `WARN skipping configuration snippet on Ingress 'default/app': keyword 'server' not allowed`,
},
// 2
{
disabled: []string{"*"},
config: " server srv001 127.0.0.1:8080",
logging: `WARN skipping configuration snippet on Ingress 'default/app': custom configuration is disabled`,
},
// 3
{
disabled: []string{"http-response"},
config: `
http-request set-header x-id 1 if { path / }
`,
expected: []string{"", " http-request set-header x-id 1 if { path / }"},
},
// 4
{
disabled: []string{"http-response"},
config: `
acl rootpath path /
http-request set-header x-id 1 if rootpath
`,
expected: []string{"", " acl rootpath path /", " http-request set-header x-id 1 if rootpath"},
},
// 5
{
disabled: []string{"http-response", "acl"},
config: `
acl rootpath path /
http-request set-header x-id 1 if rootpath
`,
logging: `WARN skipping configuration snippet on Ingress 'default/app': keyword 'acl' not allowed`,
},
// 6
{
disabled: []string{"http"},
config: " http-request set-header x-id 1 if { path / }",
expected: []string{" http-request set-header x-id 1 if { path / }"},
},
}
for i, test := range testCases {
c := setup(t)
source := &Source{
Type: "Ingress",
Namespace: "default",
Name: "app",
}
ann := map[string]map[string]string{
"/": {ingtypes.BackConfigBackend: test.config},
}
d := c.createBackendMappingData("default/app", source, map[string]string{}, ann, []string{"/"})
updater := c.createUpdater()
updater.options.DisableKeywords = test.disabled
updater.buildBackendCustomConfig(d)
c.compareObjects("custom config", i, d.backend.CustomConfig, test.expected)
c.logger.CompareLogging(test.logging)
c.teardown()
}
}

func TestFirstToken(t *testing.T) {
testCases := []struct {
line string
expected string
}{
// 0
{
line: "",
expected: "",
},
// 1
{
line: "server",
expected: "server",
},
// 2
{
line: "server svc",
expected: "server",
},
// 3
{
line: "\tserver\tsvc",
expected: "server",
},
// 4
{
line: " \tserver \tsvc",
expected: "server",
},
// 5
{
line: " server svc",
expected: "server",
},
}
for i, test := range testCases {
c := setup(t)
c.compareObjects("token", i, firstToken(test.line), test.expected)
c.teardown()
}
}

func TestHeaders(t *testing.T) {
testCases := []struct {
headers string
Expand Down
2 changes: 1 addition & 1 deletion pkg/converters/ingress/annotations/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ func (c *updater) UpdateBackendConfig(backend *hatypes.Backend, mapper *Mapper)
}
// TODO check ModeTCP with HTTP annotations
backend.BalanceAlgorithm = mapper.Get(ingtypes.BackBalanceAlgorithm).Value
backend.CustomConfig = utils.LineToSlice(mapper.Get(ingtypes.BackConfigBackend).Value)
backend.Server.MaxConn = mapper.Get(ingtypes.BackMaxconnServer).Int()
backend.Server.MaxQueue = mapper.Get(ingtypes.BackMaxQueueServer).Int()
c.buildBackendAffinity(data)
Expand All @@ -222,6 +221,7 @@ func (c *updater) UpdateBackendConfig(backend *hatypes.Backend, mapper *Mapper)
c.buildBackendBlueGreenSelector(data)
c.buildBackendBodySize(data)
c.buildBackendCors(data)
c.buildBackendCustomConfig(data)
c.buildBackendDNS(data)
c.buildBackendDynamic(data)
c.buildBackendAgentCheck(data)
Expand Down
1 change: 1 addition & 0 deletions pkg/converters/types/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type ConverterOptions struct {
FakeCrtFile CrtFile
FakeCAFile CrtFile
AnnotationPrefix []string
DisableKeywords []string
AcmeTrackTLSAnn bool
HasGateway bool
}
Expand Down

0 comments on commit 3380f04

Please sign in to comment.