From 15716a0772a8e8331785969f224e76b5d8122525 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 30 Sep 2015 09:32:17 +0200 Subject: [PATCH] Fix STGUIAPIKEY and STGUIADDR overrides (fixes #2335) Also removes STGUIAUTH and corresponding --gui-authentication as this seems fundamentally insecure and I'm unsure of the actual use case for it? --- cmd/syncthing/gui.go | 9 ++ cmd/syncthing/gui_auth.go | 1 - cmd/syncthing/main.go | 168 +++++++++++++++----------------------- 3 files changed, 73 insertions(+), 105 deletions(-) diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index fdbc9b0b9..d9798f6c6 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -85,6 +85,11 @@ func newAPISvc(id protocol.DeviceID, cfg *config.Wrapper, assetDir string, m *mo } func (s *apiSvc) getListener(cfg config.GUIConfiguration) (net.Listener, error) { + if guiAddress != "" { + // Override from the environment + cfg.Address = guiAddress + } + cert, err := tls.LoadX509KeyPair(locations[locHTTPSCertFile], locations[locHTTPSKeyFile]) if err != nil { l.Infoln("Loading HTTPS certificate:", err) @@ -196,6 +201,10 @@ func (s *apiSvc) Serve() { }) guiCfg := s.cfg.GUI() + if guiAPIKey != "" { + // Override from the environment + guiCfg.APIKey = guiAPIKey + } // Wrap everything in CSRF protection. The /rest prefix should be // protected, other requests will grant cookies. diff --git a/cmd/syncthing/gui_auth.go b/cmd/syncthing/gui_auth.go index c910dbac7..f37b1483f 100644 --- a/cmd/syncthing/gui_auth.go +++ b/cmd/syncthing/gui_auth.go @@ -25,7 +25,6 @@ var ( ) func basicAuthAndSessionMiddleware(cookieName string, cfg config.GUIConfiguration, next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if cfg.APIKey != "" && r.Header.Get("X-API-Key") == cfg.APIKey { next.ServeHTTP(w, r) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 30a784fd0..a11c8e1f5 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -43,7 +43,6 @@ import ( "github.com/syndtr/goleveldb/leveldb/errors" "github.com/syndtr/goleveldb/leveldb/opt" "github.com/thejerf/suture" - "golang.org/x/crypto/bcrypt" ) var ( @@ -188,28 +187,27 @@ are mostly useful for developers. Use with care. // Command line and environment options var ( - reset bool - showVersion bool - doUpgrade bool - doUpgradeCheck bool - upgradeTo string - noBrowser bool - noConsole bool - generateDir string - logFile string - auditEnabled bool - verbose bool - paused bool - noRestart = os.Getenv("STNORESTART") != "" - noUpgrade = os.Getenv("STNOUPGRADE") != "" - guiAddress = os.Getenv("STGUIADDRESS") // legacy - guiAuthentication = os.Getenv("STGUIAUTH") // legacy - guiAPIKey = os.Getenv("STGUIAPIKEY") // legacy - profiler = os.Getenv("STPROFILER") - guiAssets = os.Getenv("STGUIASSETS") - cpuProfile = os.Getenv("STCPUPROFILE") != "" - stRestarting = os.Getenv("STRESTART") != "" - innerProcess = os.Getenv("STNORESTART") != "" || os.Getenv("STMONITORED") != "" + reset bool + showVersion bool + doUpgrade bool + doUpgradeCheck bool + upgradeTo string + noBrowser bool + noConsole bool + generateDir string + logFile string + auditEnabled bool + verbose bool + paused bool + noRestart = os.Getenv("STNORESTART") != "" + noUpgrade = os.Getenv("STNOUPGRADE") != "" + guiAddress = os.Getenv("STGUIADDRESS") // legacy + guiAPIKey = os.Getenv("STGUIAPIKEY") // legacy + profiler = os.Getenv("STPROFILER") + guiAssets = os.Getenv("STGUIASSETS") + cpuProfile = os.Getenv("STCPUPROFILE") != "" + stRestarting = os.Getenv("STRESTART") != "" + innerProcess = os.Getenv("STNORESTART") != "" || os.Getenv("STMONITORED") != "" ) func main() { @@ -226,7 +224,6 @@ func main() { flag.StringVar(&generateDir, "generate", "", "Generate key and config in specified dir, then exit") flag.StringVar(&guiAddress, "gui-address", guiAddress, "Override GUI address") - flag.StringVar(&guiAuthentication, "gui-authentication", guiAuthentication, "Override GUI authentication; username:password") flag.StringVar(&guiAPIKey, "gui-apikey", guiAPIKey, "Override GUI API key") flag.StringVar(&confDir, "home", "", "Set configuration directory") flag.IntVar(&logFlags, "logflags", logFlags, "Select information in log line prefix") @@ -880,47 +877,52 @@ func startAuditing(mainSvc *suture.Supervisor) { } func setupGUI(mainSvc *suture.Supervisor, cfg *config.Wrapper, m *model.Model, apiSub *events.BufferedSubscription, discoverer *discover.CachingMux, relaySvc *relay.Svc) { - opts := cfg.Options() - guiCfg := overrideGUIConfig(cfg.GUI(), guiAddress, guiAuthentication, guiAPIKey) + guiCfg := cfg.GUI() - if guiCfg.Enabled && guiCfg.Address != "" { - addr, err := net.ResolveTCPAddr("tcp", guiCfg.Address) + if !guiCfg.Enabled { + return + } + if guiCfg.Address == "" { + return + } + + addr, err := net.ResolveTCPAddr("tcp", guiCfg.Address) + if err != nil { + l.Fatalf("Cannot start GUI on %q: %v", guiCfg.Address, err) + } else { + var hostOpen, hostShow string + switch { + case addr.IP == nil: + hostOpen = "localhost" + hostShow = "0.0.0.0" + case addr.IP.IsUnspecified(): + hostOpen = "localhost" + hostShow = addr.IP.String() + default: + hostOpen = addr.IP.String() + hostShow = hostOpen + } + + var proto = "http" + if guiCfg.UseTLS { + proto = "https" + } + + urlShow := fmt.Sprintf("%s://%s/", proto, net.JoinHostPort(hostShow, strconv.Itoa(addr.Port))) + l.Infoln("Starting web GUI on", urlShow) + + api, err := newAPISvc(myID, cfg, guiAssets, m, apiSub, discoverer, relaySvc) if err != nil { - l.Fatalf("Cannot start GUI on %q: %v", guiCfg.Address, err) - } else { - var hostOpen, hostShow string - switch { - case addr.IP == nil: - hostOpen = "localhost" - hostShow = "0.0.0.0" - case addr.IP.IsUnspecified(): - hostOpen = "localhost" - hostShow = addr.IP.String() - default: - hostOpen = addr.IP.String() - hostShow = hostOpen - } + l.Fatalln("Cannot start GUI:", err) + } + cfg.Subscribe(api) + mainSvc.Add(api) - var proto = "http" - if guiCfg.UseTLS { - proto = "https" - } - - urlShow := fmt.Sprintf("%s://%s/", proto, net.JoinHostPort(hostShow, strconv.Itoa(addr.Port))) - l.Infoln("Starting web GUI on", urlShow) - api, err := newAPISvc(myID, cfg, guiAssets, m, apiSub, discoverer, relaySvc) - if err != nil { - l.Fatalln("Cannot start GUI:", err) - } - cfg.Subscribe(api) - mainSvc.Add(api) - - if opts.StartBrowser && !noBrowser && !stRestarting { - urlOpen := fmt.Sprintf("%s://%s/", proto, net.JoinHostPort(hostOpen, strconv.Itoa(addr.Port))) - // Can potentially block if the utility we are invoking doesn't - // fork, and just execs, hence keep it in it's own routine. - go openURL(urlOpen) - } + if cfg.Options().StartBrowser && !noBrowser && !stRestarting { + urlOpen := fmt.Sprintf("%s://%s/", proto, net.JoinHostPort(hostOpen, strconv.Itoa(addr.Port))) + // Can potentially block if the utility we are invoking doesn't + // fork, and just execs, hence keep it in it's own routine. + go openURL(urlOpen) } } } @@ -1016,48 +1018,6 @@ func getFreePort(host string, ports ...int) (int, error) { return addr.Port, nil } -func overrideGUIConfig(cfg config.GUIConfiguration, address, authentication, apikey string) config.GUIConfiguration { - if address != "" { - cfg.Enabled = true - - if !strings.Contains(address, "//") { - // Assume just an IP was given. Don't touch he TLS setting. - cfg.Address = address - } else { - parsed, err := url.Parse(address) - if err != nil { - l.Fatalln(err) - } - cfg.Address = parsed.Host - switch parsed.Scheme { - case "http": - cfg.UseTLS = false - case "https": - cfg.UseTLS = true - default: - l.Fatalln("Unknown scheme:", parsed.Scheme) - } - } - } - - if authentication != "" { - authenticationParts := strings.SplitN(authentication, ":", 2) - - hash, err := bcrypt.GenerateFromPassword([]byte(authenticationParts[1]), 0) - if err != nil { - l.Fatalln("Invalid GUI password:", err) - } - - cfg.User = authenticationParts[0] - cfg.Password = string(hash) - } - - if apikey != "" { - cfg.APIKey = apikey - } - return cfg -} - func standbyMonitor() { restartDelay := time.Duration(60 * time.Second) now := time.Now()