From 65bf9f981ac39a718d27bd08b25c0fffd33fc9c0 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 | 5 +- ssl_permissions.go | 55 +++++++++++++++++-- ssl_permissions_test.go | 118 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 ssl_permissions_test.go diff --git a/conn.go b/conn.go index e050d535..07a621d9 100644 --- a/conn.go +++ b/conn.go @@ -31,7 +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") + ErrSSLKeyUnknownOwnership = errors.New("pq: Could not get owner information for private key, may not be properly protected.") + ErrSSLKeyNotOwnedByGroup = errors.New("pq: Private key is owned by root and a group that doesn't match the process group ID.") + ErrSSLKeyHasGroupPermissions = errors.New("pq: Private key file has group or world access. Permissions should be u=rw (0600) or less") + ErrSSLKeyHasWorldPermissions = errors.New("pq: Private key owned by root has world access. Permissions should be u=rw (0660) or less") ErrCouldNotDetectUsername = errors.New("pq: Could not detect default username. Please provide one explicitly") errUnexpectedReady = errors.New("unexpected ReadyForQuery") diff --git a/ssl_permissions.go b/ssl_permissions.go index 014af6a1..0223e965 100644 --- a/ssl_permissions.go +++ b/ssl_permissions.go @@ -3,7 +3,13 @@ package pq -import "os" +import ( + "io/fs" + "os" + "syscall" +) + +const rootUserID = uint32(0) // sslKeyPermissions checks the permissions on user-supplied ssl key files. // The key file should have very little access. @@ -14,8 +20,49 @@ func sslKeyPermissions(sslkey string) error { if err != nil { return err } - if info.Mode().Perm()&0077 != 0 { - return ErrSSLKeyHasWorldPermissions + + return hasCorrectPermissions(info) +} + +// 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 0660 (u=rw,g=rw). The file should +// never have world permissions. +// +// Returns an error when the permission check fails. +func hasCorrectPermissions(info fs.FileInfo) error { + // if file's permission matches 0600, allow access. + if info.Mode().Perm()&0177 == 0 { + return nil } - return nil + + 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, allow permission to be managed by + // the process group ID. + if unixStat.Uid == rootUserID { + if unixStat.Gid != uint32(os.Getgid()) { + return ErrSSLKeyNotOwnedByGroup + } + if info.Mode().Perm()&0117 != 0 { + return ErrSSLKeyHasWorldPermissions + } + // if the file group ID matches th process group ID and the + // file is not world readable, we are good to go + return nil + } + + return ErrSSLKeyHasGroupPermissions } diff --git a/ssl_permissions_test.go b/ssl_permissions_test.go new file mode 100644 index 00000000..a194fe79 --- /dev/null +++ b/ssl_permissions_test.go @@ -0,0 +1,118 @@ +//go:build !windows +// +build !windows + +package pq + +import ( + "io/fs" + "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() fs.FileMode { + return fs.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: 0660, + Uid: 0, + Gid: currentGID, + }, + }, + { + expectedError: ErrSSLKeyNotOwnedByGroup, + stat: syscall.Stat_t{ + Mode: 0660, + Uid: 0, + Gid: 0, + }, + }, + { + expectedError: ErrSSLKeyHasGroupPermissions, + stat: syscall.Stat_t{ + Mode: 0666, + Uid: currentUID, + Gid: currentGID, + }, + }, + { + expectedError: ErrSSLKeyHasWorldPermissions, + 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), + ) + } + } +}