Skip to content

Commit

Permalink
Merge pull request #823 from kradalby/sanitise-machine-key-url
Browse files Browse the repository at this point in the history
Protect against user injection for registration CLI page
  • Loading branch information
juanfont authored Oct 4, 2022
2 parents c00e559 + 5333df2 commit d575dac
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
- Add support for generating pre-auth keys with tags [#767](https://github.com/juanfont/headscale/pull/767)
- Add support for evaluating `autoApprovers` ACL entries when a machine is registered [#763](https://github.com/juanfont/headscale/pull/763)
- Add config flag to allow Headscale to start if OIDC provider is down [#829](https://github.com/juanfont/headscale/pull/829)
Expand Down
32 changes: 30 additions & 2 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/gorilla/mux"
"github.com/rs/zerolog/log"
"tailscale.com/types/key"
)

const (
Expand Down Expand Up @@ -93,7 +94,34 @@ func (h *Headscale) RegisterWebAPI(
) {
vars := mux.Vars(req)
nodeKeyStr, ok := vars["nkey"]
if !ok || nodeKeyStr == "" {

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.
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"))
Expand Down Expand Up @@ -130,7 +158,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().
Expand Down
7 changes: 6 additions & 1 deletion utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"os"
"path/filepath"
"reflect"
"regexp"
"strconv"
"strings"

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -325,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
Expand Down

0 comments on commit d575dac

Please sign in to comment.