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 front proxy in front of API #12803

Merged
merged 6 commits into from
Mar 3, 2017
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
19 changes: 19 additions & 0 deletions pkg/cmd/server/api/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ func GetMasterFileReferences(config *MasterConfig) []*string {
refs = append(refs, &config.KubernetesMasterConfig.ProxyClientInfo.KeyFile)
}

if config.AuthConfig.RequestHeader != nil {
refs = append(refs, &config.AuthConfig.RequestHeader.ClientCA)
}

refs = append(refs, &config.ServiceAccountConfig.MasterCA)
refs = append(refs, &config.ServiceAccountConfig.PrivateKeyFile)
for i := range config.ServiceAccountConfig.PublicKeyFiles {
Expand Down Expand Up @@ -457,6 +461,21 @@ func GetOAuthClientCertCAs(options MasterConfig) ([]*x509.Certificate, error) {
return allCerts, nil
}

func GetRequestHeaderClientCertCAs(options MasterConfig) ([]*x509.Certificate, error) {
if !UseTLS(options.ServingInfo.ServingInfo) {
return nil, nil
}
if options.AuthConfig.RequestHeader == nil {
return nil, nil
}

certs, err := cmdutil.CertificatesFromFile(options.AuthConfig.RequestHeader.ClientCA)
if err != nil {
return nil, fmt.Errorf("Error reading %s: %s", options.AuthConfig.RequestHeader.ClientCA, err)
}
return certs, nil
}

func getAPIClientCertCAs(options MasterConfig) ([]*x509.Certificate, error) {
if !UseTLS(options.ServingInfo.ServingInfo) {
return nil, nil
Expand Down
27 changes: 27 additions & 0 deletions pkg/cmd/server/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ type MasterConfig struct {
// ServingInfo describes how to start serving
ServingInfo HTTPServingInfo

// AuthConfig configures authentication options in addition to the standard
// oauth token and client certificate authenticators
AuthConfig MasterAuthConfig

// CORSAllowedOrigins
CORSAllowedOrigins []string

Expand Down Expand Up @@ -342,6 +346,29 @@ type MasterConfig struct {
AuditConfig AuditConfig
}

// MasterAuthConfig configures authentication options in addition to the standard
// oauth token and client certificate authenticators
type MasterAuthConfig struct {
// RequestHeader holds options for setting up a front proxy against the the API. It is optional.
RequestHeader *RequestHeaderAuthenticationOptions
}

// RequestHeaderAuthenticationOptions provides options for setting up a front proxy against the entire
// API instead of against the /oauth endpoint.
type RequestHeaderAuthenticationOptions struct {
// ClientCA is a file with the trusted signer certs. It is required.
ClientCA string
// ClientCommonNames is a required list of common names to require a match from.
ClientCommonNames []string

// UsernameHeaders is the list of headers to check for user information. First hit wins.
UsernameHeaders []string
// GroupNameHeader is the set of headers to check for group information. All are unioned.
GroupHeaders []string
// ExtraHeaderPrefixes is the set of request header prefixes to inspect for user extra. X-Remote-Extra- is suggested.
ExtraHeaderPrefixes []string
}

// AuditConfig holds configuration for the audit capabilities
type AuditConfig struct {
// If this flag is set, audit log will be printed in the logs.
Expand Down
23 changes: 23 additions & 0 deletions pkg/cmd/server/api/v1/swagger_doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,15 @@ func (LocalQuota) SwaggerDoc() map[string]string {
return map_LocalQuota
}

var map_MasterAuthConfig = map[string]string{
"": "MasterAuthConfig configures authentication options in addition to the standard oauth token and client certificate authenticators",
"requestHeader": "RequestHeader holds options for setting up a front proxy against the the API. It is optional.",
}

func (MasterAuthConfig) SwaggerDoc() map[string]string {
return map_MasterAuthConfig
}

var map_MasterClients = map[string]string{
"": "MasterClients holds references to `.kubeconfig` files that qualify master clients for OpenShift and Kubernetes",
"openshiftLoopbackKubeConfig": "OpenShiftLoopbackKubeConfig is a .kubeconfig filename for system components to loopback to this master",
Expand All @@ -446,6 +455,7 @@ func (MasterClients) SwaggerDoc() map[string]string {
var map_MasterConfig = map[string]string{
"": "MasterConfig holds the necessary configuration options for the OpenShift master",
"servingInfo": "ServingInfo describes how to start serving",
"authConfig": "AuthConfig configures authentication options in addition to the standard oauth token and client certificate authenticators",
"corsAllowedOrigins": "CORSAllowedOrigins",
"apiLevels": "APILevels is a list of API levels that should be enabled on startup: v1 as examples",
"masterPublicURL": "MasterPublicURL is how clients can access the OpenShift API server",
Expand Down Expand Up @@ -701,6 +711,19 @@ func (RemoteConnectionInfo) SwaggerDoc() map[string]string {
return map_RemoteConnectionInfo
}

var map_RequestHeaderAuthenticationOptions = map[string]string{
"": "RequestHeaderAuthenticationOptions provides options for setting up a front proxy against the entire API instead of against the /oauth endpoint.",
"clientCA": "ClientCA is a file with the trusted signer certs. It is required.",
"clientCommonNames": "ClientCommonNames is a required list of common names to require a match from.",
"usernameHeaders": "UsernameHeaders is the list of headers to check for user information. First hit wins.",
"groupHeaders": "GroupNameHeader is the set of headers to check for group information. All are unioned.",
"extraHeaderPrefixes": "ExtraHeaderPrefixes is the set of request header prefixes to inspect for user extra. X-Remote-Extra- is suggested.",
}

func (RequestHeaderAuthenticationOptions) SwaggerDoc() map[string]string {
return map_RequestHeaderAuthenticationOptions
}

var map_RequestHeaderIdentityProvider = map[string]string{
"": "RequestHeaderIdentityProvider provides identities for users authenticating using request header credentials",
"loginURL": "LoginURL is a URL to redirect unauthenticated /authorize requests to Unauthenticated requests from OAuth clients which expect interactive logins will be redirected here ${url} is replaced with the current URL, escaped to be safe in a query parameter\n https://www.example.com/sso-login?then=${url}\n${query} is replaced with the current query string\n https://www.example.com/auth-proxy/oauth/authorize?${query}",
Expand Down
27 changes: 27 additions & 0 deletions pkg/cmd/server/api/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ type MasterConfig struct {
// ServingInfo describes how to start serving
ServingInfo HTTPServingInfo `json:"servingInfo"`

// AuthConfig configures authentication options in addition to the standard
// oauth token and client certificate authenticators
AuthConfig MasterAuthConfig `json:"authConfig"`

// CORSAllowedOrigins
CORSAllowedOrigins []string `json:"corsAllowedOrigins"`

Expand Down Expand Up @@ -264,6 +268,29 @@ type MasterConfig struct {
AuditConfig AuditConfig `json:"auditConfig"`
}

// MasterAuthConfig configures authentication options in addition to the standard
// oauth token and client certificate authenticators
type MasterAuthConfig struct {
// RequestHeader holds options for setting up a front proxy against the the API. It is optional.
RequestHeader *RequestHeaderAuthenticationOptions `json:"requestHeader"`
}

// RequestHeaderAuthenticationOptions provides options for setting up a front proxy against the entire
// API instead of against the /oauth endpoint.
type RequestHeaderAuthenticationOptions struct {
// ClientCA is a file with the trusted signer certs. It is required.
ClientCA string `json:"clientCA"`
// ClientCommonNames is a required list of common names to require a match from.
ClientCommonNames []string `json:"clientCommonNames"`

// UsernameHeaders is the list of headers to check for user information. First hit wins.
UsernameHeaders []string `json:"usernameHeaders"`
// GroupNameHeader is the set of headers to check for group information. All are unioned.
GroupHeaders []string `json:"groupHeaders"`
// ExtraHeaderPrefixes is the set of request header prefixes to inspect for user extra. X-Remote-Extra- is suggested.
ExtraHeaderPrefixes []string `json:"extraHeaderPrefixes"`
}

// AuditConfig holds configuration for the audit capabilities
type AuditConfig struct {
// If this flag is set, audit log will be printed in the logs.
Expand Down
2 changes: 2 additions & 0 deletions pkg/cmd/server/api/v1/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ auditConfig:
maximumFileRetentionDays: 0
maximumFileSizeMegabytes: 0
maximumRetainedFiles: 0
authConfig:
requestHeader: null
controllerConfig:
serviceServingCert:
signer: null
Expand Down
28 changes: 28 additions & 0 deletions pkg/cmd/server/api/validation/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,34 @@ func ValidateMasterConfig(config *api.MasterConfig, fldPath *field.Path) Validat

validationResults.Append(ValidateAuditConfig(config.AuditConfig, fldPath.Child("auditConfig")))

validationResults.Append(ValidateMasterAuthConfig(config.AuthConfig, fldPath.Child("authConfig")))

return validationResults
}

func ValidateMasterAuthConfig(config api.MasterAuthConfig, fldPath *field.Path) ValidationResults {
validationResults := ValidationResults{}

if config.RequestHeader == nil {
return validationResults
}

if len(config.RequestHeader.ClientCA) == 0 {
validationResults.AddErrors(field.Required(fldPath.Child("requestHeader.clientCA"), "must be specified for a secure connection"))
}
if len(config.RequestHeader.ClientCommonNames) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this supposed to be optional?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's net new, so we can require it if we want. I'd rather start by requiring it and relax later if we want

validationResults.AddErrors(field.Required(fldPath.Child("requestHeader.clientCommonNames"), "must be specified for a secure connection"))
}
if len(config.RequestHeader.UsernameHeaders) == 0 {
validationResults.AddErrors(field.Required(fldPath.Child("requestHeader.usernameHeaders"), "must be specified for a secure connection"))
}
if len(config.RequestHeader.GroupHeaders) == 0 {
validationResults.AddErrors(field.Required(fldPath.Child("requestHeader.groupHeaders"), "must be specified for a secure connection"))
}
if len(config.RequestHeader.ExtraHeaderPrefixes) == 0 {
validationResults.AddErrors(field.Required(fldPath.Child("requestHeader.extraHeaderPrefixes"), "must be specified for a secure connection"))
}

return validationResults
}

Expand Down
34 changes: 34 additions & 0 deletions pkg/cmd/server/kubernetes/master_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"crypto/tls"
"errors"
"fmt"
"io/ioutil"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -112,6 +113,7 @@ func BuildDefaultAPIServer(options configapi.MasterConfig) (*apiserveroptions.Se
server.GenericServerRunOptions.TLSCertFile = options.ServingInfo.ServerCert.CertFile
server.GenericServerRunOptions.TLSPrivateKeyFile = options.ServingInfo.ServerCert.KeyFile
server.GenericServerRunOptions.ClientCAFile = options.ServingInfo.ClientCA

server.GenericServerRunOptions.MaxRequestsInFlight = options.ServingInfo.MaxRequestsInFlight
server.GenericServerRunOptions.MinRequestTimeout = options.ServingInfo.RequestTimeoutSeconds
for _, nc := range options.ServingInfo.NamedCertificates {
Expand Down Expand Up @@ -305,6 +307,14 @@ func BuildKubernetesMasterConfig(options configapi.MasterConfig, requestContextM
for _, cert := range oAuthClientCertCAs {
genericConfig.SecureServingInfo.ClientCA.AddCert(cert)
}
requestHeaderCACerts, err := configapi.GetRequestHeaderClientCertCAs(options)
if err != nil {
glog.Fatalf("Error setting up request header client certificates: %v", err)
}
for _, cert := range requestHeaderCACerts {
genericConfig.SecureServingInfo.ClientCA.AddCert(cert)
}

url, err := url.Parse(options.MasterPublicURL)
if err != nil {
glog.Fatalf("Error parsing master public url %q: %v", options.MasterPublicURL, err)
Expand Down Expand Up @@ -545,3 +555,27 @@ func getAPIResourceConfig(options configapi.MasterConfig) genericapiserver.APIRe

return resourceConfig
}

// TODO remove this func in 1.6 when we get rid of the hack above
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deads2k you forgot to remove concatenateFiles...

func concatenateFiles(prefix, separator string, files ...string) (string, error) {
data := []byte{}
for _, file := range files {
fileBytes, err := ioutil.ReadFile(file)
if err != nil {
return "", err
}
data = append(data, fileBytes...)
data = append(data, []byte(separator)...)
}
tmpFile, err := ioutil.TempFile("", prefix)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

permissions on this file are important, given what write access to it allows

if err != nil {
return "", err
}
if _, err := tmpFile.Write(data); err != nil {
return "", err
}
if err := tmpFile.Close(); err != nil {
return "", err
}
return tmpFile.Name(), nil
}
29 changes: 21 additions & 8 deletions pkg/cmd/server/origin/master_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"k8s.io/kubernetes/plugin/pkg/admission/namespace/lifecycle"
saadmit "k8s.io/kubernetes/plugin/pkg/admission/serviceaccount"
storageclassdefaultadmission "k8s.io/kubernetes/plugin/pkg/admission/storageclass/default"
"k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/headerrequest"

"github.com/openshift/origin/pkg/auth/authenticator"
"github.com/openshift/origin/pkg/auth/authenticator/anonymous"
Expand Down Expand Up @@ -664,16 +665,28 @@ func newAuthenticator(config configapi.MasterConfig, restOptionsGetter restoptio
authenticators = append(authenticators, certauth)
}

ret := &unionrequest.Authenticator{
FailOnError: true,
Handlers: []authenticator.Request{
// if you change this, have a look at the impersonationFilter where we attach groups to the impersonated user
group.NewGroupAdder(&unionrequest.Authenticator{FailOnError: true, Handlers: authenticators}, []string{bootstrappolicy.AuthenticatedGroup}),
anonymous.NewAuthenticator(),
},
topLevelAuthenticators := []authenticator.Request{}
// if we have a front proxy providing authentication configuration, wire it up and it should come first
if config.AuthConfig.RequestHeader != nil {
requestHeaderAuthenticator, err := headerrequest.NewSecure(
config.AuthConfig.RequestHeader.ClientCA,
config.AuthConfig.RequestHeader.ClientCommonNames,
config.AuthConfig.RequestHeader.UsernameHeaders,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only gets defaulted if you round trip the config right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only gets defaulted if you round trip the config right?

We do, don't we?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think we do so in integration tests (which is why I had to specify it directly).

config.AuthConfig.RequestHeader.GroupHeaders,
config.AuthConfig.RequestHeader.ExtraHeaderPrefixes,
)
if err != nil {
return nil, fmt.Errorf("Error building front proxy auth config: %v", err)
}
topLevelAuthenticators = append(topLevelAuthenticators, requestHeaderAuthenticator)
}
topLevelAuthenticators = append(topLevelAuthenticators, group.NewGroupAdder(&unionrequest.Authenticator{FailOnError: true, Handlers: authenticators}, []string{bootstrappolicy.AuthenticatedGroup}))
topLevelAuthenticators = append(topLevelAuthenticators, anonymous.NewAuthenticator())

return ret, nil
return &unionrequest.Authenticator{
FailOnError: true,
Handlers: topLevelAuthenticators,
}, nil
}

func newProjectAuthorizationCache(authorizer authorizer.Authorizer, kubeClient *kclientset.Clientset, informerFactory shared.InformerFactory) *projectauth.AuthorizationCache {
Expand Down
10 changes: 3 additions & 7 deletions test/integration/auth_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
configapi "github.com/openshift/origin/pkg/cmd/server/api"
"github.com/openshift/origin/pkg/cmd/server/origin"
oauthapi "github.com/openshift/origin/pkg/oauth/api"
clientregistry "github.com/openshift/origin/pkg/oauth/registry/oauthclient"
testutil "github.com/openshift/origin/test/util"
testserver "github.com/openshift/origin/test/util/server"
)
Expand Down Expand Up @@ -90,6 +89,9 @@ func TestAuthProxyOnAuthorize(t *testing.T) {

// make our authorize request again, but this time our transport has properly set the auth info for the front proxy
req, err := http.NewRequest("GET", rawAuthorizeRequest, nil)
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
_, err = httpClient.Do(req)
if err != nil {
t.Errorf("Unexpected error: %v", err)
Expand All @@ -108,12 +110,6 @@ func TestAuthProxyOnAuthorize(t *testing.T) {
}
}

func createClient(t *testing.T, clientRegistry clientregistry.Registry, client *oauthapi.OAuthClient) {
if _, err := clientRegistry.CreateClient(kapi.NewContext(), client); err != nil {
t.Errorf("Error creating client: %v due to %v\n", client, err)
}
}

type checkRedirect func(req *http.Request, via []*http.Request) error

func getRedirectMethod(t *testing.T, redirectRecord *[]url.URL) checkRedirect {
Expand Down
Loading