Skip to content

Commit 81fe0bd

Browse files
DevinCarrDevinCarr
committed
TUN-7628: Correct Host parsing for Access
Will no longer provide full hostname with path from provided `--hostname` flag for cloudflared access to the Host header field. This addresses certain issues caught from a security fix in go 1.19.11 and 1.20.6 in the net/http URL parsing.
1 parent bfeaa34 commit 81fe0bd

File tree

85 files changed

+22869
-4438
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

85 files changed

+22869
-4438
lines changed

‎cmd/cloudflared/access/carrier.go‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,14 @@ func ssh(c *cli.Context) error {
7070

7171
// get the hostname from the cmdline and error out if its not provided
7272
rawHostName := c.String(sshHostnameFlag)
73-
hostname, err := validation.ValidateHostname(rawHostName)
74-
if err != nil || rawHostName == "" {
73+
url, err := parseURL(rawHostName)
74+
if err != nil {
75+
log.Err(err).Send()
7576
return cli.ShowCommandHelp(c, "ssh")
7677
}
77-
originURL := ensureURLScheme(hostname)
7878

7979
// get the headers from the cmdline and add them
80-
headers := buildRequestHeaders(c.StringSlice(sshHeaderFlag))
80+
headers := parseRequestHeaders(c.StringSlice(sshHeaderFlag))
8181
if c.IsSet(sshTokenIDFlag) {
8282
headers.Set(cfAccessClientIDHeader, c.String(sshTokenIDFlag))
8383
}
@@ -89,9 +89,9 @@ func ssh(c *cli.Context) error {
8989
carrier.SetBastionDest(headers, c.String(sshDestinationFlag))
9090

9191
options := &carrier.StartOptions{
92-
OriginURL: originURL,
92+
OriginURL: url.String(),
9393
Headers: headers,
94-
Host: hostname,
94+
Host: url.Host,
9595
}
9696

9797
if connectTo := c.String(sshConnectTo); connectTo != "" {
@@ -138,20 +138,9 @@ func ssh(c *cli.Context) error {
138138
// default to 10 if provided but unset
139139
maxMessages = 10
140140
}
141-
logger := log.With().Str("host", hostname).Logger()
141+
logger := log.With().Str("host", url.Host).Logger()
142142
s = stream.NewDebugStream(s, &logger, maxMessages)
143143
}
144144
carrier.StartClient(wsConn, s, options)
145145
return nil
146146
}
147-
148-
func buildRequestHeaders(values []string) http.Header {
149-
headers := make(http.Header)
150-
for _, valuePair := range values {
151-
header, value, found := strings.Cut(valuePair, ":")
152-
if found {
153-
headers.Add(strings.TrimSpace(header), strings.TrimSpace(value))
154-
}
155-
}
156-
return headers
157-
}

‎cmd/cloudflared/access/carrier_test.go‎

Lines changed: 0 additions & 19 deletions
This file was deleted.

‎cmd/cloudflared/access/cmd.go‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,7 @@ func login(c *cli.Context) error {
222222
log := logger.CreateLoggerFromContext(c, logger.EnableTerminalLog)
223223

224224
args := c.Args()
225-
rawURL := ensureURLScheme(args.First())
226-
appURL, err := url.Parse(rawURL)
225+
appURL, err := parseURL(args.First())
227226
if args.Len() < 1 || err != nil {
228227
log.Error().Msg("Please provide the url of the Access application")
229228
return err
@@ -252,16 +251,6 @@ func login(c *cli.Context) error {
252251
return nil
253252
}
254253

255-
// ensureURLScheme prepends a URL with https:// if it doesn't have a scheme. http:// URLs will not be converted.
256-
func ensureURLScheme(url string) string {
257-
url = strings.Replace(strings.ToLower(url), "http://", "https://", 1)
258-
if !strings.HasPrefix(url, "https://") {
259-
url = fmt.Sprintf("https://%s", url)
260-
261-
}
262-
return url
263-
}
264-
265254
// curl provides a wrapper around curl, passing Access JWT along in request
266255
func curl(c *cli.Context) error {
267256
err := sentry.Init(sentry.ClientOptions{
@@ -345,7 +334,7 @@ func generateToken(c *cli.Context) error {
345334
if err != nil {
346335
return err
347336
}
348-
appURL, err := url.Parse(ensureURLScheme(c.String("app")))
337+
appURL, err := parseURL(c.String("app"))
349338
if err != nil || c.NumFlags() < 1 {
350339
fmt.Fprintln(os.Stderr, "Please provide a url.")
351340
return err
@@ -398,7 +387,7 @@ func sshGen(c *cli.Context) error {
398387
return cli.ShowCommandHelp(c, "ssh-gen")
399388
}
400389

401-
originURL, err := url.Parse(ensureURLScheme(hostname))
390+
originURL, err := parseURL(hostname)
402391
if err != nil {
403392
return err
404393
}
@@ -499,7 +488,7 @@ func isFileThere(candidate string) bool {
499488
// Then makes a request to to the origin with the token to ensure it is valid.
500489
// Returns nil if token is valid.
501490
func verifyTokenAtEdge(appUrl *url.URL, appInfo *token.AppInfo, c *cli.Context, log *zerolog.Logger) error {
502-
headers := buildRequestHeaders(c.StringSlice(sshHeaderFlag))
491+
headers := parseRequestHeaders(c.StringSlice(sshHeaderFlag))
503492
if c.IsSet(sshTokenIDFlag) {
504493
headers.Add(cfAccessClientIDHeader, c.String(sshTokenIDFlag))
505494
}

‎cmd/cloudflared/access/cmd_test.go‎

Lines changed: 0 additions & 25 deletions
This file was deleted.

‎cmd/cloudflared/access/validation.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package access
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"net/http"
7+
"net/url"
8+
"strings"
9+
10+
"golang.org/x/net/http/httpguts"
11+
)
12+
13+
// parseRequestHeaders will take user-provided header values as strings "Content-Type: application/json" and create
14+
// a http.Header object.
15+
func parseRequestHeaders(values []string) http.Header {
16+
headers := make(http.Header)
17+
for _, valuePair := range values {
18+
header, value, found := strings.Cut(valuePair, ":")
19+
if found {
20+
headers.Add(strings.TrimSpace(header), strings.TrimSpace(value))
21+
}
22+
}
23+
return headers
24+
}
25+
26+
// parseHostname will attempt to convert a user provided URL string into a string with some light error checking on
27+
// certain expectations from the URL.
28+
// Will convert all HTTP URLs to HTTPS
29+
func parseURL(input string) (*url.URL, error) {
30+
if input == "" {
31+
return nil, errors.New("no input provided")
32+
}
33+
if !strings.HasPrefix(input, "https://") && !strings.HasPrefix(input, "http://") {
34+
input = fmt.Sprintf("https://%s", input)
35+
}
36+
url, err := url.ParseRequestURI(input)
37+
if err != nil {
38+
return nil, fmt.Errorf("failed to parse as URL: %w", err)
39+
}
40+
if url.Scheme != "https" {
41+
url.Scheme = "https"
42+
}
43+
if url.Host == "" {
44+
return nil, errors.New("failed to parse Host")
45+
}
46+
host, err := httpguts.PunycodeHostPort(url.Host)
47+
if err != nil || host == "" {
48+
return nil, err
49+
}
50+
if !httpguts.ValidHostHeader(host) {
51+
return nil, errors.New("invalid Host provided")
52+
}
53+
url.Host = host
54+
return url, nil
55+
}

‎cmd/cloudflared/access/validation_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package access
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestParseRequestHeaders(t *testing.T) {
11+
values := parseRequestHeaders([]string{"client: value", "secret: safe-value", "trash", "cf-trace-id: 000:000:0:1:asd"})
12+
assert.Len(t, values, 3)
13+
assert.Equal(t, "value", values.Get("client"))
14+
assert.Equal(t, "safe-value", values.Get("secret"))
15+
assert.Equal(t, "000:000:0:1:asd", values.Get("cf-trace-id"))
16+
}
17+
18+
func TestParseURL(t *testing.T) {
19+
schemes := []string{
20+
"http://",
21+
"https://",
22+
"",
23+
}
24+
hosts := []struct {
25+
input string
26+
expected string
27+
}{
28+
{"localhost", "localhost"},
29+
{"127.0.0.1", "127.0.0.1"},
30+
{"127.0.0.1:9090", "127.0.0.1:9090"},
31+
{"::1", "::1"},
32+
{"::1:8080", "::1:8080"},
33+
{"[::1]", "[::1]"},
34+
{"[::1]:8080", "[::1]:8080"},
35+
{":8080", ":8080"},
36+
{"example.com", "example.com"},
37+
{"hello.example.com", "hello.example.com"},
38+
{"bücher.example.com", "xn--bcher-kva.example.com"},
39+
}
40+
paths := []string{
41+
"",
42+
"/test",
43+
"/example.com?qwe=123",
44+
}
45+
for i, scheme := range schemes {
46+
for j, host := range hosts {
47+
for k, path := range paths {
48+
t.Run(fmt.Sprintf("%d_%d_%d", i, j, k), func(t *testing.T) {
49+
input := fmt.Sprintf("%s%s%s", scheme, host.input, path)
50+
expected := fmt.Sprintf("%s%s%s", "https://", host.expected, path)
51+
url, err := parseURL(input)
52+
assert.NoError(t, err, "input: %s\texpected: %s", input, expected)
53+
assert.Equal(t, expected, url.String())
54+
assert.Equal(t, host.expected, url.Host)
55+
assert.Equal(t, "https", url.Scheme)
56+
})
57+
}
58+
}
59+
}
60+
61+
t.Run("no input", func(t *testing.T) {
62+
_, err := parseURL("")
63+
assert.ErrorContains(t, err, "no input provided")
64+
})
65+
66+
t.Run("missing host", func(t *testing.T) {
67+
_, err := parseURL("https:///host")
68+
assert.ErrorContains(t, err, "failed to parse Host")
69+
})
70+
71+
t.Run("invalid path only", func(t *testing.T) {
72+
_, err := parseURL("/host")
73+
assert.ErrorContains(t, err, "failed to parse Host")
74+
})
75+
76+
t.Run("invalid parse URL", func(t *testing.T) {
77+
_, err := parseURL("https://host\\host")
78+
assert.ErrorContains(t, err, "failed to parse as URL")
79+
})
80+
}

‎go.mod‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ require (
3636
go.opentelemetry.io/otel/trace v1.6.3
3737
go.opentelemetry.io/proto/otlp v0.15.0
3838
go.uber.org/automaxprocs v1.4.0
39-
golang.org/x/crypto v0.8.0
40-
golang.org/x/net v0.9.0
39+
golang.org/x/crypto v0.11.0
40+
golang.org/x/net v0.12.0
4141
golang.org/x/sync v0.1.0
42-
golang.org/x/sys v0.7.0
43-
golang.org/x/term v0.7.0
42+
golang.org/x/sys v0.10.0
43+
golang.org/x/term v0.10.0
4444
google.golang.org/protobuf v1.28.1
4545
gopkg.in/natefinch/lumberjack.v2 v2.0.0
4646
gopkg.in/yaml.v3 v3.0.1
@@ -91,7 +91,7 @@ require (
9191
golang.org/x/exp v0.0.0-20221205204356-47842c84f3db // indirect
9292
golang.org/x/mod v0.8.0 // indirect
9393
golang.org/x/oauth2 v0.6.0 // indirect
94-
golang.org/x/text v0.9.0 // indirect
94+
golang.org/x/text v0.11.0 // indirect
9595
golang.org/x/tools v0.6.0 // indirect
9696
google.golang.org/appengine v1.6.7 // indirect
9797
google.golang.org/genproto v0.0.0-20221202195650-67e5cbc046fd // indirect

‎go.sum‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -404,8 +404,8 @@ golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7/go.mod h1:yigFU9vqHzYiE8U
404404
golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
405405
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
406406
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
407-
golang.org/x/crypto v0.8.0 h1:pd9TJtTueMTVQXzk8E2XESSMQDj/U7OUu0PqJqPXQjQ=
408-
golang.org/x/crypto v0.8.0/go.mod h1:mRqEX+O9/h5TFCrQhkgjo2yKi0yYA+9ecGkdQoHrywE=
407+
golang.org/x/crypto v0.11.0 h1:6Ewdq3tDic1mg5xRO4milcWCfMVQhI4NkqWWvqejpuA=
408+
golang.org/x/crypto v0.11.0/go.mod h1:xgJhtzW8F9jGdVFWZESrid1U1bjeNy4zgy5cRr/CIio=
409409
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
410410
golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
411411
golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
@@ -475,8 +475,8 @@ golang.org/x/net v0.0.0-20210726213435-c6fcb2dbf985/go.mod h1:9nx3DQGgdP8bBQD5qx
475475
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
476476
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
477477
golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
478-
golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM=
479-
golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
478+
golang.org/x/net v0.12.0 h1:cfawfvKITfUsFCeJIHJrbSxpeu/E81khclypR0GVT50=
479+
golang.org/x/net v0.12.0/go.mod h1:zEVYFnQC7m/vmpQFELhcD1EWkZlX69l4oqgmer6hfKA=
480480
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
481481
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
482482
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@@ -545,12 +545,12 @@ golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBc
545545
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
546546
golang.org/x/sys v0.0.0-20220808155132-1c4a2a72c664/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
547547
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
548-
golang.org/x/sys v0.7.0 h1:3jlCCIQZPdOYu1h8BkNvLz8Kgwtae2cagcG/VamtZRU=
549-
golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
548+
golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA=
549+
golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
550550
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
551551
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
552-
golang.org/x/term v0.7.0 h1:BEvjmm5fURWqcfbSKTdpkDXYBrUS1c0m8agp14W48vQ=
553-
golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY=
552+
golang.org/x/term v0.10.0 h1:3R7pNqamzBraeqj/Tj8qt1aQ2HpmlC+Cx/qL/7hn4/c=
553+
golang.org/x/term v0.10.0/go.mod h1:lpqdcUyK/oCiQxvxVrppt5ggO2KCZ5QblwqPnfZ6d5o=
554554
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
555555
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
556556
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@@ -559,8 +559,8 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
559559
golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
560560
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
561561
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
562-
golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE=
563-
golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
562+
golang.org/x/text v0.11.0 h1:LAntKIrcmeSKERyiOh0XMV39LXS8IE9UL2yP7+f5ij4=
563+
golang.org/x/text v0.11.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE=
564564
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
565565
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
566566
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=

0 commit comments

Comments
 (0)