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

Extend redirect URI validation with protocol check #850

Merged
merged 1 commit into from
Feb 21, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ In the example below, NGINX responds with a redirect when a response from an ups
codes: [404]
redirect:
code: 301
url: ${scheme}://cafe.example.com/error_${status}.html
url: ${scheme}://cafe.example.com/error.html
Rulox marked this conversation as resolved.
Show resolved Hide resolved
```

```eval_rst
Expand All @@ -1008,7 +1008,7 @@ redirect:
- ``int``
- No
* - ``url``
- The URL to redirect the request to. Supported NGINX variables: ``$scheme``\ , ``$status``\ and ``$http_x_forwarded_proto``\. Variables must be inclosed in curly braces. For example: ``${scheme}``.
- The URL to redirect the request to. Supported NGINX variables: ``$scheme``\ and ``$http_x_forwarded_proto``\. Variables must be inclosed in curly braces. For example: ``${scheme}``.
- ``string``
- Yes
```
Expand All @@ -1026,8 +1026,8 @@ return:
type: application/json
body: "{\"msg\": \"You don't have permission to do this\"}"
headers:
- name: x-debug-original-status
value: ${status}
- name: x-debug-original-statuses
value: ${upstream_status}
Rulox marked this conversation as resolved.
Show resolved Hide resolved
Rulox marked this conversation as resolved.
Show resolved Hide resolved
```

```eval_rst
Expand All @@ -1047,7 +1047,7 @@ return:
- ``string``
- No
* - ``body``
- The body of the response. Supported NGINX variable: ``$status`` \ . Variables must be inclosed in curly braces. For example: ``${status}``.
- The body of the response. Supported NGINX variable: ``$upstream_status`` \ . Variables must be inclosed in curly braces. For example: ``${upstream_status}``.
- ``string``
- Yes
* - ``headers``
Expand All @@ -1061,8 +1061,8 @@ return:
The header defines an HTTP Header for a canned response in an errorPage:

```yaml
name: x-debug-original-status
value: ${status}
name: x-debug-original-statuses
value: ${upstream_status}
```

```eval_rst
Expand All @@ -1078,7 +1078,7 @@ value: ${status}
- ``string``
- Yes
* - ``value``
- The value of the header. Supported NGINX variables: ``$status`` \ . Variables must be inclosed in curly braces. For example: ``${status}``.
- The value of the header. Supported NGINX variable: ``$upstream_status`` \ . Variables must be inclosed in curly braces. For example: ``${upstream_status}``.
- ``string``
- No
```
Expand Down
10 changes: 7 additions & 3 deletions pkg/apis/configuration/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ func validateErrorPage(errorPage v1.ErrorPage, fieldPath *field.Path) field.Erro
return allErrs
}

var errorPageReturnBodyVariable = map[string]bool{"status": true}
var errorPageReturnBodyVariable = map[string]bool{"upstream_status": true}

func validateErrorPageReturn(r *v1.ErrorPageReturn, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand All @@ -659,7 +659,7 @@ func validateErrorPageReturn(r *v1.ErrorPageReturn, fieldPath *field.Path) field
return allErrs
}

var errorPageHeaderValueVariables = map[string]bool{"status": true}
var errorPageHeaderValueVariables = map[string]bool{"upstream_status": true}

func validateErrorPageHeader(h v1.Header, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand All @@ -682,7 +682,7 @@ func validateErrorPageHeader(h v1.Header, fieldPath *field.Path) field.ErrorList
return allErrs
}

var validErrorPageRedirectVariables = map[string]bool{"scheme": true, "status": true, "http_x_forwarded_proto": true}
var validErrorPageRedirectVariables = map[string]bool{"scheme": true, "http_x_forwarded_proto": true}

func validateErrorPageRedirect(r *v1.ErrorPageRedirect, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down Expand Up @@ -801,6 +801,10 @@ func validateRedirectURL(redirectURL string, fieldPath *field.Path, validVars ma
return append(allErrs, field.Required(fieldPath, "must specify a url"))
}

if !strings.Contains(redirectURL, "://") {
Rulox marked this conversation as resolved.
Show resolved Hide resolved
return append(allErrs, field.Invalid(fieldPath, redirectURL, "must contain the protocol with '://', for example http://, https:// or ${scheme}://"))
}

if !escapedStringsFmtRegexp.MatchString(redirectURL) {
msg := validation.RegexError(escapedStringsErrMsg, escapedStringsFmt, "http://www.nginx.com", "${scheme}://${host}/green/", `\"http://www.nginx.com\"`)
return append(allErrs, field.Invalid(fieldPath, redirectURL, msg))
Expand Down
14 changes: 9 additions & 5 deletions pkg/apis/configuration/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ func TestValidateRedirectURL(t *testing.T) {
msg: "x-forwarded-proto redirect url use case",
},
{
redirectURL: "${host}${request_uri}",
redirectURL: "http://${host}${request_uri}",
Rulox marked this conversation as resolved.
Show resolved Hide resolved
msg: "use multi variables, no scheme set",
},
{
Expand All @@ -962,7 +962,7 @@ func TestValidateRedirectURL(t *testing.T) {
msg: "url with escaped quotes",
},
{
redirectURL: "{abc}",
redirectURL: "http://{abc}",
msg: "url with curly braces with no $ prefix",
},
}
Expand All @@ -985,6 +985,10 @@ func TestValidateRedirectURLFails(t *testing.T) {
redirectURL: "",
msg: "url is blank",
},
{
redirectURL: "/uri",
msg: "url does not start with http://, https:// or ${scheme}://",
},
{
redirectURL: "$scheme://www.$host.com",
msg: "usage of nginx variable in url without ${}",
Expand Down Expand Up @@ -3041,7 +3045,7 @@ func TestValidateErrorPageReturn(t *testing.T) {
ActionReturn: v1.ActionReturn{
Code: 0,
Type: "",
Body: "Could not process request, try again. Status ${status}",
Body: "Could not process request, try again. Upstream status ${upstream_status}",
},
Headers: []v1.Header{
{
Expand All @@ -3054,7 +3058,7 @@ func TestValidateErrorPageReturn(t *testing.T) {
ActionReturn: v1.ActionReturn{
Code: 200,
Type: "application/json",
Body: `{\"message\": \"Could not process request, try again\", \"status\": \"${status}\"}`,
Body: `{\"message\": \"Could not process request, try again\", \"upstream_status\": \"${upstream_status}\"}`,
},
Headers: nil,
},
Expand Down Expand Up @@ -3207,7 +3211,7 @@ func TestValidateErrorPageHeader(t *testing.T) {
},
{
Name: "Header-Name",
Value: "${status}",
Value: "${upstream_status}",
},
}

Expand Down