diff --git a/terraform/test/local-secure/run_local_secure.sh b/terraform/test/local-secure/run_local_secure.sh index 5e5588d83..ffe606c5c 100755 --- a/terraform/test/local-secure/run_local_secure.sh +++ b/terraform/test/local-secure/run_local_secure.sh @@ -113,7 +113,7 @@ done wait_http "http://127.0.0.1:$VPORT/healthz" 60 && ok "volume /healthz up (mTLS)" || bad "volume /healthz" launch filer-f0 -wait_http "http://127.0.0.1:$FPORT/" 30 && ok "filer / up (mTLS)" || bad "filer /" +wait_http "http://127.0.0.1:$FPORT/healthz" 30 && ok "filer /healthz up (mTLS)" || bad "filer /healthz" # [jwt.filer_signing] is active, so the filer requires a signed JWT for writes. # An unsigned write MUST be rejected with 401 -- this proves the JWT signing key # rendered into security.toml is enforced (positive security assertion). diff --git a/weed/command/filer.go b/weed/command/filer.go index 66c5a5813..6b3503aac 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -391,6 +391,7 @@ func (fo *FilerOptions) startFiler() { DownloadMaxBytesPs: int64(*fo.downloadMaxMBps) * 1024 * 1024, DiskType: *fo.diskType, AllowedOrigins: strings.Split(*fo.allowedOrigins, ","), + ExposeDirectoryData: *fo.exposeDirectoryData, TusBasePath: *fo.tusBasePath, TusMaxSize: int64(*fo.tusMaxSizeMB) * 1024 * 1024, TusSessionExpiry: *fo.tusSessionExpiry, diff --git a/weed/filer/filechunk_manifest.go b/weed/filer/filechunk_manifest.go index 83bc90135..f7c69766a 100644 --- a/weed/filer/filechunk_manifest.go +++ b/weed/filer/filechunk_manifest.go @@ -109,7 +109,7 @@ func fetchWholeChunk(ctx context.Context, bytesBuffer *bytes.Buffer, lookupFileI glog.ErrorfCtx(ctx, "operation LookupFileId %s failed, err: %v", fileId, err) return err } - jwt := JwtForVolumeServer(fileId) + jwt := ChunkReadJwt(urlStrings, fileId) if _, err = retriedStreamFetchChunkData(ctx, bytesBuffer, urlStrings, jwt, cipherKey, isGzipped, true, 0, 0, refreshUrls(ctx, invalidator, lookupFileIdFn, fileId)); err == nil { return nil } diff --git a/weed/filer/stream.go b/weed/filer/stream.go index 096f46b17..a910061f6 100644 --- a/weed/filer/stream.go +++ b/weed/filer/stream.go @@ -51,6 +51,18 @@ func JwtForVolumeServer(fileId string) string { return string(security.GenJwtForVolumeServer(jwtSigningReadKey, jwtSigningReadKeyExpires, fileId)) } +// ChunkReadJwt returns the credential for reading fileId from urlStrings. A +// lookup answers with the volume servers holding the needle or with a filer +// proxying it, never a mix. A proxied chunk is a request to the filer, which +// authorizes it and attaches the volume credential itself, so the token there +// is a filer one. +func ChunkReadJwt(urlStrings []string, fileId string) string { + if len(urlStrings) > 0 && util_http.IsProxyChunkUrl(urlStrings[0]) { + return util_http.JwtForFilerServer(false) + } + return JwtForVolumeServer(fileId) +} + func HasData(entry *filer_pb.Entry) bool { if len(entry.Content) > 0 { @@ -501,7 +513,7 @@ func (c *ChunkStreamReader) fetchChunkToBuffer(chunkView *ChunkView) error { // pre-size to the known chunk size; avoids bytes.Buffer's doubling regrowth buffer.Grow(int(chunkView.ViewSize)) var shouldRetry bool - jwt := JwtForVolumeServer(chunkView.FileId) + jwt := ChunkReadJwt(urlStrings, chunkView.FileId) for _, urlString := range urlStrings { shouldRetry, err = util_http.ReadUrlAsStream(context.Background(), util_http.AppendQueryParameter(urlString, "readDeleted", "true"), jwt, chunkView.CipherKey, chunkView.IsGzipped, chunkView.IsFullChunk(), chunkView.OffsetInChunk, int(chunkView.ViewSize), func(data []byte) { buffer.Write(data) diff --git a/weed/filer/stream_jwt_test.go b/weed/filer/stream_jwt_test.go new file mode 100644 index 000000000..0b09eb2a1 --- /dev/null +++ b/weed/filer/stream_jwt_test.go @@ -0,0 +1,43 @@ +package filer + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/security" + "github.com/seaweedfs/seaweedfs/weed/util" +) + +// A chunk read through a filer is a request to the filer, so it carries a filer +// token; a read straight from the volume server holding the needle carries a +// volume token scoped to that file id. Sending the wrong one is not a +// degradation but a 401, so the two must not be confused. +func TestChunkReadJwt(t *testing.T) { + const ( + volumeReadKey = "volume-read-key" + filerReadKey = "filer-read-key" + fileId = "3,01637037d6" + ) + util.GetViper().Set("jwt.filer_signing.read.key", filerReadKey) + loadJwtConfigOnce.Do(func() {}) + previousKey, previousExpires := jwtSigningReadKey, jwtSigningReadKeyExpires + t.Cleanup(func() { jwtSigningReadKey, jwtSigningReadKeyExpires = previousKey, previousExpires }) + jwtSigningReadKey, jwtSigningReadKeyExpires = security.SigningKey(volumeReadKey), 60 + + t.Run("through a filer", func(t *testing.T) { + token := security.EncodedJwt(ChunkReadJwt([]string{"http://filer:8888/?proxyChunkId=" + fileId}, fileId)) + if _, err := security.DecodeJwt(security.SigningKey(filerReadKey), token, &security.SeaweedFilerClaims{}); err != nil { + t.Fatalf("token does not validate against the filer read key: %v", err) + } + }) + + t.Run("straight to a volume server", func(t *testing.T) { + token := security.EncodedJwt(ChunkReadJwt([]string{"http://volume:8080/" + fileId}, fileId)) + claims := &security.SeaweedFileIdClaims{} + if _, err := security.DecodeJwt(security.SigningKey(volumeReadKey), token, claims); err != nil { + t.Fatalf("token does not validate against the volume read key: %v", err) + } + if claims.Fid != fileId { + t.Fatalf("token authorizes file %q, want %q", claims.Fid, fileId) + } + }) +} diff --git a/weed/mount/weedfs.go b/weed/mount/weedfs.go index b915d6a7b..e16584c50 100644 --- a/weed/mount/weedfs.go +++ b/weed/mount/weedfs.go @@ -29,6 +29,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/util" "github.com/seaweedfs/seaweedfs/weed/util/chunk_cache" "github.com/seaweedfs/seaweedfs/weed/util/grace" + util_http "github.com/seaweedfs/seaweedfs/weed/util/http" "github.com/seaweedfs/seaweedfs/weed/util/version" "github.com/seaweedfs/seaweedfs/weed/wdclient" ) @@ -960,7 +961,7 @@ func (wfs *WFS) invalidateOpenFileHandle(invalidation meta_cache.EntryInvalidati func (wfs *WFS) LookupFn() wdclient.LookupFileIdFunctionType { if wfs.option.VolumeServerAccess == "filerProxy" { return func(ctx context.Context, fileId string) (targetUrls []string, err error) { - return []string{"http://" + wfs.getCurrentFiler().ToHttpAddress() + "/?proxyChunkId=" + fileId}, nil + return []string{util_http.ProxyChunkUrl(string(wfs.getCurrentFiler().ToHttpAddress()), fileId)}, nil } } // Use the cached FilerClient for efficient lookups with singleflight and cache history diff --git a/weed/mount/weedfs_write.go b/weed/mount/weedfs_write.go index 8bb6a4157..26c4b50a1 100644 --- a/weed/mount/weedfs_write.go +++ b/weed/mount/weedfs_write.go @@ -9,6 +9,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/operation" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/util" + util_http "github.com/seaweedfs/seaweedfs/weed/util/http" ) func (wfs *WFS) saveDataAsChunk(fullPath util.FullPath) filer.SaveDataAsChunkFunctionType { @@ -38,7 +39,7 @@ func (wfs *WFS) saveDataAsChunk(fullPath util.FullPath) filer.SaveDataAsChunkFun if wfs.option.VolumeServerAccess == "filerProxy" { // getCurrentFiler() can change on failover, so read it per attempt. uploadOption.GenUploadUrl = func(host, fileId string) string { - return fmt.Sprintf("http://%s/?proxyChunkId=%s", wfs.getCurrentFiler(), fileId) + return util_http.ProxyChunkUrl(string(wfs.getCurrentFiler()), fileId) } } diff --git a/weed/mq/broker/broker_write.go b/weed/mq/broker/broker_write.go index 8c4b42dae..9fd8019b5 100644 --- a/weed/mq/broker/broker_write.go +++ b/weed/mq/broker/broker_write.go @@ -13,6 +13,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/operation" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/util" + util_http "github.com/seaweedfs/seaweedfs/weed/util/http" ) func (b *MessageQueueBroker) appendToFile(targetFile string, data []byte) error { @@ -167,7 +168,7 @@ func (b *MessageQueueBroker) assignAndUpload(targetFile string, data []byte) (fi if b.option.VolumeServerAccess == "filerProxy" { // b.currentFiler can change on failover, so read it per attempt. uploadOption.GenUploadUrl = func(host, fileId string) string { - return fmt.Sprintf("http://%s/?proxyChunkId=%s", b.currentFiler, fileId) + return util_http.ProxyChunkUrl(string(b.currentFiler), fileId) } } diff --git a/weed/operation/upload_content.go b/weed/operation/upload_content.go index 860b7f654..5c02c77ac 100644 --- a/weed/operation/upload_content.go +++ b/weed/operation/upload_content.go @@ -42,7 +42,7 @@ func GenUploadUrlProxy(filerAddress string) func(host, fileId string) string { if filerAddress == "" { return fmt.Sprintf("http://%s/%s", host, fileId) } - return fmt.Sprintf("http://%s/?proxyChunkId=%s", filerAddress, fileId) + return util_http.ProxyChunkUrl(filerAddress, fileId) } } @@ -209,6 +209,12 @@ func (uploader *Uploader) uploadWithRetryData(assignFn func() (fileId string, ho } uploadOption.UploadUrl = genUrl(host, fileId) uploadOption.Jwt = auth + if util_http.IsProxyChunkUrl(uploadOption.UploadUrl) { + // The request addresses the filer, which authorizes it and mints the + // volume credential itself. The AssignVolume token is not a filer + // credential and gets the caller nowhere here. + uploadOption.Jwt = security.EncodedJwt(util_http.JwtForFilerServer(true)) + } uploadResult, err = uploader.retriedUploadData(context.Background(), data, uploadOption) return err diff --git a/weed/replication/repl_util/replication_util.go b/weed/replication/repl_util/replication_util.go index 6e13e0359..b1394c850 100644 --- a/weed/replication/repl_util/replication_util.go +++ b/weed/replication/repl_util/replication_util.go @@ -80,7 +80,7 @@ func copyChunkViews(chunkViews *filer.IntervalList[*filer.ChunkView], filerSourc var writeErr error var shouldRetry bool - jwt := filer.JwtForVolumeServer(chunk.FileId) + jwt := filer.ChunkReadJwt(fileUrls, chunk.FileId) for _, fileUrl := range fileUrls { shouldRetry, err = util_http.ReadUrlAsStream(context.Background(), fileUrl, jwt, chunk.CipherKey, chunk.IsGzipped, chunk.IsFullChunk(), chunk.OffsetInChunk, int(chunk.ViewSize), func(data []byte) { diff --git a/weed/replication/source/filer_source.go b/weed/replication/source/filer_source.go index a40e2a462..9b834b0fa 100644 --- a/weed/replication/source/filer_source.go +++ b/weed/replication/source/filer_source.go @@ -110,7 +110,7 @@ func (fs *FilerSource) LookupFileId(ctx context.Context, part string) (fileUrls } } } else { - fileUrls = append(fileUrls, fmt.Sprintf("http://%s/?proxyChunkId=%s", fs.address, part)) + fileUrls = append(fileUrls, util_http.ProxyChunkUrl(fs.address, part)) } return @@ -125,8 +125,8 @@ func (fs *FilerSource) ReadPart(fileId string, offset int64) (filename string, h } if fs.proxyByFiler { - fileUrl := "http://" + fs.address + "/?proxyChunkId=" + fileId - filename, header, resp, err = downloadFn(fileUrl, "", offset) + fileUrl := util_http.ProxyChunkUrl(fs.address, fileId) + filename, header, resp, err = downloadFn(fileUrl, util_http.JwtForFilerServer(false), offset) if err == nil { err = readPartStatusError(fileUrl, resp) } diff --git a/weed/server/filer_jwt_test.go b/weed/server/filer_jwt_test.go index 69a96cb50..74f5bb9bc 100644 --- a/weed/server/filer_jwt_test.go +++ b/weed/server/filer_jwt_test.go @@ -114,19 +114,27 @@ func TestFilerServer_maybeCheckJwtAuthorization_Scoped(t *testing.T) { expectAuthorized: true, }, { - name: "root path with prefix restriction", + name: "root listing denied to a prefix restricted token", token: genToken([]string{"/data"}, nil), method: "GET", path: "/", isWrite: false, - expectAuthorized: true, + expectAuthorized: false, }, { - name: "root path without token", + name: "root listing denied without token", token: "", method: "GET", path: "/", isWrite: false, + expectAuthorized: false, + }, + { + name: "root listing allowed with an unrestricted token", + token: genToken(nil, nil), + method: "GET", + path: "/", + isWrite: false, expectAuthorized: true, }, { diff --git a/weed/server/filer_server.go b/weed/server/filer_server.go index 44792feeb..c5e323897 100644 --- a/weed/server/filer_server.go +++ b/weed/server/filer_server.go @@ -180,9 +180,11 @@ func NewFilerServer(defaultMux, readonlyMux *http.ServeMux, option *FilerOption) domains := strings.Split(allowedOrigins, ",") option.AllowedOrigins = domains + // -exposeDirectoryData and filer.expose_directory_metadata both default to + // on, and either one turning it off has to hold: this is what keeps the + // directory listing off a filer whose reads are otherwise unauthenticated. v.SetDefault("filer.expose_directory_metadata.enabled", true) - returnDirMetadata := v.GetBool("filer.expose_directory_metadata.enabled") - option.ExposeDirectoryData = returnDirMetadata + option.ExposeDirectoryData = option.ExposeDirectoryData && v.GetBool("filer.expose_directory_metadata.enabled") fs = &FilerServer{ option: option, diff --git a/weed/server/filer_server_handlers.go b/weed/server/filer_server_handlers.go index 7050aee1f..1ef55de43 100644 --- a/weed/server/filer_server_handlers.go +++ b/weed/server/filer_server_handlers.go @@ -15,6 +15,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/security" "github.com/seaweedfs/seaweedfs/weed/stats" + util_http "github.com/seaweedfs/seaweedfs/weed/util/http" "github.com/seaweedfs/seaweedfs/weed/util/version" ) @@ -56,17 +57,6 @@ func (fs *FilerServer) filerHandler(w http.ResponseWriter, r *http.Request) { return } - // proxy to volume servers - var fileId string - if r.URL.Path == "/" { - fileId = r.URL.Query().Get("proxyChunkId") - } - if fileId != "" { - fs.proxyToVolumeServer(w, r, fileId) - stats.FilerHandlerCounter.WithLabelValues(stats.ChunkProxy).Inc() - stats.FilerRequestHistogram.WithLabelValues(stats.ChunkProxy).Observe(time.Since(start).Seconds()) - return - } requestMethod := r.Method defer func(method *string) { stats.FilerRequestCounter.WithLabelValues(*method, strconv.Itoa(statusRecorder.Status)).Inc() @@ -79,6 +69,19 @@ func (fs *FilerServer) filerHandler(w http.ResponseWriter, r *http.Request) { return } + // proxy to volume servers, after the gate: this is the one port operators + // expose, and the branch reaches any needle in the cluster by file id. + if r.URL.Path == "/" { + if fileId := r.URL.Query().Get(util_http.ProxyChunkIdParam); fileId != "" { + fs.proxyToVolumeServer(w, r, fileId) + stats.FilerHandlerCounter.WithLabelValues(stats.ChunkProxy).Inc() + // Name the deferred observation after the proxy rather than + // observing a second time, which would count the request twice. + requestMethod = stats.ChunkProxy + return + } + } + w.Header().Set("Server", "SeaweedFS "+version.VERSION) switch r.Method { @@ -211,11 +214,6 @@ func OptionsHandler(w http.ResponseWriter, r *http.Request, isReadOnly bool) { // maybeCheckJwtAuthorization returns true if access should be granted, false if it should be denied func (fs *FilerServer) maybeCheckJwtAuthorization(r *http.Request, isWrite bool) bool { - - if !isWrite && r.URL.Path == "/" { - return true - } - return fs.checkJwtAuthorization(r, isWrite, jwtScopedRequestPaths(r)) } diff --git a/weed/server/filer_server_handlers_proxy.go b/weed/server/filer_server_handlers_proxy.go index 66a76a6c1..cf6766533 100644 --- a/weed/server/filer_server_handlers_proxy.go +++ b/weed/server/filer_server_handlers_proxy.go @@ -127,15 +127,12 @@ func (fs *FilerServer) proxyToVolumeServerURL(w http.ResponseWriter, r *http.Req // (e.g. http://server:8080/6,08136bdce4). Forward the caller's query params // (e.g. readDeleted=true from weed mount) but drop the internal proxyChunkId. query := r.URL.Query() - query.Del("proxyChunkId") - if isProxyReadMethod(r.Method) { - // On a read the filer decides the volume credential below, and - // security.GetJwt reads the "jwt" query parameter before the - // Authorization header -- so leaving a caller-supplied one in place - // would silently outrank the token we attach. Writes keep theirs: the - // proxy forwards a writer's own credential either way. - query.Del("jwt") - } + query.Del(util_http.ProxyChunkIdParam) + // "jwt" here is the filer credential that got the caller past the gate, and + // a volume server has no business seeing one. security.GetJwt reads that + // parameter before the Authorization header, so relaying it would also + // hide the volume credential the filer attaches below. + query.Del("jwt") if encoded := query.Encode(); encoded != "" { targetURL += "?" + encoded } @@ -148,10 +145,9 @@ func (fs *FilerServer) proxyToVolumeServerURL(w http.ResponseWriter, r *http.Req } // Limit concurrent reads per volume server to prevent overload. Writes are - // deliberately exempt: a proxied write carries the caller's AssignVolume - // token, which expires 10s after the assign by default, so queueing one here - // can push it past expiry and turn it into a 401 the uploader does not - // re-assign on. + // deliberately exempt: the bursts this exists to contain are replication + // reads, and an upload queued behind them stalls a caller that is holding a + // volume assignment open. if isProxyReadMethod(r.Method) { volumeHost := proxyReq.URL.Host if err := acquireProxySemaphore(ctx, volumeHost); err != nil { @@ -173,23 +169,15 @@ func (fs *FilerServer) proxyToVolumeServerURL(w http.ResponseWriter, r *http.Req } // Decide the volume credential explicitly rather than letting the copied - // header stand, because the two directions need opposite handling. - // - // Reads: the volume server may require a read JWT even though the proxy - // endpoint doesn't, so mint one. When there is nothing to mint, drop the - // caller's Authorization instead of relaying it -- on this path it is a - // filer credential, and a volume server has no business seeing one. - // - // Writes: never mint. This branch runs ahead of the filer's JWT gate, so a - // token minted here would be signed for an unauthenticated caller. A - // legitimate writer already carries its own volume JWT from AssignVolume, - // so that one is forwarded untouched. - if isProxyReadMethod(r.Method) { - if jwt := fs.maybeGetVolumeReadJwtAuthorizationToken(fileId); jwt != "" { - proxyReq.Header.Set("Authorization", security.BearerPrefix+jwt) - } else { - proxyReq.Header.Del("Authorization") - } + // header stand. The caller's Authorization is the filer credential that got + // them past the gate: a volume server has no business seeing one, and it + // would not honour it anyway. Mint the credential the volume server does + // ask for, at the access level this request needs, and drop the caller's + // when there is nothing to mint. + if jwt := fs.maybeGetVolumeJwtAuthorizationToken(fileId, !isProxyReadMethod(r.Method)); jwt != "" { + proxyReq.Header.Set("Authorization", security.BearerPrefix+jwt) + } else { + proxyReq.Header.Del("Authorization") } proxyResponse, postErr := util_http.GetGlobalHttpClient().Do(proxyReq) diff --git a/weed/server/filer_server_handlers_proxy_test.go b/weed/server/filer_server_handlers_proxy_test.go index d6babddad..30fd18c03 100644 --- a/weed/server/filer_server_handlers_proxy_test.go +++ b/weed/server/filer_server_handlers_proxy_test.go @@ -117,31 +117,50 @@ func TestProxyReadDropsCallerJwtQueryParam(t *testing.T) { } } -// A writer's credential is its own either way, so the query parameter is left -// alone on writes -- stripping it would break a caller that presents its volume -// JWT that way. -func TestProxyWriteKeepsCallerJwtQueryParam(t *testing.T) { +// On a write the query parameter carries the filer credential that got the +// caller past the gate. Relaying it would hand a volume server a filer token +// and, since security.GetJwt reads it first, hide the minted volume token +// behind one the volume server cannot validate. +func TestProxyWriteDropsCallerJwtQueryParam(t *testing.T) { volume := newProxyTestVolume(t) fs := &FilerServer{volumeGuard: security.NewGuard([]string{}, proxyTestWriteKey, 10, proxyTestReadKey, 10)} r := httptest.NewRequest(http.MethodPost, - "http://filer:8888/?proxyChunkId="+proxyTestFileId+"&jwt=caller-supplied", nil) + "http://filer:8888/?proxyChunkId="+proxyTestFileId+"&jwt=filer-credential", nil) fs.proxyToVolumeServerURL(httptest.NewRecorder(), r, proxyTestFileId, volume.URL+"/"+proxyTestFileId) volume.requireReached(t) - if got := volume.seenEffectiveJwt(); got != "caller-supplied" { - t.Fatalf("writer's own jwt query param was altered: got %q", got) + if q := volume.seenRawQuery(); strings.Contains(q, "jwt=") { + t.Fatalf("filer credential survived in the forwarded query: %q", q) + } + claims := &security.SeaweedFileIdClaims{} + if _, err := security.DecodeJwt(security.SigningKey(proxyTestWriteKey), + security.EncodedJwt(volume.seenEffectiveJwt()), claims); err != nil { + t.Fatalf("volume server would evaluate %q, which does not validate against the write key: %v", + volume.seenEffectiveJwt(), err) } } -// Everything the filer can hand a caller on the proxy path is reachable without -// authentication, because the branch runs ahead of the filer's JWT gate. With -// only a write key configured it must therefore mint nothing at all. -func TestProxyMintsNothingWithoutReadKey(t *testing.T) { - fs := &FilerServer{volumeGuard: security.NewGuard([]string{}, proxyTestWriteKey, 10, "", 10)} +// The token is minted for the caller only once the filer has authorized them, +// so a key that is not configured means no token: on a read that is the write +// key, which would hand out more authority than the read needs. +func TestProxyMintsNothingWithoutKey(t *testing.T) { + for _, tc := range []struct { + name string + isWrite bool + writeKey string + readKey string + }{ + {"read with only a write key", false, proxyTestWriteKey, ""}, + {"write with only a read key", true, "", proxyTestReadKey}, + } { + t.Run(tc.name, func(t *testing.T) { + fs := &FilerServer{volumeGuard: security.NewGuard([]string{}, tc.writeKey, 10, tc.readKey, 10)} - if jwt := fs.maybeGetVolumeReadJwtAuthorizationToken(proxyTestFileId); jwt != "" { - t.Fatalf("minted %q with no read key configured", jwt) + if jwt := fs.maybeGetVolumeJwtAuthorizationToken(proxyTestFileId, tc.isWrite); jwt != "" { + t.Fatalf("minted %q with no key configured for that access level", jwt) + } + }) } } @@ -169,40 +188,31 @@ func TestProxyReadTokenIsReadOnly(t *testing.T) { } } -// Writes must reach the volume server carrying the caller's own AssignVolume -// token and nothing else. POST is the method every in-tree proxied uploader -// actually sends, so it leads the table. -func TestProxyWriteCarriesOnlyCallerCredential(t *testing.T) { - callerToken := security.BearerPrefix + string(security.GenJwtForVolumeServer(security.SigningKey(proxyTestWriteKey), 10, proxyTestFileId)) +// A proxied write reaches the volume server on a token the filer minted for +// this file id, never on the caller's own Authorization -- that one is the +// filer credential the caller was authorized with. POST is the method every +// in-tree proxied uploader actually sends, so it leads the table. +func TestProxyWriteCarriesMintedWriteToken(t *testing.T) { + vs := &VolumeServer{guard: security.NewGuard([]string{}, proxyTestWriteKey, 10, proxyTestReadKey, 10)} - for _, tc := range []struct { - name string - method string - readKey string - sent string - want string - }{ - {"anonymous post", http.MethodPost, "", "", ""}, - {"anonymous post with read key", http.MethodPost, proxyTestReadKey, "", ""}, - {"anonymous delete", http.MethodDelete, "", "", ""}, - {"anonymous delete with read key", http.MethodDelete, proxyTestReadKey, "", ""}, - {"anonymous put with read key", http.MethodPut, proxyTestReadKey, "", ""}, - {"caller token forwarded on post", http.MethodPost, proxyTestReadKey, callerToken, callerToken}, - {"caller token forwarded on delete", http.MethodDelete, "", callerToken, callerToken}, - } { - t.Run(tc.name, func(t *testing.T) { + for _, method := range []string{http.MethodPost, http.MethodPut, http.MethodDelete} { + t.Run(method, func(t *testing.T) { volume := newProxyTestVolume(t) - fs := &FilerServer{volumeGuard: security.NewGuard([]string{}, proxyTestWriteKey, 10, tc.readKey, 10)} + fs := &FilerServer{volumeGuard: security.NewGuard([]string{}, proxyTestWriteKey, 10, proxyTestReadKey, 10)} - r := httptest.NewRequest(tc.method, "http://filer:8888/?proxyChunkId="+proxyTestFileId, nil) - if tc.sent != "" { - r.Header.Set("Authorization", tc.sent) - } + r := httptest.NewRequest(method, "http://filer:8888/?proxyChunkId="+proxyTestFileId, nil) + r.Header.Set("Authorization", security.BearerPrefix+"filer-credential") fs.proxyToVolumeServerURL(httptest.NewRecorder(), r, proxyTestFileId, volume.URL+"/"+proxyTestFileId) volume.requireReached(t) - if got := volume.seenAuth(); got != tc.want { - t.Fatalf("volume server saw Authorization %q, want %q", got, tc.want) + seen := volume.seenAuth() + if seen == security.BearerPrefix+"filer-credential" { + t.Fatal("caller's filer credential reached the volume server") + } + check := httptest.NewRequest(method, "http://volume:8080/"+proxyTestFileId, nil) + check.Header.Set("Authorization", seen) + if !vs.maybeCheckJwtAuthorization(check, proxyTestVid, proxyTestFid, true) { + t.Fatalf("forwarded token %q did not authorize the write", seen) } }) } @@ -233,14 +243,14 @@ func TestProxyReadReplacesCallerCredential(t *testing.T) { } } -// With no read key there is nothing to mint, and the caller's Authorization on -// the read path is a filer credential -- it must be dropped, not relayed to a -// volume server that has no business seeing it. -func TestProxyReadDropsCallerCredentialWhenNothingMinted(t *testing.T) { - for _, method := range []string{http.MethodGet, http.MethodHead} { +// With no volume key there is nothing to mint, and the caller's Authorization +// is a filer credential -- it must be dropped, not relayed to a volume server +// that has no business seeing it. +func TestProxyDropsCallerCredentialWhenNothingMinted(t *testing.T) { + for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodPost, http.MethodDelete} { t.Run(method, func(t *testing.T) { volume := newProxyTestVolume(t) - fs := &FilerServer{volumeGuard: security.NewGuard([]string{}, proxyTestWriteKey, 10, "", 10)} + fs := &FilerServer{volumeGuard: security.NewGuard([]string{}, "", 10, "", 10)} r := httptest.NewRequest(method, "http://filer:8888/?proxyChunkId="+proxyTestFileId, nil) r.Header.Set("Authorization", security.BearerPrefix+"filer-credential") @@ -378,6 +388,54 @@ func TestProxyRejectsTraversalBeforeLookup(t *testing.T) { } } +// The proxy branch reaches any needle in the cluster by file id, on the filer +// port jwt.filer_signing exists to make safe to expose, so it has to sit behind +// the same gate as every other request. +func TestFilerHandlerGatesChunkProxy(t *testing.T) { + signingKey := "secret" + fs := &FilerServer{ + option: &FilerOption{}, + filerGuard: security.NewGuard(nil, signingKey, 0, signingKey, 0), + } + + for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodPut, http.MethodPost, http.MethodDelete} { + t.Run(method, func(t *testing.T) { + r := httptest.NewRequest(method, "http://filer:8888/?proxyChunkId="+proxyTestFileId, nil) + w := httptest.NewRecorder() + + // fs.filer is nil: reaching the lookup would panic, so surviving + // this call is itself proof the request was refused first. + fs.filerHandler(w, r) + + if w.Code != http.StatusUnauthorized { + t.Errorf("anonymous %s of a chunk returned %d, want 401", method, w.Code) + } + }) + } +} + +// ... and once past it the branch still runs. A malformed fid is refused by +// validateProxyChunkId ahead of any lookup, which is as far as this can go +// without a cluster behind the filer. +func TestFilerHandlerProxiesChunkForAuthorizedCaller(t *testing.T) { + signingKey := "secret" + fs := &FilerServer{ + option: &FilerOption{}, + filerGuard: security.NewGuard(nil, signingKey, 0, signingKey, 0), + } + token := security.GenJwtForFilerServer(security.SigningKey(signingKey), 60) + + r := httptest.NewRequest(http.MethodGet, "http://filer:8888/?proxyChunkId=3,not-a-fid", nil) + r.Header.Set("Authorization", security.BearerPrefix+string(token)) + w := httptest.NewRecorder() + + fs.filerHandler(w, r) + + if w.Code != http.StatusBadRequest { + t.Errorf("authorized chunk proxy returned %d, want 400 from the fid check", w.Code) + } +} + func TestProxySemaphore_LimitsConcurrency(t *testing.T) { host := "test-volume:8080" defer proxySemaphores.Delete(host) diff --git a/weed/server/filer_server_handlers_read.go b/weed/server/filer_server_handlers_read.go index 8de704aaa..c3c597b6e 100644 --- a/weed/server/filer_server_handlers_read.go +++ b/weed/server/filer_server_handlers_read.go @@ -331,11 +331,21 @@ func (fs *FilerServer) maybeGetVolumeReadJwtAuthorizationToken(fileId string) st // solely when jwt.signing.read.key is set, so falling back to the write key // buys no access on a read -- it only hands out a token that would authorize // a write. - key := fs.volumeGuard.ReadSigningKey() + return fs.maybeGetVolumeJwtAuthorizationToken(fileId, false) +} + +// maybeGetVolumeJwtAuthorizationToken mints the volume credential for one file +// id at the requested access level, empty when that key is unset -- which is +// also when the volume server asks for nothing. +func (fs *FilerServer) maybeGetVolumeJwtAuthorizationToken(fileId string, isWrite bool) string { + key, expiresAfterSec := fs.volumeGuard.ReadSigningKey(), fs.volumeGuard.ReadExpiresAfterSec() + if isWrite { + key, expiresAfterSec = fs.volumeGuard.SigningKey(), fs.volumeGuard.ExpiresAfterSec() + } if len(key) == 0 { return "" } // Claim the base fid: the volume server strips a _N delta suffix before // comparing, so a token claiming the suffixed form never matches. - return string(security.GenJwtForVolumeServer(key, fs.volumeGuard.ReadExpiresAfterSec(), baseFileId(fileId))) + return string(security.GenJwtForVolumeServer(key, expiresAfterSec, baseFileId(fileId))) } diff --git a/weed/util/http/chunk_proxy.go b/weed/util/http/chunk_proxy.go new file mode 100644 index 000000000..f83130b09 --- /dev/null +++ b/weed/util/http/chunk_proxy.go @@ -0,0 +1,62 @@ +package http + +import ( + "fmt" + "strings" + "sync" + + "github.com/seaweedfs/seaweedfs/weed/security" + "github.com/seaweedfs/seaweedfs/weed/util" +) + +// ProxyChunkIdParam is the query parameter a filer reads to serve one chunk +// from the volume server holding it, for callers that cannot reach volume +// servers directly. +const ProxyChunkIdParam = "proxyChunkId" + +var ( + filerSigningKey security.SigningKey + filerSigningKeyExpires int + filerReadSigningKey security.SigningKey + filerReadSigningKeyExpires int + loadFilerJwtConfigOnce sync.Once +) + +func loadFilerJwtConfig() { + v := util.GetViper() + filerSigningKey = security.SigningKey(v.GetString("jwt.filer_signing.key")) + filerSigningKeyExpires = v.GetInt("jwt.filer_signing.expires_after_seconds") + if filerSigningKeyExpires == 0 { + filerSigningKeyExpires = 10 + } + filerReadSigningKey = security.SigningKey(v.GetString("jwt.filer_signing.read.key")) + filerReadSigningKeyExpires = v.GetInt("jwt.filer_signing.read.expires_after_seconds") + if filerReadSigningKeyExpires == 0 { + filerReadSigningKeyExpires = 60 + } +} + +// JwtForFilerServer generates a JWT for the filer's HTTP API if jwt.filer_signing +// is configured for that access level, the way filer.JwtForVolumeServer does for +// volume servers. Empty when the key is unset, which is also when the filer +// serves the request unsigned. +func JwtForFilerServer(isWrite bool) string { + loadFilerJwtConfigOnce.Do(loadFilerJwtConfig) + if isWrite { + return string(security.GenJwtForFilerServer(filerSigningKey, filerSigningKeyExpires)) + } + return string(security.GenJwtForFilerServer(filerReadSigningKey, filerReadSigningKeyExpires)) +} + +// ProxyChunkUrl builds the request that reads or writes one chunk through a +// filer instead of the volume server holding it. +func ProxyChunkUrl(filerAddress string, fileId string) string { + return fmt.Sprintf("http://%s/?%s=%s", filerAddress, ProxyChunkIdParam, fileId) +} + +// IsProxyChunkUrl reports whether a chunk URL addresses a filer's chunk proxy +// rather than a volume server. Such a request is authorized by the filer, which +// attaches the volume credential itself, so the token it carries is a filer one. +func IsProxyChunkUrl(urlString string) bool { + return strings.Contains(urlString, ProxyChunkIdParam+"=") +} diff --git a/weed/util/http/chunk_proxy_test.go b/weed/util/http/chunk_proxy_test.go new file mode 100644 index 000000000..498faac52 --- /dev/null +++ b/weed/util/http/chunk_proxy_test.go @@ -0,0 +1,74 @@ +package http + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/security" + "github.com/seaweedfs/seaweedfs/weed/util" +) + +func TestIsProxyChunkUrl(t *testing.T) { + for _, tc := range []struct { + urlString string + want bool + }{ + {ProxyChunkUrl("filer:8888", "3,01637037d6"), true}, + {"http://filer:8888/?proxyChunkId=3,01637037d6&readDeleted=true", true}, + {"http://volume:8080/3,01637037d6", false}, + {"http://volume:8080/3,01637037d6?readDeleted=true", false}, + {"", false}, + } { + if got := IsProxyChunkUrl(tc.urlString); got != tc.want { + t.Errorf("IsProxyChunkUrl(%q) = %v, want %v", tc.urlString, got, tc.want) + } + } +} + +// The credential a proxied chunk carries is a filer one, signed with the key +// for the access level the caller is about to use. +func TestJwtForFilerServer(t *testing.T) { + const ( + writeKey = "filer-write-key" + readKey = "filer-read-key" + ) + v := util.GetViper() + v.Set("jwt.filer_signing.key", writeKey) + v.Set("jwt.filer_signing.read.key", readKey) + + for _, tc := range []struct { + name string + isWrite bool + signedBy string + otherKey string + }{ + {"read", false, readKey, writeKey}, + {"write", true, writeKey, readKey}, + } { + t.Run(tc.name, func(t *testing.T) { + token := security.EncodedJwt(JwtForFilerServer(tc.isWrite)) + claims := &security.SeaweedFilerClaims{} + if _, err := security.DecodeJwt(security.SigningKey(tc.signedBy), token, claims); err != nil { + t.Fatalf("token does not validate against the %s key: %v", tc.name, err) + } + if claims.ExpiresAt == nil { + t.Fatal("token never expires") + } + if _, err := security.DecodeJwt(security.SigningKey(tc.otherKey), token, &security.SeaweedFilerClaims{}); err == nil { + t.Fatal("token also validates against the other access level's key") + } + }) + } +} + +func TestJwtForFilerServerWithoutKeys(t *testing.T) { + loadFilerJwtConfigOnce.Do(func() {}) + write, read := filerSigningKey, filerReadSigningKey + t.Cleanup(func() { filerSigningKey, filerReadSigningKey = write, read }) + filerSigningKey, filerReadSigningKey = nil, nil + + for _, isWrite := range []bool{false, true} { + if token := JwtForFilerServer(isWrite); token != "" { + t.Errorf("unconfigured signing key still produced %q", token) + } + } +} diff --git a/weed/util/http/http_global_client_util.go b/weed/util/http/http_global_client_util.go index 23794e45e..ab75c2ad9 100644 --- a/weed/util/http/http_global_client_util.go +++ b/weed/util/http/http_global_client_util.go @@ -601,10 +601,14 @@ type RefreshUrlsFunc func() []string // list for the reads that follow. func RetriedFetchChunkData(ctx context.Context, buffer []byte, urlStrings []string, cipherKey []byte, isGzipped bool, isFullChunk bool, offset int64, fileId string, refreshUrls RefreshUrlsFunc) (n int, err error) { - loadJwtConfigOnce.Do(loadJwtConfig) var jwt security.EncodedJwt - if cfg := jwtSigningReadConfigPtr.Load(); cfg != nil && len(cfg.key) > 0 { - jwt = security.GenJwtForVolumeServer(cfg.key, cfg.expires, fileId) + if len(urlStrings) > 0 && IsProxyChunkUrl(urlStrings[0]) { + jwt = security.EncodedJwt(JwtForFilerServer(false)) + } else { + loadJwtConfigOnce.Do(loadJwtConfig) + if cfg := jwtSigningReadConfigPtr.Load(); cfg != nil && len(cfg.key) > 0 { + jwt = security.GenJwtForVolumeServer(cfg.key, cfg.expires, fileId) + } } // For unencrypted, non-gzipped full chunks, use direct buffer read