oauthutil: fix security problem when running with two users on the same machine

Before this change two users could run `rclone config` for the same
backend on the same machine at the same time.

User A would get as far as starting the web server.  User B would then
fail to start the webserver, but it would open the browser on the
/auth URL which would redirect the user to the login.  This would then
cause user B to authenticate to user A's rclone.

This changes fixes the problem in two ways.

Firstly it passes the state to the /auth call before redirecting and
checks it there, erroring with a 403 error if it doesn't match.  This
would have fixed the problem on its own.

Secondly it delays the opening of the web browser until after the auth
webserver has started which prevents the user entering the credentials
if another auth server is running.

Fixes #3573
This commit is contained in:
Nick Craig-Wood 2019-09-27 14:59:56 +01:00
parent fef8b98be2
commit 349112df6b

View File

@ -6,7 +6,6 @@ import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"html/template" "html/template"
"log"
"net" "net"
"net/http" "net/http"
"strings" "strings"
@ -445,9 +444,13 @@ func doConfig(id, name string, m configmap.Mapper, errorHandler func(*http.Reque
if useWebServer { if useWebServer {
server.code = make(chan string, 1) server.code = make(chan string, 1)
server.err = make(chan error, 1) server.err = make(chan error, 1)
go server.Start() err := server.Init()
if err != nil {
return errors.Wrap(err, "failed to start auth webserver")
}
go server.Serve()
defer server.Stop() defer server.Stop()
authURL = "http://" + bindAddress + "/auth" authURL = "http://" + bindAddress + "/auth?state=" + state
} }
// Generate a URL for the user to visit for authorization. // Generate a URL for the user to visit for authorization.
@ -517,8 +520,8 @@ type AuthResponseData struct {
AuthError AuthError
} }
// startWebServer runs an internal web server to receive config details // Init gets the internal web server ready to receive config details
func (s *authServer) Start() { func (s *authServer) Init() error {
fs.Debugf(nil, "Starting auth server on %s", s.bindAddress) fs.Debugf(nil, "Starting auth server on %s", s.bindAddress)
mux := http.NewServeMux() mux := http.NewServeMux()
s.server = &http.Server{ s.server = &http.Server{
@ -531,6 +534,12 @@ func (s *authServer) Start() {
return return
}) })
mux.HandleFunc("/auth", func(w http.ResponseWriter, req *http.Request) { mux.HandleFunc("/auth", func(w http.ResponseWriter, req *http.Request) {
state := req.FormValue("state")
if state != s.state {
fs.Debugf(nil, "State did not match: want %q got %q", s.state, state)
http.Error(w, "State did not match - please try again", 403)
return
}
http.Redirect(w, req, s.authURL, http.StatusTemporaryRedirect) http.Redirect(w, req, s.authURL, http.StatusTemporaryRedirect)
return return
}) })
@ -585,12 +594,18 @@ func (s *authServer) Start() {
var err error var err error
s.listener, err = net.Listen("tcp", s.bindAddress) s.listener, err = net.Listen("tcp", s.bindAddress)
if err != nil { if err != nil {
log.Fatalf("Failed to start auth webserver: %v", err) return err
} }
err = s.server.Serve(s.listener) return nil
}
// Serve the auth server, doesn't return
func (s *authServer) Serve() {
err := s.server.Serve(s.listener)
fs.Debugf(nil, "Closed auth server with error: %v", err) fs.Debugf(nil, "Closed auth server with error: %v", err)
} }
// Stop the auth server by closing its socket
func (s *authServer) Stop() { func (s *authServer) Stop() {
fs.Debugf(nil, "Closing auth server") fs.Debugf(nil, "Closing auth server")
if s.code != nil { if s.code != nil {