From 5971c00a4f8e09235d75bd1d5d3d84c32048cba6 Mon Sep 17 00:00:00 2001 From: Antony Male Date: Fri, 29 Jan 2016 17:16:01 +0000 Subject: [PATCH 1/6] Support multiple API keys (command-line and config) (fixes #2747) --- cmd/syncthing/gui.go | 2 +- cmd/syncthing/gui_auth.go | 3 +-- cmd/syncthing/gui_csrf.go | 5 +++-- cmd/syncthing/main.go | 2 +- lib/config/guiconfiguration.go | 16 ++++++++++++---- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index 7e7cc4f51..de4150438 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -238,7 +238,7 @@ func (s *apiService) Serve() { // Wrap everything in CSRF protection. The /rest prefix should be // protected, other requests will grant cookies. - handler := csrfMiddleware(s.id.String()[:5], "/rest", guiCfg.APIKey(), mux) + handler := csrfMiddleware(s.id.String()[:5], "/rest", guiCfg, mux) // Add the CORS handling handler = corsMiddleware(handler) diff --git a/cmd/syncthing/gui_auth.go b/cmd/syncthing/gui_auth.go index 1344a43e4..82a634e4e 100644 --- a/cmd/syncthing/gui_auth.go +++ b/cmd/syncthing/gui_auth.go @@ -33,9 +33,8 @@ func emitLoginAttempt(success bool, username string) { } func basicAuthAndSessionMiddleware(cookieName string, cfg config.GUIConfiguration, next http.Handler) http.Handler { - apiKey := cfg.APIKey() return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if apiKey != "" && r.Header.Get("X-API-Key") == apiKey { + if cfg.IsValidAPIKey(r.Header.Get("X-API-Key")) { next.ServeHTTP(w, r) return } diff --git a/cmd/syncthing/gui_csrf.go b/cmd/syncthing/gui_csrf.go index 74e425717..00e2d3e93 100644 --- a/cmd/syncthing/gui_csrf.go +++ b/cmd/syncthing/gui_csrf.go @@ -13,6 +13,7 @@ import ( "os" "strings" + "github.com/syncthing/syncthing/lib/config" "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/sync" ) @@ -30,11 +31,11 @@ const maxCsrfTokens = 25 // Check for CSRF token on /rest/ URLs. If a correct one is not given, reject // the request with 403. For / and /index.html, set a new CSRF cookie if none // is currently set. -func csrfMiddleware(unique, prefix, apiKey string, next http.Handler) http.Handler { +func csrfMiddleware(unique string, prefix string, cfg config.GUIConfiguration, next http.Handler) http.Handler { loadCsrfTokens() return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Allow requests carrying a valid API key - if apiKey != "" && r.Header.Get("X-API-Key") == apiKey { + if cfg.IsValidAPIKey(r.Header.Get("X-API-Key")) { next.ServeHTTP(w, r) return } diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 755896863..145b3084b 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -478,7 +478,7 @@ func upgradeViaRest() error { cfg, _ := loadConfig() target := cfg.GUI().URL() r, _ := http.NewRequest("POST", target+"/rest/system/upgrade", nil) - r.Header.Set("X-API-Key", cfg.GUI().APIKey()) + r.Header.Set("X-API-Key", cfg.GUI().RawAPIKey) tr := &http.Transport{ Dial: dialer.Dial, diff --git a/lib/config/guiconfiguration.go b/lib/config/guiconfiguration.go index 3cda0eb34..14b824577 100644 --- a/lib/config/guiconfiguration.go +++ b/lib/config/guiconfiguration.go @@ -76,9 +76,17 @@ func (c GUIConfiguration) URL() string { return u.String() } -func (c GUIConfiguration) APIKey() string { - if override := os.Getenv("STGUIAPIKEY"); override != "" { - return override +// Returns whether the given API key is valid, including both the value in config +// and any overrides +func (c GUIConfiguration) IsValidAPIKey(apiKey string) bool { + switch apiKey { + case "": + return false + + case c.RawAPIKey, os.Getenv("STGUIAPIKEY"): + return true + + default: + return false } - return c.RawAPIKey } From ae36fada6b72dc92274b17b0dcdb81d910462c2e Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 1 Feb 2016 06:46:35 +0100 Subject: [PATCH 2/6] Remove old reference to moved protocol --- protocol/README.md | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 protocol/README.md diff --git a/protocol/README.md b/protocol/README.md deleted file mode 100644 index bc77bc2be..000000000 --- a/protocol/README.md +++ /dev/null @@ -1,2 +0,0 @@ -Syncthing uses the protocols defined in -https://github.com/syncthing/specs/. From eb55d19786cab625842350b9c4fb8e5546040470 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 1 Feb 2016 09:10:14 +0100 Subject: [PATCH 3/6] Clean up error handling a bit in protocol.readMessage --- lib/protocol/protocol.go | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/lib/protocol/protocol.go b/lib/protocol/protocol.go index 3f4324a76..334ba28ab 100644 --- a/lib/protocol/protocol.go +++ b/lib/protocol/protocol.go @@ -430,36 +430,20 @@ func (c *rawConnection) readMessage() (hdr header, msg encodable, err error) { } } - // We check each returned error for the XDRError.IsEOF() method. - // IsEOF()==true here means that the message contained fewer fields than - // expected. It does not signify an EOF on the socket, because we've - // successfully read a size value and that many bytes already. New fields - // we expected but the other peer didn't send should be interpreted as - // zero/nil, and if that's not valid we'll verify it somewhere else. - switch hdr.msgType { case messageTypeIndex, messageTypeIndexUpdate: var idx IndexMessage err = idx.UnmarshalXDR(msgBuf) - if xdrErr, ok := err.(isEofer); ok && xdrErr.IsEOF() { - err = nil - } msg = idx case messageTypeRequest: var req RequestMessage err = req.UnmarshalXDR(msgBuf) - if xdrErr, ok := err.(isEofer); ok && xdrErr.IsEOF() { - err = nil - } msg = req case messageTypeResponse: var resp ResponseMessage err = resp.UnmarshalXDR(msgBuf) - if xdrErr, ok := err.(isEofer); ok && xdrErr.IsEOF() { - err = nil - } msg = resp case messageTypePing: @@ -468,23 +452,28 @@ func (c *rawConnection) readMessage() (hdr header, msg encodable, err error) { case messageTypeClusterConfig: var cc ClusterConfigMessage err = cc.UnmarshalXDR(msgBuf) - if xdrErr, ok := err.(isEofer); ok && xdrErr.IsEOF() { - err = nil - } msg = cc case messageTypeClose: var cm CloseMessage err = cm.UnmarshalXDR(msgBuf) - if xdrErr, ok := err.(isEofer); ok && xdrErr.IsEOF() { - err = nil - } msg = cm default: err = fmt.Errorf("protocol error: %s: unknown message type %#x", c.id, hdr.msgType) } + // We check the returned error for the XDRError.IsEOF() method. + // IsEOF()==true here means that the message contained fewer fields than + // expected. It does not signify an EOF on the socket, because we've + // successfully read a size value and then that many bytes from the wire. + // New fields we expected but the other peer didn't send should be + // interpreted as zero/nil, and if that's not valid we'll verify it + // somewhere else. + if xdrErr, ok := err.(isEofer); ok && xdrErr.IsEOF() { + err = nil + } + return } From 39c16d1cc479714d3de6cf9cc9122d479606aec0 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 2 Feb 2016 10:27:01 +0100 Subject: [PATCH 4/6] Add -paths option to print config, key, database paths --- cmd/syncthing/main.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 145b3084b..372530204 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -122,11 +122,6 @@ var ( const ( usage = "syncthing [options]" extraUsage = ` -The default configuration directory is: - - %s - - The -logflags value is a sum of the following: 1 Date @@ -199,6 +194,7 @@ type RuntimeOptions struct { confDir string reset bool showVersion bool + showPaths bool doUpgrade bool doUpgradeCheck bool upgradeTo string @@ -260,6 +256,7 @@ func parseCommandLineOptions() RuntimeOptions { flag.BoolVar(&options.doUpgrade, "upgrade", false, "Perform upgrade") flag.BoolVar(&options.doUpgradeCheck, "upgrade-check", false, "Check for available upgrade") flag.BoolVar(&options.showVersion, "version", false, "Show version") + flag.BoolVar(&options.showPaths, "paths", false, "Show configuration paths") flag.StringVar(&options.upgradeTo, "upgrade-to", options.upgradeTo, "Force upgrade directly from specified URL") flag.BoolVar(&options.auditEnabled, "audit", false, "Write events to audit file") flag.BoolVar(&options.verbose, "verbose", false, "Print verbose log output") @@ -270,7 +267,7 @@ func parseCommandLineOptions() RuntimeOptions { flag.BoolVar(&options.hideConsole, "no-console", false, "Hide console window") } - longUsage := fmt.Sprintf(extraUsage, baseDirs["config"], debugFacilities()) + longUsage := fmt.Sprintf(extraUsage, debugFacilities()) flag.Usage = usageFor(flag.CommandLine, usage, longUsage) flag.Parse() @@ -320,6 +317,11 @@ func main() { return } + if options.showPaths { + showPaths() + return + } + if options.browserOnly { openGUI() return @@ -1218,3 +1220,13 @@ func checkShortIDs(cfg *config.Wrapper) error { } return nil } + +func showPaths() { + fmt.Printf("Configuration file:\n\t%s\n\n", locations[locConfigFile]) + fmt.Printf("Database directory:\n\t%s\n\n", locations[locDatabase]) + fmt.Printf("Device private key & certificate files:\n\t%s\n\t%s\n\n", locations[locKeyFile], locations[locCertFile]) + fmt.Printf("HTTPS private key & certificate files:\n\t%s\n\t%s\n\n", locations[locHTTPSKeyFile], locations[locHTTPSCertFile]) + fmt.Printf("Log file:\n\t%s\n\n", locations[locLogFile]) + fmt.Printf("GUI override directory:\n\t%s\n\n", locations[locGUIAssets]) + fmt.Printf("Default sync folder directory:\n\t%s\n\n", locations[locDefFolder]) +} From e93c766c4245734baac27fe533edf16361bc1f23 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 2 Feb 2016 11:12:25 +0100 Subject: [PATCH 5/6] Rename RawAPIKey -> APIKey in GUIConfiguration --- cmd/syncthing/main.go | 2 +- lib/config/config.go | 4 ++-- lib/config/config_test.go | 2 +- lib/config/guiconfiguration.go | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 372530204..319131d96 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -480,7 +480,7 @@ func upgradeViaRest() error { cfg, _ := loadConfig() target := cfg.GUI().URL() r, _ := http.NewRequest("POST", target+"/rest/system/upgrade", nil) - r.Header.Set("X-API-Key", cfg.GUI().RawAPIKey) + r.Header.Set("X-API-Key", cfg.GUI().APIKey) tr := &http.Transport{ Dial: dialer.Dial, diff --git a/lib/config/config.go b/lib/config/config.go index be3b9b024..6d0920c17 100644 --- a/lib/config/config.go +++ b/lib/config/config.go @@ -229,8 +229,8 @@ func (cfg *Configuration) prepare(myID protocol.DeviceID) { cfg.Options.ReconnectIntervalS = 5 } - if cfg.GUI.RawAPIKey == "" { - cfg.GUI.RawAPIKey = randomString(32) + if cfg.GUI.APIKey == "" { + cfg.GUI.APIKey = randomString(32) } } diff --git a/lib/config/config_test.go b/lib/config/config_test.go index a6bfe71c9..008402d26 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -485,7 +485,7 @@ func TestCopy(t *testing.T) { cfg.Devices[0].Addresses[0] = "wrong" cfg.Folders[0].Devices[0].DeviceID = protocol.DeviceID{0, 1, 2, 3} cfg.Options.ListenAddress[0] = "wrong" - cfg.GUI.RawAPIKey = "wrong" + cfg.GUI.APIKey = "wrong" bsChanged, err := json.MarshalIndent(cfg, "", " ") if err != nil { diff --git a/lib/config/guiconfiguration.go b/lib/config/guiconfiguration.go index 14b824577..fb6c3764c 100644 --- a/lib/config/guiconfiguration.go +++ b/lib/config/guiconfiguration.go @@ -18,7 +18,7 @@ type GUIConfiguration struct { User string `xml:"user,omitempty" json:"user"` Password string `xml:"password,omitempty" json:"password"` RawUseTLS bool `xml:"tls,attr" json:"useTLS"` - RawAPIKey string `xml:"apikey,omitempty" json:"apiKey"` + APIKey string `xml:"apikey,omitempty" json:"apiKey"` InsecureAdminAccess bool `xml:"insecureAdminAccess,omitempty" json:"insecureAdminAccess"` Theme string `xml:"theme" json:"theme" default:"default"` } @@ -76,14 +76,14 @@ func (c GUIConfiguration) URL() string { return u.String() } -// Returns whether the given API key is valid, including both the value in config -// and any overrides +// IsValidAPIKey returns true when the given API key is valid, including both +// the value in config and any overrides func (c GUIConfiguration) IsValidAPIKey(apiKey string) bool { switch apiKey { case "": return false - case c.RawAPIKey, os.Getenv("STGUIAPIKEY"): + case c.APIKey, os.Getenv("STGUIAPIKEY"): return true default: From a7a9d7d85c60f0b46c383da6d4c2d8738adfabb9 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 2 Feb 2016 12:40:42 +0100 Subject: [PATCH 6/6] Return correct content type for /rest/events --- cmd/syncthing/gui.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index de4150438..79f6585dc 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -893,8 +893,10 @@ func (s *apiService) getEvents(w http.ResponseWriter, r *http.Request) { s.fss.gotEventRequest() - // Flush before blocking, to indicate that we've received the request - // and that it should not be retried. + // Flush before blocking, to indicate that we've received the request and + // that it should not be retried. Must set Content-Type header before + // flushing. + w.Header().Set("Content-Type", "application/json; charset=utf-8") f := w.(http.Flusher) f.Flush()