Просмотр исходного кода

bugfix: #173 Websocket fixes 1) The upgrader was refusing reverse proxies, 2) The upgrader was "listening" on /, not /websocket (#177)

James Read 2 лет назад
Родитель
Сommit
3db8ae53b5
2 измененных файлов с 27 добавлено и 8 удалено
  1. 7 7
      internal/httpservers/singleFrontend.go
  2. 20 1
      internal/websocket/websocket.go

+ 7 - 7
internal/httpservers/singleFrontend.go

@@ -15,7 +15,6 @@ import (
 	"net/http"
 	"net/http/httputil"
 	"net/url"
-	"strings"
 )
 
 // StartSingleHTTPFrontend will create a reverse proxy that proxies the API
@@ -38,13 +37,14 @@ func StartSingleHTTPFrontend(cfg *config.Config) {
 		apiProxy.ServeHTTP(w, r)
 	})
 
+	mux.HandleFunc("/websocket", func(w http.ResponseWriter, r *http.Request) {
+		log.Debugf("websocket req: %q", r.URL)
+		websocket.HandleWebsocket(w, r)
+	})
+
 	mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
-		if strings.Contains(r.Header.Get("Connection"), "Upgrade") {
-			websocket.HandleWebsocket(w, r)
-		} else {
-			log.Debugf("ui req: %q", r.URL)
-			webuiProxy.ServeHTTP(w, r)
-		}
+		log.Debugf("ui req: %q", r.URL)
+		webuiProxy.ServeHTTP(w, r)
 	})
 
 	srv := &http.Server{

+ 20 - 1
internal/websocket/websocket.go

@@ -9,7 +9,9 @@ import (
 	"net/http"
 )
 
-var upgrader = ws.Upgrader{}
+var upgrader = ws.Upgrader{
+	CheckOrigin: checkOriginPermissive,
+}
 
 type WebsocketClient struct {
 	conn *ws.Conn
@@ -35,6 +37,23 @@ func (WebsocketExecutionListener) OnExecutionStarted(title string) {
 	*/
 }
 
+/*
+The default checkOrigin function checks that the origin (browser) matches the
+request origin. However in OliveTin we expect many users to deliberately proxy
+the connection with reverse proxies.
+
+So, we just permit any origin. After some searching I'm not sure if this exposes
+OliveTin to security issues, but it seems probably not. It would be possible to
+create a config option like PermitWebsocketConnectionsFrom or something, but
+I'd prefer if OliveTin works as much as possible "out of the box".
+
+If this does expose OliveTin to security issues, it will be changed in the
+future obviously.
+*/
+func checkOriginPermissive(r *http.Request) bool {
+	return true
+}
+
 func (WebsocketExecutionListener) OnExecutionFinished(logEntry *executor.InternalLogEntry) {
 	le := &pb.LogEntry{
 		ActionTitle:       logEntry.ActionTitle,