Skip to content

Commit

Permalink
feat: Always encrypt passwords rather than sending them plaintext
Browse files Browse the repository at this point in the history
This also avoids them showing up in error messages from pgperms.
  • Loading branch information
Jille committed Jan 24, 2023
1 parent cc76e83 commit 4e2cad3
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 13 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ sequence_privileges:
We'll happily accept your contributions! There's still a lot of things not supported:

- Permissions on columns, domains, foreign data wrappers, foreign servers, routines, languages, large objects, tablespaces or types.
- If plain-text passwords are stored in the config file, they're sent to PostgreSQL in plain-text as well. It would be nicer to encrypt passwords locally.
- Set up default privileges so that newly created tables already have the correct permissions without having to run pgperms?
- Allow "ALL PRIVILEGES" to be used in the config file.
- A config setting to automatically manage all users (and thus delete any unlisted users without needing to tombstone them).
Expand Down
4 changes: 4 additions & 0 deletions dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package pgperms
import (
"bytes"
"context"
"fmt"

"github.com/Jille/dfr"
"github.com/samber/lo"
Expand Down Expand Up @@ -104,6 +105,9 @@ func Sync(ctx context.Context, conns *Connections, desired []byte, ss SyncSink)
if err != nil {
return err
}
if err := encryptPasswordsInConfig(ctx, conns.primary, d.Roles); err != nil {
return fmt.Errorf("failed to encrypt plain-text passwords in the config: %v", err)
}

SyncDatabases(ss, d.Databases, d.TombstonedDatabases, actual.Databases)
ss.AddBarrier()
Expand Down
89 changes: 81 additions & 8 deletions passwords.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,35 @@ package pgperms

import (
"bytes"
"context"
"crypto/hmac"
"crypto/md5"
"crypto/rand"
"crypto/sha256"
"encoding/base64"
"encoding/hex"
"fmt"
"io"
"regexp"
"strconv"
"strings"

"github.com/jackc/pgx/v4"
"github.com/xdg-go/pbkdf2"
"github.com/xdg-go/scram"
)

// TODO: Use `SELECT setting FROM pg_settings WHERE name='password_encryption'` and only send encrypted passwords.

var scramRe = regexp.MustCompile(`^SCRAM-(SHA-1|SHA-256|SHA-512)\$(\d+):([^$]+)\$([^:]+):(.+)$`)
var (
md5Re = regexp.MustCompile(`^md5[0-9a-f]{32}$`)
scramRe = regexp.MustCompile(`^SCRAM-(SHA-1|SHA-256|SHA-512)\$(\d+):([^$]+)\$([^:]+):(.+)$`)
)

// verifyPassword returns whether the hashed password belongs to the given user and password.
func verifyPassword(hashed, username, plain string) bool {
if hashed == plain {
return true
}
if strings.HasPrefix(hashed, "md5") {
b := md5.Sum([]byte(plain + username))
h := hex.EncodeToString(b[:])
return h == hashed[3:]
if md5Re.MatchString(hashed) {
return hashed == MD5Password(username, plain)
}
if m := scramRe.FindStringSubmatch(hashed); m != nil {
hgf := getScramHash(m[1])
Expand Down Expand Up @@ -127,3 +131,72 @@ func getSum(hgf scram.HashGeneratorFcn, key []byte) []byte {
_, _ = h.Write(key)
return h.Sum(nil)
}

type PasswordHasher func(username, password string) (string, error)

func SelectPasswordHasher(ctx context.Context, conn *pgx.Conn) (PasswordHasher, error) {
var method string
if err := conn.QueryRow(ctx, "SELECT setting FROM pg_settings WHERE name='password_encryption'").Scan(&method); err != nil {
return nil, err
}
switch method {
case "scram-sha-256":
return func(username, password string) (string, error) {
return ScramSha256Password(password)
}, nil
case "md5":
return func(username, password string) (string, error) {
return MD5Password(username, password), nil
}, nil
default:
return nil, fmt.Errorf("unknown password_encryption %q. File a feature request at https://github.com/SnoozeThis-org/pgperms/issues or don't use plaintext passwords in the config file", method)
}
}

func MD5Password(username, password string) string {
b := md5.Sum([]byte(password + username))
return "md5" + hex.EncodeToString(b[:])
}

func ScramSha256Password(password string) (string, error) {
const iterationCnt = 4096
const keyLen = 32
salt := make([]byte, 16)
if _, err := io.ReadFull(rand.Reader, salt); err != nil {
return "", err
}
digestKey := pbkdf2.Key([]byte(password), salt, iterationCnt, keyLen, sha256.New)
clientKey := getHMACSum(scram.SHA256, digestKey, []byte("Client Key"))
storedKey := getSum(scram.SHA256, clientKey)
serverKey := getHMACSum(scram.SHA256, digestKey, []byte("Server Key"))

return fmt.Sprintf("SCRAM-SHA-256$%d:%s$%s:%s",
iterationCnt,
base64.StdEncoding.EncodeToString(salt),
base64.StdEncoding.EncodeToString(storedKey),
base64.StdEncoding.EncodeToString(serverKey),
), nil
}

func encryptPasswordsInConfig(ctx context.Context, conn *pgx.Conn, roles map[string]RoleAttributes) error {
var hasher PasswordHasher
for r, ra := range roles {
if ra.Password == nil || md5Re.MatchString(*ra.Password) || scramRe.MatchString(*ra.Password) {
continue
}
if hasher == nil {
var err error
hasher, err = SelectPasswordHasher(ctx, conn)
if err != nil {
return err
}
}
hash, err := hasher(r, *ra.Password)
if err != nil {
return err
}
ra.hashedPassword = hash
roles[r] = ra
}
return nil
}
16 changes: 16 additions & 0 deletions passwords_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ func TestPasswords(t *testing.T) {
hashed: "md5abc",
want: true,
},
{
username: "someuser",
plain: "somepassword",
hashed: "md5036f87626dc9bdf7b4b353ecca2556d0",
want: true,
},
{
username: "quis",
plain: "SCRAM-SHA-256$4096:R1uviLmvs+9Ap6DAS1WOnQ==$mxR4jEPmRr3wePTVxZYB98KyS+mfZ9Jv0AMXbTDBTmk=:SXj6NmnPJFTuN5HLoGowDacCwKj4XmemeQYXEcsPye4=",
Expand Down Expand Up @@ -62,3 +68,13 @@ func TestPasswords(t *testing.T) {
})
}
}

func TestScramEncryption(t *testing.T) {
hash, err := ScramSha256Password("hackme")
if err != nil {
t.Fatalf("Failed to encrypt password: %v", err)
}
if !verifyPassword(hash, "username", "hackme") {
t.Fatalf("Encrypted password didn't verify: %v", err)
}
}
13 changes: 11 additions & 2 deletions roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ type RoleAttributes struct {
Password *string `yaml:"password,omitempty"`
ValidUntil *time.Time `yaml:"validuntil,omitempty"`
MemberOf []string `yaml:"member_of,omitempty"`

// hashedPassword is precalculated (to fail early on) before syncing roles. If Password is already hashed, this'll be empty.
hashedPassword string
}

func (r RoleAttributes) GetInherit() bool {
Expand Down Expand Up @@ -77,7 +80,10 @@ func (r RoleAttributes) CreateSQL(username string) string {
q += fmt.Sprintf(" CONNECTION LIMIT %d", *r.ConnectionLimit)
}
if r.Password != nil && *r.Password != "" {
q += " PASSWORD " + Escape(*r.Password)
if r.hashedPassword == "" {
r.hashedPassword = *r.Password
}
q += " PASSWORD " + Escape(r.hashedPassword)
}
if r.ValidUntil != nil {
q += " VALID UNTIL " + Escape(r.ValidUntil.Format("2006-01-02T15:04:05Z"))
Expand Down Expand Up @@ -144,7 +150,10 @@ func alterRole(ss SyncSink, username string, o, n RoleAttributes) {
}
} else {
if !verifyPassword(*o.Password, username, *n.Password) {
q += " PASSWORD " + Escape(*n.Password)
if n.hashedPassword == "" {
n.hashedPassword = *n.Password
}
q += " PASSWORD " + Escape(n.hashedPassword)
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion testdata/alter-users.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
preparation:
- SET password_encryption TO 'md5'
- CREATE ROLE orphan LOGIN
- CREATE ROLE superman CREATEDB LOGIN
- CREATE ROLE replication LOGIN REPLICATION
Expand All @@ -24,5 +25,5 @@ config:
expected:
- "/* */ ALTER ROLE orphan NOINHERIT"
- "/* */ ALTER ROLE replication CONNECTION LIMIT 5"
- "/* */ ALTER ROLE someuser PASSWORD 'hackme' VALID UNTIL '2000-01-01T13:00:00Z' LOGIN"
- "/* */ ALTER ROLE someuser PASSWORD 'md50157c149a37c410a5595f1cba1b0ab9c' VALID UNTIL '2000-01-01T13:00:00Z' LOGIN"
- "/* */ ALTER ROLE superman SUPERUSER CREATEROLE BYPASSRLS"
4 changes: 3 additions & 1 deletion testdata/create-users.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
preparation:
- SET password_encryption TO 'md5'
config:
roles:
someuser:
Expand All @@ -19,5 +21,5 @@ expected:
- "/* */ CREATE ROLE orphan NOINHERIT LOGIN"
- "/* */ CREATE ROLE replication LOGIN REPLICATION CONNECTION LIMIT 5"
- "/* */ CREATE ROLE somerole"
- "/* */ CREATE ROLE someuser LOGIN PASSWORD 'hackme' VALID UNTIL '2000-01-01T13:00:00Z'"
- "/* */ CREATE ROLE someuser LOGIN PASSWORD 'md50157c149a37c410a5595f1cba1b0ab9c' VALID UNTIL '2000-01-01T13:00:00Z'"
- "/* */ CREATE ROLE superman SUPERUSER CREATEDB CREATEROLE LOGIN BYPASSRLS"

0 comments on commit 4e2cad3

Please sign in to comment.