Fix some security issues

* The CSS styles were leaking information about tunnels, even for
  things like the login page, which can be sent to anyone.
* Tokens could be created for any user by any user.
This commit is contained in:
Anders Pitman 2020-10-26 12:18:51 -06:00
parent 8e8045cde7
commit b3f1636be6
5 changed files with 84 additions and 90 deletions

22
api.go
View File

@ -160,6 +160,28 @@ func (a *Api) DeleteTunnel(tokenData TokenData, params url.Values) error {
return nil return nil
} }
func (a *Api) CreateToken(tokenData TokenData, params url.Values) (string, error) {
owner := params.Get("owner")
if owner == "" {
return "", errors.New("Invalid owner paramater")
}
if tokenData.Owner != owner {
user, _ := a.db.GetUser(tokenData.Owner)
if !user.IsAdmin {
return "", errors.New("Unauthorized")
}
}
token, err := a.db.AddToken(owner)
if err != nil {
return "", errors.New("Failed to create token")
}
return token, nil
}
func (a *Api) handleTunnels(w http.ResponseWriter, r *http.Request) { func (a *Api) handleTunnels(w http.ResponseWriter, r *http.Request) {
token, err := extractToken("access_token", r) token, err := extractToken("access_token", r)

View File

@ -1,7 +1,7 @@
# 31 Oct 2020 Launch List # 31 Oct 2020 Launch List
- [ ] I think it's possible to create tokens for arbitrary user, even if you're not that user. - [ ] I think it's possible to create tokens for arbitrary user, even if you're not that user.
- [ ] Responses to unauthorized requests are leaking information about the current tunnels through the genereated CSS. - [ ] QR codes for admin are broken
- [ ] General security review. - [ ] General security review.
- [ ] Invalid database is wiping out tunnels - [ ] Invalid database is wiping out tunnels
- [ ] Improve SSH key download UI. - [ ] Improve SSH key download UI.
@ -11,6 +11,8 @@
- [ ] Demo video - [ ] Demo video
- [ ] Demo auto email signup - [ ] Demo auto email signup
- [ ] Post on /r/selfhosted - [ ] Post on /r/selfhosted
- [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 genereated CSS.
# Eventually # Eventually

View File

@ -72,10 +72,6 @@ type HeadData struct {
Styles template.CSS Styles template.CSS
} }
type StylesData struct {
Tunnels map[string]Tunnel
}
type MenuData struct { type MenuData struct {
IsAdmin bool IsAdmin bool
} }
@ -107,6 +103,39 @@ func (h *WebUiHandler) handleWebUiRequest(w http.ResponseWriter, r *http.Request
homePath := "/#/tunnel" homePath := "/#/tunnel"
// Note: h.box and h.headHtml need to be ready before pretty much
// everything else, including sendLoginPage
box, err := rice.FindBox("webui")
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error opening webui")
return
}
h.box = box
stylesText, err := box.String("styles.css")
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error reading styles.css")
return
}
headTmplStr, err := box.String("head.tmpl")
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error reading head.tmpl")
return
}
headTmpl, err := template.New("head").Parse(headTmplStr)
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error compiling head.tmpl")
return
}
var headBuilder strings.Builder
headTmpl.Execute(&headBuilder, HeadData{Styles: template.CSS(stylesText)})
h.headHtml = template.HTML(headBuilder.String())
token, err := extractToken("access_token", r) token, err := extractToken("access_token", r)
if err != nil { if err != nil {
h.sendLoginPage(w, r, 401) h.sendLoginPage(w, r, 401)
@ -121,29 +150,6 @@ func (h *WebUiHandler) handleWebUiRequest(w http.ResponseWriter, r *http.Request
user, _ := h.db.GetUser(tokenData.Owner) user, _ := h.db.GetUser(tokenData.Owner)
box, err := rice.FindBox("webui")
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error opening webui")
return
}
h.box = box
stylesText, err := box.String("styles.css")
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error reading styles.css")
return
}
stylesTmpl, err := template.New("styles").Parse(stylesText)
if err != nil {
w.WriteHeader(500)
h.alertDialog(w, r, err.Error(), homePath)
return
}
tunnels := h.api.GetTunnels(tokenData) tunnels := h.api.GetTunnels(tokenData)
for domain, tun := range tunnels { for domain, tun := range tunnels {
@ -152,28 +158,6 @@ func (h *WebUiHandler) handleWebUiRequest(w http.ResponseWriter, r *http.Request
tunnels[domain] = tun tunnels[domain] = tun
} }
var stylesBuilder strings.Builder
stylesTmpl.Execute(&stylesBuilder, StylesData{Tunnels: tunnels})
headTmplStr, err := box.String("head.tmpl")
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error reading head.tmpl")
return
}
headTmpl, err := template.New("head").Parse(headTmplStr)
if err != nil {
w.WriteHeader(500)
io.WriteString(w, "Error compiling head.tmpl")
return
}
var headBuilder strings.Builder
headTmpl.Execute(&headBuilder, HeadData{Styles: template.CSS(stylesBuilder.String())})
h.headHtml = template.HTML(headBuilder.String())
switch r.URL.Path { switch r.URL.Path {
case "/login": case "/login":
h.handleLogin(w, r) h.handleLogin(w, r)
@ -396,26 +380,10 @@ func (h *WebUiHandler) handleTokens(w http.ResponseWriter, r *http.Request, user
r.ParseForm() r.ParseForm()
if len(r.Form["owner"]) != 1 { _, err := h.api.CreateToken(tokenData, r.Form)
w.WriteHeader(400)
h.alertDialog(w, r, "Invalid owner parameter", "/#/tokens")
return
}
owner := r.Form["owner"][0]
users := h.db.GetUsers()
_, exists := users[owner]
if !exists {
w.WriteHeader(400)
h.alertDialog(w, r, "Owner doesn't exist", "/#/tokens")
return
}
_, err := h.db.AddToken(owner)
if err != nil { if err != nil {
w.WriteHeader(500) w.WriteHeader(500)
h.alertDialog(w, r, "Failed creating token", "/#/tokens") h.alertDialog(w, r, err.Error(), "/#/tokens")
return return
} }
@ -578,7 +546,7 @@ func (h *WebUiHandler) sendLoginPage(w http.ResponseWriter, r *http.Request, cod
return return
} }
loginTemplate, err := template.New("test").Parse(loginTemplateStr) loginTemplate, err := template.New("login").Parse(loginTemplateStr)
if err != nil { if err != nil {
w.WriteHeader(500) w.WriteHeader(500)
io.WriteString(w, "Error compiling login.tmpl") io.WriteString(w, "Error compiling login.tmpl")

View File

@ -2,6 +2,29 @@
<html> <html>
<head> <head>
{{.Head}} {{.Head}}
<style>
{{range $domain, $tunnel:= .Tunnels}}
#toggle-tunnel-delete-dialog-{{$tunnel.CssId}} {
display: none;
}
#toggle-tunnel-delete-dialog-{{$tunnel.CssId}}:checked + .dialog {
display: block;
}
#toggle-tunnel-hide-deleted-{{$tunnel.CssId}}:checked + .list-item {
/* This is a trick to make the delete request after the delete button is
* clicked. The background will never actually be displayed, because it's
* moved offscreen. */
position: absolute;
left: -999em;
background: url("/delete-tunnel?domain={{$domain}}");
}
#toggle-tunnel-hide-deleted-{{$tunnel.CssId}}:checked ~ .dialog {
display: none;
}
{{end}}
</style>
</head> </head>
<body> <body>

View File

@ -229,27 +229,6 @@ main *:target {
z-index: 1010; z-index: 1010;
} }
{{range $domain, $tunnel:= .Tunnels}}
#toggle-tunnel-delete-dialog-{{$tunnel.CssId}} {
display: none;
}
#toggle-tunnel-delete-dialog-{{$tunnel.CssId}}:checked + .dialog {
display: block;
}
#toggle-tunnel-hide-deleted-{{$tunnel.CssId}}:checked + .list-item {
/* This is a trick to make the delete request after the delete button is
* clicked. The background will never actually be displayed, because it's
* moved offscreen. */
position: absolute;
left: -999em;
background: url("/delete-tunnel?domain={{$domain}}");
}
#toggle-tunnel-hide-deleted-{{$tunnel.CssId}}:checked ~ .dialog {
display: none;
}
{{end}}
@media (min-width: 640px) { @media (min-width: 640px) {
main { main {