From ae66916226f4dad3e13c1a52dffc34ce41859879 Mon Sep 17 00:00:00 2001
From: Sahdev Zala <spzala@us.ibm.com>
Date: Sun, 23 Aug 2020 20:20:16 -0400
Subject: [PATCH] pkg: file stat warning (#12242)

Provide warning and doc instead of enforcing file permission.
---
 CHANGELOG-3.5.md                   |  3 +--
 Documentation/op-guide/security.md |  3 +++
 pkg/fileutil/fileutil.go           | 10 ++++++++--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG-3.5.md b/CHANGELOG-3.5.md
index b7464da6c..1f3702356 100644
--- a/CHANGELOG-3.5.md
+++ b/CHANGELOG-3.5.md
@@ -60,8 +60,6 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.4.0...v3.5.0) and
 - Changed `pkg/flags` function signature to [support structured logger](https://github.com/etcd-io/etcd/pull/11616).
   - Previously, `SetFlagsFromEnv(prefix string, fs *flag.FlagSet) error`, now `SetFlagsFromEnv(lg *zap.Logger, prefix string, fs *flag.FlagSet) error`.
   - Previously, `SetPflagsFromEnv(prefix string, fs *pflag.FlagSet) error`, now `SetPflagsFromEnv(lg *zap.Logger, prefix string, fs *pflag.FlagSet) error`.
-- Changed behavior on [existing dir permission](https://github.com/etcd-io/etcd/pull/11798).
-  - Previously, the permission was not checked on existing data directory and the directory used for automatically generating self-signed certificates for TLS connections with clients. Now a check is added to make sure those directories, if already exist, has a desired permission of 700 on Linux and 777 on Windows.
 
 ### `etcdctl`
 
@@ -72,6 +70,7 @@ See [code changes](https://github.com/etcd-io/etcd/compare/v3.4.0...v3.5.0) and
 - Add [`TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256` and `TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256` to `etcd --cipher-suites`](https://github.com/etcd-io/etcd/pull/11864).
 - Changed [the format of WAL entries related to auth for not keeping password as a plain text](https://github.com/etcd-io/etcd/pull/11943).
 - Add third party [Security Audit Report](https://github.com/etcd-io/etcd/pull/12201).
+- A [log warning](https://github.com/etcd-io/etcd/pull/12242) is added when etcd use any existing directory that has a permission different than 700 on Linux and 777 on Windows.
 
 ### Metrics, Monitoring
 
diff --git a/Documentation/op-guide/security.md b/Documentation/op-guide/security.md
index c4cb883e9..87343fb3b 100644
--- a/Documentation/op-guide/security.md
+++ b/Documentation/op-guide/security.md
@@ -431,6 +431,9 @@ No. etcd doesn't encrypt key/value data stored on disk drives. If a user need to
 * Let client applications encrypt and decrypt the data
 * Use a feature of underlying storage systems for encrypting stored data like [dm-crypt]
 
+### I’m seeing a log warning that "directory X exist without recommended permission -rwx------"
+When etcd create certain new directories it sets file permission to 700 to prevent unprivileged access as possible. However, if user has already created a directory with own preference, etcd uses the existing directory and logs a warning message if the permission is different than 700.
+
 [cfssl]: https://github.com/cloudflare/cfssl
 [tls-setup]: ../../hack/tls-setup
 [tls-guide]: https://github.com/coreos/docs/blob/master/os/generate-self-signed-certificates.md
diff --git a/pkg/fileutil/fileutil.go b/pkg/fileutil/fileutil.go
index 01030b213..657d55841 100644
--- a/pkg/fileutil/fileutil.go
+++ b/pkg/fileutil/fileutil.go
@@ -20,6 +20,8 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+
+	"go.uber.org/zap"
 )
 
 const (
@@ -45,7 +47,11 @@ func TouchDirAll(dir string) error {
 	if Exist(dir) {
 		err := CheckDirPermission(dir, PrivateDirMode)
 		if err != nil {
-			return err
+			lg, _ := zap.NewProduction()
+			if lg == nil {
+				lg = zap.NewExample()
+			}
+			lg.Warn("check file permission", zap.Error(err))
 		}
 	} else {
 		err := os.MkdirAll(dir, PrivateDirMode)
@@ -124,7 +130,7 @@ func CheckDirPermission(dir string, perm os.FileMode) error {
 	}
 	dirMode := dirInfo.Mode().Perm()
 	if dirMode != perm {
-		err = fmt.Errorf("directory %q,%q exist without desired file permission %q.", dir, dirInfo.Mode(), os.FileMode(PrivateDirMode))
+		err = fmt.Errorf("directory %q exist, but the permission is %q. The recommended permission is %q to prevent possible unprivileged access to the data.", dir, dirInfo.Mode(), os.FileMode(PrivateDirMode))
 		return err
 	}
 	return nil