diff --git a/logger-file-hook.go b/logger-file-hook.go index a526bbaed..2f626f706 100644 --- a/logger-file-hook.go +++ b/logger-file-hook.go @@ -39,7 +39,8 @@ func enableFileLogger() { return } - file, err := os.OpenFile(flogger.Filename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) + // Creates the named file with mode 0666, honors system umask. + file, err := os.OpenFile(flogger.Filename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666) fatalIf(err, "Unable to open log file.") // Add a local file hook. diff --git a/posix-utils_nix_test.go b/posix-utils_nix_test.go new file mode 100644 index 000000000..013562c18 --- /dev/null +++ b/posix-utils_nix_test.go @@ -0,0 +1,126 @@ +// +build linux darwin dragonfly freebsd netbsd openbsd + +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * 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 main + +import ( + "io/ioutil" + "os" + "path" + "syscall" + "testing" +) + +// Based on `man getumask` a vaporware GNU extension to glibc. +// returns file mode creation mask. +func getUmask() int { + mask := syscall.Umask(0) + syscall.Umask(mask) + return mask +} + +// Tests if the directory and file creations happen with proper umask. +func TestIsValidUmaskVol(t *testing.T) { + tmpPath, err := ioutil.TempDir(os.TempDir(), "minio-") + if err != nil { + t.Fatalf("Initializing temporary directory failed with %s.", err) + } + testCases := []struct { + volName string + expectedUmask int + }{ + {"is-this-valid", getUmask()}, + } + testCase := testCases[0] + + // Initialize a new posix layer. + disk, err := newPosix(tmpPath) + if err != nil { + t.Fatalf("Initializing posix failed with %s.", err) + } + + // Attempt to create a volume to verify the permissions later. + // MakeVol creates 0777. + if err = disk.MakeVol(testCase.volName); err != nil { + t.Fatalf("Creating a volume failed with %s expected to pass.", err) + } + defer removeAll(tmpPath) + + // Stat to get permissions bits. + st, err := os.Stat(path.Join(tmpPath, testCase.volName)) + if err != nil { + t.Fatalf("Stat failed with %s expected to pass.", err) + } + + // Get umask of the bits stored. + currentUmask := 0777 - uint32(st.Mode().Perm()) + + // Verify if umask is correct. + if int(currentUmask) != testCase.expectedUmask { + t.Fatalf("Umask check failed expected %d, got %d", testCase.expectedUmask, currentUmask) + } +} + +// Tests if the file creations happen with proper umask. +func TestIsValidUmaskFile(t *testing.T) { + tmpPath, err := ioutil.TempDir(os.TempDir(), "minio-") + if err != nil { + t.Fatalf("Initializing temporary directory failed with %s.", err) + } + testCases := []struct { + volName string + expectedUmask int + }{ + {"is-this-valid", getUmask()}, + } + testCase := testCases[0] + + // Initialize a new posix layer. + disk, err := newPosix(tmpPath) + if err != nil { + t.Fatalf("Initializing posix failed with %s.", err) + } + + // Attempt to create a volume to verify the permissions later. + // MakeVol creates directory with 0777 perms. + if err = disk.MakeVol(testCase.volName); err != nil { + t.Fatalf("Creating a volume failed with %s expected to pass.", err) + } + + defer removeAll(tmpPath) + + // Attempt to create a file to verify the permissions later. + // AppendFile creates file with 0666 perms. + if err = disk.AppendFile(testCase.volName, "hello-world.txt", []byte("Hello World")); err != nil { + t.Fatalf("Create a file `test` failed with %s expected to pass.", err) + } + + // StatFile - stat the file. + fi, err := disk.StatFile(testCase.volName, "hello-world.txt") + if err != nil { + t.Fatalf("Stat failed with %s expected to pass.", err) + } + + // Get umask of the bits stored. + currentUmask := 0666 - uint32(fi.Mode.Perm()) + + // Verify if umask is correct. + if int(currentUmask) != testCase.expectedUmask { + t.Fatalf("Umask check failed expected %d, got %d", testCase.expectedUmask, currentUmask) + } +} diff --git a/posix-utils_test.go b/posix-utils_test.go new file mode 100644 index 000000000..87db13da8 --- /dev/null +++ b/posix-utils_test.go @@ -0,0 +1,73 @@ +/* + * Minio Cloud Storage, (C) 2016 Minio, Inc. + * + * 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 main + +import "testing" + +// Tests validate volume name. +func TestIsValidVolname(t *testing.T) { + testCases := []struct { + volName string + shouldPass bool + }{ + // Cases which should pass the test. + // passing in valid bucket names. + {"lol", true}, + {"1-this-is-valid", true}, + {"1-this-too-is-valid-1", true}, + {"this.works.too.1", true}, + {"1234567", true}, + {"123", true}, + {"s3-eu-west-1.amazonaws.com", true}, + {"ideas-are-more-powerful-than-guns", true}, + {"testbucket", true}, + {"1bucket", true}, + {"bucket1", true}, + {"$this-is-not-valid-too", true}, + {"contains-$-dollar", true}, + {"contains-^-carrot", true}, + {"contains-$-dollar", true}, + {"contains-$-dollar", true}, + {".starts-with-a-dot", true}, + {"ends-with-a-dot.", true}, + {"ends-with-a-dash-", true}, + {"-starts-with-a-dash", true}, + {"THIS-BEINGS-WITH-UPPERCASe", true}, + {"tHIS-ENDS-WITH-UPPERCASE", true}, + {"ThisBeginsAndEndsWithUpperCase", true}, + {"una ñina", true}, + // cases for which test should fail. + // passing invalid bucket names. + {"", false}, + {"/", false}, + {"a", false}, + {"ab", false}, + {"ab/", false}, + {"......", true}, + {"lalalallalallalalalallalallalala-theString-size-is-greater-than-64", false}, + } + + for i, testCase := range testCases { + isValidVolname := isValidVolname(testCase.volName) + if testCase.shouldPass && !isValidVolname { + t.Errorf("Test case %d: Expected \"%s\" to be a valid bucket name", i+1, testCase.volName) + } + if !testCase.shouldPass && isValidVolname { + t.Errorf("Test case %d: Expected bucket name \"%s\" to be invalid", i+1, testCase.volName) + } + } +} diff --git a/posix-utils_windows_test.go b/posix-utils_windows_test.go index c07d63b45..d63d687bd 100644 --- a/posix-utils_windows_test.go +++ b/posix-utils_windows_test.go @@ -108,7 +108,7 @@ func TestUNCPathDiskName(t *testing.T) { var err error // Instantiate posix object to manage a disk longPathDisk := `\\?\c:\testdisk` - err = mkdirAll(longPathDisk, 0700) + err = mkdirAll(longPathDisk, 0777) if err != nil { t.Fatal(err) } @@ -147,7 +147,7 @@ func Test32kUNCPath(t *testing.T) { remaining := 32767 - 25 - len(longDiskName) longDiskName = longDiskName + `\` + strings.Repeat("a", remaining) } - err = mkdirAll(longDiskName, 0700) + err = mkdirAll(longDiskName, 0777) if err != nil { t.Fatal(err) } diff --git a/posix.go b/posix.go index e8d47e52c..226dfcff8 100644 --- a/posix.go +++ b/posix.go @@ -204,8 +204,8 @@ func (s *posix) MakeVol(volume string) (err error) { if err != nil { return err } - // Make a volume entry. - err = os.Mkdir(preparePath(volumeDir), 0700) + // Make a volume entry, with mode 0777 mkdir honors system umask. + err = os.Mkdir(preparePath(volumeDir), 0777) if err != nil && os.IsExist(err) { return errVolumeExists } @@ -472,10 +472,14 @@ func (s *posix) AppendFile(volume, path string, buf []byte) (err error) { } } // Create top level directories if they don't exist. - if err = mkdirAll(filepath.Dir(filePath), 0700); err != nil { + // with mode 0777 mkdir honors system umask. + if err = mkdirAll(filepath.Dir(filePath), 0777); err != nil { return err } - w, err := os.OpenFile(preparePath(filePath), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600) + + // Creates the named file with mode 0666 (before umask), or starts appending + // to an existig file. + w, err := os.OpenFile(preparePath(filePath), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666) if err != nil { // File path cannot be verified since one of the parents is a file. if strings.Contains(err.Error(), "not a directory") { @@ -483,6 +487,7 @@ func (s *posix) AppendFile(volume, path string, buf []byte) (err error) { } return err } + // Close upon return. defer w.Close() @@ -689,7 +694,8 @@ func (s *posix) RenameFile(srcVolume, srcPath, dstVolume, dstPath string) (err e } // Destination does not exist, hence proceed with the rename. } - if err = mkdirAll(preparePath(slashpath.Dir(dstFilePath)), 0755); err != nil { + // Creates all the parent directories, with mode 0777 mkdir honors system umask. + if err = mkdirAll(preparePath(slashpath.Dir(dstFilePath)), 0777); err != nil { // File path cannot be verified since one of the parents is a file. if strings.Contains(err.Error(), "not a directory") { return errFileAccessDenied @@ -701,6 +707,7 @@ func (s *posix) RenameFile(srcVolume, srcPath, dstVolume, dstPath string) (err e } return err } + // Finally attempt a rename. err = os.Rename(preparePath(srcFilePath), preparePath(dstFilePath)) if err != nil { if os.IsNotExist(err) {