From 851c238d1eec7afef267b1415eaa1d2aea3a8c05 Mon Sep 17 00:00:00 2001 From: Matt Robenolt Date: Tue, 28 Jan 2025 16:44:23 -0800 Subject: [PATCH] Replace uses of os.Create with os2.Create within backup/restore workflows This abstracts this ever so slightly into a new `os2.Create` to replace usages of `os.Create` across packages. I didn't want to address every single use of `os.Create` within this PR, but ideally we'd review other uses and swap them to use this version. This prevents files from being created and written with world read/write privileges. I strongly don't think any of these cases, that behavior was intentional, rather implicit due to using `os.Create` which internally uses 0666 permissions. Fixes #17647 Signed-off-by: Matt Robenolt --- go/os2/file.go | 25 ++++++++++++++++++++++++ go/vt/mysqlctl/backupengine.go | 3 ++- go/vt/mysqlctl/blackbox/utils.go | 3 ++- go/vt/mysqlctl/builtinbackupengine.go | 3 ++- go/vt/mysqlctl/filebackupstorage/file.go | 3 ++- 5 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 go/os2/file.go diff --git a/go/os2/file.go b/go/os2/file.go new file mode 100644 index 00000000000..a2892bad551 --- /dev/null +++ b/go/os2/file.go @@ -0,0 +1,25 @@ +/* +Copyright 2025 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package os2 + +import "os" + +// Create is identical to os.Create except uses 0660 permission +// rather than 0666, to exclude world read/write bit. +func Create(name string) (*os.File, error) { + return os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0660) +} diff --git a/go/vt/mysqlctl/backupengine.go b/go/vt/mysqlctl/backupengine.go index eeb14039d01..c5700a3cfd4 100644 --- a/go/vt/mysqlctl/backupengine.go +++ b/go/vt/mysqlctl/backupengine.go @@ -31,6 +31,7 @@ import ( "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/replication" + "vitess.io/vitess/go/os2" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/mysqlctl/backupstats" "vitess.io/vitess/go/vt/mysqlctl/backupstorage" @@ -723,7 +724,7 @@ func createStateFile(cnf *Mycnf) error { // rename func to openStateFile // change to return a *File fname := filepath.Join(cnf.TabletDir(), RestoreState) - fd, err := os.Create(fname) + fd, err := os2.Create(fname) if err != nil { return fmt.Errorf("unable to create file: %v", err) } diff --git a/go/vt/mysqlctl/blackbox/utils.go b/go/vt/mysqlctl/blackbox/utils.go index c7d34ae3cf6..ff7236a41b8 100644 --- a/go/vt/mysqlctl/blackbox/utils.go +++ b/go/vt/mysqlctl/blackbox/utils.go @@ -30,6 +30,7 @@ import ( "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/capabilities" + "vitess.io/vitess/go/os2" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/mysqlctl" "vitess.io/vitess/go/vt/mysqlctl/backupstats" @@ -182,7 +183,7 @@ func createBackupDir(root string, dirs ...string) error { func createBackupFiles(root string, fileCount int, ext string) error { for i := 0; i < fileCount; i++ { - f, err := os.Create(path.Join(root, fmt.Sprintf("%d.%s", i, ext))) + f, err := os2.Create(path.Join(root, fmt.Sprintf("%d.%s", i, ext))) if err != nil { return err } diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 2046a238400..597d95362bb 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -39,6 +39,7 @@ import ( "vitess.io/vitess/go/ioutil" "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/replication" + "vitess.io/vitess/go/os2" "vitess.io/vitess/go/protoutil" "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/logutil" @@ -209,7 +210,7 @@ func (fe *FileEntry) open(cnf *Mycnf, readOnly bool) (*os.File, error) { if err := os.MkdirAll(dir, os.ModePerm); err != nil { return nil, vterrors.Wrapf(err, "cannot create destination directory %v", dir) } - if fd, err = os.Create(name); err != nil { + if fd, err = os2.Create(name); err != nil { return nil, vterrors.Wrapf(err, "cannot create destination file %v", name) } } diff --git a/go/vt/mysqlctl/filebackupstorage/file.go b/go/vt/mysqlctl/filebackupstorage/file.go index bd73c55e70c..85d962a0198 100644 --- a/go/vt/mysqlctl/filebackupstorage/file.go +++ b/go/vt/mysqlctl/filebackupstorage/file.go @@ -27,6 +27,7 @@ import ( "github.com/spf13/pflag" + "vitess.io/vitess/go/os2" "vitess.io/vitess/go/vt/mysqlctl/errors" "vitess.io/vitess/go/ioutil" @@ -96,7 +97,7 @@ func (fbh *FileBackupHandle) AddFile(ctx context.Context, filename string, files return nil, fmt.Errorf("AddFile cannot be called on read-only backup") } p := path.Join(FileBackupStorageRoot, fbh.dir, fbh.name, filename) - f, err := os.Create(p) + f, err := os2.Create(p) if err != nil { return nil, err }