Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
server: set Secure on cookies if cluster is secure
Browse files Browse the repository at this point in the history
Cookie `Secure` setting is based on `disableTLSForHTTP` passed down
from the server.

Part of CRDB-36034
Epic: None

Release note (security update): DB Console cookies are marked `Secure`
for the browser when the cluster is running in secure mode.
dhartunian committed Feb 14, 2024
1 parent 92c04a3 commit 50998f5
Showing 4 changed files with 15 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/ccl/serverccl/server_controller_test.go
Original file line number Diff line number Diff line change
@@ -336,6 +336,7 @@ func TestServerControllerDefaultHTTPTenant(t *testing.T) {
for _, c := range resp.Cookies() {
if c.Name == authserver.TenantSelectCookieName {
tenantCookie = c.Value
require.True(t, c.Secure)
}
}
require.Equal(t, "hello", tenantCookie)
@@ -554,6 +555,7 @@ func TestServerControllerLoginLogout(t *testing.T) {
for i, c := range resp.Cookies() {
cookieNames[i] = c.Name
cookieValues[i] = c.Value
require.True(t, c.Secure)
if c.Name == "session" {
require.True(t, c.HttpOnly)
}
@@ -581,6 +583,7 @@ func TestServerControllerLoginLogout(t *testing.T) {
for i, c := range resp.Cookies() {
cookieNames[i] = c.Name
cookieValues[i] = c.Value
require.True(t, c.Secure)
if c.Name == "session" {
require.True(t, c.HttpOnly)
}
@@ -609,6 +612,7 @@ func TestServerControllerLoginLogout(t *testing.T) {
for i, c := range resp.Cookies() {
cookieNames[i] = c.Name
cookieValues[i] = c.Value
require.True(t, c.Secure)
if c.Name == "session" {
require.True(t, c.HttpOnly)
}
1 change: 1 addition & 0 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
@@ -1233,6 +1233,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf
systemTenantNameContainer,
pgPreServer.SendRoutingError,
tenantCapabilitiesWatcher,
cfg.BaseConfig.DisableTLSForHTTP,
)
drain.serverCtl = sc

4 changes: 4 additions & 0 deletions pkg/server/server_controller.go
Original file line number Diff line number Diff line change
@@ -90,6 +90,8 @@ type serverController struct {

watcher *tenantcapabilitieswatcher.Watcher

disableTLSForHTTP bool

mu struct {
syncutil.RWMutex

@@ -124,6 +126,7 @@ func newServerController(
systemTenantNameContainer *roachpb.TenantNameContainer,
sendSQLRoutingError func(ctx context.Context, conn net.Conn, tenantName roachpb.TenantName),
watcher *tenantcapabilitieswatcher.Watcher,
disableTLSForHTTP bool,
) *serverController {
c := &serverController{
AmbientContext: ambientCtx,
@@ -136,6 +139,7 @@ func newServerController(
watcher: watcher,
tenantWaiter: singleflight.NewGroup("tenant server poller", "poll"),
drainCh: make(chan struct{}),
disableTLSForHTTP: disableTLSForHTTP,
}
c.orchestrator = newServerOrchestrator(parentStopper, c)
c.mu.servers = map[roachpb.TenantName]serverState{
6 changes: 6 additions & 0 deletions pkg/server/server_controller_http.go
Original file line number Diff line number Diff line change
@@ -103,13 +103,15 @@ func (c *serverController) httpMux(w http.ResponseWriter, r *http.Request) {
Value: "",
Path: "/",
HttpOnly: true,
Secure: !c.disableTLSForHTTP,
Expires: timeutil.Unix(0, 0),
})
http.SetCookie(w, &http.Cookie{
Name: authserver.TenantSelectCookieName,
Value: "",
Path: "/",
HttpOnly: false,
Secure: !c.disableTLSForHTTP,
Expires: timeutil.Unix(0, 0),
})
// Fall back to serving requests from the default tenant. This helps us serve
@@ -236,6 +238,7 @@ func (c *serverController) attemptLoginToAllTenants() http.Handler {
Value: sessionsStr,
Path: "/",
HttpOnly: true,
Secure: !c.disableTLSForHTTP,
}
http.SetCookie(w, &cookie)
// The tenant cookie needs to be set at some point in order for
@@ -257,6 +260,7 @@ func (c *serverController) attemptLoginToAllTenants() http.Handler {
Value: tenantSelection,
Path: "/",
HttpOnly: false,
Secure: !c.disableTLSForHTTP,
}
http.SetCookie(w, &cookie)
if r.Header.Get(AcceptHeader) == JSONContentType {
@@ -354,6 +358,7 @@ func (c *serverController) attemptLogoutFromAllTenants() http.Handler {
Value: "",
Path: "/",
HttpOnly: true,
Secure: !c.disableTLSForHTTP,
Expires: timeutil.Unix(0, 0),
}
http.SetCookie(w, &cookie)
@@ -362,6 +367,7 @@ func (c *serverController) attemptLogoutFromAllTenants() http.Handler {
Value: "",
Path: "/",
HttpOnly: false,
Secure: !c.disableTLSForHTTP,
Expires: timeutil.Unix(0, 0),
}
http.SetCookie(w, &cookie)

0 comments on commit 50998f5

Please sign in to comment.