Skip to content

Commit

Permalink
Consistently use constant-time comparison of password hashes
Browse files Browse the repository at this point in the history
As per golang/go#47001 even subtle.ConstantTimeCompare should never be used with variable-length inputs, as it will return 0 if the lengths do not match. Switch to consistently using constant-time comparisons of hashes for password checks to avoid any possible side-channel leaks that could be combined with other vectors to discover password lengths.

Signed-off-by: Brad Davidson <[email protected]>
  • Loading branch information
brandond committed May 9, 2023
1 parent 9ec1789 commit 239021e
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 20 deletions.
16 changes: 10 additions & 6 deletions pkg/authenticator/passwordfile/passwordfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ package passwordfile

import (
"context"
"crypto/subtle"
"encoding/csv"
"fmt"
"io"
"os"
"strings"

"github.com/k3s-io/k3s/pkg/nodepassword"
"k8s.io/klog/v2"

"k8s.io/apiserver/pkg/authentication/authenticator"
Expand All @@ -37,8 +37,8 @@ type PasswordAuthenticator struct {
}

type userPasswordInfo struct {
info *user.DefaultInfo
password string
info *user.DefaultInfo
hash string
}

// NewCSV returns a PasswordAuthenticator, populated from a CSV file.
Expand All @@ -65,9 +65,13 @@ func NewCSV(path string) (*PasswordAuthenticator, error) {
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))
}
hash, err := nodepassword.Hasher.CreateHash(record[0])
if err != nil {
return nil, fmt.Errorf("failed to hash password for username '%s' in password file '%s': %v", record[1], path, err)
}
obj := &userPasswordInfo{
info: &user.DefaultInfo{Name: record[1], UID: record[2]},
password: record[0],
info: &user.DefaultInfo{Name: record[1], UID: record[2]},
hash: hash,
}
if len(record) >= 4 {
obj.info.Groups = strings.Split(record[3], ",")
Expand All @@ -88,7 +92,7 @@ func (a *PasswordAuthenticator) AuthenticatePassword(ctx context.Context, userna
if !ok {
return nil, false, nil
}
if subtle.ConstantTimeCompare([]byte(user.password), []byte(password)) == 0 {
if err := nodepassword.Hasher.VerifyHash(user.hash, password); err != nil {
return nil, false, nil
}
return &authenticator.Response{User: user.info}, true, nil
Expand Down
8 changes: 4 additions & 4 deletions pkg/nodepassword/nodepassword.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import (
)

var (
// hasher provides the algorithm for generating and verifying hashes
hasher = hash.NewSCrypt()
// Hasher provides the algorithm for generating and verifying hashes
Hasher = hash.NewSCrypt()
)

func getSecretName(nodeName string) string {
Expand All @@ -33,7 +33,7 @@ func verifyHash(secretClient coreclient.SecretClient, nodeName, pass string) err
return err
}
if hash, ok := secret.Data["hash"]; ok {
if err := hasher.VerifyHash(string(hash), pass); err != nil {
if err := Hasher.VerifyHash(string(hash), pass); err != nil {
return errors.Wrapf(err, "unable to verify hash for node '%s'", nodeName)
}
return nil
Expand All @@ -47,7 +47,7 @@ func Ensure(secretClient coreclient.SecretClient, nodeName, pass string) error {
return err
}

hash, err := hasher.CreateHash(pass)
hash, err := Hasher.CreateHash(pass)
if err != nil {
return errors.Wrapf(err, "unable to create hash for node '%s'", nodeName)
}
Expand Down
8 changes: 0 additions & 8 deletions pkg/passwd/passwd.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ func Read(file string) (*Passwd, error) {
return result, nil
}

func (p *Passwd) Check(name, pass string) (matches bool, exists bool) {
e, ok := p.names[name]
if !ok {
return false, false
}
return e.pass == pass, true
}

func (p *Passwd) EnsureUser(name, role, passwd string) error {
tokenPrefix := "::" + name + ":"
idx := strings.Index(passwd, tokenPrefix)
Expand Down
8 changes: 6 additions & 2 deletions pkg/server/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,12 @@ func verifyLocalPassword(ctx context.Context, config *Config, mu *sync.Mutex, de
return "", http.StatusInternalServerError, errors.Wrap(err, "unable to read node password file")
}

password := strings.TrimSpace(string(passBytes))
if password != node.Password {
passHash, err := nodepassword.Hasher.CreateHash(strings.TrimSpace(string(passBytes)))
if err != nil {
return "", http.StatusInternalServerError, errors.Wrap(err, "unable to hash node password file")
}

if err := nodepassword.Hasher.VerifyHash(passHash, node.Password); err != nil {
return "", http.StatusForbidden, errors.Wrapf(err, "unable to verify local password for node '%s'", node.Name)
}

Expand Down

0 comments on commit 239021e

Please sign in to comment.