From a0b1272cc3f7eefb55d37292939e41708ba7ec8f Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 4 Sep 2026 16:39:36 -0700 Subject: [PATCH] filer: authorize the chunk proxy and the root listing like the rest of the filer port (#11152) * filer: require a read token for the root listing maybeCheckJwtAuthorization waved through every GET/HEAD on "/", so a filer with jwt.filer_signing.read.key set still served its root directory listing -- entry names, sizes and chunks[].file_id -- to a caller holding no token at all, and served the same listing to a token restricted by allowed_prefixes. The exemption was added for health checks before the filer had /healthz and /readyz. Both are registered on the default and read-only muxes ahead of the "/" handler and answer without a token, so drop it. Point the mTLS harness at /healthz, which is what it was probing for. * filer: keep the jwt query parameter out of a proxied chunk request The proxy stripped "jwt" from the forwarded query on reads only, on the grounds that a writer's own credential travels there. It does not: an uploader carries its AssignVolume token in the Authorization header, and the query parameter on this path holds a filer credential. Strip it for every method. A volume server has no business seeing a filer token, and because security.GetJwt reads the query before the header, relaying one would hide the writer's own token behind it. * filer: dispatch the chunk proxy after the JWT gate The ?proxyChunkId= branch returned before maybeCheckJwtAuthorization ran, so GET, PUT, POST and DELETE against any needle in the cluster were reachable on the filer's HTTP port with no filer credential, on a filer where every other request answered 401. An anonymous caller read a stored object, replaced its bytes, or deleted the needle, which the master's next vacuum makes permanent. #10434 stopped the filer from minting a volume write token for that caller, which closes the write half only where the volume server has a jwt.signing.key of its own -- not the shipped default, and not what scaffold/security.toml recommends for a filer deployment. The read half stayed open in every configuration, because the filer mints the read token itself. Move the dispatch below the gate. A file id carries no path, so a token restricted by allowed_prefixes cannot be scoped against one and is refused here; every consumer of this endpoint holds an unrestricted token. * filer: mint the volume credential for a proxied write too The proxy minted a volume token on reads and forwarded whatever the caller sent on writes. #10434 made it that way because the branch ran ahead of the JWT gate, so a token minted here would have been signed for an unauthenticated caller; the branch now runs behind the gate, and the credential the caller presents there is a filer one, which a volume server cannot validate and has no business seeing. Mint at the access level the request needs, and drop the caller's Authorization when there is no key to mint from. A proxied uploader then needs only the filer credential, instead of holding one for each hop with a single header to put them in. * mount, mq, filer.sync: send the filer credential for a proxied chunk Every in-tree consumer of ?proxyChunkId= reached the filer anonymously: mount and the broker put the AssignVolume token in the Authorization header, which is a volume credential, and filer.sync sent nothing at all. That was enough only while the branch ran ahead of the filer's JWT gate. Build the URL through one helper, and pick the credential from the URL it returns: a chunk proxied through a filer is a request to the filer, which authorizes it and attaches the volume credential itself, so the token there is a filer one at the access level the request needs. * filer: honor -exposeDirectoryData The flag was declared on all three commands that start a filer and read by none of them: FilerOption.ExposeDirectoryData was only ever assigned from filer.expose_directory_metadata in security.toml, so -exposeDirectoryData=false silently left the listing exposed. Only the TOML key had any effect. Plumb the flag through and let either switch turn the listing off. * filer: count a proxied chunk request once Moving the dispatch below the gate put it after the deferred request observation, so every proxied chunk now landed in FilerRequestHistogram twice, once under its HTTP method and once under chunkProxy. Name the deferred one after the proxy instead, the way the unsupported-method branch already does, which also gives the endpoint the status codes FilerRequestCounter records. --- .../test/local-secure/run_local_secure.sh | 2 +- weed/command/filer.go | 1 + weed/filer/filechunk_manifest.go | 2 +- weed/filer/stream.go | 14 +- weed/filer/stream_jwt_test.go | 43 +++++ weed/mount/weedfs.go | 3 +- weed/mount/weedfs_write.go | 3 +- weed/mq/broker/broker_write.go | 3 +- weed/operation/upload_content.go | 8 +- .../replication/repl_util/replication_util.go | 2 +- weed/replication/source/filer_source.go | 6 +- weed/server/filer_jwt_test.go | 14 +- weed/server/filer_server.go | 6 +- weed/server/filer_server_handlers.go | 30 ++-- weed/server/filer_server_handlers_proxy.go | 48 ++---- .../filer_server_handlers_proxy_test.go | 154 ++++++++++++------ weed/server/filer_server_handlers_read.go | 14 +- weed/util/http/chunk_proxy.go | 62 +++++++ weed/util/http/chunk_proxy_test.go | 74 +++++++++ weed/util/http/http_global_client_util.go | 10 +- 20 files changed, 384 insertions(+), 115 deletions(-) create mode 100644 weed/filer/stream_jwt_test.go create mode 100644 weed/util/http/chunk_proxy.go create mode 100644 weed/util/http/chunk_proxy_test.go 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