Skip to content

Commit

Permalink
Restore previous handling of long passwords
Browse files Browse the repository at this point in the history
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]: golang/go#36546
  • Loading branch information
silvestre committed Jan 17, 2023
1 parent 42dffe3 commit 5e630ad
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
14 changes: 14 additions & 0 deletions src/autoscaler/api/brokerserver/broker_server.go
Expand Up @@ -75,6 +75,7 @@ func NewBrokerServer(logger lager.Logger, conf *config.Config, bindingdb db.Bind
var middleWareBrokerCredentials []MiddleWareBrokerCredentials

for _, brokerCredential := range conf.BrokerCredentials {
brokerCredential = 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
Expand Down Expand Up @@ -153,6 +154,19 @@ func NewBrokerServer(logger lager.Logger, conf *config.Config, bindingdb db.Bind
return runner, nil
}

func restrictToMaxBcryptLength(logger lager.Logger, brokerCredential config.BrokerCredentialsConfig) 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]
}

return brokerCredential
}

func GetHealth(w http.ResponseWriter, _ *http.Request) {
handlers.WriteJSONResponse(w, http.StatusOK, []byte(`{"alive":"true"}`))
}
8 changes: 8 additions & 0 deletions src/autoscaler/healthendpoint/server.go
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions templates/app-autoscaler.yml
Expand Up @@ -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
Expand Down

0 comments on commit 5e630ad

Please sign in to comment.