Skip to content

Commit

Permalink
Merge pull request #1081 from catj-cockroach/add-kubernetes-secret-su…
Browse files Browse the repository at this point in the history
…pport

adds support for kubernetes mounted private keys
  • Loading branch information
rafiss committed May 16, 2022
2 parents 54a3a4b + d8917fa commit 8c6de56
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 6 deletions.
6 changes: 4 additions & 2 deletions conn.go
Expand Up @@ -31,8 +31,10 @@ var (
ErrNotSupported = errors.New("pq: Unsupported command")
ErrInFailedTransaction = errors.New("pq: Could not complete operation in a failed transaction")
ErrSSLNotSupported = errors.New("pq: SSL is not enabled on the server")
ErrSSLKeyHasWorldPermissions = errors.New("pq: Private key file has group or world access. Permissions should be u=rw (0600) or less")
ErrCouldNotDetectUsername = errors.New("pq: Could not detect default username. Please provide one explicitly")
ErrSSLKeyUnknownOwnership = errors.New("pq: Could not get owner information for private key, may not be properly protected")
ErrSSLKeyHasWorldPermissions = errors.New("pq: Private key has world access. Permissions should be u=rw,g=r (0640) if owned by root, or u=rw (0600), or less")

ErrCouldNotDetectUsername = errors.New("pq: Could not detect default username. Please provide one explicitly")

errUnexpectedReady = errors.New("unexpected ReadyForQuery")
errNoRowsAffected = errors.New("no RowsAffected available after the empty statement")
Expand Down
80 changes: 76 additions & 4 deletions ssl_permissions.go
Expand Up @@ -3,7 +3,28 @@

package pq

import "os"
import (
"errors"
"os"
"syscall"
)

const (
rootUserID = uint32(0)

// The maximum permissions that a private key file owned by a regular user
// is allowed to have. This translates to u=rw.
maxUserOwnedKeyPermissions os.FileMode = 0600

// The maximum permissions that a private key file owned by root is allowed
// to have. This translates to u=rw,g=r.
maxRootOwnedKeyPermissions os.FileMode = 0640
)

var (
errSSLKeyHasUnacceptableUserPermissions = errors.New("permissions for files not owned by root should be u=rw (0600) or less")
errSSLKeyHasUnacceptableRootPermissions = errors.New("permissions for root owned files should be u=rw,g=r (0640) or less")
)

// sslKeyPermissions checks the permissions on user-supplied ssl key files.
// The key file should have very little access.
Expand All @@ -14,8 +35,59 @@ func sslKeyPermissions(sslkey string) error {
if err != nil {
return err
}
if info.Mode().Perm()&0077 != 0 {
return ErrSSLKeyHasWorldPermissions

err = hasCorrectPermissions(info)

// return ErrSSLKeyHasWorldPermissions for backwards compatability with
// existing code.
if err == errSSLKeyHasUnacceptableUserPermissions || err == errSSLKeyHasUnacceptableRootPermissions {
err = ErrSSLKeyHasWorldPermissions
}
return nil
return err
}

// hasCorrectPermissions checks the file info (and the unix-specific stat_t
// output) to verify that the permissions on the file are correct.
//
// If the file is owned by the same user the process is running as,
// the file should only have 0600 (u=rw). If the file is owned by root,
// and the group matches the group that the process is running in, the
// permissions cannot be more than 0640 (u=rw,g=r). The file should
// never have world permissions.
//
// Returns an error when the permission check fails.
func hasCorrectPermissions(info os.FileInfo) error {
// if file's permission matches 0600, allow access.
userPermissionMask := (os.FileMode(0777) ^ maxUserOwnedKeyPermissions)

// regardless of if we're running as root or not, 0600 is acceptable,
// so we return if we match the regular user permission mask.
if info.Mode().Perm()&userPermissionMask == 0 {
return nil
}

// We need to pull the Unix file information to get the file's owner.
// If we can't access it, there's some sort of operating system level error
// and we should fail rather than attempting to use faulty information.
sysInfo := info.Sys()
if sysInfo == nil {
return ErrSSLKeyUnknownOwnership
}

unixStat, ok := sysInfo.(*syscall.Stat_t)
if !ok {
return ErrSSLKeyUnknownOwnership
}

// if the file is owned by root, we allow 0640 (u=rw,g=r) to match what
// Postgres does.
if unixStat.Uid == rootUserID {
rootPermissionMask := (os.FileMode(0777) ^ maxRootOwnedKeyPermissions)
if info.Mode().Perm()&rootPermissionMask != 0 {
return errSSLKeyHasUnacceptableRootPermissions
}
return nil
}

return errSSLKeyHasUnacceptableUserPermissions
}
109 changes: 109 additions & 0 deletions ssl_permissions_test.go
@@ -0,0 +1,109 @@
//go:build !windows
// +build !windows

package pq

import (
"os"
"syscall"
"testing"
"time"
)

type stat_t_wrapper struct {
stat syscall.Stat_t
}

func (stat_t *stat_t_wrapper) Name() string {
return "pem.key"
}

func (stat_t *stat_t_wrapper) Size() int64 {
return int64(100)
}

func (stat_t *stat_t_wrapper) Mode() os.FileMode {
return os.FileMode(stat_t.stat.Mode)
}

func (stat_t *stat_t_wrapper) ModTime() time.Time {
return time.Now()
}

func (stat_t *stat_t_wrapper) IsDir() bool {
return true
}

func (stat_t *stat_t_wrapper) Sys() interface{} {
return &stat_t.stat
}

func TestHasCorrectRootGroupPermissions(t *testing.T) {
currentUID := uint32(os.Getuid())
currentGID := uint32(os.Getgid())

testData := []struct {
expectedError error
stat syscall.Stat_t
}{
{
expectedError: nil,
stat: syscall.Stat_t{
Mode: 0600,
Uid: currentUID,
Gid: currentGID,
},
},
{
expectedError: nil,
stat: syscall.Stat_t{
Mode: 0640,
Uid: 0,
Gid: currentGID,
},
},
{
expectedError: errSSLKeyHasUnacceptableUserPermissions,
stat: syscall.Stat_t{
Mode: 0666,
Uid: currentUID,
Gid: currentGID,
},
},
{
expectedError: errSSLKeyHasUnacceptableRootPermissions,
stat: syscall.Stat_t{
Mode: 0666,
Uid: 0,
Gid: currentGID,
},
},
}

for _, test := range testData {
wrapper := &stat_t_wrapper{
stat: test.stat,
}

if test.expectedError != hasCorrectPermissions(wrapper) {
if test.expectedError == nil {
t.Errorf(
"file owned by %d:%d with %s should not have failed check with error \"%s\"",
test.stat.Uid,
test.stat.Gid,
wrapper.Mode(),
hasCorrectPermissions(wrapper),
)
continue
}
t.Errorf(
"file owned by %d:%d with %s, expected \"%s\", got \"%s\"",
test.stat.Uid,
test.stat.Gid,
wrapper.Mode(),
test.expectedError,
hasCorrectPermissions(wrapper),
)
}
}
}

0 comments on commit 8c6de56

Please sign in to comment.