Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow dynamic header rewrite for host header on HTTPProxy routes. #5678

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion apis/projectcontour/v1/httpproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,18 @@ type Route struct {
// +optional
PathRewritePolicy *PathRewritePolicy `json:"pathRewritePolicy,omitempty"`
// The policy for managing request headers during proxying.
//
// You may dynamically rewrite the Host header to be forwarded
// upstream to the content of a request header using
// the below format "%REQ(X-Header-Name)%". If the value of the header
// is empty, it is ignored.
//
// *NOTE: Pay attention to the potential security implications of using this option.
// Provided header must come from trusted source.
//
// **NOTE: The header rewrite is only done while forwarding and has no bearing
// on the routing decision.
//
// +optional
RequestHeadersPolicy *HeadersPolicy `json:"requestHeadersPolicy,omitempty"`
// The policy for managing response headers during proxying.
Expand Down Expand Up @@ -1268,7 +1280,7 @@ type LoadBalancerPolicy struct {
}

// HeadersPolicy defines how headers are managed during forwarding.
// The `Host` header is treated specially and if set in a HTTP response
// The `Host` header is treated specially and if set in a HTTP request
// will be used as the SNI server name when forwarding over TLS. It is an
// error to attempt to set the `Host` header in a HTTP response.
type HeadersPolicy struct {
Expand Down
24 changes: 24 additions & 0 deletions changelogs/unreleased/5678-clayton-gonsalves-minor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## HTTPProxy: Allow Host header rewrite with dynamic headers.

This Change allows the host header to be rewritten on requests using dynamic headers on the only route level.

#### Example
```yaml
apiVersion: projectcontour.io/v1
kind: HTTPProxy
metadata:
name: dynamic-host-header-rewrite
spec:
fqdn: local.projectcontour.io
routes:
- conditions:
- prefix: /
services:
- name: s1
port: 80
- requestHeaderPolicy:
set:
- name: host
value: "%REQ(x-rewrite-header)%"
```

11 changes: 9 additions & 2 deletions examples/contour/01-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5883,8 +5883,15 @@ spec:
type: object
type: object
requestHeadersPolicy:
description: The policy for managing request headers during
proxying.
description: "The policy for managing request headers during
proxying. \n You may dynamically rewrite the Host header to
be forwarded upstream to the content of a request header using
the below format \"%REQ(X-Header-Name)%\". If the value of
the header is empty, it is ignored. \n *NOTE: Pay attention
to the potential security implications of using this option.
Provided header must come from trusted source. \n **NOTE:
The header rewrite is only done while forwarding and has no
bearing on the routing decision."
properties:
remove:
description: Remove specifies a list of HTTP header names
Expand Down
11 changes: 9 additions & 2 deletions examples/render/contour-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6096,8 +6096,15 @@ spec:
type: object
type: object
requestHeadersPolicy:
description: The policy for managing request headers during
proxying.
description: "The policy for managing request headers during
proxying. \n You may dynamically rewrite the Host header to
be forwarded upstream to the content of a request header using
the below format \"%REQ(X-Header-Name)%\". If the value of
the header is empty, it is ignored. \n *NOTE: Pay attention
to the potential security implications of using this option.
Provided header must come from trusted source. \n **NOTE:
The header rewrite is only done while forwarding and has no
bearing on the routing decision."
properties:
remove:
description: Remove specifies a list of HTTP header names
Expand Down
11 changes: 9 additions & 2 deletions examples/render/contour-gateway-provisioner.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5897,8 +5897,15 @@ spec:
type: object
type: object
requestHeadersPolicy:
description: The policy for managing request headers during
proxying.
description: "The policy for managing request headers during
proxying. \n You may dynamically rewrite the Host header to
be forwarded upstream to the content of a request header using
the below format \"%REQ(X-Header-Name)%\". If the value of
the header is empty, it is ignored. \n *NOTE: Pay attention
to the potential security implications of using this option.
Provided header must come from trusted source. \n **NOTE:
The header rewrite is only done while forwarding and has no
bearing on the routing decision."
properties:
remove:
description: Remove specifies a list of HTTP header names
Expand Down
11 changes: 9 additions & 2 deletions examples/render/contour-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6102,8 +6102,15 @@ spec:
type: object
type: object
requestHeadersPolicy:
description: The policy for managing request headers during
proxying.
description: "The policy for managing request headers during
proxying. \n You may dynamically rewrite the Host header to
be forwarded upstream to the content of a request header using
the below format \"%REQ(X-Header-Name)%\". If the value of
the header is empty, it is ignored. \n *NOTE: Pay attention
to the potential security implications of using this option.
Provided header must come from trusted source. \n **NOTE:
The header rewrite is only done while forwarding and has no
bearing on the routing decision."
properties:
remove:
description: Remove specifies a list of HTTP header names
Expand Down
11 changes: 9 additions & 2 deletions examples/render/contour.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6096,8 +6096,15 @@ spec:
type: object
type: object
requestHeadersPolicy:
description: The policy for managing request headers during
proxying.
description: "The policy for managing request headers during
proxying. \n You may dynamically rewrite the Host header to
be forwarded upstream to the content of a request header using
the below format \"%REQ(X-Header-Name)%\". If the value of
the header is empty, it is ignored. \n *NOTE: Pay attention
to the potential security implications of using this option.
Provided header must come from trusted source. \n **NOTE:
The header rewrite is only done while forwarding and has no
bearing on the routing decision."
properties:
remove:
description: Remove specifies a list of HTTP header names
Expand Down
4 changes: 4 additions & 0 deletions internal/dag/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,10 @@ type HeadersPolicy struct {
// HostRewrite defines if a host should be rewritten on upstream requests
HostRewrite string

// HostRewriteHeader defines if a host should be rewritten on upstream requests
// via a header value. only applicable for routes.
HostRewriteHeader string

Add map[string]string
Set map[string]string
Remove []string
Expand Down
36 changes: 31 additions & 5 deletions internal/dag/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ const (
LoadBalancerPolicyRequestHash = "RequestHash"
)

// match "%REQ(<X-Foo-Bar>)%"
var hostRewriteHeaderRegex = regexp.MustCompile(`%REQ\(([A-Za-z0-9-]+)\)%`)

// retryOn transforms a slice of retry on values to a comma-separated string.
// CRD validation ensures that all retry on values are valid.
func retryOn(ron []contour_api_v1.RetryOn) string {
Expand Down Expand Up @@ -127,6 +130,11 @@ func headersPolicyService(defaultPolicy *HeadersPolicy, policy *contour_api_v1.H
return nil, fmt.Errorf("rewriting %q header is not supported", key)
}
if len(userPolicy.HostRewrite) == 0 {
// check for the hostRewriteHeader on the service. Return error if set since this
// is not supported on envoy.
if HostRewriteHeader := extractHostRewriteHeaderValue(v); HostRewriteHeader != "" {
return nil, fmt.Errorf("rewriting %q host header with dynamic value is not supported on service", key)
}
userPolicy.HostRewrite = v
}
continue
Expand Down Expand Up @@ -164,6 +172,7 @@ func headersPolicyRoute(policy *contour_api_v1.HeadersPolicy, allowHostRewrite b

set := make(map[string]string, len(policy.Set))
hostRewrite := ""
hostRewriteHeader := ""
for _, entry := range policy.Set {
key := http.CanonicalHeaderKey(entry.Name)
if _, ok := set[key]; ok {
Expand All @@ -173,8 +182,13 @@ func headersPolicyRoute(policy *contour_api_v1.HeadersPolicy, allowHostRewrite b
if !allowHostRewrite {
return nil, fmt.Errorf("rewriting %q header is not supported", key)
}
hostRewrite = entry.Value
continue
if extractedHostRewriteHeader := extractHostRewriteHeaderValue(entry.Value); extractedHostRewriteHeader != "" {
hostRewriteHeader = http.CanonicalHeaderKey(extractedHostRewriteHeader)
continue
} else {
hostRewrite = entry.Value
continue
}
}
if msgs := validation.IsHTTPHeaderName(key); len(msgs) != 0 {
return nil, fmt.Errorf("invalid set header %q: %v", key, msgs)
Expand Down Expand Up @@ -203,12 +217,24 @@ func headersPolicyRoute(policy *contour_api_v1.HeadersPolicy, allowHostRewrite b
}

return &HeadersPolicy{
Set: set,
HostRewrite: hostRewrite,
Remove: rl,
Set: set,
HostRewrite: hostRewrite,
HostRewriteHeader: hostRewriteHeader,
Remove: rl,
}, nil
}

// extractHostRewriteHeaderValue returns the value of the header
func extractHostRewriteHeaderValue(s string) string {
matches := hostRewriteHeaderRegex.FindStringSubmatch(s)

if len(matches) == 2 {
return strings.TrimSpace(matches[1])
}

return ""
}

// headersPolicyGatewayAPI builds a *HeaderPolicy for the supplied HTTPHeaderFilter.
// TODO: Take care about the order of operators once https://github.com/kubernetes-sigs/gateway-api/issues/480 was solved.
func headersPolicyGatewayAPI(hf *gatewayapi_v1beta1.HTTPHeaderFilter, headerPolicyType string) (*HeadersPolicy, error) {
Expand Down
131 changes: 131 additions & 0 deletions internal/dag/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package dag

import (
"errors"
"fmt"
"io"
"testing"
"time"
Expand Down Expand Up @@ -1249,6 +1250,15 @@ func TestValidateHeaderAlteration(t *testing.T) {
"K-Foo": "100%%",
},
},
}, {
name: "Host header rewrite via dynamic header",
in: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{
Name: "Host",
Value: "%REQ(foo)%",
}},
},
wantErr: fmt.Errorf("rewriting \"Host\" header is not supported"),
}}

for _, test := range tests {
Expand All @@ -1259,3 +1269,124 @@ func TestValidateHeaderAlteration(t *testing.T) {
})
}
}

func TestExtractHeaderValue(t *testing.T) {
tests := map[string]string{
"%REQ(X-Header-Name)%": "X-Header-Name",
"%req(X-Header-Name)%": "",
"%REQ( Content-Type )%": "",
"REQ(Content-Type)": "",
"%REQ(Content-Type%": "",
"SomeOtherValue": "",
}

for input, expected := range tests {
t.Run(input, func(t *testing.T) {
actual := extractHostRewriteHeaderValue(input)
if actual != expected {
t.Errorf("For input %q, expected %q, got %q", input, expected, actual)
}
})
}
}

func TestHeadersPolicyRoute(t *testing.T) {
tests := []struct {
name string
policy *contour_api_v1.HeadersPolicy
allowRewrite bool
dynHeaders map[string]string
expected *HeadersPolicy
expectedErr error
}{
{
name: "nil policy",
policy: nil,
expected: nil,
},
{
name: "duplicate set headers",
policy: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{Name: "X-Header", Value: "Test"}, {Name: "X-Header", Value: "Test2"}},
},
expectedErr: fmt.Errorf("duplicate header addition: %q", "X-Header"),
},
{
name: "host rewrite not allowed",
policy: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "Test"}},
},
allowRewrite: false,
expectedErr: fmt.Errorf("rewriting %q header is not supported", "Host"),
},
{
name: "host rewrite allowed",
policy: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "Test"}},
},
allowRewrite: true,
expected: &HeadersPolicy{
HostRewrite: "Test",
Remove: nil,
},
},
{
name: "host rewrite allowed, by header",
policy: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "%REQ(Test)%"}},
},
allowRewrite: true,
expected: &HeadersPolicy{
HostRewrite: "",
HostRewriteHeader: "Test",
Remove: nil,
},
},
{
name: "host rewrite allowed, by header. invalid",
policy: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{Name: "Host", Value: "%REQ (Test"}},
},
allowRewrite: true,
expected: &HeadersPolicy{
HostRewrite: "%REQ (Test",
sunjayBhatia marked this conversation as resolved.
Show resolved Hide resolved
HostRewriteHeader: "",
Remove: nil,
},
},
{
name: "invalid header name",
policy: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{Name: " Invalid-Header ", Value: "Test"}},
},
expectedErr: fmt.Errorf(`invalid set header " Invalid-Header ": [a valid HTTP header must consist of alphanumeric characters or '-' (e.g. 'X-Header-Name', regex used for validation is '[-A-Za-z0-9]+')]`),
},
{
name: "duplicate remove headers",
policy: &contour_api_v1.HeadersPolicy{
Remove: []string{"X-Header", "X-Header"},
},
expectedErr: fmt.Errorf("duplicate header removal: %q", "X-Header"),
},
{
name: "valid set and remove headers",
policy: &contour_api_v1.HeadersPolicy{
Set: []contour_api_v1.HeaderValue{{Name: "X-Header", Value: "Test"}},
Remove: []string{"Y-Header"},
},
expected: &HeadersPolicy{
Set: map[string]string{"X-Header": "Test"},
HostRewrite: "",
Remove: []string{"Y-Header"},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result, err := headersPolicyRoute(tc.policy, tc.allowRewrite, tc.dynHeaders)
assert.Equal(t, tc.expected, result)
assert.Equal(t, tc.expectedErr, err)
})
}
}
9 changes: 8 additions & 1 deletion internal/envoy/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,20 @@ import (
"google.golang.org/protobuf/types/known/durationpb"
)

func HostReplaceHeader(hp *dag.HeadersPolicy) string {
func HostRewriteLiteral(hp *dag.HeadersPolicy) string {
if hp == nil {
return ""
}
return hp.HostRewrite
}

func HostRewriteHeader(hp *dag.HeadersPolicy) string {
if hp == nil {
return ""
}
return hp.HostRewriteHeader
}

// Timeout converts a timeout.Setting to a protobuf.Duration
// that's appropriate for Envoy. In general (though there are
// exceptions), Envoy uses the following semantics:
Expand Down
Loading