From 2456d5c3bfb0513aefc17d5107fa4362d97700aa Mon Sep 17 00:00:00 2001
From: Nicola Murino <nicola.murino@gmail.com>
Date: Mon, 29 Jul 2019 08:53:22 +0200
Subject: [PATCH] document the new banner configuration

use SFTPGo as default banner and ensure that the banner is not empty
---
 README.md             |  2 ++
 config/config.go      | 10 +++++++---
 config/config_test.go | 24 ++++++++++++++++++++++++
 sftpgo.conf           |  1 +
 4 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index 87cfd9b0..451f1f12 100644
--- a/README.md
+++ b/README.md
@@ -64,6 +64,7 @@ The `sftpgo.conf` configuration file contains the following sections:
     - `idle_timeout`, integer. Time in minutes after which an idle client will be disconnected. Default: 15
     - `max_auth_tries` integer. Maximum number of authentication attempts permitted per connection. If set to a negative number, the number of attempts are unlimited. If set to zero, the number of attempts are limited to 6.
     - `umask`, string. Umask for the new files and directories. This setting has no effect on Windows. Default: "0022"
+    - `banner`, string. Identification string used by the server. Default "SFTPGo"
     - `actions`, struct. It contains the command to execute and/or the HTTP URL to notify and the trigger conditions
         - `execute_on`, list of strings. Valid values are `download`, `upload`, `delete`, `rename`. On folder deletion a `delete` notification will be sent for each deleted file. Leave empty to disable actions.
         - `command`, string. Absolute path to the command to execute. Leave empty to disable. The command is invoked with the following arguments: 
@@ -104,6 +105,7 @@ Here is a full example showing the default config:
         "idle_timeout":15,
         "max_auth_tries":0,
         "umask":"0022",
+        "banner":"SFTPGo",
         "actions":{
             "execute_on":[],
             "command":"",
diff --git a/config/config.go b/config/config.go
index 8aeca3e4..48935bbf 100644
--- a/config/config.go
+++ b/config/config.go
@@ -3,6 +3,7 @@ package config
 import (
 	"encoding/json"
 	"os"
+	"strings"
 
 	"github.com/drakkan/sftpgo/api"
 	"github.com/drakkan/sftpgo/dataprovider"
@@ -11,7 +12,8 @@ import (
 )
 
 const (
-	logSender = "config"
+	logSender     = "config"
+	defaultBanner = "SFTPGo"
 )
 
 var (
@@ -28,7 +30,7 @@ func init() {
 	// create a default configuration to use if no config file is provided
 	globalConf = globalConfig{
 		SFTPD: sftpd.Configuration{
-			Banner:       "SFTPServer",
+			Banner:       defaultBanner,
 			BindPort:     2022,
 			BindAddress:  "",
 			IdleTimeout:  15,
@@ -78,7 +80,6 @@ func GetProviderConf() dataprovider.Config {
 // LoadConfig loads configuration from sftpgo.conf
 func LoadConfig(configPath string) error {
 	logger.Debug(logSender, "load config from path: %v", configPath)
-	//globalConf.basePath = basePath
 	file, err := os.Open(configPath)
 	if err != nil {
 		logger.Warn(logSender, "error loading configuration file: %v. Default configuration will be used: %+v", err, globalConf)
@@ -90,6 +91,9 @@ func LoadConfig(configPath string) error {
 		logger.Warn(logSender, "error parsing config file: %v. Default configuration will be used: %+v", err, globalConf)
 		return err
 	}
+	if strings.TrimSpace(globalConf.SFTPD.Banner) == "" {
+		globalConf.SFTPD.Banner = defaultBanner
+	}
 	logger.Debug(logSender, "config loaded: %+v", globalConf)
 	return err
 }
diff --git a/config/config_test.go b/config/config_test.go
index 81ed8134..19ceb09e 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -1,9 +1,11 @@
 package config_test
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/drakkan/sftpgo/api"
@@ -45,3 +47,25 @@ func TestLoadConfigTest(t *testing.T) {
 	}
 	os.Remove(configFilePath)
 }
+
+func TestEmptyBanner(t *testing.T) {
+	configDir := ".."
+	confName := "temp.conf"
+	configFilePath := filepath.Join(configDir, confName)
+	config.LoadConfig(configFilePath)
+	sftpdConf := config.GetSFTPDConfig()
+	sftpdConf.Banner = " "
+	c := make(map[string]sftpd.Configuration)
+	c["sftpd"] = sftpdConf
+	jsonConf, _ := json.Marshal(c)
+	err := ioutil.WriteFile(configFilePath, jsonConf, 0666)
+	if err != nil {
+		t.Errorf("error saving temporary configuration")
+	}
+	config.LoadConfig(configFilePath)
+	sftpdConf = config.GetSFTPDConfig()
+	if strings.TrimSpace(sftpdConf.Banner) == "" {
+		t.Errorf("SFTPD banner cannot be empty")
+	}
+	os.Remove(configFilePath)
+}
diff --git a/sftpgo.conf b/sftpgo.conf
index 5de10a32..6f47da49 100644
--- a/sftpgo.conf
+++ b/sftpgo.conf
@@ -5,6 +5,7 @@
         "idle_timeout":15,
         "max_auth_tries":0,
         "umask":"0022",
+        "banner":" ",
         "actions":{
             "execute_on":[],
             "command":"",