From db8e9bb0c69620ee6c2e58c34c138556a3cf70ce Mon Sep 17 00:00:00 2001 From: Louis Ruch Date: Wed, 1 Sep 2021 07:22:45 -0700 Subject: [PATCH 1/2] refactor(worker): refactors the proxy handler to use endpoint scheme (#1491) --- internal/cmd/commands/connect/connect.go | 2 +- internal/servers/worker/handler.go | 16 ++++++++++++---- internal/servers/worker/proxy/tcp/tcp.go | 3 +-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/cmd/commands/connect/connect.go b/internal/cmd/commands/connect/connect.go index 50d2ec40d..9c50f4ad8 100644 --- a/internal/cmd/commands/connect/connect.go +++ b/internal/cmd/commands/connect/connect.go @@ -501,7 +501,7 @@ func (c *Command) Run(args []string) (retCode int) { // The only way a user will be able to connect to the session is by // connecting directly to the port and address we report to them here. sessInfo := SessionInfo{ - Protocol: "tcp", + Protocol: c.sessionAuthzData.GetType(), Address: c.listenerAddr.IP.String(), Port: c.listenerAddr.Port, Expiration: c.expiration, diff --git a/internal/servers/worker/handler.go b/internal/servers/worker/handler.go index 47af24e9d..27ab8935d 100644 --- a/internal/servers/worker/handler.go +++ b/internal/servers/worker/handler.go @@ -5,6 +5,7 @@ import ( "errors" "net" "net/http" + "net/url" "strconv" "github.com/hashicorp/boundary/globals" @@ -75,7 +76,6 @@ func (w *Worker) handleProxy() http.HandlerFunc { tofuToken := si.LookupSessionResponse.GetTofuToken() version := si.LookupSessionResponse.GetVersion() endpoint := si.LookupSessionResponse.GetEndpoint() - // userId := si.LookupSessionResponse.GetAuthorization() sessStatus := si.Status si.RUnlock() @@ -163,10 +163,18 @@ func (w *Worker) handleProxy() http.HandlerFunc { return } - // Verify the subprotocol has a supported proxy before calling AuthorizeConnection - handleProxyFn, err := proxyHandlers.GetHandler(conn.Subprotocol()) + // Verify the protocol has a supported proxy before calling AuthorizeConnection + endpointUrl, err := url.Parse(endpoint) if err != nil { - event.WriteError(ctx, op, err, event.WithInfoMsg("worker received request for unsupported protocol %s", conn.Subprotocol())) + event.WriteError(ctx, op, err, event.WithInfoMsg("worker failed to parse target endpoint", "endpoint", endpoint)) + if err = conn.Close(websocket.StatusProtocolError, "unsupported-protocol"); err != nil { + event.WriteError(ctx, op, err, event.WithInfoMsg("error closing client connection")) + } + return + } + handleProxyFn, err := proxyHandlers.GetHandler(endpointUrl.Scheme) + if err != nil { + event.WriteError(ctx, op, err, event.WithInfoMsg("worker received request for unsupported protocol", "protocol", endpointUrl.Scheme)) if err = conn.Close(websocket.StatusProtocolError, "unsupported-protocol"); err != nil { event.WriteError(ctx, op, err, event.WithInfoMsg("error closing client connection")) } diff --git a/internal/servers/worker/proxy/tcp/tcp.go b/internal/servers/worker/proxy/tcp/tcp.go index f2ff1e718..0eff4bbc8 100644 --- a/internal/servers/worker/proxy/tcp/tcp.go +++ b/internal/servers/worker/proxy/tcp/tcp.go @@ -7,7 +7,6 @@ import ( "net/url" "sync" - "github.com/hashicorp/boundary/globals" pbs "github.com/hashicorp/boundary/internal/gen/controller/servers/services" "github.com/hashicorp/boundary/internal/observability/event" "github.com/hashicorp/boundary/internal/servers/worker/proxy" @@ -16,7 +15,7 @@ import ( ) func init() { - err := proxy.RegisterHandler(globals.TcpProxyV1, handleTcpProxyV1) + err := proxy.RegisterHandler("tcp", handleTcpProxyV1) if err != nil { panic(err) } From 030c09c6b9699581c1d6d6919103deb09e8c4f6c Mon Sep 17 00:00:00 2001 From: Louis Ruch Date: Wed, 1 Sep 2021 09:16:47 -0700 Subject: [PATCH 2/2] bug(targets): Fix panic in authorize-session (#1496) When renaming credential libraries -> credential sources the target service added the credential library information to the SessionCredential response, while the target cmd was parsing the credential source. This prevents a panic like the following: boundary targets authorize-session -id ttcp_5PS2dktESb panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x1f99a31] goroutine 1 [running]: github.com/hashicorp/boundary/internal/cmd/commands/targetscmd.printCustomActionOutputImpl(0xc00046c0a0) /Users/louisruch/boundary/internal/cmd/commands/targetscmd/funcs.go:822 +0x9d1 github.com/hashicorp/boundary/internal/cmd/commands/targetscmd.(*Command).Run(0xc00046c0a0, {0xc000134030, 0x2, 0x2}) /Users/louisruch/boundary/internal/cmd/commands/targetscmd/targets.gen.go:324 +0x188b github.com/mitchellh/cli.(*CLI).Run(0xc0001c6500) /Users/louisruch/go/pkg/mod/github.com/mitchellh/cli@v1.1.2/cli.go:262 +0x5f8 github.com/hashicorp/boundary/internal/cmd.RunCustom({0xc000134010, 0x60, 0x0}, 0x0) /Users/louisruch/boundary/internal/cmd/main.go:186 +0x9d6 github.com/hashicorp/boundary/internal/cmd.Run(...) /Users/louisruch/boundary/internal/cmd/main.go:92 main.main() /Users/louisruch/boundary/cmd/boundary/main.go:13 +0xc9 Fixes #1488 --- CHANGELOG.md | 6 ++++++ .../handlers/targets/target_service.go | 7 +++++++ .../handlers/targets/target_service_test.go | 17 ++++++++++++----- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fb2f5afa..3d8372f25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. ## Next +### Bug Fixes + +* targets: Fix panic when using `boundary targets authorize-session` + ([issue](https://github.com/hashicorp/boundary/issues/1488), + [PR](https://github.com/hashicorp/boundary/pull/1496)). + ## 0.5.1 (2021/08/16) ### New and Improved diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index a483da7f5..f60b5a8e3 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -1098,6 +1098,13 @@ HostSourceIterationLoop: CredentialStoreId: l.GetStoreId(), Type: credential.SubtypeFromId(l.GetPublicId()).String(), }, + CredentialSource: &pb.CredentialSource{ + Id: l.GetPublicId(), + Name: l.GetName(), + Description: l.GetDescription(), + CredentialStoreId: l.GetStoreId(), + Type: credential.SubtypeFromId(l.GetPublicId()).String(), + }, Secret: &pb.SessionSecret{ Raw: base64.StdEncoding.EncodeToString(jSecret), Decoded: sSecret, diff --git a/internal/servers/controller/handlers/targets/target_service_test.go b/internal/servers/controller/handlers/targets/target_service_test.go index fc289402f..19f9ac908 100644 --- a/internal/servers/controller/handlers/targets/target_service_test.go +++ b/internal/servers/controller/handlers/targets/target_service_test.go @@ -2545,11 +2545,11 @@ func TestAuthorizeSession(t *testing.T) { }}) require.NoError(t, err) - _, err = s.AddTargetCredentialLibraries(ctx, - &pbs.AddTargetCredentialLibrariesRequest{ - Id: tar.GetPublicId(), - ApplicationCredentialLibraryIds: []string{clsResp.GetItem().GetId()}, - Version: apiTar.GetItem().GetVersion(), + _, err = s.AddTargetCredentialSources(ctx, + &pbs.AddTargetCredentialSourcesRequest{ + Id: tar.GetPublicId(), + ApplicationCredentialSourceIds: []string{clsResp.GetItem().GetId()}, + Version: apiTar.GetItem().GetVersion(), }) require.NoError(t, err) @@ -2596,6 +2596,13 @@ func TestAuthorizeSession(t *testing.T) { CredentialStoreId: store.GetPublicId(), Type: vault.Subtype.String(), }, + CredentialSource: &pb.CredentialSource{ + Id: clsResp.GetItem().GetId(), + Name: clsResp.GetItem().GetName().GetValue(), + Description: clsResp.GetItem().GetDescription().GetValue(), + CredentialStoreId: store.GetPublicId(), + Type: vault.Subtype.String(), + }, }}, // TODO: validate the contents of the authorization token is what is expected }