diff --git a/sherlock/go.mod b/sherlock/go.mod index c912261d..f892e906 100644 --- a/sherlock/go.mod +++ b/sherlock/go.mod @@ -38,7 +38,7 @@ require ( github.com/sethvargo/go-password v0.3.1 github.com/slack-go/slack v0.14.0 github.com/stretchr/testify v1.9.0 - github.com/swaggo/files v1.0.1 + github.com/swaggo/files/v2 v2.0.2-0.20240712141554-0590c09f83eb github.com/swaggo/gin-swagger v1.6.1-0.20231210095754-aa92a0ac3f26 github.com/swaggo/swag v1.16.3 github.com/zitadel/oidc/v3 v3.30.1 diff --git a/sherlock/go.sum b/sherlock/go.sum index 28ebb7ea..e2c37bd2 100644 --- a/sherlock/go.sum +++ b/sherlock/go.sum @@ -657,6 +657,9 @@ github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8 github.com/stvp/go-udp-testing v0.0.0-20201019212854-469649b16807/go.mod h1:7jxmlfBCDBXRzr0eAQJ48XC1hBu1np4CS5+cHEYfwpc= github.com/swaggo/files v1.0.1 h1:J1bVJ4XHZNq0I46UU90611i9/YzdrF7x92oX1ig5IdE= github.com/swaggo/files v1.0.1/go.mod h1:0qXmMNH6sXNf+73t65aKeB+ApmgxdnkQzVTAj2uaMUg= +github.com/swaggo/files/v2 v2.0.1/go.mod h1:24kk2Y9NYEJ5lHuCra6iVwkMjIekMCaFq/0JQj66kyM= +github.com/swaggo/files/v2 v2.0.2-0.20240712141554-0590c09f83eb h1:rAois/AXxsf+CK1cBbGg6hQdauV62sDimiepyJa3BTM= +github.com/swaggo/files/v2 v2.0.2-0.20240712141554-0590c09f83eb/go.mod h1:24kk2Y9NYEJ5lHuCra6iVwkMjIekMCaFq/0JQj66kyM= github.com/swaggo/gin-swagger v1.6.1-0.20231210095754-aa92a0ac3f26 h1:7APoRVOgfnHlp4HfZ/5YudTmNZCEgPAmjJpDzESCUOY= github.com/swaggo/gin-swagger v1.6.1-0.20231210095754-aa92a0ac3f26/go.mod h1:BG00cCEy294xtVpyIAHG6+e2Qzj/xKlRdOqDkvq0uzo= github.com/swaggo/swag v1.16.3 h1:PnCYjPCah8FK4I26l2F/KQ4yz3sILcVUN3cTlBFA9Pg= diff --git a/sherlock/internal/boot/router.go b/sherlock/internal/boot/router.go index 7ec27200..381c5d14 100644 --- a/sherlock/internal/boot/router.go +++ b/sherlock/internal/boot/router.go @@ -20,10 +20,13 @@ import ( "github.com/broadinstitute/sherlock/sherlock/internal/middleware/security" "github.com/broadinstitute/sherlock/sherlock/internal/oidc_models" "github.com/gin-gonic/gin" - swaggo_files "github.com/swaggo/files" + swaggo_files "github.com/swaggo/files/v2" swaggo_gin "github.com/swaggo/gin-swagger" + "golang.org/x/net/webdav" "gorm.io/gorm" + "io/fs" "net/http" + "os" "strings" ) @@ -83,12 +86,16 @@ func BuildRouter(ctx context.Context, db *gorm.DB) *gin.Engine { router.StaticFS("/static", http.FS(html.StaticHtmlFiles)) - router.GET("/swagger/*any", swaggo_gin.WrapHandler(swaggo_files.Handler, func(c *swaggo_gin.Config) { - c.Title = "Sherlock Swagger UI" - c.URL = "/swagger/doc.json" - c.DefaultModelsExpandDepth = 2 - c.DocExpansion = "none" - })) + router.GET("/swagger/*any", swaggo_gin.WrapHandler( + &webdav.Handler{ + FileSystem: &webdavFilesystem{http.FS(swaggo_files.FS)}, + LockSystem: webdav.NewMemLS(), + }, func(c *swaggo_gin.Config) { + c.Title = "Sherlock Swagger UI" + c.URL = "/swagger/doc.json" + c.DefaultModelsExpandDepth = 2 + c.DocExpansion = "none" + })) router.GET("", func(ctx *gin.Context) { ctx.Redirect(http.StatusMovedPermanently, "/swagger/index.html") }) if config.Config.Bool("oidc.enable") { @@ -117,3 +124,56 @@ func BuildRouter(ctx context.Context, db *gorm.DB) *gin.Engine { return router } + +// webdavFilesystem adapts a http.FileSystem to a webdav.FileSystem. This is unfortunately necessary because +// swaggo_gin.WrapHandler expects a webdav.Handler that needs a webdav.FileSystem (even though we don't need +// most of what webdav.FileSystem provides). There's actually a package online that does this but it's not +// popular and this is so small I judge it makes sense to just write what we actually need ourselves. +// +// For context -- why does swaggo_gin.WrapHandler expect a webdav.Handler if swaggo_files.FS is a http.FileSystem? +// Because we're using an updated version of swaggo_files to pull in a more recent non-vulnerable version of +// swagger-ui. swaggo_gin never got an update to match, so we're stuck with this weirdness. +type webdavFilesystem struct { + http.FileSystem +} + +func (w *webdavFilesystem) Mkdir(_ context.Context, _ string, _ os.FileMode) error { + return os.ErrPermission +} + +func (w *webdavFilesystem) RemoveAll(_ context.Context, _ string) error { + return os.ErrPermission +} + +func (w *webdavFilesystem) Rename(_ context.Context, _, _ string) error { + return os.ErrPermission +} + +func (w *webdavFilesystem) Stat(_ context.Context, name string) (os.FileInfo, error) { + if f, err := w.FileSystem.Open(name); err != nil { + return nil, err + } else { + defer func(f fs.File) { + _ = f.Close() + }(f) + return f.Stat() + } +} + +func (w *webdavFilesystem) OpenFile(_ context.Context, name string, flag int, _ os.FileMode) (webdav.File, error) { + if flag != os.O_RDONLY { + return nil, os.ErrPermission + } else if f, err := w.FileSystem.Open(name); err != nil { + return nil, err + } else { + return &webdavFile{f}, nil + } +} + +type webdavFile struct { + http.File +} + +func (w *webdavFile) Write(_ []byte) (n int, err error) { + return 0, &os.PathError{Op: "write", Err: os.ErrPermission} +} diff --git a/sherlock/internal/middleware/security/middleware.go b/sherlock/internal/middleware/security/middleware.go index 389a028f..a82ab358 100644 --- a/sherlock/internal/middleware/security/middleware.go +++ b/sherlock/internal/middleware/security/middleware.go @@ -1,26 +1,11 @@ package security import ( - "crypto/sha256" - "encoding/base64" - "fmt" "github.com/gin-contrib/secure" "github.com/gin-gonic/gin" - "strings" ) -var styleAttributeValuesToAllow = []string{ - "position:absolute;width:0;height:0", -} - func Security() gin.HandlerFunc { - styleAttributeHashes := make([]string, len(styleAttributeValuesToAllow)) - for i, value := range styleAttributeValuesToAllow { - hash := sha256.Sum256([]byte(value)) - styleAttributeHashes[i] = fmt.Sprintf("'sha256-%s'", base64.StdEncoding.EncodeToString(hash[:])) - } - styleDirective := fmt.Sprintf("style-src 'self' 'unsafe-hashes' %s; ", strings.Join(styleAttributeHashes, " ")) - c := secure.Config{ // TLS is terminated at the proxy and/or load balancer, so we // don't enable any TLS-related behavior. Some of the rest of @@ -30,7 +15,7 @@ func Security() gin.HandlerFunc { BrowserXssFilter: true, ContentSecurityPolicy: "default-src 'self'; " + "img-src 'self' data:; " + - styleDirective + + "style-src 'self' 'unsafe-inline'; " + "frame-ancestors 'none'; ", IENoOpen: true, ReferrerPolicy: "strict-origin-when-cross-origin", diff --git a/sherlock/internal/middleware/security/middleware_test.go b/sherlock/internal/middleware/security/middleware_test.go index 01bfcf64..50b18520 100644 --- a/sherlock/internal/middleware/security/middleware_test.go +++ b/sherlock/internal/middleware/security/middleware_test.go @@ -24,6 +24,6 @@ func TestSecurity(t *testing.T) { router.ServeHTTP(recorder, request) assert.Contains(t, recorder.Header().Get("Content-Security-Policy"), "default-src 'self'; ") - assert.Contains(t, recorder.Header().Get("Content-Security-Policy"), "style-src 'self' 'unsafe-hashes' 'sha256-") + assert.Contains(t, recorder.Header().Get("Content-Security-Policy"), "style-src 'self' 'unsafe-inline'; ") }) }