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 a8838e7 commit 5afc104
Show file tree
Hide file tree
Showing 9 changed files with 191 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 @@ -164,6 +165,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 @@ -75,6 +75,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 @@ -176,6 +176,12 @@ func NewIngressController(backend ingress.Controller) *GenericController {
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 @@ -393,6 +399,7 @@ func NewIngressController(backend ingress.Controller) *GenericController {
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"
"time"

"github.com/golang/glog"
Expand Down Expand Up @@ -148,6 +149,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,
}
}
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 @@ -429,6 +429,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 @@ -938,6 +938,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 @@ -182,7 +182,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 @@ -191,6 +190,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
2 changes: 2 additions & 0 deletions pkg/converters/ingress/annotations/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

conv_helper "github.com/jcmoraisjr/haproxy-ingress/pkg/converters/helper_test"
"github.com/jcmoraisjr/haproxy-ingress/pkg/converters/ingress/tracker"
"github.com/jcmoraisjr/haproxy-ingress/pkg/converters/ingress/types"
convtypes "github.com/jcmoraisjr/haproxy-ingress/pkg/converters/types"
"github.com/jcmoraisjr/haproxy-ingress/pkg/haproxy"
hatypes "github.com/jcmoraisjr/haproxy-ingress/pkg/haproxy/types"
Expand Down Expand Up @@ -135,6 +136,7 @@ func (c *testConfig) teardown() {
func (c *testConfig) createUpdater() *updater {
return &updater{
haproxy: c.haproxy,
options: &types.ConverterOptions{},
cache: c.cache,
logger: c.logger,
tracker: c.tracker,
Expand Down
1 change: 1 addition & 0 deletions pkg/converters/ingress/types/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,6 @@ type ConverterOptions struct {
FakeCrtFile convtypes.CrtFile
FakeCAFile convtypes.CrtFile
AnnotationPrefix string
DisableKeywords []string
AcmeTrackTLSAnn bool
}

0 comments on commit 5afc104

Please sign in to comment.