From 4fd830167f60b89ccc4746a632d736414cc15c1b Mon Sep 17 00:00:00 2001 From: Anders Pitman Date: Tue, 27 Oct 2020 15:21:56 -0600 Subject: [PATCH] Fix more security holes --- api.go | 84 ++++++++++++++---------------------------------- client.go | 7 +++- todo.md | 5 ++- ui_handler.go | 1 + webui/index.tmpl | 1 + 5 files changed, 36 insertions(+), 62 deletions(-) diff --git a/api.go b/api.go index ec0a4ec..da8fc1b 100644 --- a/api.go +++ b/api.go @@ -83,6 +83,18 @@ func (a *Api) CreateTunnel(tokenData TokenData, params url.Values) (*Tunnel, err return nil, errors.New("Invalid domain parameter") } + owner := params.Get("owner") + if owner == "" { + return nil, errors.New("Invalid owner parameter") + } + + if tokenData.Owner != owner { + user, _ := a.db.GetUser(tokenData.Owner) + if !user.IsAdmin { + return nil, errors.New("Unauthorized") + } + } + sshKeyId := params.Get("ssh-key-id") var sshKey SshKey @@ -263,73 +275,25 @@ func (a *Api) handleTunnels(w http.ResponseWriter, r *http.Request) { w.Write([]byte(body)) case "POST": - a.handleCreateTunnel(w, r) + r.ParseForm() + _, err := a.CreateTunnel(tokenData, r.Form) + if err != nil { + w.WriteHeader(500) + w.Write([]byte(err.Error())) + } case "DELETE": - a.handleDeleteTunnel(w, r) + r.ParseForm() + err := a.DeleteTunnel(tokenData, r.Form) + if err != nil { + w.WriteHeader(500) + w.Write([]byte(err.Error())) + } default: w.WriteHeader(405) w.Write([]byte("Invalid method for /tunnels")) } } -func (a *Api) handleCreateTunnel(w http.ResponseWriter, r *http.Request) { - query := r.URL.Query() - - if len(query["domain"]) != 1 { - w.WriteHeader(400) - w.Write([]byte("Invalid domain parameter")) - return - } - domain := query["domain"][0] - - if len(query["owner"]) != 1 { - w.WriteHeader(400) - w.Write([]byte("Invalid owner parameter")) - return - } - owner := query["owner"][0] - - request := Tunnel{ - Domain: domain, - Owner: owner, - } - - tunnel, err := a.tunMan.RequestCreateTunnel(request) - if err != nil { - w.WriteHeader(400) - io.WriteString(w, err.Error()) - return - } - - tunnelJson, err := json.MarshalIndent(tunnel, "", " ") - if err != nil { - w.WriteHeader(500) - io.WriteString(w, "Error encoding tunnel") - return - } - - w.Write(tunnelJson) -} - -func (a *Api) handleDeleteTunnel(w http.ResponseWriter, r *http.Request) { - - query := r.URL.Query() - - if len(query["domain"]) != 1 { - w.WriteHeader(400) - w.Write([]byte("Invalid domain parameter")) - return - } - domain := query["domain"][0] - - err := a.tunMan.DeleteTunnel(domain) - if err != nil { - w.WriteHeader(500) - io.WriteString(w, "Failed to delete tunnel") - return - } -} - func (a *Api) GetSshKeys(tokenData TokenData) map[string]SshKey { user, _ := a.db.GetUser(tokenData.Owner) diff --git a/client.go b/client.go index 377507e..1556748 100644 --- a/client.go +++ b/client.go @@ -85,6 +85,9 @@ func (c *BoringProxyClient) RunPuppetClient() { } func (c *BoringProxyClient) PollTunnels() error { + + log.Println("PollTunnels") + url := fmt.Sprintf("https://%s/api/tunnels?client-name=%s", c.server, c.clientName) listenReq, err := http.NewRequest("GET", url, nil) @@ -128,7 +131,7 @@ func (c *BoringProxyClient) PollTunnels() error { } func (c *BoringProxyClient) SyncTunnels(serverTunnels map[string]Tunnel) { - fmt.Println("SyncTunnels") + log.Println("SyncTunnels") // update tunnels to match server for k, newTun := range serverTunnels { @@ -174,6 +177,8 @@ func (c *BoringProxyClient) SyncTunnels(serverTunnels map[string]Tunnel) { func (c *BoringProxyClient) BoreTunnel(tunnel Tunnel) context.CancelFunc { + log.Println("BoreTunnel", tunnel.Domain) + ctx, cancelFunc := context.WithCancel(context.Background()) go func() { diff --git a/todo.md b/todo.md index 7aebf65..f9ce0e8 100644 --- a/todo.md +++ b/todo.md @@ -1,6 +1,5 @@ # 31 Oct 2020 Launch List -- [ ] General security review. - [ ] Invalid database is wiping out tunnels - [ ] Improve SSH key download UI. - [ ] Improve token list UI. @@ -12,6 +11,10 @@ - [x] Head can be rendered before h.headHtml is ever set, ie if login page is visited before any other page - [x] Responses to unauthorized requests are leaking information about the current tunnels through the generated CSS. - [x] I think it's possible to create tokens for arbitrary user, even if you're not that user. +- [x] Anyone can delete tunnels +- [x] Anyone can delete tokens +- [x] QR codes for admin are broken +- [x] General security review. # Eventually diff --git a/ui_handler.go b/ui_handler.go index ad3740c..e1c565b 100644 --- a/ui_handler.go +++ b/ui_handler.go @@ -212,6 +212,7 @@ func (h *WebUiHandler) handleWebUiRequest(w http.ResponseWriter, r *http.Request var tokens map[string]TokenData var users map[string]User + // TODO: handle security checks in api if user.IsAdmin { tokens = h.db.GetTokens() users = h.db.GetUsers() diff --git a/webui/index.tmpl b/webui/index.tmpl index 4e86fbc..78bf781 100644 --- a/webui/index.tmpl +++ b/webui/index.tmpl @@ -87,6 +87,7 @@
+