From 93082b809267b8f543452a336f9d8e69c35348fb Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 23 Sep 2022 10:39:42 +0200 Subject: [PATCH 1/4] Protect against user injection for registration CLI page This commit addresses a potential issue where we allowed unsanitised content to be passed through a go template without validation. We now try to unmarshall the incoming node key and fails to render the template if it is not a valid node key. Signed-off-by: Kristoffer Dalby --- api.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/api.go b/api.go index f5de5038..23aa7c5a 100644 --- a/api.go +++ b/api.go @@ -9,6 +9,7 @@ import ( "github.com/gorilla/mux" "github.com/rs/zerolog/log" + "tailscale.com/types/key" ) const ( @@ -93,7 +94,18 @@ func (h *Headscale) RegisterWebAPI( ) { vars := mux.Vars(req) nodeKeyStr, ok := vars["nkey"] - if !ok || nodeKeyStr == "" { + + // We need to make sure we dont open for XSS style injections, if the parameter that + // is passed as a key is not parsable/validated as a NodePublic key, then fail to render + // the template and log an error. + var nodeKey key.NodePublic + err := nodeKey.UnmarshalText( + []byte(NodePublicKeyEnsurePrefix(nodeKeyStr)), + ) + + if !ok || nodeKeyStr == "" || err != nil { + log.Warn().Err(err).Msg("Failed to parse incoming nodekey") + writer.Header().Set("Content-Type", "text/plain; charset=utf-8") writer.WriteHeader(http.StatusBadRequest) _, err := writer.Write([]byte("Wrong params")) @@ -130,7 +142,7 @@ func (h *Headscale) RegisterWebAPI( writer.Header().Set("Content-Type", "text/html; charset=utf-8") writer.WriteHeader(http.StatusOK) - _, err := writer.Write(content.Bytes()) + _, err = writer.Write(content.Bytes()) if err != nil { log.Error(). Caller(). From 75a8fc8b3e503d1c9d5eaeb481b689351cbbc97e Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 23 Sep 2022 10:44:29 +0200 Subject: [PATCH 2/4] Update changelog Signed-off-by: Kristoffer Dalby --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 443305da..2b4f6260 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Give a warning when running Headscale with reverse proxy improperly configured for WebSockets [#788](https://github.com/juanfont/headscale/pull/788) - Fix subnet routers with Primary Routes [#811](https://github.com/juanfont/headscale/pull/811) - Added support for JSON logs [#653](https://github.com/juanfont/headscale/issues/653) +- Sanitise the node key passed to registration url [#823](https://github.com/juanfont/headscale/pull/823) ## 0.16.4 (2022-08-21) From 2bb34751d19dcdfdf188db437e93a75ed9b6c14a Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 23 Sep 2022 11:51:38 +0200 Subject: [PATCH 3/4] Validate the incoming nodekey with regex before attempting to parse Signed-off-by: Kristoffer Dalby --- api.go | 16 ++++++++++++++++ utils.go | 3 +++ 2 files changed, 19 insertions(+) diff --git a/api.go b/api.go index 23aa7c5a..4c3a6ca8 100644 --- a/api.go +++ b/api.go @@ -95,6 +95,22 @@ func (h *Headscale) RegisterWebAPI( vars := mux.Vars(req) nodeKeyStr, ok := vars["nkey"] + if !NodePublicKeyRegex.Match([]byte(nodeKeyStr)) { + log.Warn().Str("node_key", nodeKeyStr).Msg("Invalid node key passed to registration url") + + writer.Header().Set("Content-Type", "text/plain; charset=utf-8") + writer.WriteHeader(http.StatusUnauthorized) + _, err := writer.Write([]byte("Unauthorized")) + if err != nil { + log.Error(). + Caller(). + Err(err). + Msg("Failed to write response") + } + + return + } + // We need to make sure we dont open for XSS style injections, if the parameter that // is passed as a key is not parsable/validated as a NodePublic key, then fail to render // the template and log an error. diff --git a/utils.go b/utils.go index 666683cf..7ef054de 100644 --- a/utils.go +++ b/utils.go @@ -17,6 +17,7 @@ import ( "os" "path/filepath" "reflect" + "regexp" "strconv" "strings" @@ -64,6 +65,8 @@ const ( ZstdCompression = "zstd" ) +var NodePublicKeyRegex = regexp.MustCompile("nodekey:[a-fA-F0-9]+") + func MachinePublicKeyStripPrefix(machineKey key.MachinePublic) string { return strings.TrimPrefix(machineKey.String(), machinePublicHexPrefix) } From 8be14ef6feb160bf67f83be54624d0a83af95a16 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 23 Sep 2022 11:51:46 +0200 Subject: [PATCH 4/4] gofumpt Signed-off-by: Kristoffer Dalby --- utils.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/utils.go b/utils.go index 7ef054de..fc1fafa2 100644 --- a/utils.go +++ b/utils.go @@ -328,7 +328,9 @@ func GenerateRandomStringDNSSafe(size int) (string, error) { if err != nil { return "", err } - str = strings.ToLower(strings.ReplaceAll(strings.ReplaceAll(str, "_", ""), "-", "")) + str = strings.ToLower( + strings.ReplaceAll(strings.ReplaceAll(str, "_", ""), "-", ""), + ) } return str[:size], nil