Skip to content

Commit

Permalink
server: disable security on /api/v2/ when running insecure
Browse files Browse the repository at this point in the history 
Previously, the session validation log in `/api/v2/` would still run even when
the cluster was running in "insecure" mode. This made it cumbersome to test new
features while developing, and also caused new challenges when developing DB
Console features that use `/api/v2/` endpoints.

Now, when the cluster has the insecure flag set to true, web session check
failures won't cause failures on endpoints and the session username will be set
to "root" automatically in the context.

Release note (security update): HTTP API endpoints under the `/api/v2/` prefix,
will allow requests through when the cluster is running in "insecure" mode.
When the cluster is running in "insecure" mode requests to these endpoints will
have the username set to "root".

Release justification: low-risk high-benefit change to existing functionality.
  • Loading branch information
@dhartunian
dhartunian committed Aug 18, 2022
1 parent 7c38417 commit 109aac2
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 91 deletions.
49 changes: 27 additions & 22 deletions pkg/server/api_v2_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,16 @@ func (a *authenticationV2Server) ServeHTTP(w http.ResponseWriter, r *http.Reques
// and the request isn't routed through to the inner handler. On success, the
// username is set on the request context for use in the inner handler.
type authenticationV2Mux struct {
s *authenticationV2Server
inner http.Handler
s *authenticationV2Server
inner http.Handler
allowAnonymous bool
}

func newAuthenticationV2Mux(s *authenticationV2Server, inner http.Handler) *authenticationV2Mux {
return &authenticationV2Mux{
s: s,
inner: inner,
s: s,
inner: inner,
allowAnonymous: s.sqlServer.cfg.Insecure,
}
}

Expand All @@ -293,14 +295,13 @@ const apiV2UseCookieBasedAuth = "cookie"
// and also sends the error over http using w.
func (a *authenticationV2Mux) getSession(
w http.ResponseWriter, req *http.Request,
) (string, *serverpb.SessionCookie, error) {
) (string, *serverpb.SessionCookie, int, error) {
ctx := req.Context()
// Validate the returned session header or cookie.
rawSession := req.Header.Get(apiV2AuthHeader)
if len(rawSession) == 0 {
err := errors.New("invalid session header")
http.Error(w, err.Error(), http.StatusUnauthorized)
return "", nil, err
return "", nil, http.StatusUnauthorized, err
}

possibleSessions := []string{}
Expand Down Expand Up @@ -335,36 +336,40 @@ func (a *authenticationV2Mux) getSession(
}
if err != nil {
err := errors.New("invalid session header")
http.Error(w, err.Error(), http.StatusBadRequest)
return "", nil, err
return "", nil, http.StatusBadRequest, err
}
valid, username, err := a.s.authServer.verifySession(req.Context(), sessionCookie)
if err != nil {
apiV2InternalError(req.Context(), err, w)
return "", nil, err
return "", nil, http.StatusInternalServerError, err
}
if !valid {
err := errors.New("the provided authentication session could not be validated")
http.Error(w, err.Error(), http.StatusUnauthorized)
return "", nil, err
return "", nil, http.StatusUnauthorized, err
}

return username, sessionCookie, nil
return username, sessionCookie, http.StatusOK, nil
}

func (a *authenticationV2Mux) ServeHTTP(w http.ResponseWriter, req *http.Request) {
username, cookie, err := a.getSession(w, req)
if err == nil {
// Valid session found. Set the username in the request context, so
// child http.Handlers can access it.
ctx := req.Context()
ctx = context.WithValue(ctx, webSessionUserKey{}, username)
ctx = context.WithValue(ctx, webSessionIDKey{}, cookie.ID)
req = req.WithContext(ctx)
} else {
u, cookie, errStatus, err := a.getSession(w, req)
if err != nil && !a.allowAnonymous {
// getSession writes an error to w if err != nil.
http.Error(w, err.Error(), errStatus)
return
}
if a.allowAnonymous {
u = username.RootUser
}
// Valid session found, or insecure. Set the username in the request context,
// so child http.Handlers can access it.
ctx := req.Context()
ctx = context.WithValue(ctx, webSessionUserKey{}, u)
if cookie != nil {
ctx = context.WithValue(ctx, webSessionIDKey{}, cookie.ID)
}
req = req.WithContext(ctx)

a.inner.ServeHTTP(w, req)
}

Expand Down
151 changes: 82 additions & 69 deletions pkg/server/api_v2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -191,77 +192,89 @@ func TestAuthV2(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

testCluster := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{})
ctx := context.Background()
defer testCluster.Stopper().Stop(ctx)

ts := testCluster.Server(0)
client, err := ts.GetUnauthenticatedHTTPClient()
require.NoError(t, err)
testutils.RunTrueAndFalse(t, "insecure", func(t *testing.T, insecure bool) {
testCluster := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{
ServerArgs: base.TestServerArgs{
Insecure: insecure,
},
})
ctx := context.Background()
defer testCluster.Stopper().Stop(ctx)

session, err := ts.GetAuthSession(true)
require.NoError(t, err)
sessionBytes, err := protoutil.Marshal(session)
require.NoError(t, err)
sessionEncoded := base64.StdEncoding.EncodeToString(sessionBytes)

for _, tc := range []struct {
name string
header string
cookie string
expectedStatus int
}{
{
name: "no auth",
expectedStatus: http.StatusUnauthorized,
},
{
name: "session in header",
header: sessionEncoded,
expectedStatus: http.StatusOK,
},
{
name: "cookie auth with correct magic header",
cookie: sessionEncoded,
header: apiV2UseCookieBasedAuth,
expectedStatus: http.StatusOK,
},
{
name: "cookie auth but missing header",
cookie: sessionEncoded,
expectedStatus: http.StatusUnauthorized,
},
{
name: "cookie auth but wrong magic header",
cookie: sessionEncoded,
header: "yes",
// Bad Request and not Unauthorized because the session cannot be decoded.
expectedStatus: http.StatusBadRequest,
},
} {
t.Run(tc.name, func(t *testing.T) {
req, err := http.NewRequest("GET", ts.AdminURL()+apiV2Path+"sessions/", nil)
require.NoError(t, err)
if tc.header != "" {
req.Header.Set(apiV2AuthHeader, tc.header)
}
if tc.cookie != "" {
req.AddCookie(&http.Cookie{
Name: SessionCookieName,
Value: tc.cookie,
})
}
resp, err := client.Do(req)
require.NoError(t, err)
require.NotNil(t, resp)
defer resp.Body.Close()
ts := testCluster.Server(0)
client, err := ts.GetUnauthenticatedHTTPClient()
require.NoError(t, err)

if tc.expectedStatus != resp.StatusCode {
body, err := ioutil.ReadAll(resp.Body)
session, err := ts.GetAuthSession(true)
require.NoError(t, err)
sessionBytes, err := protoutil.Marshal(session)
require.NoError(t, err)
sessionEncoded := base64.StdEncoding.EncodeToString(sessionBytes)

for _, tc := range []struct {
name string
header string
cookie string
expectedStatus int
}{
{
name: "no auth",
expectedStatus: http.StatusUnauthorized,
},
{
name: "session in header",
header: sessionEncoded,
expectedStatus: http.StatusOK,
},
{
name: "cookie auth with correct magic header",
cookie: sessionEncoded,
header: apiV2UseCookieBasedAuth,
expectedStatus: http.StatusOK,
},
{
name: "cookie auth but missing header",
cookie: sessionEncoded,
expectedStatus: http.StatusUnauthorized,
},
{
name: "cookie auth but wrong magic header",
cookie: sessionEncoded,
header: "yes",
// Bad Request and not Unauthorized because the session cannot be decoded.
expectedStatus: http.StatusBadRequest,
},
} {
t.Run(tc.name, func(t *testing.T) {
req, err := http.NewRequest("GET", ts.AdminURL()+apiV2Path+"sessions/", nil)
require.NoError(t, err)
t.Fatalf("expected status: %d but got: %d with body: %s", tc.expectedStatus, resp.StatusCode, string(body))
}
})
}
if tc.header != "" {
req.Header.Set(apiV2AuthHeader, tc.header)
}
if tc.cookie != "" {
req.AddCookie(&http.Cookie{
Name: SessionCookieName,
Value: tc.cookie,
})
}
resp, err := client.Do(req)
require.NoError(t, err)
require.NotNil(t, resp)
defer resp.Body.Close()

if !insecure && tc.expectedStatus != resp.StatusCode {
body, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
t.Fatalf("expected status: %d but got: %d with body: %s", tc.expectedStatus, resp.StatusCode, string(body))
}
if insecure && http.StatusOK != resp.StatusCode {
body, err := ioutil.ReadAll(resp.Body)
require.NoError(t, err)
t.Fatalf("expected status: %d but got: %d with body: %s", http.StatusOK, resp.StatusCode, string(body))
}
})
}

})

}

0 comments on commit 109aac2

Please sign in to comment.