Skip to content

(v6) Fix app access websockets support #6028

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 19, 2021
Merged

(v6) Fix app access websockets support #6028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
13f4b02
Fix app access websockets support
r0mant Mar 6, 2021
f240877
Merge branch 'branch/v6' into roman/ws
r0mant Mar 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ require (
github.com/gravitational/form v0.0.0-20151109031454-c4048f792f70
github.com/gravitational/kingpin v2.1.11-0.20190130013101-742f2714c145+incompatible
github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a
github.com/gravitational/oxy v0.0.0-20200916204440-3eb06d921a1d
github.com/gravitational/oxy v0.0.0-20210316180922-c73d80d27348
github.com/gravitational/reporting v0.0.0-20180907002058-ac7b85c75c4c
github.com/gravitational/roundtrip v1.0.0
github.com/gravitational/teleport/api v0.0.0
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,10 @@ github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a h1:PN5vAN1ZA
github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a/go.mod h1:jaxS7X2ouXfNd2Pxpybd01qNQK15UmkixKj4vtpp7f8=
github.com/gravitational/oxy v0.0.0-20200916204440-3eb06d921a1d h1:IsbTjCQ4u5mr30ceWZ4GNcrQkp/Y/J9G+s9prmJm1ac=
github.com/gravitational/oxy v0.0.0-20200916204440-3eb06d921a1d/go.mod h1:ESOxlf8BB2yG3zJ0SfZe9U6wpYu3YF3znxIICg73FYA=
github.com/gravitational/oxy v0.0.0-20210315213158-2e1c3a1fc5c7 h1:AE15exmSIKcYvwfQRTeCRTOZ5oZlb0BxNe64DB4oPac=
github.com/gravitational/oxy v0.0.0-20210315213158-2e1c3a1fc5c7/go.mod h1:ESOxlf8BB2yG3zJ0SfZe9U6wpYu3YF3znxIICg73FYA=
github.com/gravitational/oxy v0.0.0-20210316180922-c73d80d27348 h1:QFHxHmxOeQT+MwFjNHkTVI7mkgPIabTMqkmHG5SiP2Y=
github.com/gravitational/oxy v0.0.0-20210316180922-c73d80d27348/go.mod h1:ESOxlf8BB2yG3zJ0SfZe9U6wpYu3YF3znxIICg73FYA=
github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf h1:MQ4e8XcxvZTeuOmRl7yE519vcWc2h/lyvYzsvt41cdY=
github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf/go.mod h1:SlYgWuQ5SjCEi6WLHjHCa1yvBfUnHcTbrrZtXPKa29o=
github.com/gravitational/reporting v0.0.0-20180907002058-ac7b85c75c4c h1:UwN3jo2EfZSGDchLVqH/EJ2A5GWvKROx3NJNUI6/plg=
Expand Down
158 changes: 157 additions & 1 deletion integration/app_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ import (
"github.com/gravitational/teleport/lib/web/app"

"github.com/gravitational/trace"

"github.com/pborman/uuid"
"github.com/stretchr/testify/require"
"golang.org/x/net/websocket"
)

// TestForward tests that requests get forwarded to the target application
Expand Down Expand Up @@ -93,6 +93,57 @@ func TestForward(t *testing.T) {
}
}

// TestAppAccessWebsockets makes sure that websocket requests get forwarded.
func TestAppAccessWebsockets(t *testing.T) {
// Create cluster, user, sessions, and credentials package.
pack := setup(t)

tests := []struct {
desc string
inCookie string
outMessage string
err error
}{
{
desc: "root cluster, valid application session cookie, successful websocket (ws://) request",
inCookie: pack.createAppSession(t, pack.rootWSPublicAddr, pack.rootAppClusterName),
outMessage: pack.rootWSMessage,
},
{
desc: "root cluster, valid application session cookie, successful secure websocket (wss://) request",
inCookie: pack.createAppSession(t, pack.rootWSSPublicAddr, pack.rootAppClusterName),
outMessage: pack.rootWSSMessage,
},
{
desc: "leaf cluster, valid application session cookie, successful websocket (ws://) request",
inCookie: pack.createAppSession(t, pack.leafWSPublicAddr, pack.leafAppClusterName),
outMessage: pack.leafWSMessage,
},
{
desc: "leaf cluster, valid application session cookie, successful secure websocket (wss://) request",
inCookie: pack.createAppSession(t, pack.leafWSSPublicAddr, pack.leafAppClusterName),
outMessage: pack.leafWSSMessage,
},
{
desc: "invalid application session cookie, websocket request fails to dial",
inCookie: "ABCDEFGHIJKLMNOPQRSTUVWXYZ",
err: &websocket.DialError{},
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
tt := tt
body, err := pack.makeWebsocketRequest(tt.inCookie, "/")
if tt.err != nil {
require.IsType(t, tt.err, trace.Unwrap(err))
} else {
require.NoError(t, err)
require.Equal(t, tt.outMessage, body)
}
})
}
}

// TestForwardModes ensures that requests are forwarded to applications even
// when the cluster is in proxy recording mode.
func TestForwardModes(t *testing.T) {
Expand Down Expand Up @@ -259,6 +310,14 @@ type pack struct {
rootAppClusterName string
rootMessage string

rootWSAppName string
rootWSPublicAddr string
rootWSMessage string

rootWSSAppName string
rootWSSPublicAddr string
rootWSSMessage string

jwtAppName string
jwtAppPublicAddr string
jwtAppClusterName string
Expand All @@ -272,6 +331,14 @@ type pack struct {
leafAppClusterName string
leafMessage string

leafWSAppName string
leafWSPublicAddr string
leafWSMessage string

leafWSSAppName string
leafWSSPublicAddr string
leafWSSMessage string

headerAppName string
headerAppPublicAddr string
headerAppClusterName string
Expand All @@ -296,11 +363,27 @@ func setup(t *testing.T) *pack {
rootAppClusterName: "example.com",
rootMessage: uuid.New(),

rootWSAppName: "ws-01",
rootWSPublicAddr: "ws-01.example.com",
rootWSMessage: uuid.New(),

rootWSSAppName: "wss-01",
rootWSSPublicAddr: "wss-01.example.com",
rootWSSMessage: uuid.New(),

leafAppName: "app-02",
leafAppPublicAddr: "app-02.example.com",
leafAppClusterName: "leaf.example.com",
leafMessage: uuid.New(),

leafWSAppName: "ws-02",
leafWSPublicAddr: "ws-02.example.com",
leafWSMessage: uuid.New(),

leafWSSAppName: "wss-02",
leafWSSPublicAddr: "wss-02.example.com",
leafWSSMessage: uuid.New(),

jwtAppName: "app-03",
jwtAppPublicAddr: "app-03.example.com",
jwtAppClusterName: "example.com",
Expand All @@ -315,10 +398,34 @@ func setup(t *testing.T) *pack {
fmt.Fprintln(w, p.rootMessage)
}))
t.Cleanup(rootServer.Close)
// Websockets server in root cluster (ws://).
rootWSServer := httptest.NewServer(websocket.Handler(func(conn *websocket.Conn) {
conn.Write([]byte(p.rootWSMessage))
conn.Close()
}))
t.Cleanup(rootWSServer.Close)
// Secure websockets server in root cluster (wss://).
rootWSSServer := httptest.NewTLSServer(websocket.Handler(func(conn *websocket.Conn) {
conn.Write([]byte(p.rootWSSMessage))
conn.Close()
}))
t.Cleanup(rootWSSServer.Close)
leafServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, p.leafMessage)
}))
t.Cleanup(leafServer.Close)
// Websockets server in leaf cluster (ws://).
leafWSServer := httptest.NewServer(websocket.Handler(func(conn *websocket.Conn) {
conn.Write([]byte(p.leafWSMessage))
conn.Close()
}))
t.Cleanup(leafWSServer.Close)
// Secure websockets server in leaf cluster (wss://).
leafWSSServer := httptest.NewTLSServer(websocket.Handler(func(conn *websocket.Conn) {
conn.Write([]byte(p.leafWSSMessage))
conn.Close()
}))
t.Cleanup(leafWSSServer.Close)
jwtServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprintln(w, r.Header.Get(teleport.AppJWTHeader))
}))
Expand Down Expand Up @@ -429,6 +536,16 @@ func setup(t *testing.T) *pack {
URI: rootServer.URL,
PublicAddr: p.rootAppPublicAddr,
},
{
Name: p.rootWSAppName,
URI: rootWSServer.URL,
PublicAddr: p.rootWSPublicAddr,
},
{
Name: p.rootWSSAppName,
URI: rootWSSServer.URL,
PublicAddr: p.rootWSSPublicAddr,
},
{
Name: p.jwtAppName,
URI: jwtServer.URL,
Expand Down Expand Up @@ -467,6 +584,16 @@ func setup(t *testing.T) *pack {
URI: leafServer.URL,
PublicAddr: p.leafAppPublicAddr,
},
{
Name: p.leafWSAppName,
URI: leafWSServer.URL,
PublicAddr: p.leafWSPublicAddr,
},
{
Name: p.leafWSSAppName,
URI: leafWSSServer.URL,
PublicAddr: p.leafWSSPublicAddr,
},
}
p.leafAppServer, err = p.leafCluster.StartApp(laConf)
require.NoError(t, err)
Expand Down Expand Up @@ -622,6 +749,35 @@ func (p *pack) makeRequest(sessionCookie string, method string, endpoint string)
return p.sendRequest(req)
}

// makeWebsocketRequest makes a websocket request with the given session cookie.
func (p *pack) makeWebsocketRequest(sessionCookie, endpoint string) (string, error) {
config, err := websocket.NewConfig(
fmt.Sprintf("wss://%s%s", net.JoinHostPort(Loopback, p.rootCluster.GetPortWeb()), endpoint),
"https://localhost")
if err != nil {
return "", trace.Wrap(err)
}
if sessionCookie != "" {
config.Header.Set("Cookie", (&http.Cookie{
Name: app.CookieName,
Value: sessionCookie,
}).String())
}
config.TlsConfig = &tls.Config{
InsecureSkipVerify: true,
}
conn, err := websocket.DialConfig(config)
if err != nil {
return "", trace.Wrap(err)
}
defer conn.Close()
data, err := ioutil.ReadAll(conn)
if err != nil {
return "", trace.Wrap(err)
}
return string(data), nil
}

// assembleRootProxyURL returns the URL string of an endpoint at the root
// cluster's proxy web.
func (p *pack) assembleRootProxyURL(endpoint string) string {
Expand Down
3 changes: 2 additions & 1 deletion integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5190,7 +5190,8 @@ func TestWebProxyInsecure(t *testing.T) {

// Web proxy endpoint should just respond with 200 when called over http://,
// content doesn't matter.
resp, err := http.Get(fmt.Sprintf("http://%v", net.JoinHostPort(Loopback, rc.GetPortWeb())))
resp, err := http.Get(fmt.Sprintf("http://%v/webapi/ping", net.JoinHostPort(Loopback, rc.GetPortWeb())))
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
require.NoError(t, resp.Body.Close())
}
13 changes: 11 additions & 2 deletions lib/reversetunnel/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"golang.org/x/crypto/ssh"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/services"
Expand Down Expand Up @@ -297,8 +298,16 @@ func (p *transport) start() {
return
}

p.log.Debug("Handing off connection to a local SSH service")
p.server.HandleConnection(utils.NewChConn(p.sconn, p.channel))
p.log.Debugf("Handing off connection to a local %q service.", dreq.ConnType)
switch dreq.ConnType {
case types.AppTunnel:
// Use channel connection wrapper that supports deadlines when
// proxying applications. This is required for websockets to
// work over Application Access.
p.server.HandleConnection(utils.NewCancelableChConn(p.sconn, p.channel))
default:
p.server.HandleConnection(utils.NewChConn(p.sconn, p.channel))
}
Comment on lines +301 to +310
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why you separated these two and created CancelableChConn instead of just adding deadline support to ChConn?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@russjones No good reason other than out of abundance of caution. I can just update ChConn if you feel strongly about it, but I just felt that this functionality is only required for app access and wanted to avoid the risk of potentially introducing some side-effects to other types of services like ssh (and esp. since it's going to a minor release).

Copy link
Contributor

@russjones russjones Mar 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense for 6.1. What do you think updating ChConn on master to support deadlines but keeping them separate on branch/v6? master is still 6 months away from becoming 7.0 so we would get a fair amount of testing in during that time plus you would get any additional testing for Application Access on 6.1.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@russjones Yeah, that was one of my alternative ideas as well. We can do that. I'll rebase this PR off of branch/v6 and open the one to master with just updated ChConn.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, looks good to me, merge it in after rebase off of branch/v6.

return
}
// If this is a proxy and not an SSH node, try finding an inbound
Expand Down
2 changes: 2 additions & 0 deletions lib/srv/app/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ func (s *Server) newSession(ctx context.Context, identity *tlsca.Identity, app *
fwd, err := forward.New(
forward.RoundTripper(transport),
forward.Logger(logrus.StandardLogger()),
forward.WebsocketRewriter(transport.ws),
forward.WebsocketDial(transport.ws.dialer),
)
if err != nil {
return nil, trace.Wrap(err)
Expand Down
40 changes: 40 additions & 0 deletions lib/srv/app/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/utils"

"github.com/gravitational/oxy/forward"
"github.com/gravitational/trace"
)

Expand Down Expand Up @@ -77,6 +78,8 @@ type transport struct {
tr http.RoundTripper

uri *url.URL

ws *websocketTransport
}

// newTransport creates a new transport.
Expand Down Expand Up @@ -106,6 +109,7 @@ func newTransport(ctx context.Context, c *transportConfig) (*transport, error) {
c: c,
uri: uri,
tr: tr,
ws: newWebsocketTransport(uri, tr.TLSClientConfig),
}, nil
}

Expand Down Expand Up @@ -280,3 +284,39 @@ func isRedirect(code int) bool {
}
return false
}

// websocketTransport combines parameters for websockets transport.
//
// Implements forward.ReqRewriter.
type websocketTransport struct {
uri *url.URL
dialer forward.Dialer
}

// newWebsocketTransport returns transport that knows how to rewrite and
// dial websocket requests.
func newWebsocketTransport(uri *url.URL, tlsConfig *tls.Config) *websocketTransport {
return &websocketTransport{
uri: uri,
dialer: func(network, address string) (net.Conn, error) {
// Request is going to "wss://".
if uri.Scheme == "https" {
return tls.Dial(network, address, tlsConfig)
}
// Request is going to "ws://".
return net.Dial(network, address)
},
}
}

// Rewrite rewrites the websocket request.
func (r *websocketTransport) Rewrite(req *http.Request) {
// Update scheme and host to those of the target app's to make sure
// it's forwarded correctly.
req.URL.Scheme = "ws"
if r.uri.Scheme == "https" {
req.URL.Scheme = "wss"
}
req.URL.Host = r.uri.Host
req.Host = r.uri.Host
}
Loading