Repository: hashicorp/boundary

Issue:
Creating multiple target connections with the same client port hangs the boundary CLI
<!-- Please reserve GitHub issues for bug reports and feature requests.

For questions, the best place to get answers is on our [discussion forum](https://discuss.hashicorp.com/c/boundary), as they will get more visibility from experienced users than the issue tracker.

Please note: We take Boundary's security and our users' trust very seriously. If you believe you have found a security issue in Boundary, please responsibly disclose by contacting us at security@hashicorp.com. Our PGP key is available at [our security page](https://www.hashicorp.com/security/).

-->

**Describe the bug**
When using Boundary CLI to create a connections with the same client port to listen to, Boundary CLI gets unresponsive.

**To Reproduce**
Steps to reproduce the behavior:
1. Run `boundary authenticate  ...`
2. Run `boundary connect ... -listen-port 2222`. You should get a "Proxy listening information" output.
3. In a different terminal, run `boundary connect ... -listen-port 2222` (notice the same port). This time you don't get any output, and the CLI get unresponsive. You can't even `CTRL+C` out of it. I have to do `kill -9` on the last connection.

**Expected behavior**
I would expect that the boundary CLI would give an error message on that the port is already occupied and exit with non-zero.

**Additional context**
This happens often when a `default client port` is configured and the user is not sure that a connection is already establish.

Discussion:

Solution patch:
diff --git a/.github/workflows/enos-run.yml b/.github/workflows/enos-run.yml
index 0e84231fa..8a290d4b7 100644
--- a/.github/workflows/enos-run.yml
+++ b/.github/workflows/enos-run.yml
@@ -152,14 +152,14 @@ jobs:
       - name: Configure GCP credentials
         if: contains(matrix.filter, 'gcp')
         id: gcp_auth
-        uses: google-github-actions/auth@ba79af03959ebeac9769e648f473a284504d9193 # v2.1.10
+        uses: google-github-actions/auth@140bb5113ffb6b65a7e9b937a81fa96cf5064462 # v2.1.11
         with:
           credentials_json: ${{ secrets.GCP_CREDENTIALS }}
           access_token_lifetime: '3600s'
           project_id: ${{ secrets.GCP_PROJECT_ID_CI }}
       - name: 'Set up GCP Cloud SDK'
         if: contains(matrix.filter, 'gcp')
-        uses: google-github-actions/setup-gcloud@77e7a554d41e2ee56fc945c52dfd3f33d12def9a # v2.1.4
+        uses: google-github-actions/setup-gcloud@6a7c903a70c8625ed6700fa299f5ddb4ca6022e9 # v2.1.5
       - name: Set up Enos
         uses: hashicorp/action-setup-enos@v1    # TSCCR: loading action configs: failed to query HEAD reference: failed to get advertised references: authorization failed
         with:
diff --git a/enos/scripts/test_e2e_env.sh b/enos/scripts/test_e2e_env.sh
index d89417760..b09f4d54f 100644
--- a/enos/scripts/test_e2e_env.sh
+++ b/enos/scripts/test_e2e_env.sh
@@ -7,7 +7,33 @@ DIR=$(pwd)
 SCRIPTS_DIR=$( cd -- "$( dirname -- "$0" )" &> /dev/null && pwd )
 STATEDIR=$(ls -td $SCRIPTS_DIR/../.enos/*/ | head -1) # get latest directory
 
+# Default to linux format
+FORMAT="linux"
+
+# Parse flag
+while getopts "f:" opt; do
+  case ${opt} in
+    f)
+      if [[ "$OPTARG" == "linux" || "$OPTARG" == "windows" ]]; then
+        FORMAT="$OPTARG"
+      else
+        echo "Invalid format. Use 'linux' or 'windows'."
+        exit 1
+      fi
+      ;;
+    *)
+      echo "Usage: $0 [-f linux|windows]"
+      exit 1
+      ;;
+  esac
+done
+
 cd $STATEDIR
-terraform show -json terraform.tfstate | jq -r '.values.root_module.child_modules[].resources[] | select(.address=="module.run_e2e_test.enos_local_exec.run_e2e_test") | .values.environment | to_entries[] | select(.value != "") | "export \(.key)=\(.value|@sh)"'
+
+if [[ "$FORMAT" == "windows" ]]; then
+  terraform show -json terraform.tfstate | jq -r '.values.root_module.child_modules[].resources[] | select(.address=="module.run_e2e_test.enos_local_exec.run_e2e_test") | .values.environment | to_entries[] | select(.value != "") | "$env:\(.key)=\(.value|@sh);"'
+else
+  terraform show -json terraform.tfstate | jq -r '.values.root_module.child_modules[].resources[] | select(.address=="module.run_e2e_test.enos_local_exec.run_e2e_test") | .values.environment | to_entries[] | select(.value != "") | "export \(.key)=\(.value|@sh);"'
+fi
 
 cd $DIR
diff --git a/internal/cmd/commands/connect/connect.go b/internal/cmd/commands/connect/connect.go
index 41a660c7e..a6df08865 100644
--- a/internal/cmd/commands/connect/connect.go
+++ b/internal/cmd/commands/connect/connect.go
@@ -434,6 +434,7 @@ func (c *Command) Run(args []string) (retCode int) {
 		c.PrintCliError(fmt.Errorf("Error parsing listen address: %w", err))
 		return base.CommandCliError
 	}
+
 	listenAddr = netip.AddrPortFrom(addr, uint16(c.flagListenPort))
 
 	connsLeftCh := make(chan int32)
@@ -458,7 +459,10 @@ func (c *Command) Run(args []string) (retCode int) {
 	proxyError := new(atomic.Error)
 	go func() {
 		defer close(clientProxyCloseCh)
-		proxyError.Store(clientProxy.Start())
+		if err = clientProxy.Start(); err != nil {
+			c.proxyCancel()
+			proxyError.Store(err)
+		}
 	}()
 	go func() {
 		defer close(connCountCloseCh)
@@ -482,7 +486,15 @@ 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.
 
-		proxyAddr := clientProxy.ListenerAddress(context.Background())
+		proxyAddr := clientProxy.ListenerAddress(c.proxyCtx)
+		if proxyAddr == "" {
+			if err := proxyError.Load(); err != nil {
+				c.PrintCliError(fmt.Errorf("Error starting proxy: %w", err))
+				return base.CommandCliError
+			}
+			c.PrintCliError(fmt.Errorf("Error starting proxy: no address returned"))
+			return base.CommandCliError
+		}
 		var clientProxyHost, clientProxyPort string
 		clientProxyHost, clientProxyPort, err = util.SplitHostPort(proxyAddr)
 		if err != nil && !errors.Is(err, util.ErrMissingPort) {
diff --git a/internal/gen/controller/servers/services/credential.pb.go b/internal/gen/controller/servers/services/credential.pb.go
index 85b71ddd5..b6162c85b 100644
--- a/internal/gen/controller/servers/services/credential.pb.go
+++ b/internal/gen/controller/servers/services/credential.pb.go
@@ -126,7 +126,7 @@ func (*Credential_SshCertificate) isCredential_Credential() {}
 type UsernamePassword struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
 	// The username of the credential
-	Username string `protobuf:"bytes,10,opt,name=username,proto3" json:"username,omitempty"` // @gotags: `class:"public"`
+	Username string `protobuf:"bytes,10,opt,name=username,proto3" json:"username,omitempty"` // @gotags: `class:"sensitive"`
 	// The password of the credential
 	Password      string `protobuf:"bytes,20,opt,name=password,proto3" json:"password,omitempty"` // @gotags: `class:"secret"`
 	unknownFields protoimpl.UnknownFields
diff --git a/internal/proto/controller/api/resources/sessionrecordings/v1/session_recording.proto b/internal/proto/controller/api/resources/sessionrecordings/v1/session_recording.proto
index 5eb48f66f..90866e07d 100644
--- a/internal/proto/controller/api/resources/sessionrecordings/v1/session_recording.proto
+++ b/internal/proto/controller/api/resources/sessionrecordings/v1/session_recording.proto
@@ -314,7 +314,7 @@ message Credential {
 // The attributes of a UsernamePassword Credential.
 message UsernamePasswordCredentialAttributes {
   // The username associated with the credential.
-  string username = 1; // @gotags: class:"public"
+  string username = 1; // @gotags: class:"sensitive"
 
   // The hmac value of the password.
   string password_hmac = 2; // @gotags: class:"public"
diff --git a/internal/proto/controller/servers/services/v1/credential.proto b/internal/proto/controller/servers/services/v1/credential.proto
index 331adc5c7..63e11bc8b 100644
--- a/internal/proto/controller/servers/services/v1/credential.proto
+++ b/internal/proto/controller/servers/services/v1/credential.proto
@@ -21,7 +21,7 @@ message Credential {
 // UsernamePassword is a credential containing a username and a password.
 message UsernamePassword {
   // The username of the credential
-  string username = 10; // @gotags: `class:"public"`
+  string username = 10; // @gotags: `class:"sensitive"`
 
   // The password of the credential
   string password = 20; // @gotags: `class:"secret"`
diff --git a/sdk/pbs/controller/api/resources/session_recordings/session_recording.pb.go b/sdk/pbs/controller/api/resources/session_recordings/session_recording.pb.go
index 6cf8d9f41..efa8ef1dd 100644
--- a/sdk/pbs/controller/api/resources/session_recordings/session_recording.pb.go
+++ b/sdk/pbs/controller/api/resources/session_recordings/session_recording.pb.go
@@ -1285,7 +1285,7 @@ func (*Credential_JsonAttributes) isCredential_Attrs() {}
 type UsernamePasswordCredentialAttributes struct {
 	state protoimpl.MessageState `protogen:"open.v1"`
 	// The username associated with the credential.
-	Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty" class:"public"` // @gotags: class:"public"
+	Username string `protobuf:"bytes,1,opt,name=username,proto3" json:"username,omitempty" class:"sensitive"` // @gotags: class:"sensitive"
 	// The hmac value of the password.
 	PasswordHmac  string `protobuf:"bytes,2,opt,name=password_hmac,json=passwordHmac,proto3" json:"password_hmac,omitempty" class:"public"` // @gotags: class:"public"
 	unknownFields protoimpl.UnknownFields
diff --git a/website/content/docs/concepts/domain-model/aliases.mdx b/website/content/docs/concepts/domain-model/aliases.mdx
index 248fe91f6..2bafd3b5f 100644
--- a/website/content/docs/concepts/domain-model/aliases.mdx
+++ b/website/content/docs/concepts/domain-model/aliases.mdx
@@ -7,7 +7,7 @@ description: >-
 
 # Aliases
 
-An alias is a globally unique, DNS-like string that is associated wtih a destination resource.
+An alias is a globally unique, DNS-like string that is associated with a destination resource.
 Examples of valid aliases are `webserver` and `webserver.boundary`.
 You can establish a session to a target by referencing its alias, instead of having to provide a target ID or target name and scope ID.
 For example, if you have an alias `boundary.dev`, you can use it to connect to a target with the following command: `boundary connect ssh boundary.dev`.
@@ -33,4 +33,4 @@ The target subtype has the following additional attributes:
 
  ## More information
 
- - [Aliases](/boundary/docs/concepts/aliases)
\ No newline at end of file
+ - [Aliases](/boundary/docs/concepts/aliases)
diff --git a/website/content/docs/concepts/domain-model/index.mdx b/website/content/docs/concepts/domain-model/index.mdx
index 40cd8d7eb..c12ff6560 100644
--- a/website/content/docs/concepts/domain-model/index.mdx
+++ b/website/content/docs/concepts/domain-model/index.mdx
@@ -83,6 +83,11 @@ significant number of resources exist.
   issued from a configured [authentication method][]
   which can be used to establish the identity of a [user][].
 
+- **[Alias][]** :
+  A resource
+  that represents a globally unique, DNS-like string that is
+  associated with a destination resource, such as a [target][].
+
 - **[Credential][]** :
   A data structure containing one or more secrets
   that binds an identity to a set of permissions or capabilities
@@ -152,6 +157,11 @@ significant number of resources exist.
    A storage bucket is used to store [session recordings][].
    The storage bucket represents a bucket in an external object store.
 
+- **[Storage policy][]** :
+   A storage policy defines the rules for storing
+   [session recordings][] in a [storage bucket][].
+   The storage policy may specify the retention period for the recordings.
+
 - **[Target][]** :
   A resource
   that represents a networked service
@@ -178,6 +188,7 @@ Refer to the [Scopes] section to help you understand the structure of resources
 [iam]: https://en.wikipedia.org/wiki/Identity_management
 [account]: /boundary/docs/concepts/domain-model/accounts
 [accounts]: /boundary/docs/concepts/domain-model/accounts
+[alias]: /boundary/docs/concepts/domain-model/aliases
 [authentication method]: /boundary/docs/concepts/domain-model/auth-methods
 [authentication methods]: /boundary/docs/concepts/domain-model/auth-methods
 [credential library]: /boundary/docs/concepts/domain-model/credential-libraries
@@ -204,6 +215,7 @@ Refer to the [Scopes] section to help you understand the structure of resources
 [session recordings]: /boundary/docs/concepts/domain-model/session-recordings
 [sessions]: /boundary/docs/concepts/domain-model/sessions
 [storage bucket]: /boundary/docs/concepts/domain-model/storage-buckets
+[storage policy]: /boundary/docs/concepts/domain-model/storage-policy
 [target]: /boundary/docs/concepts/domain-model/targets
 [targets]: /boundary/docs/concepts/domain-model/targets
 [user]: /boundary/docs/concepts/domain-model/users
diff --git a/website/content/docs/release-notes/v0_19_0.mdx b/website/content/docs/release-notes/v0_19_0.mdx
index 2a71cfe8e..105fb87d6 100644
--- a/website/content/docs/release-notes/v0_19_0.mdx
+++ b/website/content/docs/release-notes/v0_19_0.mdx
@@ -86,9 +86,11 @@ description: >-
     <td style={{verticalAlign: 'middle'}}>
     You may have redundant grant scopes if you applied a grant to a scope and the scope also inherited the grant from the <code>this</code>, <code>children</code>, or <code>descendants</code> options. As of Boundary version 0.19.3, redundant grant scopes are no longer permitted.
     <br /><br />
-    When you upgrade a cluster to Boundary version 0.19.3, the migration will fail with a message if the database contains any redundant grant scopes. The migration tool provides a command that automatically removes any redundant grant scopes so that you can proceed with the upgrade.
+    When you upgrade a Boundary Enterprise or Community edition cluster to version 0.19.3 or later, the migration will fail with a message if the database contains any redundant grant scopes. The migration tool provides a command that automatically removes any redundant grant scopes so that you can proceed with the upgrade.
     <br /><br />
-    Learn more about removing redundant grant scopes and upgrading to version 0.19.3:&nbsp; <a href="#redundant-grants">Known issues and breaking changes </a>
+    For HCP Boundary users, the redundant grant scopes are automatically removed as part of the migration process.
+    <br /><br />
+    Learn more about removing redundant grant scopes and upgrading to version 0.19.3 or later:&nbsp; <a href="#redundant-grants">Known issues and breaking changes </a>
     </td>
   </tr>
 
@@ -406,12 +408,14 @@ description: >-
     <td style={{verticalAlign: 'middle'}}>
     You may have redundant grant scopes if you applied a grant to a scope and the scope also inherited the grant from the <code>this</code>, <code>children</code>, or <code>descendants</code> options. Redundant grant scopes could cause performance issues. As of Boundary version 0.19.3, redundant grant scopes are no longer permitted.
     <br /><br />
-    When you run the <code>boundary database migrate</code> command to upgrade a cluster to Boundary version 0.19.3, the migration will fail with a message if the database contains any redundant grant scopes. The migration tool provides the following command to automatically repair the grant scopes:
+    When you run the <code>boundary database migrate</code> command to upgrade a Boundary Enterprise or Community edition cluster to version 0.19.3 or later, the migration will fail with a message if the database contains any redundant grant scopes. The migration tool provides the following command to automatically repair the grant scopes:
     <br /><br />
     <code>boundary database migrate -repair=oss:97001</code>
     <br /><br />
     Run the command to remove any redundant grant scopes from roles. Any individually granted scopes that are already covered by <code>descendants</code> or <code>children</code> grants are considered invalid and removed. When the migration is complete, the migration tool produces a message that details any changes.
     <br /><br />
+    HCP Boundary automatically removes the redudant grant scopes as part of the upgrade process. No further action is required.
+    <br /><br />
     Learn more:
       <ul>
       <li><a href="/boundary/docs/concepts/domain-model/scopes">Scopes</a></li>



Test patch:
diff --git a/testing/internal/e2e/tests/base/target_tcp_connect_listen_port_test.go b/testing/internal/e2e/tests/base/target_tcp_connect_listen_port_test.go
new file mode 100644
index 000000000..90bef7d63
--- /dev/null
+++ b/testing/internal/e2e/tests/base/target_tcp_connect_listen_port_test.go
@@ -0,0 +1,134 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package base_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/hashicorp/boundary/internal/session"
+	"github.com/hashicorp/boundary/internal/target"
+	"github.com/hashicorp/boundary/testing/internal/e2e"
+	"github.com/hashicorp/boundary/testing/internal/e2e/boundary"
+)
+
+// TestCliTcpTargetConnectListenPort uses the boundary cli to create a target and
+// connect to it multiple times on the same listen port. The cli should return an error when
+// a port is already in use.
+func TestCliTcpTargetConnectListenPort(t *testing.T) {
+	e2e.MaybeSkipTest(t)
+	c, err := loadTestConfig()
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	boundary.AuthenticateAdminCli(t, ctx)
+	orgId, err := boundary.CreateOrgCli(t, ctx)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		ctx := context.Background()
+		boundary.AuthenticateAdminCli(t, ctx)
+		output := e2e.RunCommand(ctx, "boundary", e2e.WithArgs("scopes", "delete", "-id", orgId))
+		require.NoError(t, output.Err, string(output.Stderr))
+	})
+	projectId, err := boundary.CreateProjectCli(t, ctx, orgId)
+	require.NoError(t, err)
+	targetId, err := boundary.CreateTargetCli(t, ctx, projectId, c.TargetPort,
+		target.WithAddress(c.TargetAddress),
+	)
+	require.NoError(t, err)
+
+	// Connect to target with client port 3333
+	const ListenPort = "3333"
+
+	ctxCancel, cancel := context.WithCancel(ctx)
+	sessionChannel := make(chan *e2e.CommandResult)
+
+	go func() {
+		sessionChannel <- e2e.RunCommand(ctxCancel, "boundary",
+			e2e.WithArgs(
+				"connect",
+				"-target-id", targetId,
+				"-listen-port", ListenPort,
+			),
+		)
+	}()
+	t.Cleanup(cancel)
+
+	s := boundary.WaitForSessionCli(t, ctx, projectId)
+	boundary.WaitForSessionStatusCli(t, ctx, s.Id, session.StatusPending.String())
+	t.Logf("Successfully connected to target with listen port %s", ListenPort)
+
+	t.Logf("Starting new connection to target with same listen port %s", ListenPort)
+	output := e2e.RunCommand(ctx, "boundary",
+		e2e.WithArgs(
+			"connect",
+			"-target-id", targetId,
+			"-listen-port", ListenPort,
+		),
+	)
+
+	require.Error(t, output.Err, "Expected error when connecting to target with same listen port")
+	require.Contains(t, string(output.Stderr), "address already in use")
+	require.Equal(t, 2, output.ExitCode)
+}
+
+// TestCliTcpTargetConnectDefaultClientPort uses the boundary cli to create a target and
+// connect to it multiple times on the same client port. The test is similar to the one above,
+// but sets DefaultClientPort instead of ListenPort
+func TestCliTcpTargetConnectDefaultClientPort(t *testing.T) {
+	const DefaultClientPort = 3333
+	e2e.MaybeSkipTest(t)
+	c, err := loadTestConfig()
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	boundary.AuthenticateAdminCli(t, ctx)
+	orgId, err := boundary.CreateOrgCli(t, ctx)
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		ctx := context.Background()
+		boundary.AuthenticateAdminCli(t, ctx)
+		output := e2e.RunCommand(ctx, "boundary", e2e.WithArgs("scopes", "delete", "-id", orgId))
+		require.NoError(t, output.Err, string(output.Stderr))
+	})
+	projectId, err := boundary.CreateProjectCli(t, ctx, orgId)
+	require.NoError(t, err)
+	targetId, err := boundary.CreateTargetCli(t, ctx, projectId, c.TargetPort,
+		target.WithAddress(c.TargetAddress),
+		target.WithDefaultClientPort(DefaultClientPort),
+	)
+	require.NoError(t, err)
+
+	// Connect to target with DefaultClientPort set
+	ctxCancel, cancel := context.WithCancel(ctx)
+	sessionChannel := make(chan *e2e.CommandResult)
+
+	go func() {
+		sessionChannel <- e2e.RunCommand(ctxCancel, "boundary",
+			e2e.WithArgs(
+				"connect",
+				"-target-id", targetId,
+			),
+		)
+	}()
+	t.Cleanup(cancel)
+
+	s := boundary.WaitForSessionCli(t, ctx, projectId)
+	boundary.WaitForSessionStatusCli(t, ctx, s.Id, session.StatusPending.String())
+	t.Logf("Successfully connected to target with client port %d", DefaultClientPort)
+
+	t.Logf("Starting new connection to target on client port %d", DefaultClientPort)
+	output := e2e.RunCommand(ctx, "boundary",
+		e2e.WithArgs(
+			"connect",
+			"-target-id", targetId,
+		),
+	)
+
+	require.Error(t, output.Err, "Expected error when connecting to target with same listen port")
+	require.Contains(t, string(output.Stderr), "address already in use")
+	require.Equal(t, 2, output.ExitCode)
+}