Skip to content

Commit

Permalink
Merge pull request square#95 from square/mmc-refactor
Browse files Browse the repository at this point in the history
Refactor file writing out to decouple from Secrets
  • Loading branch information
mcpherrinm committed Aug 22, 2019
2 parents b8fb1a8 + 6b9898f commit 6d9361f
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 143 deletions.
42 changes: 22 additions & 20 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,33 @@ import (
"strings"
"time"

"github.com/square/keysync/output"

yaml "gopkg.in/yaml.v2"
)

// Config is the main yaml configuration file passed to the keysync binary
type Config struct {
ClientsDir string `yaml:"client_directory"` // A directory of configuration files
SecretsDir string `yaml:"secrets_directory"` // The directory secrets will be written to
CaFile string `yaml:"ca_file"` // The CA to trust (PEM) for Keywhiz communication
YamlExt string `yaml:"yaml_ext"` // The filename extension of the yaml config files
PollInterval string `yaml:"poll_interval"` // If specified, poll at the given interval, otherwise, exit after syncing
ClientTimeout string `yaml:"client_timeout"` // If specified, timeout client connections after specified duration, otherwise use default.
MinBackoff string `yaml:"min_backoff"` // If specified, wait time before first retry, otherwise, use default.
MaxBackoff string `yaml:"max_backoff"` // If specified, max wait time before retries, otherwise, use default.
MaxRetries uint16 `yaml:"max_retries"` // If specified, retry each HTTP call after non-200 response
Server string `yaml:"server"` // The server to connect to (host:port)
Debug bool `yaml:"debug"` // Enable debugging output
DefaultUser string `yaml:"default_user"` // Default user to own files
DefaultGroup string `yaml:"default_group"` // Default group to own files
APIPort uint16 `yaml:"api_port"` // Port for API to listen on
SentryDSN string `yaml:"sentry_dsn"` // Sentry DSN
SentryCaFile string `yaml:"sentry_ca_file"` // The CA to trust (PEM) for Sentry communication
FsType Filesystem `yaml:"filesystem_type"` // Enforce writing this type of filesystem. Use value from statfs.
ChownFiles bool `yaml:"chown_files"` // Do we chown files? Set to false when running without CAP_CHOWN.
MetricsPrefix string `yaml:"metrics_prefix"` // Prefix metric names with this
Monitor MonitorConfig `yaml:"monitor"` // Config for monitoring/alerts
ClientsDir string `yaml:"client_directory"` // A directory of configuration files
SecretsDir string `yaml:"secrets_directory"` // The directory secrets will be written to
CaFile string `yaml:"ca_file"` // The CA to trust (PEM) for Keywhiz communication
YamlExt string `yaml:"yaml_ext"` // The filename extension of the yaml config files
PollInterval string `yaml:"poll_interval"` // If specified, poll at the given interval, otherwise, exit after syncing
ClientTimeout string `yaml:"client_timeout"` // If specified, timeout client connections after specified duration, otherwise use default.
MinBackoff string `yaml:"min_backoff"` // If specified, wait time before first retry, otherwise, use default.
MaxBackoff string `yaml:"max_backoff"` // If specified, max wait time before retries, otherwise, use default.
MaxRetries uint16 `yaml:"max_retries"` // If specified, retry each HTTP call after non-200 response
Server string `yaml:"server"` // The server to connect to (host:port)
Debug bool `yaml:"debug"` // Enable debugging output
DefaultUser string `yaml:"default_user"` // Default user to own files
DefaultGroup string `yaml:"default_group"` // Default group to own files
APIPort uint16 `yaml:"api_port"` // Port for API to listen on
SentryDSN string `yaml:"sentry_dsn"` // Sentry DSN
SentryCaFile string `yaml:"sentry_ca_file"` // The CA to trust (PEM) for Sentry communication
FsType output.Filesystem `yaml:"filesystem_type"` // Enforce writing this type of filesystem. Use value from statfs.
ChownFiles bool `yaml:"chown_files"` // Do we chown files? Set to false when running without CAP_CHOWN.
MetricsPrefix string `yaml:"metrics_prefix"` // Prefix metric names with this
Monitor MonitorConfig `yaml:"monitor"` // Config for monitoring/alerts
}

// The MonitorConfig has extra settings for monitoring/alerts.
Expand Down
112 changes: 112 additions & 0 deletions output/write.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package output

import (
"crypto/rand"
"encoding/hex"
"fmt"
"os"
"path/filepath"
"syscall"
)

// FileInfo returns the filesystem properties atomicWrite wrote
type FileInfo struct {
Mode os.FileMode
UID int
GID int
}

// GetFileInfo from an open file
func GetFileInfo(file *os.File) (*FileInfo, error) {
stat, err := file.Stat()
if err != nil {
return nil, fmt.Errorf("failed to stat after writing: %v", err)
}
filemode := stat.Mode()
uid := int(stat.Sys().(*syscall.Stat_t).Uid)
gid := int(stat.Sys().(*syscall.Stat_t).Gid)

return &FileInfo{filemode, uid, gid}, nil
}

// WriteFileAtomically creates a temporary file, sets perms, writes content, and renames it to filename
// This sequence ensures the following:
// 1. Nobody can open the file before we set owner/permissions properly
// 2. Nobody observes a partially-overwritten secret file.
// The returned FileInfo may not match the passed in one, especially if chownFiles is false.
func WriteFileAtomically(dir, filename string, chownFiles bool, fileInfo FileInfo, enforceFilesystem Filesystem, content []byte) (*FileInfo, error) {
if err := os.MkdirAll(dir, 0775); err != nil {
return nil, fmt.Errorf("making client directory '%s': %v", dir, err)
}

// We can't use ioutil.TempFile because we want to open 0000.
buf := make([]byte, 32)
_, err := rand.Read(buf)
if err != nil {
return nil, err
}
randSuffix := hex.EncodeToString(buf)
fullPath := filepath.Join(dir, filename)
f, err := os.OpenFile(fullPath+randSuffix, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0000)
// Try to remove the file, in event we early-return with an error.
defer os.Remove(fullPath + randSuffix)
if err != nil {
return nil, err
}

if chownFiles {
err = f.Chown(fileInfo.UID, fileInfo.GID)
if err != nil {
return nil, err
}
}

// Always Chmod after the Chown, so we don't expose secret with the wrong owner.
err = f.Chmod(fileInfo.Mode)
if err != nil {
return nil, err
}

if enforceFilesystem != 0 {
good, err := isFilesystem(f, enforceFilesystem)
if err != nil {
return nil, fmt.Errorf("checking filesystem type: %v", err)
}
if !good {
return nil, fmt.Errorf("unexpected filesystem writing %s", filename)
}
}
_, err = f.Write(content)
if err != nil {
return nil, fmt.Errorf("failed writing filesystem content: %v", err)
}

fileinfo, err := GetFileInfo(f)
if err != nil {
return nil, fmt.Errorf("failed to get file mode back from file: %v", err)
}

// While this is intended for use with tmpfs, you could write secrets to disk.
// We ignore any errors from syncing, as it's not strictly required.
_ = f.Sync()

// Rename is atomic, so nobody will observe a partially updated secret
err = os.Rename(fullPath+randSuffix, fullPath)
if err != nil {
return nil, err
}

return fileinfo, nil
}

// The Filesystem identification. On Mac, this is uint32, and int64 on linux
// So both are safe to store as an int64.
// Linux Tmpfs = 0x01021994
// Get these constants with `stat --file-system --format=%t`
type Filesystem int64

func isFilesystem(file *os.File, fs Filesystem) (bool, error) {
var statfs syscall.Statfs_t
err := syscall.Fstatfs(int(file.Fd()), &statfs)
return Filesystem(statfs.Type) == fs, err
}
12 changes: 6 additions & 6 deletions ownership/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ import (
// It is intended to be used with the implementation on Os. There's also one in mock.go that
// uses fixed data instead of operating-system sourced data.
type Lookup interface {
UID(username string) (uint32, error)
GID(groupname string) (uint32, error)
UID(username string) (int, error)
GID(groupname string) (int, error)
}

// Os implements Lookup using the os/user standard library package
type Os struct{}

var _ Lookup = Os{}

func (o Os) UID(username string) (uint32, error) {
func (o Os) UID(username string) (int, error) {
u, err := user.Lookup(username)
if err != nil {
return 0, fmt.Errorf("error resolving uid for %s: %v", username, err)
Expand All @@ -28,10 +28,10 @@ func (o Os) UID(username string) (uint32, error) {
if err != nil {
return 0, fmt.Errorf("error parsing uid %s for %s: %v", u.Uid, username, err)
}
return uint32(id), nil
return int(id), nil
}

func (o Os) GID(groupname string) (uint32, error) {
func (o Os) GID(groupname string) (int, error) {
group, err := user.LookupGroup(groupname)
if err != nil {
return 0, fmt.Errorf("error resolving gid for %s: %v", group, err)
Expand All @@ -40,5 +40,5 @@ func (o Os) GID(groupname string) (uint32, error) {
if err != nil {
return 0, fmt.Errorf("error parsing gid %s for %s: %v", group.Gid, groupname, err)
}
return uint32(id), nil
return int(id), nil
}
8 changes: 4 additions & 4 deletions ownership/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,21 @@ import "fmt"

// Mock implements the lookup interface using a fixed set of users and groups, useful for tests
type Mock struct {
Users map[string]uint32
Groups map[string]uint32
Users map[string]int
Groups map[string]int
}

var _ Lookup = &Mock{}

func (m *Mock) UID(username string) (uint32, error) {
func (m *Mock) UID(username string) (int, error) {
uid, ok := m.Users[username]
if !ok {
return 0, fmt.Errorf("unknown user %s", username)
}
return uid, nil
}

func (m *Mock) GID(username string) (uint32, error) {
func (m *Mock) GID(username string) (int, error) {
uid, ok := m.Groups[username]
if !ok {
return 0, fmt.Errorf("unknown group %s", username)
Expand Down
6 changes: 3 additions & 3 deletions ownership/ownership.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import (

// Ownership indicates the default ownership of filesystem entries.
type Ownership struct {
UID uint32
GID uint32
UID int
GID int
// Where to look up users and groups
Lookup
}
Expand All @@ -30,7 +30,7 @@ type Ownership struct {
// Logs as error anything that goes wrong, but always returns something
// Worst-case you get "0", ie root, owning things, which is safe as root can always read all files.
func NewOwnership(username, groupname, fallbackUser, fallbackGroup string, lookup Lookup, logger *logrus.Entry) Ownership {
var uid, gid uint32
var uid, gid int
var err error

if username != "" {
Expand Down
4 changes: 2 additions & 2 deletions ownership/ownership_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
var testLog = logrus.New().WithField("test", "test")

var data = Mock{
Users: map[string]uint32{"test0": 1000, "test1": 1001, "test2": 1002},
Groups: map[string]uint32{"group0": 2000, "group1": 2001, "group2": 2002},
Users: map[string]int{"test0": 1000, "test1": 1001, "test2": 1002},
Groups: map[string]int{"group0": 2000, "group1": 2001, "group2": 2002},
}

// TestNewOwnership verifies basic functionality, with no fallback or errors
Expand Down
4 changes: 2 additions & 2 deletions secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ func TestSecretModeValue(t *testing.T) {

func TestSecretOwnershipValue(t *testing.T) {
var data = ownership.Mock{
Users: map[string]uint32{"test0": 1000, "test1": 1001, "test2": 1002},
Groups: map[string]uint32{"group0": 2000, "group1": 2001, "group2": 2002},
Users: map[string]int{"test0": 1000, "test1": 1001, "test2": 1002},
Groups: map[string]int{"group0": 2000, "group1": 2001, "group2": 2002},
}
defaultOwnership := ownership.Ownership{UID: 1, GID: 1, Lookup: &data}

Expand Down
4 changes: 3 additions & 1 deletion syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"time"
"unsafe"

"github.com/square/keysync/output"

"github.com/sirupsen/logrus"
sqmetrics "github.com/square/go-sq-metrics"
)
Expand All @@ -39,7 +41,7 @@ type secretState struct {
// Checksum is the server's identifier for the contents of the hash (it's an HMAC)
Checksum string
// We store the mode we wrote to the filesystem
FileInfo
output.FileInfo
// Owner, Group, and Mode come from the Keywhiz server
Owner string
Group string
Expand Down
Loading

0 comments on commit 6d9361f

Please sign in to comment.