From d7a7823c41d8d0e90dee045b65151bfe5383b831 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 1 Oct 2019 08:35:25 -0700 Subject: [PATCH] actions/config: ensure config file is created with mode 0644 If the user has set a restrictive umask, e.g. 0077, then /etc/fscrypt.conf would be created without the world-readable bit set. Fix it by overriding the umask when creating the file. Resolves https://github.com/google/fscrypt/issues/151 --- actions/config.go | 4 ++- actions/config_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++ filesystem/path.go | 10 ++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 actions/config_test.go diff --git a/actions/config.go b/actions/config.go index 386edc4a..7fdaf5b7 100644 --- a/actions/config.go +++ b/actions/config.go @@ -31,6 +31,7 @@ import ( "golang.org/x/sys/unix" "github.com/google/fscrypt/crypto" + "github.com/google/fscrypt/filesystem" "github.com/google/fscrypt/metadata" "github.com/google/fscrypt/util" ) @@ -68,7 +69,8 @@ var ( func CreateConfigFile(target time.Duration, useLegacy bool) error { // Create the config file before computing the hashing costs, so we fail // immediately if the program has insufficient permissions. - configFile, err := os.OpenFile(ConfigFileLocation, createFlags, configPermissions) + configFile, err := filesystem.OpenFileOverridingUmask(ConfigFileLocation, + createFlags, configPermissions) switch { case os.IsExist(err): return ErrConfigFileExists diff --git a/actions/config_test.go b/actions/config_test.go new file mode 100644 index 00000000..037e4330 --- /dev/null +++ b/actions/config_test.go @@ -0,0 +1,55 @@ +/* + * config_test.go - tests for creating the config file + * + * Copyright 2019 Google LLC + * + * 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 actions + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +// Test that the global config file is created with mode 0644, regardless of the +// current umask. +func TestConfigFileIsCreatedWithCorrectMode(t *testing.T) { + oldMask := unix.Umask(0) + defer unix.Umask(oldMask) + unix.Umask(0077) + + tempDir, err := ioutil.TempDir("", "fscrypt") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(tempDir) + ConfigFileLocation = filepath.Join(tempDir, "test.conf") + + if err = CreateConfigFile(time.Millisecond, false); err != nil { + t.Fatal(err) + } + fileInfo, err := os.Stat(ConfigFileLocation) + if err != nil { + t.Fatal(err) + } + if fileInfo.Mode().Perm() != 0644 { + t.Error("Expected newly created config file to have mode 0644") + } +} diff --git a/filesystem/path.go b/filesystem/path.go index 5fd3fdf2..cfc3dc0b 100644 --- a/filesystem/path.go +++ b/filesystem/path.go @@ -24,9 +24,19 @@ import ( "os" "path/filepath" + "golang.org/x/sys/unix" + "github.com/pkg/errors" ) +// OpenFileOverridingUmask calls os.OpenFile but with the umask overridden so +// that no permission bits are masked out if the file is created. +func OpenFileOverridingUmask(name string, flag int, perm os.FileMode) (*os.File, error) { + oldMask := unix.Umask(0) + defer unix.Umask(oldMask) + return os.OpenFile(name, flag, perm) +} + // We only check the unix permissions and the sticky bit const permMask = os.ModeSticky | os.ModePerm