Switch to forked httprouter and enable UseRawPath option (#11068)

* Use forked httprouter with RawPath fix: gravitational/httprouter

* Enable UseRawPath everywhere.

* Test: allow MFA devices with `/` in names to be deleted

Co-authored-by: Przemko Robakowski <przemko.robakowski@goteleport.com>

go.mod

@@ -215,6 +215,7 @@ replace (
 	github.com/coreos/go-oidc => github.com/gravitational/go-oidc v0.0.6
 	github.com/gogo/protobuf => github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf
 	github.com/gravitational/teleport/api => ./api
+	github.com/julienschmidt/httprouter => github.com/gravitational/httprouter v1.3.1-0.20220408074523-c876c5e705a5
 	github.com/siddontang/go-mysql v1.1.0 => github.com/gravitational/go-mysql v1.1.1-teleport.2
 	github.com/sirupsen/logrus => github.com/gravitational/logrus v1.4.4-0.20210817004754-047e20245621
 )

go.sum

@@ -440,6 +440,8 @@ github.com/gravitational/go-mysql v1.1.1-teleport.2 h1:XZ36BZ7BgslA5ZCyCHjpc1wil
 github.com/gravitational/go-mysql v1.1.1-teleport.2/go.mod h1:re0JQZ1Cy5dVlIDGq0YksfDIla/GRZlxqOoC0XPSSGE=
 github.com/gravitational/go-oidc v0.0.6 h1:DCllahGYxDAvxWsq8UILgO+/i1EheQRxcNzS+D+wP5I=
 github.com/gravitational/go-oidc v0.0.6/go.mod h1:SevmOUNdOB0aD9BAIgjptZ6oHkKxMZZgA70nwPfgU/w=
+github.com/gravitational/httprouter v1.3.1-0.20220408074523-c876c5e705a5 h1:qg8FcGwRACSHortU1UxCSo9nF0t34rPWjk9Nef3j2Ic=
+github.com/gravitational/httprouter v1.3.1-0.20220408074523-c876c5e705a5/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
 github.com/gravitational/kingpin v2.1.11-0.20190130013101-742f2714c145+incompatible h1:CfyZl3nyo9K5lLqOmqvl9/IElY1UCnOWKZiQxJ8HKdA=
 github.com/gravitational/kingpin v2.1.11-0.20190130013101-742f2714c145+incompatible/go.mod h1:LWxG30M3FcrjhOn3T4zz7JmBoQJ45MWZmOXgy9Ganoc=
 github.com/gravitational/license v0.0.0-20210218173955-6d8fb49b117a h1:PN5vAN1ZA0zqdpM6wNdx6+bkdlQ5fImd75oaIHSbOhY=
@@ -560,9 +562,6 @@ github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1
 github.com/jstemmer/go-junit-report v0.9.1/go.mod h1:Brl9GWCQeLvo8nXZwPNNblvFj/XSXhF0NWZEnDohbsk=
 github.com/jtolds/gls v4.20.0+incompatible h1:xdiiI2gbIgH/gLH7ADydsJ1uDOEzR8yvV7C0MuV77Wo=
 github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU=
-github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
-github.com/julienschmidt/httprouter v1.3.0 h1:U0609e9tgbseu3rBINet9P48AI/D3oJs4dN7jwJOQ1U=
-github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
 github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0 h1:iQTw/8FWTuc7uiaSepXwyf3o52HaUYcV+Tu66S3F5GA=
 github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8=
 github.com/karrick/godirwalk v1.8.0/go.mod h1:H5KPZjojv4lE+QYImBI8xVtrBRgYrIVsaRPx4tDPEn4=

lib/auth/apiserver.go

@@ -91,6 +91,7 @@ func NewAPIServer(config *APIConfig) (http.Handler, error) {
 		Clock:     clockwork.NewRealClock(),
 	}
 	srv.Router = *httprouter.New()
+	srv.Router.UseRawPath = true
 
 	// Kubernetes extensions
 	srv.POST("/:version/kube/csr", srv.withAuth(srv.processKubeCSR))

lib/httplib/httplib_test.go

@@ -62,6 +62,7 @@ type testHandler struct {
 func newTestHandler() *testHandler {
 	h := &testHandler{}
 	h.Router = *httprouter.New()
+	h.Router.UseRawPath = true
 	h.POST("/v1/sessions/:id/stream", MakeHandler(h.postSessionChunkOriginal))
 	h.POST("/v1/namespaces/:namespace/sessions/:id/stream", MakeHandler(h.postSessionChunkNamespace))
 	return h

lib/kube/proxy/forwarder.go

@@ -239,6 +239,8 @@ func NewForwarder(cfg ForwarderConfig) (*Forwarder, error) {
 		close:             close,
 	}
 
+	fwd.router.UseRawPath = true
+
 	fwd.router.POST("/api/:ver/namespaces/:podNamespace/pods/:podName/exec", fwd.withAuth(fwd.exec))
 	fwd.router.GET("/api/:ver/namespaces/:podNamespace/pods/:podName/exec", fwd.withAuth(fwd.exec))
 

lib/web/apiserver.go

@@ -229,6 +229,9 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*WebAPIHandler, error) {
 		ClusterFeatures: cfg.ClusterFeatures,
 	}
 
+	// for properly handling url-encoded parameter values.
+	h.UseRawPath = true
+
 	for _, o := range opts {
 		if err := o(h); err != nil {
 			return nil, trace.Wrap(err)

lib/web/apiserver_test.go

@@ -2331,6 +2331,53 @@ func TestAddMFADevice(t *testing.T) {
 	}
 }
 
+func TestDeleteMFA(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	env := newWebPack(t, 1)
+	proxy := env.proxies[0]
+	pack := proxy.authPack(t, "foo@example.com")
+
+	//setting up client manually because we need sanitizer off
+	jar, err := cookiejar.New(nil)
+	require.NoError(t, err)
+	opts := []roundtrip.ClientParam{roundtrip.BearerAuth(pack.session.Token), roundtrip.CookieJar(jar), roundtrip.HTTPClient(client.NewInsecureWebClient())}
+	rclt, err := roundtrip.NewClient(proxy.webURL.String(), teleport.WebAPIVersion, opts...)
+	require.NoError(t, err)
+	clt := client.WebClient{Client: rclt}
+	jar.SetCookies(&proxy.webURL, pack.cookies)
+
+	totpCode, err := totp.GenerateCode(pack.otpSecret, env.clock.Now().Add(30*time.Second))
+	require.NoError(t, err)
+
+	// Obtain a privilege token.
+	endpoint := pack.clt.Endpoint("webapi", "users", "privilege", "token")
+	re, err := pack.clt.PostJSON(ctx, endpoint, &privilegeTokenRequest{
+		SecondFactorToken: totpCode,
+	})
+	require.NoError(t, err)
+
+	var privilegeToken string
+	require.NoError(t, json.Unmarshal(re.Bytes(), &privilegeToken))
+
+	names := []string{"x", "??", "%123/", "///", "my/device", "?/%&*1"}
+	for _, devName := range names {
+		devName := devName
+		t.Run(devName, func(t *testing.T) {
+			t.Parallel()
+			otpSecret := base32.StdEncoding.EncodeToString([]byte(devName))
+			dev, err := services.NewTOTPDevice(devName, otpSecret, env.clock.Now())
+			require.NoError(t, err)
+			err = env.server.Auth().UpsertMFADevice(ctx, pack.user, dev)
+			require.NoError(t, err)
+
+			enc := url.PathEscape(devName)
+			_, err = clt.Delete(ctx, pack.clt.Endpoint("webapi", "mfa", "token", privilegeToken, "devices", enc))
+			require.NoError(t, err)
+		})
+	}
+}
+
 func TestGetMFADevicesWithAuth(t *testing.T) {
 	t.Parallel()
 	env := newWebPack(t, 1)

lib/web/app/handler.go

@@ -122,6 +122,7 @@ func NewHandler(ctx context.Context, c *HandlerConfig) (*Handler, error) {
 
 	// Create the application routes.
 	h.router = httprouter.New()
+	h.router.UseRawPath = true
 	h.router.GET("/x-teleport-auth", makeRouterHandler(h.handleFragment))
 	h.router.POST("/x-teleport-auth", makeRouterHandler(h.handleFragment))
 	h.router.GET("/teleport-logout", h.withRouterAuth(h.handleLogout))

vendor/modules.txt

@@ -543,7 +543,7 @@ github.com/jonboulle/clockwork
 # github.com/json-iterator/go v1.1.11
 ## explicit; go 1.12
 github.com/json-iterator/go
-# github.com/julienschmidt/httprouter v1.3.0
+# github.com/julienschmidt/httprouter v1.3.0 => github.com/gravitational/httprouter v1.3.1-0.20220408074523-c876c5e705a5
 ## explicit; go 1.7
 github.com/julienschmidt/httprouter
 # github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0
@@ -1287,4 +1287,5 @@ sigs.k8s.io/yaml
 # github.com/coreos/go-oidc => github.com/gravitational/go-oidc v0.0.6
 # github.com/gogo/protobuf => github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf
 # github.com/gravitational/teleport/api => ./api
+# github.com/julienschmidt/httprouter => github.com/gravitational/httprouter v1.3.1-0.20220408074523-c876c5e705a5
 # github.com/sirupsen/logrus => github.com/gravitational/logrus v1.4.4-0.20210817004754-047e20245621