From 7fa523535d6b77302a47709fd6e37fc415aba813 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 11 Mar 2020 14:31:31 -0400 Subject: [PATCH] Remove support for basic authentication This change removes support for basic authn in v1.19 via the --basic-auth-file flag. This functionality was deprecated in v1.16 in response to ATR-K8S-002: Non-constant time password comparison. Similar functionality is available via the --token-auth-file flag for development purposes. Signed-off-by: Monis Khan Kubernetes-commit: df292749c9d063b06861d0f4f1741c37b815a2fa --- .../authenticator/interfaces.go | 15 -- pkg/endpoints/filters/authentication.go | 5 +- pkg/server/config.go | 6 +- pkg/server/options/authentication.go | 1 - plugin/pkg/authenticator/password/doc.go | 18 -- .../password/passwordfile/passwordfile.go | 95 ----------- .../passwordfile/passwordfile_test.go | 161 ------------------ .../request/basicauth/basicauth.go | 53 ------ .../request/basicauth/basicauth_test.go | 126 -------------- 9 files changed, 2 insertions(+), 478 deletions(-) delete mode 100644 plugin/pkg/authenticator/password/doc.go delete mode 100644 plugin/pkg/authenticator/password/passwordfile/passwordfile.go delete mode 100644 plugin/pkg/authenticator/password/passwordfile/passwordfile_test.go delete mode 100644 plugin/pkg/authenticator/request/basicauth/basicauth.go delete mode 100644 plugin/pkg/authenticator/request/basicauth/basicauth_test.go diff --git a/pkg/authentication/authenticator/interfaces.go b/pkg/authentication/authenticator/interfaces.go index e3b1b622c..8ff979b80 100644 --- a/pkg/authentication/authenticator/interfaces.go +++ b/pkg/authentication/authenticator/interfaces.go @@ -35,13 +35,6 @@ type Request interface { AuthenticateRequest(req *http.Request) (*Response, bool, error) } -// Password checks a username and password against a backing authentication -// store and returns a Response or an error if the password could not be -// checked. -type Password interface { - AuthenticatePassword(ctx context.Context, user, password string) (*Response, bool, error) -} - // TokenFunc is a function that implements the Token interface. type TokenFunc func(ctx context.Context, token string) (*Response, bool, error) @@ -58,14 +51,6 @@ func (f RequestFunc) AuthenticateRequest(req *http.Request) (*Response, bool, er return f(req) } -// PasswordFunc is a function that implements the Password interface. -type PasswordFunc func(ctx context.Context, user, password string) (*Response, bool, error) - -// AuthenticatePassword implements authenticator.Password. -func (f PasswordFunc) AuthenticatePassword(ctx context.Context, user, password string) (*Response, bool, error) { - return f(ctx, user, password) -} - // Response is the struct returned by authenticator interfaces upon successful // authentication. It contains information about whether the authenticator // authenticated the request, information about the context of the diff --git a/pkg/endpoints/filters/authentication.go b/pkg/endpoints/filters/authentication.go index 58430b7b3..2806edc79 100644 --- a/pkg/endpoints/filters/authentication.go +++ b/pkg/endpoints/filters/authentication.go @@ -71,11 +71,8 @@ func WithAuthentication(handler http.Handler, auth authenticator.Request, failed }) } -func Unauthorized(s runtime.NegotiatedSerializer, supportsBasicAuth bool) http.Handler { +func Unauthorized(s runtime.NegotiatedSerializer) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - if supportsBasicAuth { - w.Header().Set("WWW-Authenticate", `Basic realm="kubernetes-master"`) - } ctx := req.Context() requestInfo, found := genericapirequest.RequestInfoFrom(ctx) if !found { diff --git a/pkg/server/config.go b/pkg/server/config.go index db5368bf0..1c1b3934b 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -272,10 +272,6 @@ type AuthenticationInfo struct { APIAudiences authenticator.Audiences // Authenticator determines which subject is making the request Authenticator authenticator.Request - // SupportsBasicAuth indicates that's at least one Authenticator supports basic auth - // If this is true, a basic auth challenge is returned on authentication failure - // TODO(roberthbailey): Remove once the server no longer supports http basic auth. - SupportsBasicAuth bool } type AuthorizationInfo struct { @@ -670,7 +666,7 @@ func DefaultBuildHandlerChain(apiHandler http.Handler, c *Config) http.Handler { } handler = genericapifilters.WithImpersonation(handler, c.Authorization.Authorizer, c.Serializer) handler = genericapifilters.WithAudit(handler, c.AuditBackend, c.AuditPolicyChecker, c.LongRunningFunc) - failedHandler := genericapifilters.Unauthorized(c.Serializer, c.Authentication.SupportsBasicAuth) + failedHandler := genericapifilters.Unauthorized(c.Serializer) failedHandler = genericapifilters.WithFailedAuthenticationAudit(failedHandler, c.AuditBackend, c.AuditPolicyChecker) handler = genericapifilters.WithAuthentication(handler, c.Authentication.Authenticator, failedHandler, c.Authentication.APIAudiences) handler = genericfilters.WithCORS(handler, c.CorsAllowedOriginList, nil, nil, nil, "true") diff --git a/pkg/server/options/authentication.go b/pkg/server/options/authentication.go index 9a395c94a..1df0fccf6 100644 --- a/pkg/server/options/authentication.go +++ b/pkg/server/options/authentication.go @@ -319,7 +319,6 @@ func (s *DelegatingAuthenticationOptions) ApplyTo(authenticationInfo *server.Aut if openAPIConfig != nil { openAPIConfig.SecurityDefinitions = securityDefinitions } - authenticationInfo.SupportsBasicAuth = false return nil } diff --git a/plugin/pkg/authenticator/password/doc.go b/plugin/pkg/authenticator/password/doc.go deleted file mode 100644 index 0f917cc36..000000000 --- a/plugin/pkg/authenticator/password/doc.go +++ /dev/null @@ -1,18 +0,0 @@ -/* -Copyright 2014 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 password contains authenticator.Password implementations -package password // import "k8s.io/apiserver/plugin/pkg/authenticator/password" diff --git a/plugin/pkg/authenticator/password/passwordfile/passwordfile.go b/plugin/pkg/authenticator/password/passwordfile/passwordfile.go deleted file mode 100644 index e17a5eb0d..000000000 --- a/plugin/pkg/authenticator/password/passwordfile/passwordfile.go +++ /dev/null @@ -1,95 +0,0 @@ -/* -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 passwordfile - -import ( - "context" - "crypto/subtle" - "encoding/csv" - "fmt" - "io" - "os" - "strings" - - "k8s.io/klog" - - "k8s.io/apiserver/pkg/authentication/authenticator" - "k8s.io/apiserver/pkg/authentication/user" -) - -// PasswordAuthenticator authenticates users by password -type PasswordAuthenticator struct { - users map[string]*userPasswordInfo -} - -type userPasswordInfo struct { - info *user.DefaultInfo - password string -} - -// NewCSV returns a PasswordAuthenticator, populated from a CSV file. -// The CSV file must contain records in the format "password,username,useruid" -func NewCSV(path string) (*PasswordAuthenticator, error) { - file, err := os.Open(path) - if err != nil { - return nil, err - } - defer file.Close() - - recordNum := 0 - users := make(map[string]*userPasswordInfo) - reader := csv.NewReader(file) - reader.FieldsPerRecord = -1 - for { - record, err := reader.Read() - if err == io.EOF { - break - } - if err != nil { - return nil, err - } - if len(record) < 3 { - return nil, fmt.Errorf("password file '%s' must have at least 3 columns (password, user name, user uid), found %d", path, len(record)) - } - obj := &userPasswordInfo{ - info: &user.DefaultInfo{Name: record[1], UID: record[2]}, - password: record[0], - } - if len(record) >= 4 { - obj.info.Groups = strings.Split(record[3], ",") - } - recordNum++ - if _, exist := users[obj.info.Name]; exist { - klog.Warningf("duplicate username '%s' has been found in password file '%s', record number '%d'", obj.info.Name, path, recordNum) - } - users[obj.info.Name] = obj - } - - return &PasswordAuthenticator{users}, nil -} - -// AuthenticatePassword returns user info if authentication is successful, nil otherwise -func (a *PasswordAuthenticator) AuthenticatePassword(ctx context.Context, username, password string) (*authenticator.Response, bool, error) { - user, ok := a.users[username] - if !ok { - return nil, false, nil - } - if subtle.ConstantTimeCompare([]byte(user.password), []byte(password)) == 0 { - return nil, false, nil - } - return &authenticator.Response{User: user.info}, true, nil -} diff --git a/plugin/pkg/authenticator/password/passwordfile/passwordfile_test.go b/plugin/pkg/authenticator/password/passwordfile/passwordfile_test.go deleted file mode 100644 index e9f43d6dc..000000000 --- a/plugin/pkg/authenticator/password/passwordfile/passwordfile_test.go +++ /dev/null @@ -1,161 +0,0 @@ -/* -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 passwordfile - -import ( - "context" - "io/ioutil" - "os" - "reflect" - "testing" - - "k8s.io/apiserver/pkg/authentication/user" -) - -func TestPasswordFile(t *testing.T) { - auth, err := newWithContents(t, ` -password1,user1,uid1 -password2,user2,uid2 -password3,user3,uid3,"group1,group2" -password4,user4,uid4,"group2" -password5,user5,uid5,group5 -password6,user6,uid6,group5,otherdata -password7,user7,uid7,"group1,group2",otherdata -`) - if err != nil { - t.Fatalf("unable to read passwordfile: %v", err) - } - - testCases := []struct { - Username string - Password string - User *user.DefaultInfo - Ok bool - Err bool - }{ - { - Username: "user1", - Password: "password1", - User: &user.DefaultInfo{Name: "user1", UID: "uid1"}, - Ok: true, - }, - { - Username: "user2", - Password: "password2", - User: &user.DefaultInfo{Name: "user2", UID: "uid2"}, - Ok: true, - }, - { - Username: "user1", - Password: "password2", - }, - { - Username: "user2", - Password: "password1", - }, - { - Username: "user3", - Password: "password3", - User: &user.DefaultInfo{Name: "user3", UID: "uid3", Groups: []string{"group1", "group2"}}, - Ok: true, - }, - { - Username: "user4", - Password: "password4", - User: &user.DefaultInfo{Name: "user4", UID: "uid4", Groups: []string{"group2"}}, - Ok: true, - }, - { - Username: "user5", - Password: "password5", - User: &user.DefaultInfo{Name: "user5", UID: "uid5", Groups: []string{"group5"}}, - Ok: true, - }, - { - Username: "user6", - Password: "password6", - User: &user.DefaultInfo{Name: "user6", UID: "uid6", Groups: []string{"group5"}}, - Ok: true, - }, - { - Username: "user7", - Password: "password7", - User: &user.DefaultInfo{Name: "user7", UID: "uid7", Groups: []string{"group1", "group2"}}, - Ok: true, - }, - { - Username: "user7", - Password: "passwordbad", - }, - { - Username: "userbad", - Password: "password7", - }, - { - Username: "user8", - Password: "password8", - }, - } - for i, testCase := range testCases { - resp, ok, err := auth.AuthenticatePassword(context.Background(), testCase.Username, testCase.Password) - if err != nil { - t.Errorf("%d: unexpected error: %v", i, err) - } - if testCase.User == nil { - if resp != nil { - t.Errorf("%d: unexpected non-nil user %#v", i, resp.User) - } - } else if !reflect.DeepEqual(testCase.User, resp.User) { - t.Errorf("%d: expected user %#v, got %#v", i, testCase.User, resp.User) - } - if testCase.Ok != ok { - t.Errorf("%d: expected auth %v, got %v", i, testCase.Ok, ok) - } - } -} - -func TestBadPasswordFile(t *testing.T) { - if _, err := newWithContents(t, ` -password1,user1,uid1 -password2,user2,uid2 -password3,user3 -password4 -`); err == nil { - t.Fatalf("unexpected non error") - } -} - -func TestInsufficientColumnsPasswordFile(t *testing.T) { - if _, err := newWithContents(t, "password4\n"); err == nil { - t.Fatalf("unexpected non error") - } -} - -func newWithContents(t *testing.T, contents string) (auth *PasswordAuthenticator, err error) { - f, err := ioutil.TempFile("", "passwordfile_test") - if err != nil { - t.Fatalf("unexpected error creating passwordfile: %v", err) - } - f.Close() - defer os.Remove(f.Name()) - - if err := ioutil.WriteFile(f.Name(), []byte(contents), 0700); err != nil { - t.Fatalf("unexpected error writing passwordfile: %v", err) - } - - return NewCSV(f.Name()) -} diff --git a/plugin/pkg/authenticator/request/basicauth/basicauth.go b/plugin/pkg/authenticator/request/basicauth/basicauth.go deleted file mode 100644 index 7b8894a91..000000000 --- a/plugin/pkg/authenticator/request/basicauth/basicauth.go +++ /dev/null @@ -1,53 +0,0 @@ -/* -Copyright 2014 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 basicauth - -import ( - "errors" - "net/http" - - "k8s.io/apiserver/pkg/authentication/authenticator" -) - -// Authenticator authenticates requests using basic auth -type Authenticator struct { - auth authenticator.Password -} - -// New returns a request authenticator that validates credentials using the provided password authenticator -func New(auth authenticator.Password) *Authenticator { - return &Authenticator{auth} -} - -var errInvalidAuth = errors.New("invalid username/password combination") - -// AuthenticateRequest authenticates the request using the "Authorization: Basic" header in the request -func (a *Authenticator) AuthenticateRequest(req *http.Request) (*authenticator.Response, bool, error) { - username, password, found := req.BasicAuth() - if !found { - return nil, false, nil - } - - resp, ok, err := a.auth.AuthenticatePassword(req.Context(), username, password) - - // If the password authenticator didn't error, provide a default error - if !ok && err == nil { - err = errInvalidAuth - } - - return resp, ok, err -} diff --git a/plugin/pkg/authenticator/request/basicauth/basicauth_test.go b/plugin/pkg/authenticator/request/basicauth/basicauth_test.go deleted file mode 100644 index c23e0ce08..000000000 --- a/plugin/pkg/authenticator/request/basicauth/basicauth_test.go +++ /dev/null @@ -1,126 +0,0 @@ -/* -Copyright 2014 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 basicauth - -import ( - "context" - "errors" - "net/http" - "testing" - - "k8s.io/apiserver/pkg/authentication/authenticator" - "k8s.io/apiserver/pkg/authentication/user" -) - -type testPassword struct { - Username string - Password string - Called bool - - User user.Info - OK bool - Err error -} - -func (t *testPassword) AuthenticatePassword(ctx context.Context, user, password string) (*authenticator.Response, bool, error) { - t.Called = true - t.Username = user - t.Password = password - return &authenticator.Response{User: t.User}, t.OK, t.Err -} - -func TestBasicAuth(t *testing.T) { - testCases := map[string]struct { - Header string - Password testPassword - - ExpectedCalled bool - ExpectedUsername string - ExpectedPassword string - - ExpectedUser string - ExpectedOK bool - ExpectedErr bool - }{ - "no auth": {}, - "empty password basic header": { - ExpectedCalled: true, - ExpectedUsername: "user_with_empty_password", - ExpectedPassword: "", - ExpectedErr: true, - }, - "valid basic header": { - ExpectedCalled: true, - ExpectedUsername: "myuser", - ExpectedPassword: "mypassword:withcolon", - ExpectedErr: true, - }, - "password auth returned user": { - Password: testPassword{User: &user.DefaultInfo{Name: "returneduser"}, OK: true}, - ExpectedCalled: true, - ExpectedUsername: "myuser", - ExpectedPassword: "mypw", - ExpectedUser: "returneduser", - ExpectedOK: true, - }, - "password auth returned error": { - Password: testPassword{Err: errors.New("auth error")}, - ExpectedCalled: true, - ExpectedUsername: "myuser", - ExpectedPassword: "mypw", - ExpectedErr: true, - }, - } - - for k, testCase := range testCases { - password := testCase.Password - auth := authenticator.Request(New(&password)) - - req, _ := http.NewRequest("GET", "/", nil) - if testCase.ExpectedUsername != "" || testCase.ExpectedPassword != "" { - req.SetBasicAuth(testCase.ExpectedUsername, testCase.ExpectedPassword) - } - - resp, ok, err := auth.AuthenticateRequest(req) - - if testCase.ExpectedCalled != password.Called { - t.Errorf("%s: Expected called=%v, got %v", k, testCase.ExpectedCalled, password.Called) - continue - } - if testCase.ExpectedUsername != password.Username { - t.Errorf("%s: Expected called with username=%v, got %v", k, testCase.ExpectedUsername, password.Username) - continue - } - if testCase.ExpectedPassword != password.Password { - t.Errorf("%s: Expected called with password=%v, got %v", k, testCase.ExpectedPassword, password.Password) - continue - } - - if testCase.ExpectedErr != (err != nil) { - t.Errorf("%s: Expected err=%v, got err=%v", k, testCase.ExpectedErr, err) - continue - } - if testCase.ExpectedOK != ok { - t.Errorf("%s: Expected ok=%v, got ok=%v", k, testCase.ExpectedOK, ok) - continue - } - if testCase.ExpectedUser != "" && testCase.ExpectedUser != resp.User.GetName() { - t.Errorf("%s: Expected user.GetName()=%v, got %v", k, testCase.ExpectedUser, resp.User.GetName()) - continue - } - } -}