Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

Commit

Permalink
Refactor file writing out to decouple from Secrets
Browse files Browse the repository at this point in the history
This makes a new function which handles atomically writing files with a given
mode and permissions.  I plan to reuse this code in the future, so this is just
the first part of seperation

The package is called "output" because I plan to move the Output* code there
too, but that requires a bit more refactoring to avoid circular dependencies.
So that's another PR for later.
  • Loading branch information
mcpherrinm committed Aug 22, 2019
1 parent c6d25b7 commit f6d86f5
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 126 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
111 changes: 111 additions & 0 deletions output/write.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
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 uint32
GID uint32
}

// 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 := stat.Sys().(*syscall.Stat_t).Uid
gid := 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.
func WriteFileAtomically(dir, filename string, mode os.FileMode, chownFiles bool, uid, gid int, 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(uid, 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(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
}
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"
"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
119 changes: 14 additions & 105 deletions write.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,15 @@ package keysync

import (
"bytes"
"crypto/rand"
"crypto/sha256"
"encoding/hex"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"syscall"

"github.com/square/keysync/output"
"github.com/square/keysync/ownership"

pkgerr "github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -106,8 +103,8 @@ func (c OutputDirCollection) Cleanup(known map[string]struct{}, logger *logrus.E
type OutputDir struct {
WriteDirectory string
DefaultOwnership ownership.Ownership
EnforceFilesystem Filesystem // What filesystem type do we expect to write to?
ChownFiles bool // Do we chown the file? (Needs root or CAP_CHOWN).
EnforceFilesystem output.Filesystem // What filesystem type do we expect to write to?
ChownFiles bool // Do we chown the file? (Needs root or CAP_CHOWN).
Logger *logrus.Entry
}

Expand All @@ -133,7 +130,7 @@ func (out *OutputDir) Validate(secret *Secret, state secretState) bool {
if err != nil {
return false
}
fileinfo, err := GetFileInfo(f)
fileinfo, err := output.GetFileInfo(f)
if err != nil {
return false
}
Expand Down Expand Up @@ -192,125 +189,37 @@ func (out *OutputDir) Cleanup(secrets map[string]Secret) error {
return nil
}

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

// 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 := stat.Sys().(*syscall.Stat_t).Uid
gid := stat.Sys().(*syscall.Stat_t).Gid

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

// atomicWrite 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.
// Since keysync is intended to write to tmpfs, this function doesn't do the necessary fsyncs if it
// were persisting content to disk.
// Write puts a Secret into OutputDir
func (out *OutputDir) Write(secret *Secret) (*secretState, error) {
filename, err := secret.Filename()
if err != nil {
return nil, pkgerr.Wrap(err, "cannot write to file")
}

if err := os.MkdirAll(out.WriteDirectory, 0775); err != nil {
return nil, fmt.Errorf("making client directory '%s': %v", out.WriteDirectory, 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(out.WriteDirectory, 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 out.ChownFiles {
ownership := secret.OwnershipValue(out.DefaultOwnership)

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

mode, err := secret.ModeValue()
if err != nil {
return nil, err
}

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

}

if out.EnforceFilesystem != 0 {
good, err := isFilesystem(f, out.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(secret.Content)
if err != nil {
return nil, fmt.Errorf("failed writing filesystem content: %v", err)
}

filemode, err := GetFileInfo(f)
if err != nil {
return nil, fmt.Errorf("failed to get file mode back from file: %v", err)
var uid, gid int
if out.ChownFiles {
owner := secret.OwnershipValue(out.DefaultOwnership)
uid = int(owner.UID)
gid = int(owner.GID)
}

// 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)
fileinfo, err := output.WriteFileAtomically(out.WriteDirectory, filename, mode, out.ChownFiles, uid, gid, out.EnforceFilesystem, secret.Content)
if err != nil {
return nil, err
}

state := secretState{
ContentHash: sha256.Sum256(secret.Content),
Checksum: secret.Checksum,
FileInfo: *filemode,
FileInfo: *fileinfo,
Owner: secret.Owner,
Group: secret.Group,
Mode: secret.Mode,
}
return &state, err
}

// 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
}

0 comments on commit f6d86f5

Please sign in to comment.