diff --git a/.github/workflows/greetings.yml b/.github/workflows/greetings.yml index 2c1115f..f9d04b0 100644 --- a/.github/workflows/greetings.yml +++ b/.github/workflows/greetings.yml @@ -16,8 +16,8 @@ jobs: and [Code of Conduct](https://github.com/prologic/bitcask/blob/master/CODE_OF_CONDUCT.md) documents. - if possible please also make sure your Bug Report or Feature Request is clearly defined - with either examples or a reprodicible case (_if a bug_). + If possible please also make sure your Bug Report or Feature Request is clearly defined + with either examples or a reproducible case (_if a bug_). Thank you 😃 pr-message: |- diff --git a/AUTHORS b/AUTHORS index 6f38876..ef5e7ec 100644 --- a/AUTHORS +++ b/AUTHORS @@ -4,6 +4,7 @@ Alain Gilbert Awn Umar +Bryan Stenson Christian Muehlhaeuser Ignacio Hagopian James Mills diff --git a/bitcask.go b/bitcask.go index 2b04e89..00898c9 100644 --- a/bitcask.go +++ b/bitcask.go @@ -284,7 +284,7 @@ func (b *Bitcask) put(key, value []byte) (int64, int64, error) { id := b.curr.FileID() - df, err := data.NewDatafile(b.path, id, true, b.config.MaxKeySize, b.config.MaxValueSize) + df, err := data.NewDatafile(b.path, id, true, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask) if err != nil { return -1, 0, err } @@ -292,7 +292,7 @@ func (b *Bitcask) put(key, value []byte) (int64, int64, error) { b.datafiles[id] = df id = b.curr.FileID() + 1 - curr, err := data.NewDatafile(b.path, id, false, b.config.MaxKeySize, b.config.MaxValueSize) + curr, err := data.NewDatafile(b.path, id, false, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask) if err != nil { return -1, 0, err } @@ -307,7 +307,7 @@ func (b *Bitcask) Reopen() error { b.mu.Lock() defer b.mu.Unlock() - datafiles, lastID, err := loadDatafiles(b.path, b.config.MaxKeySize, b.config.MaxValueSize) + datafiles, lastID, err := loadDatafiles(b.path, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask) if err != nil { return err } @@ -316,7 +316,7 @@ func (b *Bitcask) Reopen() error { return err } - curr, err := data.NewDatafile(b.path, lastID, false, b.config.MaxKeySize, b.config.MaxValueSize) + curr, err := data.NewDatafile(b.path, lastID, false, b.config.MaxKeySize, b.config.MaxValueSize, b.config.FileFileModeBeforeUmask) if err != nil { return err } @@ -417,10 +417,6 @@ func Open(path string, options ...Option) (*Bitcask, error) { err error ) - if err := os.MkdirAll(path, 0755); err != nil { - return nil, err - } - configPath := filepath.Join(path, "config.json") if internal.Exists(configPath) { cfg, err = config.Load(configPath) @@ -431,6 +427,16 @@ func Open(path string, options ...Option) (*Bitcask, error) { cfg = newDefaultConfig() } + for _, opt := range options { + if err := opt(cfg); err != nil { + return nil, err + } + } + + if err := os.MkdirAll(path, cfg.DirFileModeBeforeUmask); err != nil { + return nil, err + } + bitcask := &Bitcask{ Flock: flock.New(filepath.Join(path, "lock")), config: cfg, @@ -439,12 +445,6 @@ func Open(path string, options ...Option) (*Bitcask, error) { indexer: index.NewIndexer(), } - for _, opt := range options { - if err := opt(bitcask.config); err != nil { - return nil, err - } - } - locked, err := bitcask.Flock.TryLock() if err != nil { return nil, err @@ -470,7 +470,7 @@ func Open(path string, options ...Option) (*Bitcask, error) { return bitcask, nil } -func loadDatafiles(path string, maxKeySize uint32, maxValueSize uint64) (datafiles map[int]data.Datafile, lastID int, err error) { +func loadDatafiles(path string, maxKeySize uint32, maxValueSize uint64, fileModeBeforeUmask os.FileMode) (datafiles map[int]data.Datafile, lastID int, err error) { fns, err := internal.GetDatafiles(path) if err != nil { return nil, 0, err @@ -483,7 +483,7 @@ func loadDatafiles(path string, maxKeySize uint32, maxValueSize uint64) (datafil datafiles = make(map[int]data.Datafile, len(ids)) for _, id := range ids { - datafiles[id], err = data.NewDatafile(path, id, true, maxKeySize, maxValueSize) + datafiles[id], err = data.NewDatafile(path, id, true, maxKeySize, maxValueSize, fileModeBeforeUmask) if err != nil { return } diff --git a/bitcask_test.go b/bitcask_test.go index a645743..a48fa5e 100644 --- a/bitcask_test.go +++ b/bitcask_test.go @@ -723,6 +723,166 @@ func TestStatsError(t *testing.T) { }) } +func TestDirFileModeBeforeUmask(t *testing.T) { + skipIfWindows(t) + + assert := assert.New(t) + + t.Run("Setup", func(t *testing.T) { + t.Run("Default DirFileModeBeforeUmask is 0700", func(t *testing.T) { + testdir, err := ioutil.TempDir("", "bitcask") + embeddedDir := filepath.Join(testdir, "cache") + assert.NoError(err) + defer os.RemoveAll(testdir) + + defaultTestMode := os.FileMode(0700) + + db, err := Open(embeddedDir) + assert.NoError(err) + defer db.Close() + err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error { + // skip the root directory + if path == testdir { + return nil + } + if info.IsDir() { + // perms for directory on disk are filtered through defaultTestMode, AND umask of user running test. + // this means the mkdir calls can only FURTHER restrict permissions, not grant more (preventing escalatation). + // to make this test OS agnostic, we'll skip using golang.org/x/sys/unix, inferring umask via XOR and AND NOT. + + // create anotherDir with allPerms - to infer umask + anotherDir := filepath.Join(testdir, "temp") + err := os.Mkdir(anotherDir, os.ModePerm) + assert.NoError(err) + defer os.RemoveAll(anotherDir) + + anotherStat, err := os.Stat(anotherDir) + assert.NoError(err) + + // infer umask from anotherDir + umask := os.ModePerm ^ (anotherStat.Mode() & os.ModePerm) + + assert.Equal(info.Mode()&os.ModePerm, defaultTestMode&^umask) + } + return nil + }) + assert.NoError(err) + }) + + t.Run("Dir FileModeBeforeUmask is set via options for all subdirectories", func(t *testing.T) { + testdir, err := ioutil.TempDir("", "bitcask") + embeddedDir := filepath.Join(testdir, "cache") + assert.NoError(err) + defer os.RemoveAll(testdir) + + testMode := os.FileMode(0713) + + db, err := Open(embeddedDir, WithDirFileModeBeforeUmask(testMode)) + assert.NoError(err) + defer db.Close() + err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error { + // skip the root directory + if path == testdir { + return nil + } + if info.IsDir() { + // create anotherDir with allPerms - to infer umask + anotherDir := filepath.Join(testdir, "temp") + err := os.Mkdir(anotherDir, os.ModePerm) + assert.NoError(err) + defer os.RemoveAll(anotherDir) + + anotherStat, _ := os.Stat(anotherDir) + + // infer umask from anotherDir + umask := os.ModePerm ^ (anotherStat.Mode() & os.ModePerm) + + assert.Equal(info.Mode()&os.ModePerm, testMode&^umask) + } + return nil + }) + assert.NoError(err) + }) + + }) +} + +func TestFileFileModeBeforeUmask(t *testing.T) { + skipIfWindows(t) + + assert := assert.New(t) + + t.Run("Setup", func(t *testing.T) { + t.Run("Default File FileModeBeforeUmask is 0600", func(t *testing.T) { + testdir, err := ioutil.TempDir("", "bitcask") + assert.NoError(err) + defer os.RemoveAll(testdir) + + defaultTestMode := os.FileMode(0600) + + db, err := Open(testdir) + assert.NoError(err) + defer db.Close() + err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + // the lock file is set within Flock, so ignore it + if filepath.Base(path) == "lock" { + return nil + } + // create aFile with allPerms - to infer umask + aFilePath := filepath.Join(testdir, "temp") + _, err := os.OpenFile(aFilePath, os.O_CREATE, os.ModePerm) + assert.NoError(err) + defer os.RemoveAll(aFilePath) + + fileStat, _ := os.Stat(aFilePath) + + // infer umask from anotherDir + umask := os.ModePerm ^ (fileStat.Mode() & os.ModePerm) + + assert.Equal(info.Mode()&os.ModePerm, defaultTestMode&^umask) + } + return nil + }) + assert.NoError(err) + }) + + t.Run("File FileModeBeforeUmask is set via options for all files", func(t *testing.T) { + testdir, err := ioutil.TempDir("", "bitcask") + assert.NoError(err) + defer os.RemoveAll(testdir) + + testMode := os.FileMode(0673) + + db, err := Open(testdir, WithFileFileModeBeforeUmask(testMode)) + assert.NoError(err) + defer db.Close() + err = filepath.Walk(testdir, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + // the lock file is set within Flock, so ignore it + if filepath.Base(path) == "lock" { + return nil + } + // create aFile with allPerms - to infer umask + aFilePath := filepath.Join(testdir, "temp") + _, err := os.OpenFile(aFilePath, os.O_CREATE, os.ModePerm) + assert.NoError(err) + defer os.RemoveAll(aFilePath) + + fileStat, _ := os.Stat(aFilePath) + + // infer umask from anotherDir + umask := os.ModePerm ^ (fileStat.Mode() & os.ModePerm) + + assert.Equal(info.Mode()&os.ModePerm, testMode&^umask) + } + return nil + }) + assert.NoError(err) + }) + }) +} + func TestMaxDatafileSize(t *testing.T) { var ( db *Bitcask diff --git a/go.sum b/go.sum index e54d32c..502cbf7 100644 --- a/go.sum +++ b/go.sum @@ -236,6 +236,7 @@ github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/tidwall/redcon v1.3.2 h1:8INx/Nm3VSUbDUT16TH1rMgYQsbXNqy9xcX70edHXbo= github.com/tidwall/redcon v1.3.2/go.mod h1:bdYBm4rlcWpst2XMwKVzWDF9CoUxEbUmM7CQrKeOZas= +github.com/tidwall/redcon v1.3.3 h1:BkvIOVPSMw7JnPlyjaSCckgEhvXCy3U5GgKkssveax4= github.com/tidwall/redcon v1.3.3/go.mod h1:bdYBm4rlcWpst2XMwKVzWDF9CoUxEbUmM7CQrKeOZas= github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= diff --git a/internal/config/config.go b/internal/config/config.go index 2e870c7..6ac170b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,15 +3,18 @@ package config import ( "encoding/json" "io/ioutil" + "os" ) // Config contains the bitcask configuration parameters type Config struct { - MaxDatafileSize int `json:"max_datafile_size"` - MaxKeySize uint32 `json:"max_key_size"` - MaxValueSize uint64 `json:"max_value_size"` - Sync bool `json:"sync"` - AutoRecovery bool `json:"autorecovery"` + MaxDatafileSize int `json:"max_datafile_size"` + MaxKeySize uint32 `json:"max_key_size"` + MaxValueSize uint64 `json:"max_value_size"` + Sync bool `json:"sync"` + AutoRecovery bool `json:"autorecovery"` + DirFileModeBeforeUmask os.FileMode + FileFileModeBeforeUmask os.FileMode } // Load loads a configuration from the given path @@ -38,10 +41,10 @@ func (c *Config) Save(path string) error { return err } - err = ioutil.WriteFile(path, data, 0600) + err = ioutil.WriteFile(path, data, c.FileFileModeBeforeUmask) if err != nil { return err } - + return nil } diff --git a/internal/data/datafile.go b/internal/data/datafile.go index 44d0563..1679dcc 100644 --- a/internal/data/datafile.go +++ b/internal/data/datafile.go @@ -48,7 +48,7 @@ type datafile struct { } // NewDatafile opens an existing datafile -func NewDatafile(path string, id int, readonly bool, maxKeySize uint32, maxValueSize uint64) (Datafile, error) { +func NewDatafile(path string, id int, readonly bool, maxKeySize uint32, maxValueSize uint64, fileMode os.FileMode) (Datafile, error) { var ( r *os.File ra *mmap.ReaderAt @@ -59,7 +59,7 @@ func NewDatafile(path string, id int, readonly bool, maxKeySize uint32, maxValue fn := filepath.Join(path, fmt.Sprintf(defaultDatafileFilename, id)) if !readonly { - w, err = os.OpenFile(fn, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0640) + w, err = os.OpenFile(fn, os.O_WRONLY|os.O_APPEND|os.O_CREATE, fileMode) if err != nil { return nil, err } diff --git a/options.go b/options.go index 937780a..c92e5a4 100644 --- a/options.go +++ b/options.go @@ -1,8 +1,18 @@ package bitcask -import "github.com/prologic/bitcask/internal/config" +import ( + "os" + + "github.com/prologic/bitcask/internal/config" +) const ( + // DefaultDirFileModeBeforeUmask is the default os.FileMode used when creating directories + DefaultDirFileModeBeforeUmask = os.FileMode(0700) + + // DefaultFileFileModeBeforeUmask is the default os.FileMode used when creating files + DefaultFileFileModeBeforeUmask = os.FileMode(0600) + // DefaultMaxDatafileSize is the default maximum datafile size in bytes DefaultMaxDatafileSize = 1 << 20 // 1MB @@ -31,6 +41,22 @@ func WithAutoRecovery(enabled bool) Option { } } +// WithDirFileModeBeforeUmask sets the FileMode used for each new file created. +func WithDirFileModeBeforeUmask(mode os.FileMode) Option { + return func(cfg *config.Config) error { + cfg.DirFileModeBeforeUmask = mode + return nil + } +} + +// WithFileFileModeBeforeUmask sets the FileMode used for each new file created. +func WithFileFileModeBeforeUmask(mode os.FileMode) Option { + return func(cfg *config.Config) error { + cfg.FileFileModeBeforeUmask = mode + return nil + } +} + // WithMaxDatafileSize sets the maximum datafile size option func WithMaxDatafileSize(size int) Option { return func(cfg *config.Config) error { @@ -66,9 +92,11 @@ func WithSync(sync bool) Option { func newDefaultConfig() *config.Config { return &config.Config{ - MaxDatafileSize: DefaultMaxDatafileSize, - MaxKeySize: DefaultMaxKeySize, - MaxValueSize: DefaultMaxValueSize, - Sync: DefaultSync, + MaxDatafileSize: DefaultMaxDatafileSize, + MaxKeySize: DefaultMaxKeySize, + MaxValueSize: DefaultMaxValueSize, + Sync: DefaultSync, + DirFileModeBeforeUmask: DefaultDirFileModeBeforeUmask, + FileFileModeBeforeUmask: DefaultFileFileModeBeforeUmask, } }