Skip to content

Commit 0c60bd6

Browse files
feat: nginx proxy headers security fine-tuning (#1093)
1 parent e87f3c4 commit 0c60bd6

File tree

36 files changed

+617
-436
lines changed

36 files changed

+617
-436
lines changed

golang/Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,3 +285,7 @@ test-cli:
285285
.PHONY: coverage
286286
coverage:
287287
go tool cover -func ./merged.cov
288+
289+
.PHONY: fieldalign
290+
fieldalign:
291+
fieldalign -fix ./...

golang/api/v1/deploy.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -179,17 +179,20 @@ type ContainerConfig struct {
179179
Networks []string `json:"networks"`
180180
Ports []builder.PortBinding `json:"port" binding:"dive"`
181181
PortRanges []builder.PortRangeBinding `json:"portRanges" binding:"dive"`
182-
CustomHeaders []string `json:"customHeaders,omitempty"`
183-
Mounts []string `json:"mount"`
184-
IngressPort uint16 `json:"ingressPort"`
185-
Replicas uint8 `json:"replicas"`
186-
Shared bool `json:"shared"`
187-
UseLoadBalancer bool `json:"useLoadBalancer"`
188-
Expose bool `json:"expose"`
189-
ProxyHeaders bool `json:"proxyHeaders"`
190-
ExposeTLS bool `json:"exposeTls"`
191-
IngressStripPath bool `json:"ingressPathStrip"`
192-
TTY bool `json:"tty"`
182+
// These are CORS headers, providing value will enable CORS and add these headers as extra
183+
CorsHeaders []string `json:"corsHeaders,omitempty"`
184+
// Not all headers are proxied downstream by default, this allows the listed headers to reach backends
185+
ProxyHeaders []string `json:"proxyHeaders,omitempty"`
186+
Mounts []string `json:"mount"`
187+
IngressPort uint16 `json:"ingressPort"`
188+
Replicas uint8 `json:"replicas"`
189+
Shared bool `json:"shared"`
190+
ProxyBuffering bool `json:"proxyBuffering"`
191+
UseLoadBalancer bool `json:"useLoadBalancer"`
192+
Expose bool `json:"expose"`
193+
ExposeTLS bool `json:"exposeTls"`
194+
IngressStripPath bool `json:"ingressPathStrip"`
195+
TTY bool `json:"tty"`
193196
}
194197

195198
type Metrics struct {

golang/internal/mapper/grpc.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,15 @@ func mapCraneConfig(crane *agent.CraneContainerConfig, containerConfig *v1.Conta
174174
containerConfig.DeploymentStrategy = strcase.ToCamel(crane.DeploymentStrategy.String())
175175

176176
if crane.ProxyHeaders != nil {
177-
containerConfig.ProxyHeaders = *crane.ProxyHeaders
177+
containerConfig.ProxyHeaders = crane.ProxyHeaders
178+
}
179+
180+
if crane.CorsHeaders != nil {
181+
containerConfig.CorsHeaders = crane.CorsHeaders
182+
}
183+
184+
if crane.ProxyBuffering != nil {
185+
containerConfig.ProxyBuffering = *crane.ProxyBuffering
178186
}
179187

180188
if crane.UseLoadBalancer != nil {

golang/internal/mapper/grpc_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ func testExpectedCommon(req *agent.DeployWorkloadRequest) *v1.DeployImageRequest
123123
RestartPolicy: container.RestartPolicyAlways,
124124
Networks: []string{"n1", "n2"},
125125
NetworkMode: "BRIDGE",
126-
CustomHeaders: []string(nil),
127126
Annotations: v1.Markers{
128127
Deployment: map[string]string{"annot1": "value1"},
129128
Service: map[string]string{"annot2": "value2"},
@@ -144,7 +143,7 @@ func testExpectedCommon(req *agent.DeployWorkloadRequest) *v1.DeployImageRequest
144143
Limits: v1.Resources{CPU: "250m", Memory: "512Mi"},
145144
Requests: v1.Resources{CPU: "100m", Memory: "64Mi"},
146145
},
147-
ProxyHeaders: true,
146+
ProxyHeaders: []string{"ProxyHeader"},
148147
UseLoadBalancer: true,
149148
ExtraLBAnnotations: map[string]string{"annotation1": "value1"},
150149
},
@@ -336,9 +335,8 @@ func testCraneConfig() *agent.CraneContainerConfig {
336335
sProbe := "/test-startup"
337336
port := int32(1234)
338337
return &agent.CraneContainerConfig{
339-
CustomHeaders: []string{"header1", "value1", "header2", "value2"},
340338
ExtraLBAnnotations: map[string]string{"annotation1": "value1"},
341-
ProxyHeaders: &b,
339+
ProxyHeaders: []string{"ProxyHeader"},
342340
UseLoadBalancer: &b,
343341
Labels: &agent.Marker{
344342
Deployment: map[string]string{"label1": "value1"},

golang/pkg/crane/k8s/configmap.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,26 @@ func (cm *configmap) deployConfigMapRuntime(runtimeType v1.RuntimeConfigType, na
8484
return nil
8585
}
8686

87+
func (cm *configmap) deployIngressProxyHeaders(namespace, containerName string, headers ...string) error {
88+
client, err := getConfigMapClient(namespace, cm.appConfig)
89+
if err != nil {
90+
return err
91+
}
92+
93+
headerMap := make(map[string]string)
94+
for _, item := range headers {
95+
headerMap[item] = ""
96+
}
97+
98+
_, err = client.Apply(cm.ctx,
99+
corev1.ConfigMap(containerName, namespace).
100+
WithData(headerMap),
101+
metaV1.ApplyOptions{FieldManager: cm.appConfig.FieldManagerName, Force: cm.appConfig.ForceOnConflicts},
102+
)
103+
104+
return err
105+
}
106+
87107
// delete related configmaps. note: configmaps being in use are unaffected by this
88108
func (cm *configmap) deleteConfigMaps(namespace, name string) error {
89109
client, err := getConfigMapClient(namespace, cm.appConfig)

golang/pkg/crane/k8s/deploy_facade.go

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66

77
"github.com/rs/zerolog/log"
8+
"k8s.io/apimachinery/pkg/api/errors"
89

910
v1 "github.com/dyrector-io/dyrectorio/golang/api/v1"
1011
"github.com/dyrector-io/dyrectorio/golang/internal/dogger"
@@ -13,8 +14,6 @@ import (
1314
"github.com/dyrector-io/dyrectorio/golang/internal/util"
1415
builder "github.com/dyrector-io/dyrectorio/golang/pkg/builder/container"
1516
"github.com/dyrector-io/dyrectorio/golang/pkg/crane/config"
16-
17-
"k8s.io/apimachinery/pkg/api/errors"
1817
)
1918

2019
type DeployFacade struct {
@@ -213,28 +212,39 @@ func (d *DeployFacade) Deploy() error {
213212
}
214213

215214
if d.params.ContainerConfig.Expose {
215+
if len(d.params.ContainerConfig.ProxyHeaders) > 0 {
216+
if err := d.configmap.deployIngressProxyHeaders(d.namespace.name,
217+
d.params.ContainerConfig.Container,
218+
d.params.ContainerConfig.ProxyHeaders...,
219+
); err != nil {
220+
log.Error().Err(err).Stack().Msg("Error with ingress proxy headers configmap")
221+
}
222+
}
223+
216224
if err := d.ingress.deployIngress(
217225
&DeployIngressOptions{
218226
namespace: d.namespace.name,
219227
containerName: d.params.ContainerConfig.Container,
220-
ingressName: d.params.ContainerConfig.IngressName,
221-
ingressHost: d.params.ContainerConfig.IngressHost,
222-
ingressPath: d.params.ContainerConfig.IngressPath,
223-
stripPrefix: d.params.ContainerConfig.IngressStripPath,
224-
uploadLimit: d.params.ContainerConfig.IngressUploadLimit,
225-
customHeaders: d.params.ContainerConfig.CustomHeaders,
226-
port: d.params.ContainerConfig.IngressPort,
227-
portList: d.service.portsBound,
228-
tls: d.params.ContainerConfig.ExposeTLS,
229-
proxyHeaders: d.params.ContainerConfig.ProxyHeaders,
230-
annotations: d.params.ContainerConfig.Annotations.Ingress,
231-
labels: d.params.ContainerConfig.Labels.Ingress,
228+
name: d.params.ContainerConfig.IngressName,
229+
routing: routingOptions{
230+
proxyBuffering: d.params.ContainerConfig.ProxyBuffering,
231+
ingressHost: d.params.ContainerConfig.IngressHost,
232+
ingressPath: d.params.ContainerConfig.IngressPath,
233+
stripPrefix: d.params.ContainerConfig.IngressStripPath,
234+
uploadLimit: d.params.ContainerConfig.IngressUploadLimit,
235+
proxyHeaders: d.params.ContainerConfig.ProxyHeaders,
236+
corsHeaders: d.params.ContainerConfig.CorsHeaders,
237+
port: d.params.ContainerConfig.IngressPort,
238+
portList: d.service.portsBound,
239+
tls: d.params.ContainerConfig.ExposeTLS,
240+
},
241+
annotations: d.params.ContainerConfig.Annotations.Ingress,
242+
labels: d.params.ContainerConfig.Labels.Ingress,
232243
},
233244
); err != nil {
234245
log.Error().Err(err).Stack().Msg("Error with ingress")
235246
}
236247
}
237-
238248
return nil
239249
}
240250

golang/pkg/crane/k8s/ingress.go

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,11 @@ import (
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1313
applymetav1 "k8s.io/client-go/applyconfigurations/meta/v1"
1414
netv1 "k8s.io/client-go/applyconfigurations/networking/v1"
15+
networking "k8s.io/client-go/kubernetes/typed/networking/v1"
1516

1617
"github.com/dyrector-io/dyrectorio/golang/internal/domain"
1718
"github.com/dyrector-io/dyrectorio/golang/internal/util"
1819
"github.com/dyrector-io/dyrectorio/golang/pkg/crane/config"
19-
20-
networking "k8s.io/client-go/kubernetes/typed/networking/v1"
2120
)
2221

2322
// facade object for ingress management
@@ -28,21 +27,26 @@ type ingress struct {
2827
status string
2928
}
3029

30+
type routingOptions struct {
31+
ingressHost string
32+
ingressPath string
33+
uploadLimit string
34+
proxyHeaders []string
35+
corsHeaders []string
36+
portList []int32
37+
port uint16
38+
proxyBuffering bool
39+
stripPrefix bool
40+
tls bool
41+
}
42+
3143
type DeployIngressOptions struct {
3244
annotations map[string]string
3345
labels map[string]string
3446
containerName string
35-
ingressName string
36-
ingressHost string
37-
ingressPath string
38-
uploadLimit string
47+
name string
3948
namespace string
40-
customHeaders []string
41-
portList []int32
42-
port uint16
43-
stripPrefix bool
44-
proxyHeaders bool
45-
tls bool
49+
routing routingOptions
4650
}
4751

4852
func newIngress(ctx context.Context, client *Client) *ingress {
@@ -59,29 +63,31 @@ func (ing *ingress) deployIngress(options *DeployIngressOptions) error {
5963
log.Error().Err(err).Stack().Msg("Error with ingress client")
6064
}
6165

62-
if options.port == 0 && len(options.portList) == 0 {
66+
routing := options.routing
67+
68+
if routing.port == 0 && len(routing.portList) == 0 {
6369
return errors.New("empty ports, nothing to expose")
6470
}
6571

66-
routedPort := options.port
72+
routedPort := routing.port
6773
if routedPort == 0 {
68-
routedPort = uint16(options.portList[0]) //#nosec G115
74+
routedPort = uint16(routing.portList[0]) //#nosec G115
6975
}
7076

7177
ingressDomain := domain.GetHostRule(
7278
&domain.HostRouting{
73-
Subdomain: options.ingressName,
74-
RootDomain: options.ingressHost,
79+
Subdomain: options.name,
80+
RootDomain: routing.ingressHost,
7581
ContainerName: options.containerName,
7682
Prefix: options.namespace,
7783
DomainFallback: ing.appConfig.RootDomain,
7884
})
7985

8086
ingressPath := "/"
81-
if options.ingressPath != "" {
82-
ingressPath = options.ingressPath
87+
if routing.ingressPath != "" {
88+
ingressPath = routing.ingressPath
8389
// prefix stripping works in combination with annotations
84-
if options.stripPrefix {
90+
if routing.stripPrefix {
8591
split := strings.Split(ingressPath, "/")
8692

8793
split = append(split, "?(.*)")
@@ -105,12 +111,12 @@ func (ing *ingress) deployIngress(options *DeployIngressOptions) error {
105111
),
106112
),
107113
)))
108-
tlsConf := getTLSConfig(ingressDomain, options.containerName, options.tls)
114+
tlsConf := getTLSConfig(ingressDomain, options.containerName, options.routing.tls)
109115
if tlsConf != nil {
110116
spec.WithTLS(tlsConf)
111117
}
112118

113-
annot := getIngressAnnotations(options)
119+
annot := getIngressAnnotations(options.namespace, options.containerName, &options.routing)
114120
maps.Copy(annot, options.annotations)
115121

116122
labels := map[string]string{}
@@ -153,9 +159,7 @@ func getTLSConfig(ingressPath, containerName string, enabled bool) *netv1.Ingres
153159
return nil
154160
}
155161

156-
func getIngressAnnotations(opts *DeployIngressOptions) map[string]string {
157-
corsHeaders := []string{}
158-
162+
func getIngressAnnotations(namespace, name string, opts *routingOptions) map[string]string {
159163
annotations := map[string]string{
160164
"kubernetes.io/ingress.class": "nginx",
161165
}
@@ -165,23 +169,18 @@ func getIngressAnnotations(opts *DeployIngressOptions) map[string]string {
165169
annotations["cert-manager.io/cluster-issuer"] = "letsencrypt-prod"
166170
}
167171

168-
// Add Custom Headers to the CORS Allow Header annotation if presents
169-
if len(opts.customHeaders) > 0 {
170-
corsHeaders = opts.customHeaders
172+
if len(opts.corsHeaders) > 0 {
173+
annotations["nginx.ingress.kubernetes.io/enable-cors"] = "true"
174+
annotations["nginx.ingress.kubernetes.io/cors-allow-headers"] = strings.Join(opts.proxyHeaders, ", ")
171175
}
172176

173-
if opts.proxyHeaders {
174-
extraHeaders := []string{"X-Forwarded-For", "X-Forwarded-Host", "X-Forwarded-Server", "X-Real-IP", "X-Requested-With"}
175-
corsHeaders = append(corsHeaders, extraHeaders...)
176-
177-
annotations["nginx.ingress.kubernetes.io/enable-cors"] = "true"
177+
if opts.proxyBuffering {
178178
annotations["nginx.ingress.kubernetes.io/proxy-buffering"] = "on"
179179
annotations["nginx.ingress.kubernetes.io/proxy-buffer-size"] = "256k"
180180
}
181181

182-
// Add header string to cors-allow-headers if presents any value
183-
if len(corsHeaders) > 0 {
184-
annotations["nginx.ingress.kubernetes.io/cors-allow-headers"] = strings.Join(corsHeaders, ", ")
182+
if len(opts.proxyHeaders) > 0 {
183+
annotations["nginx.ingress.kubernetes.io/proxy-set-headers"] = util.JoinV("/", namespace, name)
185184
}
186185

187186
if opts.uploadLimit != "" {

golang/pkg/dagent/utils/dockerhelper_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ func TestEventToMessageContainer(t *testing.T) {
172172

173173
assert.NoError(t, err)
174174

175+
// if this is failing locally, you have to log in to ghcr.io
175176
containerID := *builder.GetContainerID()
176177

177178
event := events.Message{

0 commit comments

Comments
 (0)