Skip to content

Commit

Permalink
Merge pull request #3506 from KoerberDigitalDevTeam/feature/extrenal-…
Browse files Browse the repository at this point in the history
…auth-security-opt-out

Improve the external authorization concept from opt-in to secure-by-default
  • Loading branch information
k8s-ci-robot authored May 7, 2019
2 parents b8fb09d + 4811168 commit b06e114
Show file tree
Hide file tree
Showing 20 changed files with 814 additions and 67 deletions.
9 changes: 9 additions & 0 deletions docs/user-guide/nginx-configuration/annotations.md
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream](#client-certificate-authentication)|"true" or "false"|
|[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string|
|[nginx.ingress.kubernetes.io/auth-snippet](#external-authentication)|string|
|[nginx.ingress.kubernetes.io/enable-global-auth](#external-authentication)|"true" or "false"|
|[nginx.ingress.kubernetes.io/backend-protocol](#backend-protocol)|string|HTTP,HTTPS,GRPC,GRPCS,AJP|
|[nginx.ingress.kubernetes.io/canary](#canary)|"true" or "false"|
|[nginx.ingress.kubernetes.io/canary-by-header](#canary)|string|
Expand Down Expand Up @@ -389,6 +390,14 @@ nginx.ingress.kubernetes.io/auth-snippet: |
!!! example
Please check the [external-auth](../../examples/auth/external-auth/README.md) example.

#### Global External Authentication

By default the controller redirects all requests to an existing service that provides authentication if `global-auth-url` is set in the NGINX ConfigMap. If you want to disable this behavior for that ingress, you can use `enable-global-auth: "false"` in the NGINX ConfigMap.
`nginx.ingress.kubernetes.io/enable-global-auth`:
indicates if GlobalExternalAuth configuration should be applied or not to this Ingress rule. Default values is set to `"true"`.

!!! note For more information please see [global-auth-url](./configmap.md#global-auth-url).

### Rate limiting

These annotations define a limit on the connections that can be opened by a single client IP address.
Expand Down
45 changes: 45 additions & 0 deletions docs/user-guide/nginx-configuration/configmap.md
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ The following table shows a configuration option's name, type, and the default v
|[limit-req-status-code](#limit-req-status-code)|int|503|
|[limit-conn-status-code](#limit-conn-status-code)|int|503|
|[no-tls-redirect-locations](#no-tls-redirect-locations)|string|"/.well-known/acme-challenge"|
|[global-auth-url](#global-auth-url)|string|""|
|[global-auth-method](#global-auth-method)|string|""|
|[global-auth-signin](#global-auth-signin)|string|""|
|[global-auth-response-headers](#global-auth-response-headers)|string|""|
|[global-auth-request-redirect](#global-auth-request-redirect)|string|""|
|[global-auth-snippet](#global-auth-snippet)|string|""|
|[no-auth-locations](#no-auth-locations)|string|"/.well-known/acme-challenge"|
|[block-cidrs](#block-cidrs)|[]string|""|
|[block-user-agents](#block-user-agents)|[]string|""|
Expand Down Expand Up @@ -864,6 +870,45 @@ Sets the [status code to return in response to rejected connections](http://ngin
A comma-separated list of locations on which http requests will never get redirected to their https counterpart.
_**default:**_ "/.well-known/acme-challenge"

## global-auth-url

A url to an existing service that provides authentication for all the locations.
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-url`.
Locations that should not get authenticated can be listed using `no-auth-locations` See [no-auth-locations](#no-auth-locations). In addition, each service can be excluded from authentication via annotation `enable-global-auth` set to "false".
_**default:**_ ""

_References:_ [https://github.com/kubernetes/ingress-nginx/blob/master/docs/user-guide/nginx-configuration/annotations.md#external-authentication](https://github.com/kubernetes/ingress-nginx/blob/master/docs/user-guide/nginx-configuration/annotations.md#external-authentication)

## global-auth-method

A HTTP method to use for an existing service that provides authentication for all the locations.
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-method`.
_**default:**_ ""

## global-auth-signin

Sets the location of the error page for an existing service that provides authentication for all the locations.
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-signin`.
_**default:**_ ""

## global-auth-response-headers

Sets the headers to pass to backend once authentication request completes. Applied to all the locations.
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-response-headers`.
_**default:**_ ""

## global-auth-request-redirect

Sets the X-Auth-Request-Redirect header value. Applied to all the locations.
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-request-redirect`.
_**default:**_ ""

## global-auth-snippet

Sets a custom snippet to use with external authentication. Applied to all the locations.
Similar to the Ingress rule annotation `nginx.ingress.kubernetes.io/auth-request-redirect`.
_**default:**_ ""

## no-auth-locations

A comma-separated list of locations that should not get authenticated.
Expand Down
3 changes: 3 additions & 0 deletions internal/ingress/annotations/annotations.go
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/ingress-nginx/internal/ingress/annotations/alias"
"k8s.io/ingress-nginx/internal/ingress/annotations/auth"
"k8s.io/ingress-nginx/internal/ingress/annotations/authreq"
"k8s.io/ingress-nginx/internal/ingress/annotations/authreqglobal"
"k8s.io/ingress-nginx/internal/ingress/annotations/authtls"
"k8s.io/ingress-nginx/internal/ingress/annotations/backendprotocol"
"k8s.io/ingress-nginx/internal/ingress/annotations/clientbodybuffersize"
Expand Down Expand Up @@ -83,6 +84,7 @@ type Ingress struct {
//TODO: Change this back into an error when https://github.com/imdario/mergo/issues/100 is resolved
Denied *string
ExternalAuth authreq.Config
EnableGlobalAuth bool
HTTP2PushPreload bool
Proxy proxy.Config
RateLimit ratelimit.Config
Expand Down Expand Up @@ -127,6 +129,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor {
"CustomHTTPErrors": customhttperrors.NewParser(cfg),
"DefaultBackend": defaultbackend.NewParser(cfg),
"ExternalAuth": authreq.NewParser(cfg),
"EnableGlobalAuth": authreqglobal.NewParser(cfg),
"HTTP2PushPreload": http2pushpreload.NewParser(cfg),
"Proxy": proxy.NewParser(cfg),
"RateLimit": ratelimit.NewParser(cfg),
Expand Down
45 changes: 29 additions & 16 deletions internal/ingress/annotations/authreq/main.go
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package authreq

import (
"fmt"
"net/url"
"regexp"
"strings"
Expand Down Expand Up @@ -84,7 +85,8 @@ var (
headerRegexp = regexp.MustCompile(`^[a-zA-Z\d\-_]+$`)
)

func validMethod(method string) bool {
// ValidMethod checks is the provided string a valid HTTP method
func ValidMethod(method string) bool {
if len(method) == 0 {
return false
}
Expand All @@ -97,7 +99,8 @@ func validMethod(method string) bool {
return false
}

func validHeader(header string) bool {
// ValidHeader checks is the provided string satisfies the header's name regex
func ValidHeader(header string) bool {
return headerRegexp.Match([]byte(header))
}

Expand All @@ -119,22 +122,13 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
return nil, err
}

authURL, err := url.Parse(urlString)
if err != nil {
return nil, err
}
if authURL.Scheme == "" {
return nil, ing_errors.NewLocationDenied("url scheme is empty")
}
if authURL.Host == "" {
return nil, ing_errors.NewLocationDenied("url host is empty")
}
if strings.Contains(authURL.Host, "..") {
return nil, ing_errors.NewLocationDenied("invalid url host")
authURL, message := ParseStringToURL(urlString)
if authURL == nil {
return nil, ing_errors.NewLocationDenied(message)
}

authMethod, _ := parser.GetStringAnnotation("auth-method", ing)
if len(authMethod) != 0 && !validMethod(authMethod) {
if len(authMethod) != 0 && !ValidMethod(authMethod) {
return nil, ing_errors.NewLocationDenied("invalid HTTP method")
}

Expand All @@ -156,7 +150,7 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
for _, header := range harr {
header = strings.TrimSpace(header)
if len(header) > 0 {
if !validHeader(header) {
if !ValidHeader(header) {
return nil, ing_errors.NewLocationDenied("invalid headers list")
}
responseHeaders = append(responseHeaders, header)
Expand All @@ -176,3 +170,22 @@ func (a authReq) Parse(ing *extensions.Ingress) (interface{}, error) {
AuthSnippet: authSnippet,
}, nil
}

// ParseStringToURL parses the provided string into URL and returns error
// message in case of failure
func ParseStringToURL(input string) (*url.URL, string) {

parsedURL, err := url.Parse(input)
if err != nil {
return nil, fmt.Sprintf("%v is not a valid URL: %v", input, err)
}
if parsedURL.Scheme == "" {
return nil, "url scheme is empty."
} else if parsedURL.Host == "" {
return nil, "url host is empty."
} else if strings.Contains(parsedURL.Host, "..") {
return nil, "invalid url host."
}
return parsedURL, ""

}
36 changes: 36 additions & 0 deletions internal/ingress/annotations/authreq/main_test.go
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package authreq

import (
"fmt"
"net/url"
"reflect"
"testing"

Expand Down Expand Up @@ -178,3 +179,38 @@ func TestHeaderAnnotations(t *testing.T) {
}
}
}

func TestParseStringToURL(t *testing.T) {
validURL := "http://bar.foo.com/external-auth"
validParsedURL, _ := url.Parse(validURL)

tests := []struct {
title string
url string
message string
parsed *url.URL
expErr bool
}{
{"empty", "", "url scheme is empty.", nil, true},
{"no scheme", "bar", "url scheme is empty.", nil, true},
{"invalid host", "http://", "url host is empty.", nil, true},
{"invalid host (multiple dots)", "http://foo..bar.com", "invalid url host.", nil, true},
{"valid URL", validURL, "", validParsedURL, false},
}

for _, test := range tests {

i, err := ParseStringToURL(test.url)
if test.expErr {
if err != test.message {
t.Errorf("%v: expected error \"%v\" but \"%v\" was returned", test.title, test.message, err)
}
continue
}

if i.String() != test.parsed.String() {
t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.parsed, i)
}
}

}
45 changes: 45 additions & 0 deletions internal/ingress/annotations/authreqglobal/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
Copyright 2015 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package authreqglobal

import (
extensions "k8s.io/api/extensions/v1beta1"

"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/resolver"
)

type authReqGlobal struct {
r resolver.Resolver
}

// NewParser creates a new authentication request annotation parser
func NewParser(r resolver.Resolver) parser.IngressAnnotation {
return authReqGlobal{r}
}

// ParseAnnotations parses the annotations contained in the ingress
// rule used to enable or disable global external authentication
func (a authReqGlobal) Parse(ing *extensions.Ingress) (interface{}, error) {

enableGlobalAuth, err := parser.GetBoolAnnotation("enable-global-auth", ing)
if err != nil {
enableGlobalAuth = true
}

return enableGlobalAuth, nil
}
82 changes: 82 additions & 0 deletions internal/ingress/annotations/authreqglobal/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
Copyright 2015 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package authreqglobal

import (
"testing"

api "k8s.io/api/core/v1"
extensions "k8s.io/api/extensions/v1beta1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/resolver"

"k8s.io/apimachinery/pkg/util/intstr"
)

func buildIngress() *extensions.Ingress {
defaultBackend := extensions.IngressBackend{
ServiceName: "default-backend",
ServicePort: intstr.FromInt(80),
}

return &extensions.Ingress{
ObjectMeta: meta_v1.ObjectMeta{
Name: "foo",
Namespace: api.NamespaceDefault,
},
Spec: extensions.IngressSpec{
Backend: &extensions.IngressBackend{
ServiceName: "default-backend",
ServicePort: intstr.FromInt(80),
},
Rules: []extensions.IngressRule{
{
Host: "foo.bar.com",
IngressRuleValue: extensions.IngressRuleValue{
HTTP: &extensions.HTTPIngressRuleValue{
Paths: []extensions.HTTPIngressPath{
{
Path: "/foo",
Backend: defaultBackend,
},
},
},
},
},
},
},
}
}

func TestAnnotation(t *testing.T) {
ing := buildIngress()

data := map[string]string{}
data[parser.GetAnnotationWithPrefix("auth-url")] = "http://foo.com/external-auth"
data[parser.GetAnnotationWithPrefix("enable-global-auth")] = "false"
ing.SetAnnotations(data)

i, _ := NewParser(&resolver.Mock{}).Parse(ing)
u, ok := i.(bool)
if !ok {
t.Errorf("expected a Config type")
}
if u {
t.Errorf("Expected false but returned true")
}
}
Loading

0 comments on commit b06e114

Please sign in to comment.