From aabf9f652e4d0fa90b9aeb81e49f1f362a5a191b Mon Sep 17 00:00:00 2001 From: Silvestre Zabala Date: Tue, 17 Jan 2023 15:11:19 +0100 Subject: [PATCH] Restore previous handling of long passwords Configured passwords for the servicebroker and the healthendpoints were previously silently truncated to the first 72 bytes [1]. This change restores the truncation to the first 72 bytes, but adds an error level warning (as `lager` does not have a dedicated warning level), documenting this truncation, so previously configured passwords longer than 72 bytes will continue to work unchanged. Prior to this PR, passwords longer than 72 bytes would cause an error. [1]: https://github.com/golang/go/issues/36546 --- src/autoscaler/api/brokerserver/broker_server.go | 12 ++++++++++++ src/autoscaler/healthendpoint/server.go | 8 ++++++++ templates/app-autoscaler.yml | 4 ++-- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/autoscaler/api/brokerserver/broker_server.go b/src/autoscaler/api/brokerserver/broker_server.go index a2b0c62562..a34688992a 100644 --- a/src/autoscaler/api/brokerserver/broker_server.go +++ b/src/autoscaler/api/brokerserver/broker_server.go @@ -75,6 +75,7 @@ func NewBrokerServer(logger lager.Logger, conf *config.Config, bindingdb db.Bind var middleWareBrokerCredentials []MiddleWareBrokerCredentials for _, brokerCredential := range conf.BrokerCredentials { + restrictToMaxBcryptLength(logger, brokerCredential) if string(brokerCredential.BrokerUsernameHash) == "" { var err error brokerCredential.BrokerUsernameHash, err = bcrypt.GenerateFromPassword([]byte(brokerCredential.BrokerUsername), bcrypt.MinCost) // use MinCost as the config already provided it as cleartext @@ -153,6 +154,17 @@ func NewBrokerServer(logger lager.Logger, conf *config.Config, bindingdb db.Bind return runner, nil } +func restrictToMaxBcryptLength(logger lager.Logger, brokerCredential config.BrokerCredentialsConfig) { + if len(brokerCredential.BrokerUsername) > 72 { + logger.Error("warning-configured-username-too-long-using-only-first-72-characters", bcrypt.ErrPasswordTooLong, lager.Data{"username-length": len(brokerCredential.BrokerUsername)}) + brokerCredential.BrokerUsername = brokerCredential.BrokerUsername[:72] + } + if len(brokerCredential.BrokerPassword) > 72 { + logger.Error("warning-configured-password-too-long-using-only-first-72-characters", bcrypt.ErrPasswordTooLong, lager.Data{"password-length": len(brokerCredential.BrokerPassword)}) + brokerCredential.BrokerPassword = brokerCredential.BrokerPassword[:72] + } +} + func GetHealth(w http.ResponseWriter, _ *http.Request) { handlers.WriteJSONResponse(w, http.StatusOK, []byte(`{"alive":"true"}`)) } diff --git a/src/autoscaler/healthendpoint/server.go b/src/autoscaler/healthendpoint/server.go index a0df8a0108..0d3b57d428 100644 --- a/src/autoscaler/healthendpoint/server.go +++ b/src/autoscaler/healthendpoint/server.go @@ -135,6 +135,10 @@ func getPasswordHashBytes(logger lager.Logger, passwordHash string, password str var passwordHashByte []byte var err error if passwordHash == "" { + if len(password) > 72 { + logger.Error("warning-configured-password-too-long-using-only-first-72-characters", bcrypt.ErrPasswordTooLong, lager.Data{"password-length": len(password)}) + password = password[:72] + } passwordHashByte, err = bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) // use MinCost as the config already provided it as cleartext if err != nil { logger.Error("failed-new-server-password", err) @@ -150,6 +154,10 @@ func getUserHashBytes(logger lager.Logger, usernameHash string, username string) var usernameHashByte []byte var err error if usernameHash == "" { + if len(username) > 72 { + logger.Error("warning-configured-username-too-long-using-only-first-72-characters", bcrypt.ErrPasswordTooLong, lager.Data{"username-length": len(username)}) + username = username[:72] + } // when username and password are set for health check usernameHashByte, err = bcrypt.GenerateFromPassword([]byte(username), bcrypt.MinCost) // use MinCost as the config already provided it as cleartext if err != nil { diff --git a/templates/app-autoscaler.yml b/templates/app-autoscaler.yml index 55d2b70e1d..e09fbbeff3 100644 --- a/templates/app-autoscaler.yml +++ b/templates/app-autoscaler.yml @@ -706,11 +706,11 @@ variables: - name: service_broker_password type: password options: - length: 72 + length: 128 - name: service_broker_password_blue type: password options: - length: 72 + length: 128 - name: autoscaler_metricsforwarder_health_password type: password - name: autoscaler_metricsgateway_health_password