Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc3f9467

Browse files
authored
fix: Strip session_token cookie from app proxy requests (coder#3528)
Fixescoder/security#1.
1 parent000e1a5 commitc3f9467

File tree

8 files changed

+94
-16
lines changed

8 files changed

+94
-16
lines changed

‎coderd/httpapi/cookie.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package httpapi
2+
3+
import (
4+
"net/textproto"
5+
"strings"
6+
7+
"github.com/coder/coder/codersdk"
8+
)
9+
10+
// StripCoderCookies removes the session token from the cookie header provided.
11+
funcStripCoderCookies(headerstring)string {
12+
header=textproto.TrimString(header)
13+
cookies:= []string{}
14+
15+
varpartstring
16+
forlen(header)>0 {// continue since we have rest
17+
part,header,_=strings.Cut(header,";")
18+
part=textproto.TrimString(part)
19+
ifpart=="" {
20+
continue
21+
}
22+
name,_,_:=strings.Cut(part,"=")
23+
ifname==codersdk.SessionTokenKey||
24+
name==codersdk.OAuth2StateKey||
25+
name==codersdk.OAuth2RedirectKey {
26+
continue
27+
}
28+
cookies=append(cookies,part)
29+
}
30+
returnstrings.Join(cookies,"; ")
31+
}

‎coderd/httpapi/cookie_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package httpapi_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
"github.com/coder/coder/coderd/httpapi"
9+
)
10+
11+
funcTestStripCoderCookies(t*testing.T) {
12+
t.Parallel()
13+
for_,tc:=range []struct {
14+
Inputstring
15+
Outputstring
16+
}{{
17+
"testing=hello; wow=test",
18+
"testing=hello; wow=test",
19+
}, {
20+
"session_token=moo; wow=test",
21+
"wow=test",
22+
}, {
23+
"another_token=wow; session_token=ok",
24+
"another_token=wow",
25+
}, {
26+
"session_token=ok; oauth_state=wow; oauth_redirect=/",
27+
"",
28+
}} {
29+
tc:=tc
30+
t.Run(tc.Input,func(t*testing.T) {
31+
t.Parallel()
32+
require.Equal(t,tc.Output,httpapi.StripCoderCookies(tc.Input))
33+
})
34+
}
35+
}

‎coderd/httpmw/oauth2.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,6 @@ import (
1313
"github.com/coder/coder/cryptorand"
1414
)
1515

16-
const (
17-
oauth2StateCookieName="oauth_state"
18-
oauth2RedirectCookieName="oauth_redirect"
19-
)
20-
2116
typeoauth2StateKeystruct{}
2217

2318
typeOAuth2Statestruct {
@@ -71,7 +66,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
7166
}
7267

7368
http.SetCookie(rw,&http.Cookie{
74-
Name:oauth2StateCookieName,
69+
Name:codersdk.OAuth2StateKey,
7570
Value:state,
7671
Path:"/",
7772
HttpOnly:true,
@@ -80,7 +75,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
8075
// Redirect must always be specified, otherwise
8176
// an old redirect could apply!
8277
http.SetCookie(rw,&http.Cookie{
83-
Name:oauth2RedirectCookieName,
78+
Name:codersdk.OAuth2RedirectKey,
8479
Value:r.URL.Query().Get("redirect"),
8580
Path:"/",
8681
HttpOnly:true,
@@ -98,10 +93,10 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
9893
return
9994
}
10095

101-
stateCookie,err:=r.Cookie(oauth2StateCookieName)
96+
stateCookie,err:=r.Cookie(codersdk.OAuth2StateKey)
10297
iferr!=nil {
10398
httpapi.Write(rw,http.StatusUnauthorized, codersdk.Response{
104-
Message:fmt.Sprintf("Cookie %q must be provided.",oauth2StateCookieName),
99+
Message:fmt.Sprintf("Cookie %q must be provided.",codersdk.OAuth2StateKey),
105100
})
106101
return
107102
}
@@ -113,7 +108,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
113108
}
114109

115110
varredirectstring
116-
stateRedirect,err:=r.Cookie(oauth2RedirectCookieName)
111+
stateRedirect,err:=r.Cookie(codersdk.OAuth2RedirectKey)
117112
iferr==nil {
118113
redirect=stateRedirect.Value
119114
}

‎coderd/httpmw/oauth2_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"golang.org/x/oauth2"
1313

1414
"github.com/coder/coder/coderd/httpmw"
15+
"github.com/coder/coder/codersdk"
1516
)
1617

1718
typetestOAuth2Providerstruct {
@@ -71,7 +72,7 @@ func TestOAuth2(t *testing.T) {
7172
t.Parallel()
7273
req:=httptest.NewRequest("GET","/?code=something&state=test",nil)
7374
req.AddCookie(&http.Cookie{
74-
Name:"oauth_state",
75+
Name:codersdk.OAuth2StateKey,
7576
Value:"mismatch",
7677
})
7778
res:=httptest.NewRecorder()
@@ -82,7 +83,7 @@ func TestOAuth2(t *testing.T) {
8283
t.Parallel()
8384
req:=httptest.NewRequest("GET","/?code=test&state=something",nil)
8485
req.AddCookie(&http.Cookie{
85-
Name:"oauth_state",
86+
Name:codersdk.OAuth2StateKey,
8687
Value:"something",
8788
})
8889
req.AddCookie(&http.Cookie{

‎coderd/userauth_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ func oauth2Callback(t *testing.T, client *codersdk.Client) *http.Response {
447447
req,err:=http.NewRequest("GET",oauthURL.String(),nil)
448448
require.NoError(t,err)
449449
req.AddCookie(&http.Cookie{
450-
Name:"oauth_state",
450+
Name:codersdk.OAuth2StateKey,
451451
Value:state,
452452
})
453453
res,err:=client.HTTPClient.Do(req)
@@ -469,7 +469,7 @@ func oidcCallback(t *testing.T, client *codersdk.Client) *http.Response {
469469
req,err:=http.NewRequest("GET",oauthURL.String(),nil)
470470
require.NoError(t,err)
471471
req.AddCookie(&http.Cookie{
472-
Name:"oauth_state",
472+
Name:codersdk.OAuth2StateKey,
473473
Value:state,
474474
})
475475
res,err:=client.HTTPClient.Do(req)

‎coderd/workspaceapps.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,12 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
170170
}
171171
deferrelease()
172172

173+
// This strips the session token from a workspace app request.
174+
cookieHeaders:=r.Header.Values("Cookie")[:]
175+
r.Header.Del("Cookie")
176+
for_,cookieHeader:=rangecookieHeaders {
177+
r.Header.Add("Cookie",httpapi.StripCoderCookies(cookieHeader))
178+
}
173179
proxy.Transport=conn.HTTPTransport()
174180
proxy.ServeHTTP(rw,r)
175181
}

‎coderd/workspaceapps_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"testing"
1010

1111
"github.com/google/uuid"
12+
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
1314

1415
"cdr.dev/slog/sloggers/slogtest"
@@ -27,6 +28,8 @@ func TestWorkspaceAppsProxyPath(t *testing.T) {
2728
require.NoError(t,err)
2829
server:= http.Server{
2930
Handler:http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {
31+
_,err:=r.Cookie(codersdk.SessionTokenKey)
32+
assert.ErrorIs(t,err,http.ErrNoCookie)
3033
w.WriteHeader(http.StatusOK)
3134
}),
3235
}

‎codersdk/client.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,15 @@ import (
1515
"nhooyr.io/websocket"
1616
)
1717

18-
// SessionTokenKey represents the name of the cookie or query parameter the API key is stored in.
19-
constSessionTokenKey="session_token"
18+
// These cookies are Coder-specific. If a new one is added or changed, the name
19+
// shouldn't be likely to conflict with any user-application set cookies.
20+
// Be sure to strip additional cookies in httpapi.StripCoder Cookies!
21+
const (
22+
// SessionTokenKey represents the name of the cookie or query parameter the API key is stored in.
23+
SessionTokenKey="session_token"
24+
OAuth2StateKey="oauth_state"
25+
OAuth2RedirectKey="oauth_redirect"
26+
)
2027

2128
// New creates a Coder client for the provided URL.
2229
funcNew(serverURL*url.URL)*Client {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp