Skip to content

Commit

Permalink
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.
  • Loading branch information
dhartunian committed Feb 15, 2024
1 parent 11345db commit a2220dd
Show file tree
Hide file tree
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
Expand Up @@ -338,6 +338,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)
Expand Down Expand Up @@ -558,6 +559,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)
}
Expand Down Expand Up @@ -585,6 +587,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)
}
Expand Down Expand Up @@ -613,6 +616,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)
}
Expand Down
1 change: 1 addition & 0 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (serverctl.ServerStartupInterf
pgPreServer.SendRoutingError,
tenantCapabilitiesWatcher,
cfg.DisableSQLServer,
cfg.BaseConfig.DisableTLSForHTTP,
)
drain.serverCtl = sc

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

watcher *tenantcapabilitieswatcher.Watcher

disableTLSForHTTP bool

mu struct {
syncutil.RWMutex

Expand Down Expand Up @@ -128,6 +130,7 @@ func newServerController(
sendSQLRoutingError func(ctx context.Context, conn net.Conn, tenantName roachpb.TenantName),
watcher *tenantcapabilitieswatcher.Watcher,
disableSQLServer bool,
disableTLSForHTTP bool,
) *serverController {
c := &serverController{
AmbientContext: ambientCtx,
Expand All @@ -141,6 +144,7 @@ func newServerController(
tenantWaiter: singleflight.NewGroup("tenant server poller", "poll"),
drainCh: make(chan struct{}),
disableSQLServer: disableSQLServer,
disableTLSForHTTP: disableTLSForHTTP,
}
c.orchestrator = newServerOrchestrator(parentStopper, c)
c.mu.servers = map[roachpb.TenantName]serverState{
Expand Down
6 changes: 6 additions & 0 deletions pkg/server/server_controller_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit a2220dd

Please sign in to comment.