coder · Emyrk · Apr 5, 2023 · Apr 4, 2023 · Apr 4, 2023 · Apr 4, 2023
diff --git a/cli/server.go b/cli/server.go
@@ -78,6 +78,7 @@ import (
 	"github.com/coder/coder/coderd/tracing"
 	"github.com/coder/coder/coderd/updatecheck"
 	"github.com/coder/coder/coderd/util/slice"
+	"github.com/coder/coder/coderd/workspaceapps"
 	"github.com/coder/coder/codersdk"
 	"github.com/coder/coder/cryptorand"
 	"github.com/coder/coder/provisioner/echo"
@@ -781,37 +782,42 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 					}
 				}
 
-				// Read the app signing key from the DB. We store it hex
-				// encoded since the config table uses strings for the value and
-				// we don't want to deal with automatic encoding issues.
-				appSigningKeyStr, err := tx.GetAppSigningKey(ctx)
+				// Read the app signing key from the DB. We store it hex encoded
+				// since the config table uses strings for the value and we
+				// don't want to deal with automatic encoding issues.
+				appSecurityKeyStr, err := tx.GetAppSecurityKey(ctx)
 				if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
 					return xerrors.Errorf("get app signing key: %w", err)
 				}
-				if appSigningKeyStr == "" {
-					// Generate 64 byte secure random string.
-					b := make([]byte, 64)
+				// If the string in the DB is an invalid hex string or the
+				// length is not equal to the current key length, generate a new
+				// one.
+				//
+				// If the key is regenerated, old signed tokens and encrypted
+				// strings will become invalid. New signed app tokens will be
+				// generated automatically on failure. Any workspace app token
+				// smuggling operations in progress may fail, although with a
+				// helpful error.
+				if decoded, err := hex.DecodeString(appSecurityKeyStr); err != nil || len(decoded) != len(workspaceapps.SecurityKey{}) {
+					b := make([]byte, len(workspaceapps.SecurityKey{}))
 					_, err := rand.Read(b)
 					if err != nil {
 						return xerrors.Errorf("generate fresh app signing key: %w", err)
 					}
 
-					appSigningKeyStr = hex.EncodeToString(b)
-					err = tx.InsertAppSigningKey(ctx, appSigningKeyStr)
+					appSecurityKeyStr = hex.EncodeToString(b)
+					err = tx.UpsertAppSecurityKey(ctx, appSecurityKeyStr)
 					if err != nil {
 						return xerrors.Errorf("insert freshly generated app signing key to database: %w", err)
 					}
 				}
 
-				appSigningKey, err := hex.DecodeString(appSigningKeyStr)
+				appSecurityKey, err := workspaceapps.KeyFromString(appSecurityKeyStr)
 				if err != nil {
-					return xerrors.Errorf("decode app signing key from database as hex: %w", err)
-				}
-				if len(appSigningKey) != 64 {
-					return xerrors.Errorf("app signing key must be 64 bytes, key in database is %d bytes", len(appSigningKey))
+					return xerrors.Errorf("decode app signing key from database: %w", err)
 				}
 
-				options.AppSigningKey = appSigningKey
+				options.AppSecurityKey = appSecurityKey
 				return nil
 			}, nil)
 			if err != nil {

diff --git a/cli/server_test.go b/cli/server_test.go
@@ -668,8 +668,7 @@ func TestServer(t *testing.T) {
 				if c.tlsListener {
 					accessURLParsed, err := url.Parse(c.requestURL)
 					require.NoError(t, err)
-					client := codersdk.New(accessURLParsed)
-					client.HTTPClient = &http.Client{
+					client := &http.Client{
 						CheckRedirect: func(req *http.Request, via []*http.Request) error {
 							return http.ErrUseLastResponse
 						},
@@ -682,11 +681,15 @@ func TestServer(t *testing.T) {
 							},
 						},
 					}
-					defer client.HTTPClient.CloseIdleConnections()
-					_, err = client.HasFirstUser(ctx)
-					if err != nil {
-						require.ErrorContains(t, err, "Invalid application URL")
-					}
+					defer client.CloseIdleConnections()
+
+					req, err := http.NewRequestWithContext(ctx, http.MethodGet, accessURLParsed.String(), nil)
+					require.NoError(t, err)
+					resp, err := client.Do(req)
+					// We don't care much about the response, just that TLS
+					// worked.
+					require.NoError(t, err)
+					defer resp.Body.Close()
 				}
 			})
 		}

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -123,9 +123,9 @@ type Options struct {
 	SwaggerEndpoint       bool
 	SetUserGroups         func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error
 	TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
-	// AppSigningKey denotes the symmetric key to use for signing temporary app
-	// tokens. The key must be 64 bytes long.
-	AppSigningKey      []byte
+	// AppSecurityKey is the crypto key used to sign and encrypt tokens related to
+	// workspace applications. It consists of both a signing and encryption key.
+	AppSecurityKey     workspaceapps.SecurityKey
 	HealthcheckFunc    func(ctx context.Context) (*healthcheck.Report, error)
 	HealthcheckTimeout time.Duration
 	HealthcheckRefresh time.Duration
@@ -241,9 +241,6 @@ func New(options *Options) *API {
 		v := schedule.NewAGPLTemplateScheduleStore()
 		options.TemplateScheduleStore.Store(&v)
 	}
-	if len(options.AppSigningKey) != 64 {
-		panic("coderd: AppSigningKey must be 64 bytes long")
-	}
 	if options.HealthcheckFunc == nil {
 		options.HealthcheckFunc = func(ctx context.Context) (*healthcheck.Report, error) {
 			return healthcheck.Run(ctx, &healthcheck.ReportOptions{
@@ -309,7 +306,7 @@ func New(options *Options) *API {
 			options.DeploymentValues,
 			oauthConfigs,
 			options.AgentInactiveDisconnectTimeout,
-			options.AppSigningKey,
+			options.AppSecurityKey,
 		),
 		metricsCache:          metricsCache,
 		Auditor:               atomic.Pointer[audit.Auditor]{},
@@ -334,6 +331,21 @@ func New(options *Options) *API {
 	api.workspaceAgentCache = wsconncache.New(api.dialWorkspaceAgentTailnet, 0)
 	api.TailnetCoordinator.Store(&options.TailnetCoordinator)
 
+	api.workspaceAppServer = &workspaceapps.Server{
+		Logger: options.Logger.Named("workspaceapps"),
+
+		DashboardURL:     api.AccessURL,
+		AccessURL:        api.AccessURL,
+		Hostname:         api.AppHostname,
+		HostnameRegex:    api.AppHostnameRegex,
+		DeploymentValues: options.DeploymentValues,
+		RealIPConfig:     options.RealIPConfig,
+
+		SignedTokenProvider: api.WorkspaceAppsProvider,
+		WorkspaceConnCache:  api.workspaceAgentCache,
+		AppSecurityKey:      options.AppSecurityKey,
+	}
+
 	apiKeyMiddleware := httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
 		DB:                          options.Database,
 		OAuth2Configs:               oauthConfigs,
@@ -366,11 +378,12 @@ func New(options *Options) *API {
 		httpmw.ExtractRealIP(api.RealIPConfig),
 		httpmw.Logger(api.Logger),
 		httpmw.Prometheus(options.PrometheusRegistry),
-		// handleSubdomainApplications checks if the first subdomain is a valid
-		// app URL. If it is, it will serve that application.
+		// SubdomainAppMW checks if the first subdomain is a valid app URL. If
+		// it is, it will serve that application.
 		//
-		// Workspace apps do their own auth.
-		api.handleSubdomainApplications(apiRateLimiter),
+		// Workspace apps do their own auth and must be BEFORE the auth
+		// middleware.
+		api.workspaceAppServer.SubdomainAppMW(apiRateLimiter),
 		// Build-Version is helpful for debugging.
 		func(next http.Handler) http.Handler {
 			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -393,16 +406,12 @@ func New(options *Options) *API {
 
 	r.Get("/healthz", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) })
 
-	apps := func(r chi.Router) {
-		// Workspace apps do their own auth.
+	// Attach workspace apps routes.
+	r.Group(func(r chi.Router) {
 		r.Use(apiRateLimiter)
-		r.HandleFunc("/*", api.workspaceAppsProxyPath)
-	}
-	// %40 is the encoded character of the @ symbol. VS Code Web does
-	// not handle character encoding properly, so it's safe to assume
-	// other applications might not as well.
-	r.Route("/%40{user}/{workspace_and_agent}/apps/{workspaceapp}", apps)
-	r.Route("/@{user}/{workspace_and_agent}/apps/{workspaceapp}", apps)
+		api.workspaceAppServer.Attach(r)
+	})
+
 	r.Route("/derp", func(r chi.Router) {
 		r.Get("/", derpHandler.ServeHTTP)
 		// This is used when UDP is blocked, and latency must be checked via HTTP(s).
@@ -644,9 +653,6 @@ func New(options *Options) *API {
 				r.Post("/report-lifecycle", api.workspaceAgentReportLifecycle)
 				r.Post("/metadata/{key}", api.workspaceAgentPostMetadata)
 			})
-			// No middleware on the PTY endpoint since it uses workspace
-			// application auth and signed app tokens.
-			r.Get("/{workspaceagent}/pty", api.workspaceAgentPTY)
 			r.Route("/{workspaceagent}", func(r chi.Router) {
 				r.Use(
 					apiKeyMiddleware,
@@ -655,11 +661,12 @@ func New(options *Options) *API {
 				)
 				r.Get("/", api.workspaceAgent)
 				r.Get("/watch-metadata", api.watchWorkspaceAgentMetadata)
-				r.Get("/pty", api.workspaceAgentPTY)
 				r.Get("/startup-logs", api.workspaceAgentStartupLogs)
 				r.Get("/listening-ports", api.workspaceAgentListeningPorts)
 				r.Get("/connection", api.workspaceAgentConnection)
 				r.Get("/coordinate", api.workspaceAgentClientCoordinate)
+
+				// PTY is part of workspaceAppServer.
 			})
 		})
 		r.Route("/workspaces", func(r chi.Router) {
@@ -792,6 +799,7 @@ type API struct {
 	workspaceAgentCache   *wsconncache.Cache
 	updateChecker         *updatecheck.Checker
 	WorkspaceAppsProvider workspaceapps.SignedTokenProvider
+	workspaceAppServer    *workspaceapps.Server
 
 	// Experiments contains the list of experiments currently enabled.
 	// This is used to gate features that are not yet ready for production.

diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -11,7 +11,6 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/base64"
-	"encoding/hex"
 	"encoding/json"
 	"encoding/pem"
 	"errors"
@@ -69,6 +68,7 @@ import (
 	"github.com/coder/coder/coderd/telemetry"
 	"github.com/coder/coder/coderd/updatecheck"
 	"github.com/coder/coder/coderd/util/ptr"
+	"github.com/coder/coder/coderd/workspaceapps"
 	"github.com/coder/coder/codersdk"
 	"github.com/coder/coder/codersdk/agentsdk"
 	"github.com/coder/coder/cryptorand"
@@ -81,9 +81,9 @@ import (
 	"github.com/coder/coder/testutil"
 )
 
-// AppSigningKey is a 64-byte key used to sign JWTs for workspace app tokens in
-// tests.
-var AppSigningKey = must(hex.DecodeString("64656164626565666465616462656566646561646265656664656164626565666465616462656566646561646265656664656164626565666465616462656566"))
+// AppSecurityKey is a 96-byte key used to sign JWTs and encrypt JWEs for
+// workspace app tokens in tests.
+var AppSecurityKey = must(workspaceapps.KeyFromString("6465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e207761732068657265206465616e2077617320686572"))
 
 type Options struct {
 	// AccessURL denotes a custom access URL. By default we use the httptest
@@ -346,7 +346,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
 			DeploymentValues:            options.DeploymentValues,
 			UpdateCheckOptions:          options.UpdateCheckOptions,
 			SwaggerEndpoint:             options.SwaggerEndpoint,
-			AppSigningKey:               AppSigningKey,
+			AppSecurityKey:              AppSecurityKey,
 			SSHConfig:                   options.ConfigSSH,
 			HealthcheckFunc:             options.HealthcheckFunc,
 			HealthcheckTimeout:          options.HealthcheckTimeout,

diff --git a/coderd/database/dbauthz/querier.go b/coderd/database/dbauthz/querier.go
@@ -379,14 +379,14 @@ func (q *querier) GetLogoURL(ctx context.Context) (string, error) {
 	return q.db.GetLogoURL(ctx)
 }
 
-func (q *querier) GetAppSigningKey(ctx context.Context) (string, error) {
+func (q *querier) GetAppSecurityKey(ctx context.Context) (string, error) {
 	// No authz checks
-	return q.db.GetAppSigningKey(ctx)
+	return q.db.GetAppSecurityKey(ctx)
 }
 
-func (q *querier) InsertAppSigningKey(ctx context.Context, data string) error {
+func (q *querier) UpsertAppSecurityKey(ctx context.Context, data string) error {
 	// No authz checks as this is done during startup
-	return q.db.InsertAppSigningKey(ctx, data)
+	return q.db.UpsertAppSecurityKey(ctx, data)
 }
 
 func (q *querier) GetServiceBanner(ctx context.Context) (string, error) {
@@ -994,6 +994,16 @@ func (q *querier) GetTemplateUserRoles(ctx context.Context, id uuid.UUID) ([]dat
 	return q.db.GetTemplateUserRoles(ctx, id)
 }
 
+func (q *querier) DeleteApplicationConnectAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error {
+	// TODO: This is not 100% correct because it omits apikey IDs.
+	err := q.authorizeContext(ctx, rbac.ActionDelete,
+		rbac.ResourceAPIKey.WithOwner(userID.String()))
+	if err != nil {
+		return err
+	}
+	return q.db.DeleteApplicationConnectAPIKeysByUserID(ctx, userID)
+}
+
 func (q *querier) DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error {
 	// TODO: This is not 100% correct because it omits apikey IDs.
 	err := q.authorizeContext(ctx, rbac.ActionDelete,

diff --git a/coderd/database/dbfake/databasefake.go b/coderd/database/dbfake/databasefake.go
@@ -143,7 +143,7 @@ type data struct {
 	lastUpdateCheck []byte
 	serviceBanner   []byte
 	logoURL         string
-	appSigningKey   string
+	appSecurityKey  string
 	lastLicenseID   int32
 }
 
@@ -679,6 +679,19 @@ func (q *fakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error {
 	return sql.ErrNoRows
 }
 
+func (q *fakeQuerier) DeleteApplicationConnectAPIKeysByUserID(_ context.Context, userID uuid.UUID) error {
+	q.mutex.Lock()
+	defer q.mutex.Unlock()
+
+	for i := len(q.apiKeys) - 1; i >= 0; i-- {
+		if q.apiKeys[i].UserID == userID && q.apiKeys[i].Scope == database.APIKeyScopeApplicationConnect {
+			q.apiKeys = append(q.apiKeys[:i], q.apiKeys[i+1:]...)
+		}
+	}
+
+	return nil
+}
+
 func (q *fakeQuerier) DeleteAPIKeysByUserID(_ context.Context, userID uuid.UUID) error {
 	q.mutex.Lock()
 	defer q.mutex.Unlock()
@@ -4463,18 +4476,18 @@ func (q *fakeQuerier) GetLogoURL(_ context.Context) (string, error) {
 	return q.logoURL, nil
 }
 
-func (q *fakeQuerier) GetAppSigningKey(_ context.Context) (string, error) {
+func (q *fakeQuerier) GetAppSecurityKey(_ context.Context) (string, error) {
 	q.mutex.RLock()
 	defer q.mutex.RUnlock()
 
-	return q.appSigningKey, nil
+	return q.appSecurityKey, nil
 }
 
-func (q *fakeQuerier) InsertAppSigningKey(_ context.Context, data string) error {
+func (q *fakeQuerier) UpsertAppSecurityKey(_ context.Context, data string) error {
 	q.mutex.Lock()
 	defer q.mutex.Unlock()
 
-	q.appSigningKey = data
+	q.appSecurityKey = data
 	return nil
 }
 

diff --git a/coderd/database/dbgen/generator.go b/coderd/database/dbgen/generator.go
@@ -77,12 +77,23 @@ func APIKey(t testing.TB, db database.Store, seed database.APIKey) (key database
 	secret, _ := cryptorand.String(22)
 	hashed := sha256.Sum256([]byte(secret))
 
+	ip := seed.IPAddress
+	if !ip.Valid {
+		ip = pqtype.Inet{
+			IPNet: net.IPNet{
+				IP:   net.IPv4(127, 0, 0, 1),
+				Mask: net.IPv4Mask(255, 255, 255, 255),
+			},
+			Valid: true,
+		}
+	}
+
 	key, err := db.InsertAPIKey(context.Background(), database.InsertAPIKeyParams{
 		ID: takeFirst(seed.ID, id),
 		// 0 defaults to 86400 at the db layer
 		LifetimeSeconds: takeFirst(seed.LifetimeSeconds, 0),
 		HashedSecret:    takeFirstSlice(seed.HashedSecret, hashed[:]),
-		IPAddress:       pqtype.Inet{},
+		IPAddress:       ip,
 		UserID:          takeFirst(seed.UserID, uuid.New()),
 		LastUsed:        takeFirst(seed.LastUsed, database.Now()),
 		ExpiresAt:       takeFirst(seed.ExpiresAt, database.Now().Add(time.Hour)),