From d8917faf2ecafe856e2cb2df95b51a96f58b3387 Mon Sep 17 00:00:00 2001 From: Cat J <87989074+catj-cockroach@users.noreply.github.com> Date: Fri, 6 May 2022 16:35:08 +0000 Subject: [PATCH] adds support for kubernetes mounted private keys --- conn.go | 6 ++- ssl_permissions.go | 80 +++++++++++++++++++++++++++-- ssl_permissions_test.go | 109 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 189 insertions(+), 6 deletions(-) create mode 100644 ssl_permissions_test.go diff --git a/conn.go b/conn.go index e050d535..b4e67bc4 100644 --- a/conn.go +++ b/conn.go @@ -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") diff --git a/ssl_permissions.go b/ssl_permissions.go index 014af6a1..d587f102 100644 --- a/ssl_permissions.go +++ b/ssl_permissions.go @@ -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. @@ -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 } diff --git a/ssl_permissions_test.go b/ssl_permissions_test.go new file mode 100644 index 00000000..b0bdca10 --- /dev/null +++ b/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), + ) + } + } +}