From 5e203d3c924a3b88958a879c23ab4fe73ed2f9f0 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 13 Oct 2019 13:28:17 +0100 Subject: [PATCH 1/4] Ensure .ssh dir exists before rewriting public keys --- models/ssh_key.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/models/ssh_key.go b/models/ssh_key.go index b7c5b4fe6e587..40846aaf5d07c 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -645,6 +645,15 @@ func rewriteAllPublicKeys(e Engine) error { sshOpLocker.Lock() defer sshOpLocker.Unlock() + // First of ensure that the RootPath is present, and if not make it with 0700 permissions + // This of course doesn't guarantee that this is the right directory for authorized_keys + // but at least if it's supposed to be this directory and it doesn't exist and we're the + // right user it will at least be created properly. + err := os.MkdirAll(setting.SSH.RootPath, 0700) + if err != nil { + return err + } + fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys") tmpPath := fPath + ".tmp" t, err := os.OpenFile(tmpPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) From 56e3eb80a0aef8fe72746892e71c1f63d28cf77d Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 13 Oct 2019 13:30:15 +0100 Subject: [PATCH 2/4] Ensure .ssh dir exists before appending to authorized_keys --- models/ssh_key.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/models/ssh_key.go b/models/ssh_key.go index 40846aaf5d07c..4b607bc66ffcd 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -358,6 +358,15 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error { sshOpLocker.Lock() defer sshOpLocker.Unlock() + // First of ensure that the RootPath is present, and if not make it with 0700 permissions + // This of course doesn't guarantee that this is the right directory for authorized_keys + // but at least if it's supposed to be this directory and it doesn't exist and we're the + // right user it will at least be created properly. + err := os.MkdirAll(setting.SSH.RootPath, 0700) + if err != nil { + return err + } + fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys") f, err := os.OpenFile(fPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600) if err != nil { From 2a6768516ad9e7e786d478c4b300c37c5ef7bf23 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 13 Oct 2019 14:09:59 +0100 Subject: [PATCH 3/4] Log the error because it would be useful to know where it is trying to MkdirAll --- models/ssh_key.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/models/ssh_key.go b/models/ssh_key.go index 4b607bc66ffcd..484925271db43 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -364,6 +364,7 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error { // right user it will at least be created properly. err := os.MkdirAll(setting.SSH.RootPath, 0700) if err != nil { + log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err) return err } @@ -660,6 +661,7 @@ func rewriteAllPublicKeys(e Engine) error { // right user it will at least be created properly. err := os.MkdirAll(setting.SSH.RootPath, 0700) if err != nil { + log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err) return err } From f943c483ab0a964b5f5b854eb6b53a68c9ccc322 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 13 Oct 2019 14:27:13 +0100 Subject: [PATCH 4/4] Only try to create RootPath if it's not empty --- models/ssh_key.go | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/models/ssh_key.go b/models/ssh_key.go index 484925271db43..d1132bf0c61a0 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -358,14 +358,16 @@ func appendAuthorizedKeysToFile(keys ...*PublicKey) error { sshOpLocker.Lock() defer sshOpLocker.Unlock() - // First of ensure that the RootPath is present, and if not make it with 0700 permissions - // This of course doesn't guarantee that this is the right directory for authorized_keys - // but at least if it's supposed to be this directory and it doesn't exist and we're the - // right user it will at least be created properly. - err := os.MkdirAll(setting.SSH.RootPath, 0700) - if err != nil { - log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err) - return err + if setting.SSH.RootPath != "" { + // First of ensure that the RootPath is present, and if not make it with 0700 permissions + // This of course doesn't guarantee that this is the right directory for authorized_keys + // but at least if it's supposed to be this directory and it doesn't exist and we're the + // right user it will at least be created properly. + err := os.MkdirAll(setting.SSH.RootPath, 0700) + if err != nil { + log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err) + return err + } } fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys") @@ -655,14 +657,16 @@ func rewriteAllPublicKeys(e Engine) error { sshOpLocker.Lock() defer sshOpLocker.Unlock() - // First of ensure that the RootPath is present, and if not make it with 0700 permissions - // This of course doesn't guarantee that this is the right directory for authorized_keys - // but at least if it's supposed to be this directory and it doesn't exist and we're the - // right user it will at least be created properly. - err := os.MkdirAll(setting.SSH.RootPath, 0700) - if err != nil { - log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err) - return err + if setting.SSH.RootPath != "" { + // First of ensure that the RootPath is present, and if not make it with 0700 permissions + // This of course doesn't guarantee that this is the right directory for authorized_keys + // but at least if it's supposed to be this directory and it doesn't exist and we're the + // right user it will at least be created properly. + err := os.MkdirAll(setting.SSH.RootPath, 0700) + if err != nil { + log.Error("Unable to MkdirAll(%s): %v", setting.SSH.RootPath, err) + return err + } } fPath := filepath.Join(setting.SSH.RootPath, "authorized_keys")