From e44c498d72583dc24f50672fddf247557540391a Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 04:33:29 +0000 Subject: [PATCH 01/17] feat(vermeer): support file credentials and native token helper --- .github/workflows/vermeer-ci.yml | 36 ++++ vermeer/Dockerfile | 4 +- vermeer/README.md | 53 ++++- vermeer/apps/auth/factor_file.go | 41 ++++ vermeer/apps/auth/factor_file_test.go | 81 ++++++++ vermeer/apps/auth/filter.go | 22 +- vermeer/apps/common/hugegraph_credentials.go | 18 +- .../apps/common/hugegraph_credentials_test.go | 72 +++++++ vermeer/apps/common/pd_peers.go | 4 +- vermeer/apps/common/pd_peers_test.go | 11 +- vermeer/apps/master/master_main.go | 4 +- vermeer/tools/token/token_generator.go | 99 +++++++-- vermeer/tools/token/token_generator_test.go | 188 ++++++++++++++++++ 13 files changed, 594 insertions(+), 39 deletions(-) create mode 100644 vermeer/apps/auth/factor_file.go create mode 100644 vermeer/apps/auth/factor_file_test.go create mode 100644 vermeer/tools/token/token_generator_test.go diff --git a/.github/workflows/vermeer-ci.yml b/.github/workflows/vermeer-ci.yml index bc93ca4cc..4e71048d0 100644 --- a/.github/workflows/vermeer-ci.yml +++ b/.github/workflows/vermeer-ci.yml @@ -74,6 +74,7 @@ jobs: - name: Test deployment credentials and graph initialization failures run: | + go test -race ./apps/auth ./tools/token go test -race ./apps/common go test -race ./apps/master/workers ./apps/master/services -run 'TestWorkerAvailability' go test -race ./apps/master/bl -run 'Test(WorkerAvailability|HugegraphLoadAuthorization)' @@ -116,3 +117,38 @@ jobs: docker run --rm --entrypoint test vermeer:ci-check -f /go/bin/release-docs/LICENSE docker run --rm --entrypoint test vermeer:ci-check -f /go/bin/release-docs/NOTICE docker run --rm --entrypoint test vermeer:ci-check -d /go/bin/release-docs/licenses + + - name: Verify native token helper and invalid factor rejection + run: | + umask 077 + TOKEN_CHECK_DIR=$(mktemp -d) + trap 'rm -rf "${TOKEN_CHECK_DIR}"' EXIT + openssl rand -hex 32 > "${TOKEN_CHECK_DIR}/factor" + docker run --rm -v "${TOKEN_CHECK_DIR}:/probe" \ + --entrypoint /go/bin/vermeer-token vermeer:ci-check \ + -factor-file /probe/factor -output-file /probe/token \ + -user admin -space '$DEFAULT' -i -quiet \ + > "${TOKEN_CHECK_DIR}/stdout" 2> "${TOKEN_CHECK_DIR}/stderr" + test -s "${TOKEN_CHECK_DIR}/token" + test "$(stat -c '%a' "${TOKEN_CHECK_DIR}/token")" = 600 + test ! -s "${TOKEN_CHECK_DIR}/stdout" + test ! -s "${TOKEN_CHECK_DIR}/stderr" + if docker run --rm -v "${TOKEN_CHECK_DIR}:/probe" \ + --entrypoint /go/bin/vermeer-token vermeer:ci-check \ + -factor-file /probe/missing -output-file /probe/rejected -quiet \ + > "${TOKEN_CHECK_DIR}/stdout" 2> "${TOKEN_CHECK_DIR}/stderr"; then + echo 'token generation accepted a missing factor file' >&2 + exit 1 + fi + test ! -e "${TOKEN_CHECK_DIR}/rejected" + test ! -s "${TOKEN_CHECK_DIR}/stdout" + if docker run --rm -e VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/missing \ + vermeer:ci-check -env=master -auth=token \ + > "${TOKEN_CHECK_DIR}/master-log" 2>&1; then + echo 'Master accepted a missing factor file' >&2 + exit 1 + fi + if grep -qF -f "${TOKEN_CHECK_DIR}/factor" "${TOKEN_CHECK_DIR}/master-log"; then + echo 'Master log exposed the factor' >&2 + exit 1 + fi diff --git a/vermeer/Dockerfile b/vermeer/Dockerfile index daa70b9f7..55676dba2 100644 --- a/vermeer/Dockerfile +++ b/vermeer/Dockerfile @@ -34,11 +34,13 @@ ARG TARGETOS ARG TARGETARCH RUN --mount=type=cache,target=/go/pkg/mod \ --mount=type=cache,target=/root/.cache/go-build \ - GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/app + GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/app && \ + GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/vermeer-token ./tools/token FROM alpine EXPOSE 8080 COPY --from=builder /go/bin/app /go/bin/app +COPY --from=builder /go/bin/vermeer-token /go/bin/vermeer-token COPY --from=builder /src/config/ /go/bin/config/ COPY --from=builder /src/release-docs/ /go/bin/release-docs/ COPY --from=builder /usr/local/go/lib/time/zoneinfo.zip / diff --git a/vermeer/README.md b/vermeer/README.md index b1990d39f..2d922aa38 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -108,12 +108,15 @@ docker pull hugegraph/vermeer:latest ``` Create a dedicated config directory (e.g., `~/vermeer-config/`) with `master.ini` and `worker.ini` files (see [Configuration](#configuration) section). +Create its `token-factor` file using [Generating Auth Token](#generating-auth-token) before starting the authenticated Master. Run with Docker: ```bash # Master node (with Web UI enabled) -docker run -v ~/vermeer-config:/go/bin/config hugegraph/vermeer --env=master -auth token -auth_token_factor 1234 +docker run -v ~/vermeer-config:/go/bin/config \ + -e VERMEER_AUTH_TOKEN_FACTOR_FILE=/go/bin/config/token-factor \ + hugegraph/vermeer --env=master -auth token # Worker node docker run -v ~/vermeer-config:/go/bin/config hugegraph/vermeer --env=worker @@ -246,23 +249,48 @@ Deployment credentials require the trusted PD configuration described below. ### Generating Auth Token -When `auth=token` is enabled, you need to generate a token for login. Use the built-in token generator: +When `auth=token` is enabled, protect the signing factor in a file instead of placing it in process arguments. +Create the file before starting Master, and keep it across restarts: + +```bash +umask 077 +openssl rand -hex 32 > config/token-factor +``` + +Set `VERMEER_AUTH_TOKEN_FACTOR_FILE=/path/to/token-factor`, or set `auth_token_factor_file` in Master's INI file. +The environment file takes precedence over the INI file, and either file takes precedence over `auth_token_factor`. +The file must contain one valid UTF-8 line with at least four characters and cannot contain only whitespace. +A terminal LF or CRLF is accepted; other spaces are preserved. An invalid configured file stops Master before it opens its listeners; +it never falls back to an inline or default factor. + +Images built from this revision include the native `/go/bin/vermeer-token` helper. Generate a protected login/service token file: ```bash # Run with Go -go run tools/token/token_generator.go -user admin -space default -factor 1234 -i +go run ./tools/token -user admin -space '$DEFAULT' \ + -factor-file config/token-factor -output-file config/admin-token -i -quiet # Or run in Docker (no local Go required) -docker run --rm -v $(pwd):/src -v $(pwd)/config:/src/config golang:1.23-alpine sh -c "cd /src && go run tools/token/token_generator.go -user admin -space default -factor 1234 -i" +docker run --rm -v "$PWD/config:/tokens" --entrypoint /go/bin/vermeer-token \ + hugegraph/vermeer -user admin -space '$DEFAULT' \ + -factor-file /tokens/token-factor -output-file /tokens/admin-token -i -quiet ``` Parameters: - `-user` — Username (default: `foo`) - `-space` — Graph space name (default: `bar`) -- `-factor` — Must match `auth_token_factor` in `master.ini` +- `-factor-file` — Read the same signing factor file used by Master. +- `-factor` — Legacy inline factor; mutually exclusive with `-factor-file`. Prefer files for deployment secrets. +- `-output-file` — Atomically write the Base58 token with mode `0600`, with no token or factor printed. +- `-quiet` — When no output file is set, print only the token for an intentional interactive retrieval. - `-client` — Client identifier (default: `hg`) - `-i` — Generate immortal token (never expires) +Hubble accepts this plain token file through `vermeer.token_file`; it must not include a `Bearer` prefix or JSON. +Normal tokens expire after 24 hours. Long-lived service tokens must be protected like passwords. +Rotating the signing factor invalidates existing tokens; regenerate dependent service tokens and restart their clients together. +Token generation reuses Vermeer's native signing and encoding implementation and does not require a running Master or Kubernetes API access. + The generated token can be used in two ways: 1. **HTTP Header**: `Authorization: Bearer ` @@ -279,7 +307,7 @@ is sent as the internal `vermeer` credential and is never added to task paramete When no file is configured, the legacy credential remains available for older PD deployments; it does not authenticate to a modern PD with a different shared secret. -When either deployment credential file is configured, configure both Master and +When any PD or HugeGraph deployment credentials are configured, configure both Master and Worker with `pd_peers=["pd.internal:8686"]`, or set `VERMEER_PD_PEERS` to that JSON array (environment takes precedence). These deployment addresses replace task PD addresses for load and output discovery. Missing or invalid deployment peers @@ -316,8 +344,17 @@ direct adapter never forwards the user's login password. Worker Schema discovery and reads use these deployment credentials without adding them to persisted task parameters. A configured missing, unreadable, empty or invalid file fails the load; it never falls back to task credentials. Without a file, the existing -`load.hugegraph_username` / `load.hugegraph_password` behavior is retained. This -setting does not configure the separate HugeGraph result writer. +`load.hugegraph_username` / `load.hugegraph_password` behavior is retained unless +the following password-file form is configured. These settings do not configure the separate HugeGraph result writer. + +To reuse a Kubernetes Secret containing a raw Server password without converting it to JSON, set +`VERMEER_HUGEGRAPH_USERNAME` and `VERMEER_HUGEGRAPH_PASSWORD_FILE` together, or configure +`hugegraph_username` and `hugegraph_password_file` in Worker's INI file. Each environment value overrides its corresponding INI value. +The explicit JSON credentials file above takes precedence over this pair. If either member of the pair is configured, both are required; +a missing, unreadable or empty password file fails the load without using task credentials. +Password-file contents are read exactly, including quotes, backslashes, spaces and terminal newlines; mount the original Secret value +and avoid adding a newline with `echo`. These credentials are only used for Schema discovery/reads through the configured trusted PD peers +and do not enter persisted task parameters. ```ini [default] diff --git a/vermeer/apps/auth/factor_file.go b/vermeer/apps/auth/factor_file.go new file mode 100644 index 000000000..a2e9f1a98 --- /dev/null +++ b/vermeer/apps/auth/factor_file.go @@ -0,0 +1,41 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package auth + +import ( + "errors" + "fmt" + "os" + "strings" + "unicode/utf8" +) + +// ReadTokenFactorFile is shared by the Master and the native token generator. +// A single terminal newline is accepted; no other whitespace is changed. +func ReadTokenFactorFile(file string) (string, error) { + data, err := os.ReadFile(file) + if err != nil { + return "", fmt.Errorf("read token factor file: %w", err) + } + factor := strings.TrimSuffix(strings.TrimSuffix(string(data), "\n"), "\r") + if !utf8.ValidString(factor) || utf8.RuneCountInString(factor) < 4 || + strings.TrimSpace(factor) == "" || strings.ContainsAny(factor, "\r\n") { + return "", errors.New("token factor file must contain one UTF-8 line of at least four characters") + } + return factor, nil +} diff --git a/vermeer/apps/auth/factor_file_test.go b/vermeer/apps/auth/factor_file_test.go new file mode 100644 index 000000000..d4ca2a8b5 --- /dev/null +++ b/vermeer/apps/auth/factor_file_test.go @@ -0,0 +1,81 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package auth + +import ( + "os" + "path/filepath" + "strings" + "testing" + "vermeer/apps/common" +) + +func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { + previousFactor := common.GetConfigDefault("auth_token_factor", "") + previousFile := common.GetConfigDefault("auth_token_factor_file", "") + previousFactory := factory + t.Cleanup(func() { + common.SetConfig("auth_token_factor", previousFactor) + common.SetConfig("auth_token_factor_file", previousFile) + factory = previousFactory + }) + common.SetConfig("auth_token_factor", "legacy-inline-factor") + configured := filepath.Join(t.TempDir(), "configured") + override := filepath.Join(t.TempDir(), "override") + if err := os.WriteFile(configured, []byte("configured-factor\r\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(override, []byte("environment-factor"), 0600); err != nil { + t.Fatal(err) + } + common.SetConfig("auth_token_factor_file", configured) + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", override) + if err := Init(); err != nil || factory.Factor != "environment-factor" { + t.Fatal("environment factor file must override config and inline factor") + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", "") + if err := Init(); err != nil || factory.Factor != "configured-factor" { + t.Fatal("configured factor file must override inline factor") + } + for _, test := range []struct{ name, data string }{ + {"missing", ""}, {"empty", ""}, {"invalid", "synthetic-secret\nsecond-line"}, + {"short-ascii", "abc"}, {"short-unicode", "密碼"}, {"invalid-utf8", "abcd\xff"}, + {"whitespace-only", " "}, + } { + t.Run(test.name, func(t *testing.T) { + file := filepath.Join(t.TempDir(), "factor") + if test.name != "missing" { + if err := os.WriteFile(file, []byte(test.data), 0600); err != nil { + t.Fatal(err) + } + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", file) + err := Init() + if err == nil || factory != nil { + t.Fatal("invalid file must not leave an active fallback factory") + } + if strings.Contains(err.Error(), "synthetic-secret") { + t.Fatal("errors must not expose factor contents") + } + }) + } + common.SetConfig("auth_token_factor_file", "") + if err := Init(); err != nil || factory.Factor != "legacy-inline-factor" { + t.Fatal("unconfigured deployments must preserve inline factors") + } +} diff --git a/vermeer/apps/auth/filter.go b/vermeer/apps/auth/filter.go index d87a34eae..4d2878db4 100644 --- a/vermeer/apps/auth/filter.go +++ b/vermeer/apps/auth/filter.go @@ -19,7 +19,9 @@ package auth import ( "encoding/json" + "errors" "net/http" + "os" "strings" "time" "vermeer/apps/common" @@ -39,9 +41,25 @@ const ( var factory *TokenFactory -func Init() { - factor := common.GetConfig("auth_token_factor").(string) +func Init() error { + factor, inlineConfigured := common.GetConfig("auth_token_factor").(string) + file := os.Getenv("VERMEER_AUTH_TOKEN_FACTOR_FILE") + if file == "" { + file, _ = common.GetConfigDefault("auth_token_factor_file", "").(string) + } + if file != "" { + var err error + factor, err = ReadTokenFactorFile(file) + if err != nil { + factory = nil + return err + } + } else if !inlineConfigured { + factory = nil + return errors.New("configure a token signing factor or factor file") + } factory = &TokenFactory{Factor: factor} + return nil } func NoneAuthFilter(ctx *gin.Context) { diff --git a/vermeer/apps/common/hugegraph_credentials.go b/vermeer/apps/common/hugegraph_credentials.go index 5f5633d2c..f6dcb2142 100644 --- a/vermeer/apps/common/hugegraph_credentials.go +++ b/vermeer/apps/common/hugegraph_credentials.go @@ -29,7 +29,23 @@ import ( func ResolveHugegraphCredentials(username, password string) (string, string, error) { file := deploymentConfig("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", "hugegraph_credentials_file") if file == "" { - return username, password, nil + deploymentUsername := deploymentConfig("VERMEER_HUGEGRAPH_USERNAME", "hugegraph_username") + passwordFile := deploymentConfig("VERMEER_HUGEGRAPH_PASSWORD_FILE", "hugegraph_password_file") + if deploymentUsername == "" && passwordFile == "" { + return username, password, nil + } + if deploymentUsername == "" || passwordFile == "" { + return "", "", errors.New("configure both HugeGraph username and password file") + } + data, err := os.ReadFile(passwordFile) + if err != nil { + return "", "", fmt.Errorf("read HugeGraph password file: %w", err) + } + if len(data) == 0 { + return "", "", errors.New("HugeGraph password file must not be empty") + } + // Password files are raw Secret values: preserve spaces, quotes and newlines. + return deploymentUsername, string(data), nil } data, err := os.ReadFile(file) if err != nil { diff --git a/vermeer/apps/common/hugegraph_credentials_test.go b/vermeer/apps/common/hugegraph_credentials_test.go index c5adae484..f6be84d78 100644 --- a/vermeer/apps/common/hugegraph_credentials_test.go +++ b/vermeer/apps/common/hugegraph_credentials_test.go @@ -95,3 +95,75 @@ func TestHugegraphCredentialsFileFailsClosed(t *testing.T) { }) } } + +func TestHugegraphPasswordFilePreservesSecretBytes(t *testing.T) { + isolatePDPeerConfig(t) + configured := filepath.Join(t.TempDir(), "configured-password") + override := filepath.Join(t.TempDir(), "environment-password") + const rawPassword = " quote\"backslash\\dollar$Unicode密碼\n" + if err := os.WriteFile(configured, []byte("config-password"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(override, []byte(rawPassword), 0600); err != nil { + t.Fatal(err) + } + SetConfig("hugegraph_username", "config-user") + SetConfig("hugegraph_password_file", configured) + t.Setenv("VERMEER_HUGEGRAPH_USERNAME", "environment-user") + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", override) + username, password, err := ResolveHugegraphCredentials("task-user", "task-password") + if err != nil || username != "environment-user" || password != rawPassword { + t.Fatal("raw Secret contents must survive file reading without escaping or trimming") + } + t.Setenv("VERMEER_HUGEGRAPH_USERNAME", "") + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", "") + username, password, err = ResolveHugegraphCredentials("task-user", "task-password") + if err != nil || username != "config-user" || password != "config-password" { + t.Fatal("config username/password file must override task credentials") + } + explicit := filepath.Join(t.TempDir(), "credentials.json") + if err := os.WriteFile(explicit, []byte(`{"username":"json-user","password":"json-password"}`), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", explicit) + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", filepath.Join(t.TempDir(), "missing")) + username, password, err = ResolveHugegraphCredentials("task-user", "task-password") + if err != nil || username != "json-user" || password != "json-password" { + t.Fatal("explicit JSON credentials must retain precedence over password-file credentials") + } +} + +func TestHugegraphPasswordFileFailsClosed(t *testing.T) { + for _, test := range []struct { + name, username, contents string + create, file bool + }{ + {name: "missing-user", contents: "synthetic-sensitive-password", create: true, file: true}, + {name: "missing-file-setting", username: "deployment-user"}, + {name: "missing-file", username: "deployment-user", file: true}, + {name: "empty-file", username: "deployment-user", create: true, file: true}, + } { + t.Run(test.name, func(t *testing.T) { + isolatePDPeerConfig(t) + file := filepath.Join(t.TempDir(), "password") + if test.create { + if err := os.WriteFile(file, []byte(test.contents), 0600); err != nil { + t.Fatal(err) + } + } + t.Setenv("VERMEER_HUGEGRAPH_USERNAME", test.username) + if test.file { + t.Setenv("VERMEER_HUGEGRAPH_PASSWORD_FILE", file) + } + username, password, err := ResolveHugegraphCredentials("task-user", "task-sensitive-password") + if err == nil || username != "" || password != "" { + t.Fatal("invalid deployment credentials must not fall back") + } + for _, secret := range []string{"synthetic-sensitive-password", "task-sensitive-password"} { + if strings.Contains(err.Error(), secret) { + t.Fatal("errors must not expose credentials") + } + } + }) + } +} diff --git a/vermeer/apps/common/pd_peers.go b/vermeer/apps/common/pd_peers.go index 3812c3f8b..162d86bfb 100644 --- a/vermeer/apps/common/pd_peers.go +++ b/vermeer/apps/common/pd_peers.go @@ -41,7 +41,9 @@ func resolvePDPeers(taskPeers []string) ([]string, error) { configured := deploymentConfig("VERMEER_PD_PEERS", "pd_peers") if configured == "" { if deploymentConfig("VERMEER_PD_PASSWORD_FILE", "pd_password_file") != "" || - deploymentConfig("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", "hugegraph_credentials_file") != "" { + deploymentConfig("VERMEER_HUGEGRAPH_CREDENTIALS_FILE", "hugegraph_credentials_file") != "" || + deploymentConfig("VERMEER_HUGEGRAPH_USERNAME", "hugegraph_username") != "" || + deploymentConfig("VERMEER_HUGEGRAPH_PASSWORD_FILE", "hugegraph_password_file") != "" { return nil, errors.New("configure pd_peers when using deployment credentials") } return taskPeers, nil diff --git a/vermeer/apps/common/pd_peers_test.go b/vermeer/apps/common/pd_peers_test.go index a79e559c1..197fe95be 100644 --- a/vermeer/apps/common/pd_peers_test.go +++ b/vermeer/apps/common/pd_peers_test.go @@ -73,12 +73,14 @@ func TestPDPeersOverrideWarning(t *testing.T) { func isolatePDPeerConfig(t *testing.T) { t.Helper() - for _, key := range []string{"pd_peers", "pd_password_file", "hugegraph_credentials_file"} { + for _, key := range []string{"pd_peers", "pd_password_file", "hugegraph_credentials_file", + "hugegraph_username", "hugegraph_password_file"} { old := GetConfigDefault(key, "") SetConfig(key, "") t.Cleanup(func() { SetConfig(key, old) }) } - for _, key := range []string{"VERMEER_PD_PEERS", "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE"} { + for _, key := range []string{"VERMEER_PD_PEERS", "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE", + "VERMEER_HUGEGRAPH_USERNAME", "VERMEER_HUGEGRAPH_PASSWORD_FILE"} { t.Setenv(key, "") } } @@ -103,8 +105,9 @@ func TestPDPeersDeploymentPrecedence(t *testing.T) { } func TestPDPeersDeploymentCredentialsRequireConfig(t *testing.T) { - for _, key := range []string{"pd_password_file", "hugegraph_credentials_file", - "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE"} { + for _, key := range []string{"pd_password_file", "hugegraph_credentials_file", "hugegraph_username", "hugegraph_password_file", + "VERMEER_PD_PASSWORD_FILE", "VERMEER_HUGEGRAPH_CREDENTIALS_FILE", + "VERMEER_HUGEGRAPH_USERNAME", "VERMEER_HUGEGRAPH_PASSWORD_FILE"} { t.Run(key, func(t *testing.T) { isolatePDPeerConfig(t) missing := filepath.Join(t.TempDir(), "not-created") diff --git a/vermeer/apps/master/master_main.go b/vermeer/apps/master/master_main.go index 91cd33180..160d5422a 100644 --- a/vermeer/apps/master/master_main.go +++ b/vermeer/apps/master/master_main.go @@ -50,7 +50,9 @@ func Main() { switch authType { case "token": sen.StaticFS("ui", asset.Assets) - auth.Init() + if err := auth.Init(); err != nil { + logrus.Fatalf("token authentication initialization failed: %s", err) + } services.SetRouters(sen, auth.TokenFilter) services.SetAdminRouters(sen, auth.TokenFilter, auth.AdminFilter) services.SetUI(sen) diff --git a/vermeer/tools/token/token_generator.go b/vermeer/tools/token/token_generator.go index f12dc8941..437cf61b9 100644 --- a/vermeer/tools/token/token_generator.go +++ b/vermeer/tools/token/token_generator.go @@ -19,18 +19,58 @@ package main import ( "encoding/json" + "errors" "flag" + "fmt" + "io" "math" + "os" + "path/filepath" "vermeer/apps/auth" ) func main() { - user := flag.String("user", "foo", "user name") - space := flag.String("space", "bar", "space name") - factor := flag.String("factor", "1234", "token factor") - client := flag.String("client", "hg", "client identifier") - immortal := flag.Bool("i", false, "immortal token") - flag.Parse() + if err := run(os.Args[1:], os.Stdout, os.Stderr); err != nil { + if errors.Is(err, flag.ErrHelp) { + return + } + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} + +func run(args []string, stdout, stderr io.Writer) error { + flags := flag.NewFlagSet("vermeer-token", flag.ContinueOnError) + flags.SetOutput(stderr) + user := flags.String("user", "foo", "user name") + space := flags.String("space", "bar", "space name") + factor := flags.String("factor", "1234", "token factor (prefer -factor-file for deployment secrets)") + factorFile := flags.String("factor-file", "", "read the token factor from this file") + outputFile := flags.String("output-file", "", "write only the token to this file with mode 0600") + quiet := flags.Bool("quiet", false, "print only the token when -output-file is not set") + client := flags.String("client", "hg", "client identifier") + immortal := flags.Bool("i", false, "immortal token") + if err := flags.Parse(args); err != nil { + return err + } + if flags.NArg() != 0 { + return errors.New("unexpected positional arguments") + } + if *user == "" { + return errors.New("user must not be empty") + } + if *factorFile != "" { + factorSpecified := false + flags.Visit(func(f *flag.Flag) { factorSpecified = factorSpecified || f.Name == "factor" }) + if factorSpecified { + return errors.New("use either -factor or -factor-file") + } + var err error + *factor, err = auth.ReadTokenFactorFile(*factorFile) + if err != nil { + return err + } + } factory := &auth.TokenFactory{Factor: *factor} tokenObj := factory.NewToken(*user, *space, *client) @@ -38,22 +78,39 @@ func main() { tokenObj.Time = math.MaxInt64 } factory.Sign(tokenObj) - jsonStr := toJson(*tokenObj) - token := auth.ToBase58(jsonStr) - - println(" user:", *user) - println(" space:", *space) - println(" factor:", *factor) - println(" client:", *client) - println("immortal:", *immortal) - println(" json:", jsonStr) - println(" token:", token) + data, err := json.Marshal(tokenObj) + if err != nil { + return errors.New("encode token") + } + token := auth.ToBase58(string(data)) + if *outputFile != "" { + return writeTokenFile(*outputFile, token) + } + if *quiet { + _, err = fmt.Fprintln(stdout, token) + return err + } + _, err = fmt.Fprintf(stdout, " user: %s\n space: %s\n client: %s\nimmortal: %t\n token: %s\n", + *user, *space, *client, *immortal, token) + return err } -func toJson(d interface{}) string { - if b, err := json.Marshal(d); err == nil { - str := string(b) - return str +func writeTokenFile(path, token string) error { + file, err := os.CreateTemp(filepath.Dir(path), ".vermeer-token-*") + if err != nil { + return fmt.Errorf("create token output: %w", err) + } + defer os.Remove(file.Name()) + if _, err = file.WriteString(token + "\n"); err != nil { + _ = file.Close() + return fmt.Errorf("write token output: %w", err) + } + if err = file.Close(); err != nil { + return fmt.Errorf("close token output: %w", err) + } + // Rename publishes a complete 0600 file and never follows a destination symlink. + if err = os.Rename(file.Name(), path); err != nil { + return fmt.Errorf("publish token output: %w", err) } - return "" + return nil } diff --git a/vermeer/tools/token/token_generator_test.go b/vermeer/tools/token/token_generator_test.go new file mode 100644 index 000000000..5946856f7 --- /dev/null +++ b/vermeer/tools/token/token_generator_test.go @@ -0,0 +1,188 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "bytes" + "encoding/json" + "math" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + "vermeer/apps/auth" + "vermeer/apps/common" + "vermeer/apps/structure" + + "github.com/gin-gonic/gin" +) + +func TestDeploymentTokenNativeAuthenticationAndRotation(t *testing.T) { + gin.SetMode(gin.TestMode) + factorFile := filepath.Join(t.TempDir(), "factor") + output := filepath.Join(t.TempDir(), "token") + const firstFactor = "synthetic-部署-factor\\with-quotes\"" + if err := os.WriteFile(factorFile, []byte(firstFactor+"\n"), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factorFile) + args := []string{"-factor-file", factorFile, "-output-file", output, "-quiet", + "-user", "admin", "-space", "$DEFAULT", "-i"} + var stdout, stderr bytes.Buffer + if err := run(args, &stdout, &stderr); err != nil { + t.Fatal(err) + } + if stdout.Len() != 0 || stderr.Len() != 0 { + t.Fatal("file output must not log credentials") + } + data, err := os.ReadFile(output) + if err != nil { + t.Fatal(err) + } + info, err := os.Stat(output) + if err != nil || info.Mode().Perm() != 0600 { + t.Fatal("token file must have mode 0600") + } + token := strings.TrimSpace(string(data)) + parsed := auth.ToTokenBase58(token) + factory := auth.TokenFactory{Factor: firstFactor} + if parsed == nil || parsed.Time != math.MaxInt64 || !factory.Verify(parsed) { + t.Fatal("helper token must use the native factory and immortal timestamp") + } + old := common.GetConfigDefault("auth_token_factor", "") + t.Cleanup(func() { common.SetConfig("auth_token_factor", old) }) + common.SetConfig("auth_token_factor", "different-config-factor") + if err := auth.Init(); err != nil { + t.Fatal(err) + } + router := gin.New() + router.GET("/protected", auth.TokenFilter, auth.AdminFilter, func(ctx *gin.Context) { + credential := ctx.MustGet(structure.CredentialKey).(*structure.Credential) + if credential.Space() != "$DEFAULT" { + t.Fatal("token service space changed") + } + ctx.Status(http.StatusOK) + }) + check := func(encoded string, want int) { + t.Helper() + request := httptest.NewRequest(http.MethodGet, "/protected", nil) + request.Header.Set("Authorization", "Bearer "+encoded) + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != want { + t.Fatalf("native filter status = %d, want %d", response.Code, want) + } + } + check(token, http.StatusOK) + parsed.Time = time.Now().Unix() - 25*60*60 + factory.Sign(parsed) + expired, err := json.Marshal(parsed) + if err != nil { + t.Fatal(err) + } + check(auth.ToBase58(string(expired)), http.StatusUnauthorized) + if err := os.WriteFile(factorFile, []byte("rotated-deployment-factor"), 0600); err != nil { + t.Fatal(err) + } + if err := auth.Init(); err != nil { + t.Fatal(err) + } + check(token, http.StatusUnauthorized) + if err := run(args, &stdout, &stderr); err != nil { + t.Fatal(err) + } + rotated, err := os.ReadFile(output) + if err != nil { + t.Fatal(err) + } + check(strings.TrimSpace(string(rotated)), http.StatusOK) +} + +func TestTokenOutputAndInputFailures(t *testing.T) { + var stdout, stderr bytes.Buffer + const secret = "synthetic-sensitive-factor" + if err := run([]string{"-factor", secret, "-quiet", "-i"}, &stdout, &stderr); err != nil { + t.Fatal(err) + } + if auth.ToTokenBase58(strings.TrimSpace(stdout.String())) == nil || stderr.Len() != 0 { + t.Fatal("quiet output must contain only the native token") + } + stdout.Reset() + if err := run([]string{"-factor", secret}, &stdout, &stderr); err != nil { + t.Fatal(err) + } + if strings.Contains(stdout.String(), secret) || strings.Contains(stdout.String(), "json:") { + t.Fatal("interactive metadata must not print the factor or signed JSON") + } + for _, content := range []string{"", "\n", "secret\nsecond-line", "secret\rsecond-line"} { + file := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(file, []byte(content), 0600); err != nil { + t.Fatal(err) + } + stdout.Reset() + stderr.Reset() + if err := run([]string{"-factor-file", file}, &stdout, &stderr); err == nil { + t.Fatal("invalid factor files must fail") + } + if stdout.Len() != 0 || stderr.Len() != 0 { + t.Fatal("invalid factors must not be printed") + } + } + for _, args := range [][]string{ + {"-factor-file", filepath.Join(t.TempDir(), "missing")}, + {"-factor", secret, "-factor-file", "unused"}, + {"-user", ""}, + {"-output-file", filepath.Join(t.TempDir(), "missing", "token")}, + } { + stdout.Reset() + stderr.Reset() + if err := run(args, &stdout, &stderr); err == nil { + t.Fatal("invalid arguments must fail") + } + if stdout.Len() != 0 { + t.Fatal("failed generation must not expose token contents") + } + } +} + +func TestTokenFileReplacesSymlinkWithoutReadingTarget(t *testing.T) { + directory := t.TempDir() + target := filepath.Join(directory, "target") + output := filepath.Join(directory, "token") + if err := os.WriteFile(target, []byte("keep-existing-file"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, output); err != nil { + t.Fatal(err) + } + var stdout, stderr bytes.Buffer + if err := run([]string{"-output-file", output, "-i"}, &stdout, &stderr); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(target) + if err != nil || string(data) != "keep-existing-file" { + t.Fatal("token output must not follow a symlink") + } + info, err := os.Lstat(output) + if err != nil || !info.Mode().IsRegular() || info.Mode().Perm() != 0600 { + t.Fatal("published token must be a private regular file") + } +} From 1d3abd467fbd8dd74fea80e039d459933864d868 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 04:38:50 +0000 Subject: [PATCH 02/17] docs(vermeer): correct the native UI entry route --- vermeer/README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vermeer/README.md b/vermeer/README.md index 2d922aa38..77f479c62 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -402,7 +402,8 @@ worker_group = default Vermeer exposes a REST API on port `6688` (configurable in `master.ini`). -> **Note**: The Web UI (`/ui/*`) is only available when `auth=token` is configured. Set `auth_token_factor` in `master.ini` or pass `-auth token -auth_token_factor ` via command line. +> **Note**: The Web UI at `/` (with static assets under `/ui/*`) is only available when `auth=token` is configured. +> Configure the signing factor through `VERMEER_AUTH_TOKEN_FACTOR_FILE` for deployments, or `auth_token_factor` for local use. ### Key Endpoints @@ -415,7 +416,7 @@ Vermeer exposes a REST API on port `6688` (configurable in `master.ini`). | `/api/v1/tasks/{task_id}` | GET | Get task status and results | | `/api/v1/workers` | GET | List connected workers | | `/api/v1/workers/availability` | GET | Read worker readiness for the authenticated space | -| `/ui/` | GET | Web UI dashboard | +| `/` | GET | Web UI dashboard | Worker availability uses the normal space credential and returns only `{"available":true|false}` for that authenticated space's allocations or the @@ -646,7 +647,7 @@ vermeer/tools/protoc/linux64/protoc vermeer/apps/protos/*.proto --go-grpc_out=ve ## Monitoring -Access the Web UI dashboard at `http://master-ip:6688/ui/master.html` for: +Access the Web UI dashboard at `http://master-ip:6688/` for: > **Note**: The Web UI requires `auth=token` in `master.ini` (or `-auth token` via command line). Without token authentication, the UI routes are not registered. From 319e02f0e2d41834b9f2c8c8e4fe0f2555670905 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 04:43:59 +0000 Subject: [PATCH 03/17] fix(vermeer): preserve executable image permissions for non-root users --- .github/workflows/vermeer-ci.yml | 16 ++++++++++++++++ vermeer/Dockerfile | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/vermeer-ci.yml b/.github/workflows/vermeer-ci.yml index 4e71048d0..87e690a12 100644 --- a/.github/workflows/vermeer-ci.yml +++ b/.github/workflows/vermeer-ci.yml @@ -152,3 +152,19 @@ jobs: echo 'Master log exposed the factor' >&2 exit 1 fi + # Exercise the same non-root identity used by the Helm test hook. + # Keep its synthetic factor and token inside a private writable /tmp directory. + docker run --rm --user 100:101 --entrypoint /bin/sh vermeer:ci-check -ec ' + umask 077 + token_dir=$(mktemp -d /tmp/vermeer-token.XXXXXX) + trap '\''rm -rf "${token_dir}"'\'' EXIT + printf "%s" synthetic-nonroot-test-factor > "${token_dir}/factor" + test -x /go/bin/app + /go/bin/vermeer-token -factor-file "${token_dir}/factor" \ + -output-file "${token_dir}/token" -user admin -space '\''$DEFAULT'\'' -i -quiet \ + > "${token_dir}/stdout" 2> "${token_dir}/stderr" + test -s "${token_dir}/token" + test "$(stat -c %a "${token_dir}/token")" = 600 + test ! -s "${token_dir}/stdout" + test ! -s "${token_dir}/stderr" + ' diff --git a/vermeer/Dockerfile b/vermeer/Dockerfile index 55676dba2..97766887c 100644 --- a/vermeer/Dockerfile +++ b/vermeer/Dockerfile @@ -39,8 +39,8 @@ RUN --mount=type=cache,target=/go/pkg/mod \ FROM alpine EXPOSE 8080 -COPY --from=builder /go/bin/app /go/bin/app -COPY --from=builder /go/bin/vermeer-token /go/bin/vermeer-token +COPY --from=builder --chmod=0755 /go/bin/app /go/bin/app +COPY --from=builder --chmod=0755 /go/bin/vermeer-token /go/bin/vermeer-token COPY --from=builder /src/config/ /go/bin/config/ COPY --from=builder /src/release-docs/ /go/bin/release-docs/ COPY --from=builder /usr/local/go/lib/time/zoneinfo.zip / From eba60f869a849fa3f81164107e001d79abddfe8b Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 04:51:05 +0000 Subject: [PATCH 04/17] fix(vermeer): sign file-backed deployment tokens with HMAC-SHA256 --- .github/workflows/vermeer-ci.yml | 2 +- vermeer/README.md | 9 ++ vermeer/apps/auth/factor_file.go | 11 +- vermeer/apps/auth/factor_file_test.go | 4 +- vermeer/apps/auth/filter.go | 10 +- vermeer/apps/auth/token.go | 42 ++++++- vermeer/apps/auth/token_v2_test.go | 111 ++++++++++++++++++ .../master/services/http_login_v2_test.go | 96 +++++++++++++++ vermeer/tools/token/token_generator.go | 11 +- vermeer/tools/token/token_generator_test.go | 16 ++- 10 files changed, 296 insertions(+), 16 deletions(-) create mode 100644 vermeer/apps/auth/token_v2_test.go create mode 100644 vermeer/apps/master/services/http_login_v2_test.go diff --git a/.github/workflows/vermeer-ci.yml b/.github/workflows/vermeer-ci.yml index 87e690a12..1224f8dda 100644 --- a/.github/workflows/vermeer-ci.yml +++ b/.github/workflows/vermeer-ci.yml @@ -76,7 +76,7 @@ jobs: run: | go test -race ./apps/auth ./tools/token go test -race ./apps/common - go test -race ./apps/master/workers ./apps/master/services -run 'TestWorkerAvailability' + go test -race ./apps/master/workers ./apps/master/services -run 'Test(WorkerAvailability|NativeLoginCookie)' go test -race ./apps/master/bl -run 'Test(WorkerAvailability|HugegraphLoadAuthorization)' go test -race ./apps/worker -run 'Test(LoadReceivers|RecvVertex|ComputeDispatchCancellation)' go test -race ./apps/structure -run 'Test(VertexStore|GraphPropagates|MemoryVertex|HugegraphSchema)' diff --git a/vermeer/README.md b/vermeer/README.md index 77f479c62..598cb8427 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -263,6 +263,12 @@ The file must contain one valid UTF-8 line with at least four characters and can A terminal LF or CRLF is accepted; other spaces are preserved. An invalid configured file stops Master before it opens its listeners; it never falls back to an inline or default factor. +File-backed factors use **v2 HMAC-SHA256** tokens. Their digest carries a `v2:` version marker inside the existing Base58-encoded token; +the signed payload preserves the boundaries of user, space, nickname, client and timestamp fields. +Master in file mode accepts only v2 signatures and compares MACs in constant time. It never falls back to legacy SHA1 verification. +The existing `auth_token_factor` string configuration and `-factor` generator option retain their historical token protocol for older clients. +Those legacy tokens and v2 tokens cannot authenticate across modes. + Images built from this revision include the native `/go/bin/vermeer-token` helper. Generate a protected login/service token file: ```bash @@ -290,6 +296,9 @@ Hubble accepts this plain token file through `vermeer.token_file`; it must not i Normal tokens expire after 24 hours. Long-lived service tokens must be protected like passwords. Rotating the signing factor invalidates existing tokens; regenerate dependent service tokens and restart their clients together. Token generation reuses Vermeer's native signing and encoding implementation and does not require a running Master or Kubernetes API access. +When upgrading an earlier file-based deployment to v2, retain its protected factor file but upgrade Master and the token helper together, +regenerate service token files, and restart dependent clients. Existing native UI cookies contain old signatures and require a new login token. +Hubble treats tokens as opaque text, so no Hubble API or token-file format change is required. The generated token can be used in two ways: diff --git a/vermeer/apps/auth/factor_file.go b/vermeer/apps/auth/factor_file.go index a2e9f1a98..37dac33e2 100644 --- a/vermeer/apps/auth/factor_file.go +++ b/vermeer/apps/auth/factor_file.go @@ -33,9 +33,16 @@ func ReadTokenFactorFile(file string) (string, error) { return "", fmt.Errorf("read token factor file: %w", err) } factor := strings.TrimSuffix(strings.TrimSuffix(string(data), "\n"), "\r") + if err := validateTokenSigningKey(factor); err != nil { + return "", err + } + return factor, nil +} + +func validateTokenSigningKey(factor string) error { if !utf8.ValidString(factor) || utf8.RuneCountInString(factor) < 4 || strings.TrimSpace(factor) == "" || strings.ContainsAny(factor, "\r\n") { - return "", errors.New("token factor file must contain one UTF-8 line of at least four characters") + return errors.New("token signing key must contain one UTF-8 line of at least four characters") } - return factor, nil + return nil } diff --git a/vermeer/apps/auth/factor_file_test.go b/vermeer/apps/auth/factor_file_test.go index d4ca2a8b5..8fc719275 100644 --- a/vermeer/apps/auth/factor_file_test.go +++ b/vermeer/apps/auth/factor_file_test.go @@ -45,11 +45,11 @@ func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { } common.SetConfig("auth_token_factor_file", configured) t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", override) - if err := Init(); err != nil || factory.Factor != "environment-factor" { + if err := Init(); err != nil || factory.Factor != "" || string(factory.signingKey) != "environment-factor" { t.Fatal("environment factor file must override config and inline factor") } t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", "") - if err := Init(); err != nil || factory.Factor != "configured-factor" { + if err := Init(); err != nil || factory.Factor != "" || string(factory.signingKey) != "configured-factor" { t.Fatal("configured factor file must override inline factor") } for _, test := range []struct{ name, data string }{ diff --git a/vermeer/apps/auth/filter.go b/vermeer/apps/auth/filter.go index 4d2878db4..8f32b87d4 100644 --- a/vermeer/apps/auth/filter.go +++ b/vermeer/apps/auth/filter.go @@ -42,19 +42,21 @@ const ( var factory *TokenFactory func Init() error { - factor, inlineConfigured := common.GetConfig("auth_token_factor").(string) file := os.Getenv("VERMEER_AUTH_TOKEN_FACTOR_FILE") if file == "" { file, _ = common.GetConfigDefault("auth_token_factor_file", "").(string) } if file != "" { - var err error - factor, err = ReadTokenFactorFile(file) + signingKey, err := ReadTokenFactorFile(file) if err != nil { factory = nil return err } - } else if !inlineConfigured { + factory, err = NewSecureTokenFactory(signingKey) + return err + } + factor, inlineConfigured := common.GetConfig("auth_token_factor").(string) + if !inlineConfigured { factory = nil return errors.New("configure a token signing factor or factor file") } diff --git a/vermeer/apps/auth/token.go b/vermeer/apps/auth/token.go index 725c46dff..a5f095929 100644 --- a/vermeer/apps/auth/token.go +++ b/vermeer/apps/auth/token.go @@ -18,8 +18,11 @@ under the License. package auth import ( + "crypto/hmac" "crypto/sha1" + "crypto/sha256" "encoding/hex" + "encoding/json" "io" "strconv" "strings" @@ -28,7 +31,10 @@ import ( var empty = "" -const defaultFactor = "!^_^!YouCanTypeWhateverYouWant" +const ( + defaultFactor = "!^_^!YouCanTypeWhateverYouWant" + secureTokenPrefix = "v2:" +) type Token struct { User string `json:"u"` // Login ID @@ -42,6 +48,16 @@ type Token struct { type TokenFactory struct { Factor string randoms *[4]string + // File-backed deployment keys never populate the legacy Factor field. + signingKey []byte +} + +// NewSecureTokenFactory uses versioned HMAC-SHA256 tokens without legacy fallback. +func NewSecureTokenFactory(signingKey string) (*TokenFactory, error) { + if err := validateTokenSigningKey(signingKey); err != nil { + return nil, err + } + return &TokenFactory{signingKey: []byte(signingKey)}, nil } func (factory *TokenFactory) getRandoms() *[4]string { @@ -82,6 +98,10 @@ func (factory *TokenFactory) Sign(a *Token) { if a == nil { return } + if factory.signingKey != nil { + a.Digest = secureTokenPrefix + hex.EncodeToString(factory.secureSignature(a)) + return + } a.Digest = digest(factory.signSource(a)) } @@ -90,9 +110,27 @@ func (factory *TokenFactory) Verify(a *Token) bool { return false } + if factory.signingKey != nil { + encoded, ok := strings.CutPrefix(a.Digest, secureTokenPrefix) + if !ok || len(encoded) != sha256.Size*2 { + return false + } + signature, err := hex.DecodeString(encoded) + return err == nil && hmac.Equal(signature, factory.secureSignature(a)) + } s := digest(factory.signSource(a)) - return a.Digest == s + return hmac.Equal([]byte(a.Digest), []byte(s)) +} + +func (factory *TokenFactory) secureSignature(token *Token) []byte { + // Fixed-field JSON preserves field boundaries and excludes the digest. + // Authenticate the nickname as well as every authorization field. + payload, _ := json.Marshal([5]interface{}{token.User, token.Space, token.Nick, token.Client, token.Time}) + mac := hmac.New(sha256.New, factory.signingKey) + _, _ = mac.Write([]byte("vermeer-token-v2\x00")) + _, _ = mac.Write(payload) + return mac.Sum(nil) } func digest(s string) string { diff --git a/vermeer/apps/auth/token_v2_test.go b/vermeer/apps/auth/token_v2_test.go new file mode 100644 index 000000000..784ab6a6b --- /dev/null +++ b/vermeer/apps/auth/token_v2_test.go @@ -0,0 +1,111 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package auth + +import ( + "math" + "strings" + "testing" +) + +func TestSecureTokenProtocolAndLegacyCompatibility(t *testing.T) { + const key = "legacy-compatibility-factor" + legacy := &TokenFactory{Factor: key} + secure, err := NewSecureTokenFactory(key) + if err != nil { + t.Fatal(err) + } + if secure.Factor != "" || secure.signingKey == nil { + t.Fatal("secure keys must be separate from legacy factor state") + } + token := &Token{User: "admin", Space: "$DEFAULT", Client: "compatibility", Time: math.MaxInt64} + legacy.Sign(token) + if token.Digest != "a89528d7bfec6d12addab5a403da64a148f03ce5" || !legacy.Verify(token) { + t.Fatal("legacy inline signing must preserve the historical protocol") + } + if secure.Verify(token) { + t.Fatal("file-backed factories must not accept legacy signatures") + } + secure.Sign(token) + // Independent HMAC-SHA256 fixture over the documented domain and JSON payload. + const expected = "v2:f567b23ada3378d0b216008b2076afd3577b2e69be49dab6efcaeb2618d338eb" + if token.Digest != expected || !secure.Verify(token) { + t.Fatal("unexpected v2 signing protocol") + } + if legacy.Verify(token) { + t.Fatal("legacy factories must not accept v2 signatures") + } + rotated, err := NewSecureTokenFactory("rotated-deployment-signing-key") + if err != nil { + t.Fatal(err) + } + if rotated.Verify(token) { + t.Fatal("rotating the signing key must invalidate old signatures") + } + if secure.Verify(nil) || legacy.Verify(nil) { + t.Fatal("nil tokens must be rejected") + } +} + +func TestSecureTokenRejectsTampering(t *testing.T) { + factory, err := NewSecureTokenFactory("synthetic-secure-signing-key") + if err != nil { + t.Fatal(err) + } + original := factory.NewToken("admin", "$DEFAULT", "hubble") + original.Nick = "display-name" + original.Time = math.MaxInt64 + factory.Sign(original) + mutations := map[string]func(*Token){ + "user": func(token *Token) { token.User = "another-user" }, + "space": func(token *Token) { token.Space = "another-space" }, + "nickname": func(token *Token) { token.Nick = "another-name" }, + "client": func(token *Token) { token.Client = "another-client" }, + "time": func(token *Token) { token.Time-- }, + "version": func(token *Token) { token.Digest = "v1:" + strings.TrimPrefix(token.Digest, "v2:") }, + "unversioned": func(token *Token) { token.Digest = strings.TrimPrefix(token.Digest, "v2:") }, + "truncated": func(token *Token) { token.Digest = token.Digest[:len(token.Digest)-1] }, + "invalid-hex": func(token *Token) { token.Digest = "v2:" + strings.Repeat("z", 64) }, + "signature": func(token *Token) { token.Digest = "v2:" + strings.Repeat("0", 64) }, + } + for name, mutate := range mutations { + t.Run(name, func(t *testing.T) { + token := *original + mutate(&token) + if factory.Verify(&token) { + t.Fatal("modified token must not authenticate") + } + }) + } + first := &Token{User: "a", Space: "bc", Client: "client", Time: 1} + second := &Token{User: "ab", Space: "c", Client: "client", Time: 1} + factory.Sign(first) + second.Digest = first.Digest + if factory.Verify(second) { + t.Fatal("signed fields must retain unambiguous boundaries") + } +} + +func TestSecureTokenFactoryRejectsInvalidKeys(t *testing.T) { + for _, key := range []string{"", "abc", "密碼", " ", "key\nsecond-line", "key\xff"} { + factory, err := NewSecureTokenFactory(key) + if err == nil || factory != nil { + t.Fatal("invalid secure keys must not create a legacy factory") + } + } +} diff --git a/vermeer/apps/master/services/http_login_v2_test.go b/vermeer/apps/master/services/http_login_v2_test.go new file mode 100644 index 000000000..755dc450b --- /dev/null +++ b/vermeer/apps/master/services/http_login_v2_test.go @@ -0,0 +1,96 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package services + +import ( + "bytes" + "encoding/json" + "math" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "vermeer/apps/auth" + "vermeer/apps/common" + + "github.com/gin-gonic/gin" +) + +func TestNativeLoginCookieUsesSecureDeploymentToken(t *testing.T) { + previousMode := gin.Mode() + previousFactor := common.GetConfig("auth_token_factor") + t.Cleanup(func() { gin.SetMode(previousMode); common.SetConfig("auth_token_factor", previousFactor) }) + gin.SetMode(gin.TestMode) + const key = "synthetic-native-ui-signing-key" + factorFile := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(factorFile, []byte(key), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factorFile) + common.SetConfig("auth_token_factor", key) + if err := auth.Init(); err != nil { + t.Fatal(err) + } + secure, err := auth.NewSecureTokenFactory(key) + if err != nil { + t.Fatal(err) + } + token := secure.NewToken("admin", "$DEFAULT", "native-ui") + token.Time = math.MaxInt64 + secure.Sign(token) + router := gin.New() + router.POST("/api/v1/login", (&LoginHandler{}).POST) + router.GET("/protected", auth.TokenFilter, auth.AdminFilter, func(ctx *gin.Context) { ctx.Status(http.StatusOK) }) + authenticate := func(token *auth.Token, want int) { + t.Helper() + encoded, err := json.Marshal(token) + if err != nil { + t.Fatal(err) + } + body, err := json.Marshal(map[string]string{"token": auth.ToBase58(string(encoded))}) + if err != nil { + t.Fatal(err) + } + login := httptest.NewRequest(http.MethodPost, "/api/v1/login", bytes.NewReader(body)) + login.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + router.ServeHTTP(response, login) + if response.Code != http.StatusOK { + t.Fatalf("native login status = %d", response.Code) + } + cookies := response.Result().Cookies() + if len(cookies) != 1 || !cookies[0].HttpOnly { + t.Fatal("native login must set its protected session cookie") + } + request := httptest.NewRequest(http.MethodGet, "/protected", nil) + request.AddCookie(cookies[0]) + checked := httptest.NewRecorder() + router.ServeHTTP(checked, request) + if checked.Code != want { + t.Fatalf("cookie authentication status = %d, want %d", checked.Code, want) + } + } + authenticate(token, http.StatusOK) + tampered := *token + tampered.Space = "another-space" + authenticate(&tampered, http.StatusUnauthorized) + legacy := &auth.TokenFactory{Factor: key} + legacy.Sign(token) + authenticate(token, http.StatusUnauthorized) +} diff --git a/vermeer/tools/token/token_generator.go b/vermeer/tools/token/token_generator.go index 437cf61b9..deaed1865 100644 --- a/vermeer/tools/token/token_generator.go +++ b/vermeer/tools/token/token_generator.go @@ -59,20 +59,25 @@ func run(args []string, stdout, stderr io.Writer) error { if *user == "" { return errors.New("user must not be empty") } + var factory *auth.TokenFactory if *factorFile != "" { factorSpecified := false flags.Visit(func(f *flag.Flag) { factorSpecified = factorSpecified || f.Name == "factor" }) if factorSpecified { return errors.New("use either -factor or -factor-file") } - var err error - *factor, err = auth.ReadTokenFactorFile(*factorFile) + signingKey, err := auth.ReadTokenFactorFile(*factorFile) if err != nil { return err } + factory, err = auth.NewSecureTokenFactory(signingKey) + if err != nil { + return err + } + } else { + factory = &auth.TokenFactory{Factor: *factor} } - factory := &auth.TokenFactory{Factor: *factor} tokenObj := factory.NewToken(*user, *space, *client) if *immortal { tokenObj.Time = math.MaxInt64 diff --git a/vermeer/tools/token/token_generator_test.go b/vermeer/tools/token/token_generator_test.go index 5946856f7..d25a4e0f5 100644 --- a/vermeer/tools/token/token_generator_test.go +++ b/vermeer/tools/token/token_generator_test.go @@ -63,8 +63,12 @@ func TestDeploymentTokenNativeAuthenticationAndRotation(t *testing.T) { } token := strings.TrimSpace(string(data)) parsed := auth.ToTokenBase58(token) - factory := auth.TokenFactory{Factor: firstFactor} - if parsed == nil || parsed.Time != math.MaxInt64 || !factory.Verify(parsed) { + factory, err := auth.NewSecureTokenFactory(firstFactor) + if err != nil { + t.Fatal(err) + } + if parsed == nil || parsed.Time != math.MaxInt64 || + !strings.HasPrefix(parsed.Digest, "v2:") || !factory.Verify(parsed) { t.Fatal("helper token must use the native factory and immortal timestamp") } old := common.GetConfigDefault("auth_token_factor", "") @@ -92,6 +96,14 @@ func TestDeploymentTokenNativeAuthenticationAndRotation(t *testing.T) { } } check(token, http.StatusOK) + legacyFactory := auth.TokenFactory{Factor: firstFactor} + legacyToken := *parsed + legacyFactory.Sign(&legacyToken) + legacyData, err := json.Marshal(legacyToken) + if err != nil { + t.Fatal(err) + } + check(auth.ToBase58(string(legacyData)), http.StatusUnauthorized) parsed.Time = time.Now().Unix() - 25*60*60 factory.Sign(parsed) expired, err := json.Marshal(parsed) From f46185037854dfd9fbf238c480fcb085dcf90fe4 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 04:58:17 +0000 Subject: [PATCH 05/17] fix(vermeer): isolate secure token signing from legacy factories --- .github/workflows/codeql-analysis.yml | 13 +++++ vermeer/apps/auth/factor_file_test.go | 12 +++-- vermeer/apps/auth/filter.go | 13 ++++- vermeer/apps/auth/secure_token.go | 75 ++++++++++++++++++++++++++ vermeer/apps/auth/token.go | 43 ++------------- vermeer/apps/auth/token_v2_test.go | 2 +- vermeer/tools/token/token_generator.go | 18 +++---- 7 files changed, 122 insertions(+), 54 deletions(-) create mode 100644 vermeer/apps/auth/secure_token.go diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 6051eb371..cf85eff6c 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -66,6 +66,19 @@ jobs: - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v3 + with: + output: ${{ runner.temp }}/codeql-results + + # Preserve paths for security findings even when the installation token + # cannot read the repository's code-scanning API. This does not filter results. + - name: Preserve Go CodeQL diagnostics + if: always() && matrix.language == 'go' + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: codeql-go-sarif-${{ github.run_attempt }} + path: ${{ runner.temp }}/codeql-results/go.sarif + if-no-files-found: ignore + retention-days: 1 # Disabled for now because dependency review is unsupported until the # repository dependency graph is enabled. diff --git a/vermeer/apps/auth/factor_file_test.go b/vermeer/apps/auth/factor_file_test.go index 8fc719275..55278c684 100644 --- a/vermeer/apps/auth/factor_file_test.go +++ b/vermeer/apps/auth/factor_file_test.go @@ -29,10 +29,12 @@ func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { previousFactor := common.GetConfigDefault("auth_token_factor", "") previousFile := common.GetConfigDefault("auth_token_factor_file", "") previousFactory := factory + previousSecureFactory := secureFactory t.Cleanup(func() { common.SetConfig("auth_token_factor", previousFactor) common.SetConfig("auth_token_factor_file", previousFile) factory = previousFactory + secureFactory = previousSecureFactory }) common.SetConfig("auth_token_factor", "legacy-inline-factor") configured := filepath.Join(t.TempDir(), "configured") @@ -45,11 +47,13 @@ func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { } common.SetConfig("auth_token_factor_file", configured) t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", override) - if err := Init(); err != nil || factory.Factor != "" || string(factory.signingKey) != "environment-factor" { + if err := Init(); err != nil || factory != nil || secureFactory == nil || + string(secureFactory.signingKey) != "environment-factor" { t.Fatal("environment factor file must override config and inline factor") } t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", "") - if err := Init(); err != nil || factory.Factor != "" || string(factory.signingKey) != "configured-factor" { + if err := Init(); err != nil || factory != nil || secureFactory == nil || + string(secureFactory.signingKey) != "configured-factor" { t.Fatal("configured factor file must override inline factor") } for _, test := range []struct{ name, data string }{ @@ -66,7 +70,7 @@ func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { } t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", file) err := Init() - if err == nil || factory != nil { + if err == nil || factory != nil || secureFactory != nil { t.Fatal("invalid file must not leave an active fallback factory") } if strings.Contains(err.Error(), "synthetic-secret") { @@ -75,7 +79,7 @@ func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { }) } common.SetConfig("auth_token_factor_file", "") - if err := Init(); err != nil || factory.Factor != "legacy-inline-factor" { + if err := Init(); err != nil || secureFactory != nil || factory == nil || factory.Factor != "legacy-inline-factor" { t.Fatal("unconfigured deployments must preserve inline factors") } } diff --git a/vermeer/apps/auth/filter.go b/vermeer/apps/auth/filter.go index 8f32b87d4..eee77d036 100644 --- a/vermeer/apps/auth/filter.go +++ b/vermeer/apps/auth/filter.go @@ -40,8 +40,11 @@ const ( ) var factory *TokenFactory +var secureFactory *SecureTokenFactory func Init() error { + factory = nil + secureFactory = nil file := os.Getenv("VERMEER_AUTH_TOKEN_FACTOR_FILE") if file == "" { file, _ = common.GetConfigDefault("auth_token_factor_file", "").(string) @@ -52,7 +55,7 @@ func Init() error { factory = nil return err } - factory, err = NewSecureTokenFactory(signingKey) + secureFactory, err = NewSecureTokenFactory(signingKey) return err } factor, inlineConfigured := common.GetConfig("auth_token_factor").(string) @@ -77,6 +80,7 @@ func TokenFilter(ctx *gin.Context) { var credential *structure.Credential var err error var text string + var valid bool header := ctx.GetHeader(HeaderKey) if header == "" { if str, err := ctx.Cookie(CookieKey); err != nil { @@ -97,7 +101,12 @@ func TokenFilter(ctx *gin.Context) { goto LOGIN } - if !factory.Verify(token) { + if secureFactory != nil { + valid = secureFactory.Verify(token) + } else if factory != nil { + valid = factory.Verify(token) + } + if !valid { text = "Invalid Token" goto LOGIN } diff --git a/vermeer/apps/auth/secure_token.go b/vermeer/apps/auth/secure_token.go new file mode 100644 index 000000000..0bc18d5cc --- /dev/null +++ b/vermeer/apps/auth/secure_token.go @@ -0,0 +1,75 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package auth + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "strings" +) + +const secureTokenPrefix = "v2:" + +// SecureTokenFactory keeps deployment signing keys out of the legacy token implementation. +// Its concrete type has no legacy Factor field or SHA1 signing path. +type SecureTokenFactory struct { + signingKey []byte +} + +// NewSecureTokenFactory uses versioned HMAC-SHA256 tokens without legacy fallback. +func NewSecureTokenFactory(signingKey string) (*SecureTokenFactory, error) { + if err := validateTokenSigningKey(signingKey); err != nil { + return nil, err + } + return &SecureTokenFactory{signingKey: []byte(signingKey)}, nil +} + +func (factory *SecureTokenFactory) NewToken(user, space, client string) *Token { + return newToken(user, space, client) +} + +func (factory *SecureTokenFactory) Sign(token *Token) { + if token == nil { + return + } + token.Digest = secureTokenPrefix + hex.EncodeToString(factory.signature(token)) +} + +func (factory *SecureTokenFactory) Verify(token *Token) bool { + if token == nil { + return false + } + encoded, ok := strings.CutPrefix(token.Digest, secureTokenPrefix) + if !ok || len(encoded) != sha256.Size*2 { + return false + } + signature, err := hex.DecodeString(encoded) + return err == nil && hmac.Equal(signature, factory.signature(token)) +} + +func (factory *SecureTokenFactory) signature(token *Token) []byte { + // Fixed-field JSON preserves field boundaries and excludes the digest. + // Authenticate the nickname as well as every authorization field. + payload, _ := json.Marshal([5]interface{}{token.User, token.Space, token.Nick, token.Client, token.Time}) + mac := hmac.New(sha256.New, factory.signingKey) + _, _ = mac.Write([]byte("vermeer-token-v2\x00")) + _, _ = mac.Write(payload) + return mac.Sum(nil) +} diff --git a/vermeer/apps/auth/token.go b/vermeer/apps/auth/token.go index a5f095929..42aba975c 100644 --- a/vermeer/apps/auth/token.go +++ b/vermeer/apps/auth/token.go @@ -20,9 +20,7 @@ package auth import ( "crypto/hmac" "crypto/sha1" - "crypto/sha256" "encoding/hex" - "encoding/json" "io" "strconv" "strings" @@ -31,10 +29,7 @@ import ( var empty = "" -const ( - defaultFactor = "!^_^!YouCanTypeWhateverYouWant" - secureTokenPrefix = "v2:" -) +const defaultFactor = "!^_^!YouCanTypeWhateverYouWant" type Token struct { User string `json:"u"` // Login ID @@ -48,16 +43,6 @@ type Token struct { type TokenFactory struct { Factor string randoms *[4]string - // File-backed deployment keys never populate the legacy Factor field. - signingKey []byte -} - -// NewSecureTokenFactory uses versioned HMAC-SHA256 tokens without legacy fallback. -func NewSecureTokenFactory(signingKey string) (*TokenFactory, error) { - if err := validateTokenSigningKey(signingKey); err != nil { - return nil, err - } - return &TokenFactory{signingKey: []byte(signingKey)}, nil } func (factory *TokenFactory) getRandoms() *[4]string { @@ -85,6 +70,10 @@ func (factory *TokenFactory) getRandoms() *[4]string { // NewToken Create a Token by login-id, space, client(e.g. IP address). func (factory *TokenFactory) NewToken(user string, space string, client string) *Token { + return newToken(user, space, client) +} + +func newToken(user, space, client string) *Token { if user == "" { return nil } @@ -98,10 +87,6 @@ func (factory *TokenFactory) Sign(a *Token) { if a == nil { return } - if factory.signingKey != nil { - a.Digest = secureTokenPrefix + hex.EncodeToString(factory.secureSignature(a)) - return - } a.Digest = digest(factory.signSource(a)) } @@ -110,29 +95,11 @@ func (factory *TokenFactory) Verify(a *Token) bool { return false } - if factory.signingKey != nil { - encoded, ok := strings.CutPrefix(a.Digest, secureTokenPrefix) - if !ok || len(encoded) != sha256.Size*2 { - return false - } - signature, err := hex.DecodeString(encoded) - return err == nil && hmac.Equal(signature, factory.secureSignature(a)) - } s := digest(factory.signSource(a)) return hmac.Equal([]byte(a.Digest), []byte(s)) } -func (factory *TokenFactory) secureSignature(token *Token) []byte { - // Fixed-field JSON preserves field boundaries and excludes the digest. - // Authenticate the nickname as well as every authorization field. - payload, _ := json.Marshal([5]interface{}{token.User, token.Space, token.Nick, token.Client, token.Time}) - mac := hmac.New(sha256.New, factory.signingKey) - _, _ = mac.Write([]byte("vermeer-token-v2\x00")) - _, _ = mac.Write(payload) - return mac.Sum(nil) -} - func digest(s string) string { if s == "" { return "" diff --git a/vermeer/apps/auth/token_v2_test.go b/vermeer/apps/auth/token_v2_test.go index 784ab6a6b..a3053adc9 100644 --- a/vermeer/apps/auth/token_v2_test.go +++ b/vermeer/apps/auth/token_v2_test.go @@ -30,7 +30,7 @@ func TestSecureTokenProtocolAndLegacyCompatibility(t *testing.T) { if err != nil { t.Fatal(err) } - if secure.Factor != "" || secure.signingKey == nil { + if secure.signingKey == nil { t.Fatal("secure keys must be separate from legacy factor state") } token := &Token{User: "admin", Space: "$DEFAULT", Client: "compatibility", Time: math.MaxInt64} diff --git a/vermeer/tools/token/token_generator.go b/vermeer/tools/token/token_generator.go index deaed1865..b148afd85 100644 --- a/vermeer/tools/token/token_generator.go +++ b/vermeer/tools/token/token_generator.go @@ -26,6 +26,7 @@ import ( "math" "os" "path/filepath" + "time" "vermeer/apps/auth" ) @@ -59,7 +60,10 @@ func run(args []string, stdout, stderr io.Writer) error { if *user == "" { return errors.New("user must not be empty") } - var factory *auth.TokenFactory + tokenObj := &auth.Token{User: *user, Space: *space, Client: *client, Time: time.Now().Unix()} + if *immortal { + tokenObj.Time = math.MaxInt64 + } if *factorFile != "" { factorSpecified := false flags.Visit(func(f *flag.Flag) { factorSpecified = factorSpecified || f.Name == "factor" }) @@ -70,19 +74,15 @@ func run(args []string, stdout, stderr io.Writer) error { if err != nil { return err } - factory, err = auth.NewSecureTokenFactory(signingKey) + secureFactory, err := auth.NewSecureTokenFactory(signingKey) if err != nil { return err } + secureFactory.Sign(tokenObj) } else { - factory = &auth.TokenFactory{Factor: *factor} - } - - tokenObj := factory.NewToken(*user, *space, *client) - if *immortal { - tokenObj.Time = math.MaxInt64 + legacyFactory := &auth.TokenFactory{Factor: *factor} + legacyFactory.Sign(tokenObj) } - factory.Sign(tokenObj) data, err := json.Marshal(tokenObj) if err != nil { return errors.New("encode token") From 5825f4b78cf01cef1a2d4c6217b1ab58ead64fc1 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 05:05:08 +0000 Subject: [PATCH 06/17] fix(vermeer): separate deployment token CLI from legacy tooling --- .github/workflows/vermeer-ci.yml | 9 +- vermeer/Dockerfile | 2 +- vermeer/README.md | 11 +- vermeer/apps/auth/secure_token.go | 6 +- vermeer/apps/auth/token_v2_test.go | 13 ++ vermeer/cmd/vermeer-token/main.go | 112 ++++++++++++++++++ .../vermeer-token/main_test.go} | 22 +++- vermeer/tools/token/token_generator.go | 110 ++++------------- 8 files changed, 186 insertions(+), 99 deletions(-) create mode 100644 vermeer/cmd/vermeer-token/main.go rename vermeer/{tools/token/token_generator_test.go => cmd/vermeer-token/main_test.go} (88%) diff --git a/.github/workflows/vermeer-ci.yml b/.github/workflows/vermeer-ci.yml index 1224f8dda..f5d2d8be5 100644 --- a/.github/workflows/vermeer-ci.yml +++ b/.github/workflows/vermeer-ci.yml @@ -74,7 +74,7 @@ jobs: - name: Test deployment credentials and graph initialization failures run: | - go test -race ./apps/auth ./tools/token + go test -race ./apps/auth ./cmd/vermeer-token go test -race ./apps/common go test -race ./apps/master/workers ./apps/master/services -run 'Test(WorkerAvailability|NativeLoginCookie)' go test -race ./apps/master/bl -run 'Test(WorkerAvailability|HugegraphLoadAuthorization)' @@ -133,6 +133,13 @@ jobs: test "$(stat -c '%a' "${TOKEN_CHECK_DIR}/token")" = 600 test ! -s "${TOKEN_CHECK_DIR}/stdout" test ! -s "${TOKEN_CHECK_DIR}/stderr" + if docker run --rm --entrypoint /go/bin/vermeer-token vermeer:ci-check \ + -factor synthetic-legacy-factor -quiet \ + > "${TOKEN_CHECK_DIR}/stdout" 2> "${TOKEN_CHECK_DIR}/stderr"; then + echo 'runtime token helper accepted a legacy inline factor' >&2 + exit 1 + fi + test ! -s "${TOKEN_CHECK_DIR}/stdout" if docker run --rm -v "${TOKEN_CHECK_DIR}:/probe" \ --entrypoint /go/bin/vermeer-token vermeer:ci-check \ -factor-file /probe/missing -output-file /probe/rejected -quiet \ diff --git a/vermeer/Dockerfile b/vermeer/Dockerfile index 97766887c..1236eb1af 100644 --- a/vermeer/Dockerfile +++ b/vermeer/Dockerfile @@ -35,7 +35,7 @@ ARG TARGETARCH RUN --mount=type=cache,target=/go/pkg/mod \ --mount=type=cache,target=/root/.cache/go-build \ GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/app && \ - GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/vermeer-token ./tools/token + GOOS=$TARGETOS GOARCH=$TARGETARCH go build -o /go/bin/vermeer-token ./cmd/vermeer-token FROM alpine EXPOSE 8080 diff --git a/vermeer/README.md b/vermeer/README.md index 598cb8427..b74b1ada8 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -266,14 +266,14 @@ it never falls back to an inline or default factor. File-backed factors use **v2 HMAC-SHA256** tokens. Their digest carries a `v2:` version marker inside the existing Base58-encoded token; the signed payload preserves the boundaries of user, space, nickname, client and timestamp fields. Master in file mode accepts only v2 signatures and compares MACs in constant time. It never falls back to legacy SHA1 verification. -The existing `auth_token_factor` string configuration and `-factor` generator option retain their historical token protocol for older clients. +The existing `auth_token_factor` string configuration and source-only legacy token tool retain their historical protocol for older clients. Those legacy tokens and v2 tokens cannot authenticate across modes. Images built from this revision include the native `/go/bin/vermeer-token` helper. Generate a protected login/service token file: ```bash # Run with Go -go run ./tools/token -user admin -space '$DEFAULT' \ +go run ./cmd/vermeer-token -user admin -space '$DEFAULT' \ -factor-file config/token-factor -output-file config/admin-token -i -quiet # Or run in Docker (no local Go required) @@ -285,13 +285,16 @@ docker run --rm -v "$PWD/config:/tokens" --entrypoint /go/bin/vermeer-token \ Parameters: - `-user` — Username (default: `foo`) - `-space` — Graph space name (default: `bar`) -- `-factor-file` — Read the same signing factor file used by Master. -- `-factor` — Legacy inline factor; mutually exclusive with `-factor-file`. Prefer files for deployment secrets. +- `-factor-file` — Required. Read the same signing factor file used by Master; inline factors are not accepted. - `-output-file` — Atomically write the Base58 token with mode `0600`, with no token or factor printed. - `-quiet` — When no output file is set, print only the token for an intentional interactive retrieval. - `-client` — Client identifier (default: `hg`) - `-i` — Generate immortal token (never expires) +The runtime helper supports only the secure file-backed protocol and rejects `-factor`. +The source-only `tools/token/token_generator.go` utility retains its original `-factor` interface and diagnostic output for legacy +inline-configured Masters. It is not distributed in the runtime image and must not be used for file-backed deployments. + Hubble accepts this plain token file through `vermeer.token_file`; it must not include a `Bearer` prefix or JSON. Normal tokens expire after 24 hours. Long-lived service tokens must be protected like passwords. Rotating the signing factor invalidates existing tokens; regenerate dependent service tokens and restart their clients together. diff --git a/vermeer/apps/auth/secure_token.go b/vermeer/apps/auth/secure_token.go index 0bc18d5cc..b73686293 100644 --- a/vermeer/apps/auth/secure_token.go +++ b/vermeer/apps/auth/secure_token.go @@ -49,11 +49,15 @@ func (factory *SecureTokenFactory) Sign(token *Token) { if token == nil { return } + if len(factory.signingKey) == 0 { + token.Digest = "" + return + } token.Digest = secureTokenPrefix + hex.EncodeToString(factory.signature(token)) } func (factory *SecureTokenFactory) Verify(token *Token) bool { - if token == nil { + if token == nil || len(factory.signingKey) == 0 { return false } encoded, ok := strings.CutPrefix(token.Digest, secureTokenPrefix) diff --git a/vermeer/apps/auth/token_v2_test.go b/vermeer/apps/auth/token_v2_test.go index a3053adc9..a9da5f00a 100644 --- a/vermeer/apps/auth/token_v2_test.go +++ b/vermeer/apps/auth/token_v2_test.go @@ -109,3 +109,16 @@ func TestSecureTokenFactoryRejectsInvalidKeys(t *testing.T) { } } } + +func TestZeroValueSecureTokenFactoryFailsClosed(t *testing.T) { + var factory SecureTokenFactory + token := &Token{User: "admin", Space: "$DEFAULT", Client: "zero-value", Time: math.MaxInt64} + token.Digest = secureTokenPrefix + strings.Repeat("0", 64) + if factory.Verify(token) { + t.Fatal("zero-value factory must never authenticate a token") + } + factory.Sign(token) + if token.Digest != "" || factory.Verify(token) { + t.Fatal("zero-value signing must clear the signature and remain unusable") + } +} diff --git a/vermeer/cmd/vermeer-token/main.go b/vermeer/cmd/vermeer-token/main.go new file mode 100644 index 000000000..5357d9766 --- /dev/null +++ b/vermeer/cmd/vermeer-token/main.go @@ -0,0 +1,112 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package main + +import ( + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "math" + "os" + "path/filepath" + "vermeer/apps/auth" +) + +func main() { + if err := run(os.Args[1:], os.Stdout, os.Stderr); err != nil { + if errors.Is(err, flag.ErrHelp) { + return + } + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } +} + +func run(args []string, stdout, stderr io.Writer) error { + flags := flag.NewFlagSet("vermeer-token", flag.ContinueOnError) + flags.SetOutput(stderr) + user := flags.String("user", "foo", "user name") + space := flags.String("space", "bar", "space name") + factorFile := flags.String("factor-file", "", "read the token factor from this file") + outputFile := flags.String("output-file", "", "write only the token to this file with mode 0600") + quiet := flags.Bool("quiet", false, "print only the token when -output-file is not set") + client := flags.String("client", "hg", "client identifier") + immortal := flags.Bool("i", false, "immortal token") + if err := flags.Parse(args); err != nil { + return err + } + if flags.NArg() != 0 { + return errors.New("unexpected positional arguments") + } + if *user == "" { + return errors.New("user must not be empty") + } + if *factorFile == "" { + return errors.New("-factor-file is required for deployment tokens") + } + signingKey, err := auth.ReadTokenFactorFile(*factorFile) + if err != nil { + return err + } + secureFactory, err := auth.NewSecureTokenFactory(signingKey) + if err != nil { + return err + } + tokenObj := secureFactory.NewToken(*user, *space, *client) + if *immortal { + tokenObj.Time = math.MaxInt64 + } + secureFactory.Sign(tokenObj) + data, err := json.Marshal(tokenObj) + if err != nil { + return errors.New("encode token") + } + token := auth.ToBase58(string(data)) + if *outputFile != "" { + return writeTokenFile(*outputFile, token) + } + if *quiet { + _, err = fmt.Fprintln(stdout, token) + return err + } + _, err = fmt.Fprintf(stdout, " user: %s\n space: %s\n client: %s\nimmortal: %t\n token: %s\n", + *user, *space, *client, *immortal, token) + return err +} + +func writeTokenFile(path, token string) error { + file, err := os.CreateTemp(filepath.Dir(path), ".vermeer-token-*") + if err != nil { + return fmt.Errorf("create token output: %w", err) + } + defer os.Remove(file.Name()) + if _, err = file.WriteString(token + "\n"); err != nil { + _ = file.Close() + return fmt.Errorf("write token output: %w", err) + } + if err = file.Close(); err != nil { + return fmt.Errorf("close token output: %w", err) + } + // Rename publishes a complete 0600 file and never follows a destination symlink. + if err = os.Rename(file.Name(), path); err != nil { + return fmt.Errorf("publish token output: %w", err) + } + return nil +} diff --git a/vermeer/tools/token/token_generator_test.go b/vermeer/cmd/vermeer-token/main_test.go similarity index 88% rename from vermeer/tools/token/token_generator_test.go rename to vermeer/cmd/vermeer-token/main_test.go index d25a4e0f5..65acdc95e 100644 --- a/vermeer/tools/token/token_generator_test.go +++ b/vermeer/cmd/vermeer-token/main_test.go @@ -131,14 +131,18 @@ func TestDeploymentTokenNativeAuthenticationAndRotation(t *testing.T) { func TestTokenOutputAndInputFailures(t *testing.T) { var stdout, stderr bytes.Buffer const secret = "synthetic-sensitive-factor" - if err := run([]string{"-factor", secret, "-quiet", "-i"}, &stdout, &stderr); err != nil { + factorFile := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(factorFile, []byte(secret), 0600); err != nil { + t.Fatal(err) + } + if err := run([]string{"-factor-file", factorFile, "-quiet", "-i"}, &stdout, &stderr); err != nil { t.Fatal(err) } if auth.ToTokenBase58(strings.TrimSpace(stdout.String())) == nil || stderr.Len() != 0 { t.Fatal("quiet output must contain only the native token") } stdout.Reset() - if err := run([]string{"-factor", secret}, &stdout, &stderr); err != nil { + if err := run([]string{"-factor-file", factorFile}, &stdout, &stderr); err != nil { t.Fatal(err) } if strings.Contains(stdout.String(), secret) || strings.Contains(stdout.String(), "json:") { @@ -159,10 +163,12 @@ func TestTokenOutputAndInputFailures(t *testing.T) { } } for _, args := range [][]string{ + {}, {"-factor-file", filepath.Join(t.TempDir(), "missing")}, - {"-factor", secret, "-factor-file", "unused"}, - {"-user", ""}, - {"-output-file", filepath.Join(t.TempDir(), "missing", "token")}, + {"-factor", secret}, + {"-factor", secret, "-factor-file", factorFile}, + {"-user", "", "-factor-file", factorFile}, + {"-factor-file", factorFile, "-output-file", filepath.Join(t.TempDir(), "missing", "token")}, } { stdout.Reset() stderr.Reset() @@ -185,8 +191,12 @@ func TestTokenFileReplacesSymlinkWithoutReadingTarget(t *testing.T) { if err := os.Symlink(target, output); err != nil { t.Fatal(err) } + factorFile := filepath.Join(directory, "factor") + if err := os.WriteFile(factorFile, []byte("synthetic-output-file-factor"), 0600); err != nil { + t.Fatal(err) + } var stdout, stderr bytes.Buffer - if err := run([]string{"-output-file", output, "-i"}, &stdout, &stderr); err != nil { + if err := run([]string{"-factor-file", factorFile, "-output-file", output, "-i"}, &stdout, &stderr); err != nil { t.Fatal(err) } data, err := os.ReadFile(target) diff --git a/vermeer/tools/token/token_generator.go b/vermeer/tools/token/token_generator.go index b148afd85..f12dc8941 100644 --- a/vermeer/tools/token/token_generator.go +++ b/vermeer/tools/token/token_generator.go @@ -19,103 +19,41 @@ package main import ( "encoding/json" - "errors" "flag" - "fmt" - "io" "math" - "os" - "path/filepath" - "time" "vermeer/apps/auth" ) func main() { - if err := run(os.Args[1:], os.Stdout, os.Stderr); err != nil { - if errors.Is(err, flag.ErrHelp) { - return - } - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } -} + user := flag.String("user", "foo", "user name") + space := flag.String("space", "bar", "space name") + factor := flag.String("factor", "1234", "token factor") + client := flag.String("client", "hg", "client identifier") + immortal := flag.Bool("i", false, "immortal token") + flag.Parse() -func run(args []string, stdout, stderr io.Writer) error { - flags := flag.NewFlagSet("vermeer-token", flag.ContinueOnError) - flags.SetOutput(stderr) - user := flags.String("user", "foo", "user name") - space := flags.String("space", "bar", "space name") - factor := flags.String("factor", "1234", "token factor (prefer -factor-file for deployment secrets)") - factorFile := flags.String("factor-file", "", "read the token factor from this file") - outputFile := flags.String("output-file", "", "write only the token to this file with mode 0600") - quiet := flags.Bool("quiet", false, "print only the token when -output-file is not set") - client := flags.String("client", "hg", "client identifier") - immortal := flags.Bool("i", false, "immortal token") - if err := flags.Parse(args); err != nil { - return err - } - if flags.NArg() != 0 { - return errors.New("unexpected positional arguments") - } - if *user == "" { - return errors.New("user must not be empty") - } - tokenObj := &auth.Token{User: *user, Space: *space, Client: *client, Time: time.Now().Unix()} + factory := &auth.TokenFactory{Factor: *factor} + tokenObj := factory.NewToken(*user, *space, *client) if *immortal { tokenObj.Time = math.MaxInt64 } - if *factorFile != "" { - factorSpecified := false - flags.Visit(func(f *flag.Flag) { factorSpecified = factorSpecified || f.Name == "factor" }) - if factorSpecified { - return errors.New("use either -factor or -factor-file") - } - signingKey, err := auth.ReadTokenFactorFile(*factorFile) - if err != nil { - return err - } - secureFactory, err := auth.NewSecureTokenFactory(signingKey) - if err != nil { - return err - } - secureFactory.Sign(tokenObj) - } else { - legacyFactory := &auth.TokenFactory{Factor: *factor} - legacyFactory.Sign(tokenObj) - } - data, err := json.Marshal(tokenObj) - if err != nil { - return errors.New("encode token") - } - token := auth.ToBase58(string(data)) - if *outputFile != "" { - return writeTokenFile(*outputFile, token) - } - if *quiet { - _, err = fmt.Fprintln(stdout, token) - return err - } - _, err = fmt.Fprintf(stdout, " user: %s\n space: %s\n client: %s\nimmortal: %t\n token: %s\n", - *user, *space, *client, *immortal, token) - return err + factory.Sign(tokenObj) + jsonStr := toJson(*tokenObj) + token := auth.ToBase58(jsonStr) + + println(" user:", *user) + println(" space:", *space) + println(" factor:", *factor) + println(" client:", *client) + println("immortal:", *immortal) + println(" json:", jsonStr) + println(" token:", token) } -func writeTokenFile(path, token string) error { - file, err := os.CreateTemp(filepath.Dir(path), ".vermeer-token-*") - if err != nil { - return fmt.Errorf("create token output: %w", err) - } - defer os.Remove(file.Name()) - if _, err = file.WriteString(token + "\n"); err != nil { - _ = file.Close() - return fmt.Errorf("write token output: %w", err) - } - if err = file.Close(); err != nil { - return fmt.Errorf("close token output: %w", err) - } - // Rename publishes a complete 0600 file and never follows a destination symlink. - if err = os.Rename(file.Name(), path); err != nil { - return fmt.Errorf("publish token output: %w", err) +func toJson(d interface{}) string { + if b, err := json.Marshal(d); err == nil { + str := string(b) + return str } - return nil + return "" } From 9e35f354f5c456178dcbe1ae1f8b8054cfa07abe Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 05:14:45 +0000 Subject: [PATCH 07/17] fix(vermeer): bound peer bootstrap and retire ended Worker sessions safely --- vermeer/apps/common/grpc_dial.go | 43 +++++ vermeer/apps/common/grpc_dial_test.go | 58 ++++++ vermeer/apps/master/bl/grpc_handlers.go | 96 ++++++---- vermeer/apps/master/bl/server_manager.go | 2 + vermeer/apps/master/bl/worker_bl.go | 31 +++- .../master/workers/bootstrap_workers_test.go | 169 ++++++++++++++++++ vermeer/apps/master/workers/worker_manager.go | 89 ++++++++- vermeer/apps/master/workers/worker_util.go | 8 +- vermeer/apps/master/workers/workers.go | 7 +- vermeer/apps/worker/service.go | 119 +++++++----- vermeer/apps/worker/service_bootstrap_test.go | 102 +++++++++++ 11 files changed, 629 insertions(+), 95 deletions(-) create mode 100644 vermeer/apps/common/grpc_dial.go create mode 100644 vermeer/apps/common/grpc_dial_test.go create mode 100644 vermeer/apps/master/workers/bootstrap_workers_test.go create mode 100644 vermeer/apps/worker/service_bootstrap_test.go diff --git a/vermeer/apps/common/grpc_dial.go b/vermeer/apps/common/grpc_dial.go new file mode 100644 index 000000000..bd6ab4180 --- /dev/null +++ b/vermeer/apps/common/grpc_dial.go @@ -0,0 +1,43 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package common + +import ( + "context" + "fmt" + "os" + "time" + + "google.golang.org/grpc" +) + +// DialGRPCPeer bounds blocking bootstrap connections without changing stream lifetimes. +func DialGRPCPeer(ctx context.Context, target string, options ...grpc.DialOption) (*grpc.ClientConn, error) { + configured := os.Getenv("VERMEER_GRPC_DIAL_TIMEOUT") + if configured == "" { + configured = fmt.Sprint(GetConfigDefault("grpc_dial_timeout", "5s")) + } + timeout, err := time.ParseDuration(configured) + if err != nil || timeout <= 0 { + return nil, fmt.Errorf("grpc_dial_timeout / VERMEER_GRPC_DIAL_TIMEOUT must be a positive duration") + } + dialCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + options = append(options, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + return grpc.DialContext(dialCtx, target, options...) +} diff --git a/vermeer/apps/common/grpc_dial_test.go b/vermeer/apps/common/grpc_dial_test.go new file mode 100644 index 000000000..a12dc78ee --- /dev/null +++ b/vermeer/apps/common/grpc_dial_test.go @@ -0,0 +1,58 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package common + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +func TestGRPCBootstrapDialHasBoundedFailure(t *testing.T) { + previous := GetConfigDefault("grpc_dial_timeout", "5s") + t.Cleanup(func() { SetConfig("grpc_dial_timeout", previous) }) + SetConfig("grpc_dial_timeout", "1s") + t.Setenv("VERMEER_GRPC_DIAL_TIMEOUT", "40ms") + started := time.Now() + connection, err := DialGRPCPeer(context.Background(), "passthrough:///unreachable", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + <-ctx.Done() + return nil, ctx.Err() + })) + if connection != nil || !errors.Is(err, context.DeadlineExceeded) || time.Since(started) > time.Second { + t.Fatalf("bootstrap dial did not stop at its configured deadline: %v", err) + } +} + +func TestGRPCBootstrapDialRejectsInvalidTimeout(t *testing.T) { + for _, configured := range []string{"bad", "0s", "-1s"} { + t.Run(configured, func(t *testing.T) { + t.Setenv("VERMEER_GRPC_DIAL_TIMEOUT", configured) + connection, err := DialGRPCPeer(context.Background(), "unreachable") + if err == nil || connection != nil { + t.Fatal("invalid timeout must fail before dialing") + } + }) + } +} diff --git a/vermeer/apps/master/bl/grpc_handlers.go b/vermeer/apps/master/bl/grpc_handlers.go index 4b5378c73..f116c8f02 100644 --- a/vermeer/apps/master/bl/grpc_handlers.go +++ b/vermeer/apps/master/bl/grpc_handlers.go @@ -24,6 +24,7 @@ import ( "strings" "sync" "time" + "vermeer/apps/common" "vermeer/apps/compute" "vermeer/apps/graphio" "vermeer/apps/master/schedules" @@ -52,6 +53,9 @@ func (h *ServerHandler) Init() { func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterReq) (*pb.HelloMasterResp, error) { h.locker.Lock() defer h.locker.Unlock() + if err := ctx.Err(); err != nil { + return nil, err + } p, _ := peer.FromContext(ctx) // check worker version, if not match, deny @@ -80,6 +84,10 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR workers := workerMgr.GetAllWorkers() var existWorkerName string for _, worker := range workers { + if workerMgr.IsAbandonedRegistration(worker) { + workerBl.KickOfflineSession(worker) + continue + } if worker.GrpcPeer == ip+":"+port { existWorkerName = worker.Name break @@ -87,9 +95,17 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR } if existWorkerName != "" { //如果worker已经存在，必须等待至grpc recv handler感知后，删除worker - for workerMgr.GetWorker(existWorkerName) != nil { + for existing := workerMgr.GetWorker(existWorkerName); existing != nil; existing = workerMgr.GetWorker(existWorkerName) { + if workerMgr.CanReplaceDisconnectedSession(existing) { + workerBl.KickOfflineSession(existing) + continue + } logrus.Infof("worker %v exist, wait one second", existWorkerName) - time.Sleep(1 * time.Second) + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(time.Second): + } } } @@ -104,6 +120,16 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR logrus.Errorf("failed to add a WorkerClient to the WorkerManager, error: %s", err) return &pb.HelloMasterResp{}, err } + registered := false + defer func() { + if !registered { + workerMgr.RemoveWorkerSession(reqWorker) + if reqWorker.Connection != nil { + _ = reqWorker.Connection.Close() + } + } + }() + _, err = Scheduler.ChangeWorkerStatus(reqWorker.Name, schedules.WorkerOngoingStatusIdle) if err != nil { logrus.Errorf("failed to change worker status to idle, error: %s", err) @@ -117,27 +143,14 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR WorkerName: reqWorker.Name, } - for _, v := range workers { - if v.GrpcPeer == ip+":"+port { - logrus.Debugf("workerMgr has a duplicate address, ignore it") - continue - } - workerInfo := pb.WorkerInfo{ - Name: v.Name, - Id: v.Id, - GrpcPeer: v.GrpcPeer, - } - resp.Workers = append(resp.Workers, &workerInfo) - } - // connect to client workerClient := workerMgr.GetWorker(reqWorker.Name) dialOptions := grpc.WithDefaultCallOptions( grpc.MaxCallSendMsgSize(1*1024*1024*1024), grpc.MaxCallRecvMsgSize(1*1024*1024*1024)) - connection, err := grpc.Dial( - ip+":"+port, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + connection, err := common.DialGRPCPeer(ctx, + ip+":"+port, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { logrus.Errorf("connect worker error: %s", err) return &resp, err @@ -149,7 +162,10 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR resp.Spaces = append(resp.Spaces, space.Name()) } - initWorkerClient(workerClient) + initWorkerClient(ctx, workerClient) + if err := ctx.Err(); err != nil { + return nil, err + } graphs := graphMgr.GetGraphsByWorker(reqWorker.Name) taskCanRecovery := make([]*structure.TaskInfo, 0) @@ -195,13 +211,27 @@ func (h *ServerHandler) SayHelloMaster(ctx context.Context, req *pb.HelloMasterR logrus.Infof("worker:%v recovery task:%v type:%v ", workerClient.Name, task.ID, task.Type) } }() + for _, v := range workerMgr.GetBootstrapWorkers() { + if v.GrpcPeer == ip+":"+port { + logrus.Debugf("workerMgr has a duplicate address, ignore it") + continue + } + workerInfo := pb.WorkerInfo{ + Name: v.Name, + Id: v.Id, + GrpcPeer: v.GrpcPeer, + } + resp.Workers = append(resp.Workers, &workerInfo) + } + + registered = true return &resp, nil } -func initWorkerClient(workerClient *workers.WorkerClient) { +func initWorkerClient(ctx context.Context, workerClient *workers.WorkerClient) { logrus.Infof("init worker: %s@%s", workerClient.Name, workerClient.GrpcPeer) // init memory limit - err := workers.SendMemLimit(workerClient, &workers.WorkerMemLimit{ + err := workers.SendMemLimitContext(ctx, workerClient, &workers.WorkerMemLimit{ MaxMem: threshold.GroupMaxMemory(workerClient.Group), MinFree: threshold.GroupMinFree(workerClient.Group), GcRatio: float32(threshold.GroupGcPct(workerClient.Group)) / 100, @@ -382,6 +412,7 @@ func (h *ServerHandler) UploadTPResult(ctx context.Context, in *pb.UploadTPResul type LoadGraphServer struct { streamServer pb.Master_LoadGraphTaskServer + worker *workers.WorkerClient sendMutex sync.Mutex } @@ -391,20 +422,16 @@ func (s *LoadGraphServer) SetServer(stream pb.Master_LoadGraphTaskServer) { func (s *LoadGraphServer) RecvHandler(workerName string) { logrus.Infof("load graph task recv handler setup: %s", workerName) + worker := s.worker for { req, err := s.streamServer.Recv() if err != nil { logrus.Errorf("load graph task recv: %s", err) + workerMgr.ClearTaskStream(worker, workers.LoadTaskStream, s.streamServer.Context()) time.Sleep(3 * time.Second) - if workerBl.KickOffline(workerName) { - break - } - // if !WorkerMgr.CheckWorkerAlive(workerName) { - // WorkerMgr.RemoveWorker(workerName) - // break - // } - continue + workerBl.KickOfflineSession(worker) + return } if req.State == graphio.LoadPartStatusPrepared { } else if req.State == graphio.LoadPartStatusDone { @@ -457,6 +484,7 @@ func (s *LoadGraphServer) AsyncLoad( type ComputeTaskServer struct { streamServer pb.Master_ComputeTaskServer + worker *workers.WorkerClient } func (s *ComputeTaskServer) SetServer(stream pb.Master_ComputeTaskServer) { @@ -465,20 +493,16 @@ func (s *ComputeTaskServer) SetServer(stream pb.Master_ComputeTaskServer) { func (s *ComputeTaskServer) RecvHandler(workerName string) { logrus.Infof("compute task recv handler setup: %s", workerName) + worker := s.worker for { req, err := s.streamServer.Recv() if err != nil { logrus.Errorf("compute task recv error: %s", err) + workerMgr.ClearTaskStream(worker, workers.ComputeTaskStream, s.streamServer.Context()) time.Sleep(3 * time.Second) - if workerBl.KickOffline(workerName) { - break - } - // if !WorkerMgr.CheckWorkerAlive(workerName) { - // WorkerMgr.RemoveWorker(workerName) - // break - // } - continue + workerBl.KickOfflineSession(worker) + return } _ = req } diff --git a/vermeer/apps/master/bl/server_manager.go b/vermeer/apps/master/bl/server_manager.go index e9856c51f..068891c2c 100644 --- a/vermeer/apps/master/bl/server_manager.go +++ b/vermeer/apps/master/bl/server_manager.go @@ -46,6 +46,7 @@ func (s *ServerManager) StartingLoadServer(workerName string, stream pb.Master_L s.locker.Lock() s.loadSevers[workerName] = loadServer worker := workerMgr.GetWorker(workerName) + loadServer.worker = worker workerMgr.SetTaskStream(worker, workers.LoadTaskStream, stream.Context()) s.locker.Unlock() defer func() { @@ -70,6 +71,7 @@ func (s *ServerManager) StartingComputeServer(workerName string, stream pb.Maste s.locker.Lock() s.computeSevers[workerName] = computeServer worker := workerMgr.GetWorker(workerName) + computeServer.worker = worker workerMgr.SetTaskStream(worker, workers.ComputeTaskStream, stream.Context()) s.locker.Unlock() defer func() { diff --git a/vermeer/apps/master/bl/worker_bl.go b/vermeer/apps/master/bl/worker_bl.go index 651e1e043..1d5fe7930 100644 --- a/vermeer/apps/master/bl/worker_bl.go +++ b/vermeer/apps/master/bl/worker_bl.go @@ -21,6 +21,7 @@ import ( "fmt" "sync" . "vermeer/apps/master/graphs" + "vermeer/apps/master/workers" "vermeer/apps/structure" "github.com/sirupsen/logrus" @@ -36,20 +37,28 @@ type WorkerBl struct { // KickOffline Checking the worker state and performing a forced termination if it's offline. // Return false if the worker is alive. func (wb *WorkerBl) KickOffline(workerName string) bool { - if !workerMgr.CheckWorkerAlive(workerName) { - - if err := wb.ReleaseWorker(workerName); err != nil { - logrus.Errorf("failed to perform ReleaseWorker with the name: %s as the worker is offline, caused by: %v", workerName, err) - } + return wb.KickOfflineSession(workerMgr.GetWorker(workerName)) +} - return true +func (wb *WorkerBl) KickOfflineSession(worker *workers.WorkerClient) bool { + if !workerMgr.ClaimOfflineSession(worker) { + return false } - - return false + if worker.Connection != nil { + _ = worker.Connection.Close() + } + if err := wb.releaseWorker(worker.Name, worker); err != nil { + logrus.Errorf("failed to release offline worker %s: %v", worker.Name, err) + } + return true } // ReleaseWorker Release the graph loaded by the worker, cancel the task running in the worker, and remove the worker from the WorkerManager. func (wb *WorkerBl) ReleaseWorker(workerName string) error { + return wb.releaseWorker(workerName, nil) +} + +func (wb *WorkerBl) releaseWorker(workerName string, session *workers.WorkerClient) error { graphs := graphMgr.GetGraphsByWorker(workerName) if len(graphs) == 0 { @@ -78,7 +87,11 @@ func (wb *WorkerBl) ReleaseWorker(workerName string) error { } } - workerMgr.RemoveWorker(workerName) + if session != nil { + workerMgr.RemoveWorkerSession(session) + } else { + workerMgr.RemoveWorker(workerName) + } return nil } diff --git a/vermeer/apps/master/workers/bootstrap_workers_test.go b/vermeer/apps/master/workers/bootstrap_workers_test.go new file mode 100644 index 000000000..85cc23f5b --- /dev/null +++ b/vermeer/apps/master/workers/bootstrap_workers_test.go @@ -0,0 +1,169 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package workers + +import ( + "context" + "errors" + "net" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials/insecure" + "vermeer/apps/common" + "vermeer/apps/storage" +) + +func TestBootstrapExcludesEndedSessionsWithoutDroppingNewPeers(t *testing.T) { + connection := readyWorkerConnection(t) + fresh := &WorkerClient{Name: "fresh", Id: 1} + active := &WorkerClient{Name: "active", Id: 2, Connection: connection, taskStreams: readyTaskStreams()} + ended := &WorkerClient{Name: "ended", Id: 3, Connection: connection, taskStreams: readyTaskStreams()} + canceledCtx, cancel := context.WithCancel(context.Background()) + canceled := &WorkerClient{Name: "canceled", Id: 4, Connection: connection, taskStreams: readyTaskStreams()} + canceled.taskStreams[LoadTaskStream] = canceledCtx + manager := workerManager{workersByName: map[string]*WorkerClient{ + fresh.Name: fresh, active.Name: active, ended.Name: ended, canceled.Name: canceled, + }} + manager.ClearTaskStream(ended, LoadTaskStream, ended.taskStreams[LoadTaskStream]) + cancel() + peers := manager.GetBootstrapWorkers() + if len(peers) != 2 || peers[0] != fresh || peers[1] != active { + t.Fatal("bootstrap must preserve fresh/active peers and omit ended streams even with ready transport") + } + if manager.CheckWorkerAlive(ended.Name) { + t.Fatal("an ended session cannot remain alive") + } +} + +func TestOfflineCleanupCannotTouchReplacementSession(t *testing.T) { + previous := common.PrometheusMetrics.WorkerCnt + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_session_cleanup"}, nil) + t.Cleanup(func() { common.PrometheusMetrics.WorkerCnt = previous }) + connection := readyWorkerConnection(t) + old := &WorkerClient{Name: "reused", Connection: connection, taskStreams: readyTaskStreams()} + manager := workerManager{workersByName: map[string]*WorkerClient{old.Name: old}} + oldCtx := old.taskStreams[LoadTaskStream] + replacementCtx, cancel := context.WithCancel(context.Background()) + defer cancel() + manager.SetTaskStream(old, LoadTaskStream, replacementCtx) + manager.ClearTaskStream(old, LoadTaskStream, oldCtx) + if len(manager.GetBootstrapWorkers()) != 1 { + t.Fatal("old stream EOF must not terminate its replacement") + } + manager.ClearTaskStream(old, LoadTaskStream, replacementCtx) + var claimed atomic.Int32 + var wait sync.WaitGroup + for range 8 { + wait.Add(1) + go func() { + defer wait.Done() + if manager.ClaimOfflineSession(old) { + claimed.Add(1) + } + }() + } + wait.Wait() + if manager.CanReplaceDisconnectedSession(old) { + t.Fatal("an already claimed cleanup must wait rather than be reclaimed") + } + if claimed.Load() != 1 { + t.Fatal("only one EOF handler may clean up a session") + } + newer := &WorkerClient{Name: old.Name, Connection: connection, taskStreams: readyTaskStreams()} + manager.workersByName[old.Name] = newer + manager.RemoveWorkerSession(old) + manager.ClearTaskStream(old, LoadTaskStream, replacementCtx) + if manager.GetWorker(newer.Name) != newer || !manager.IsWorkerReady(newer) { + t.Fatal("late cleanup or EOF must not remove or terminate a new registration") + } +} + +func TestAbandonedRegistrationCanRegisterAgain(t *testing.T) { + previous := common.PrometheusMetrics.WorkerCnt + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_abandoned_registration"}, nil) + t.Cleanup(func() { common.PrometheusMetrics.WorkerCnt = previous }) + connection, err := grpc.NewClient("passthrough:///abandoned", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return nil, errors.New("worker gone") })) + if err != nil { + t.Fatal(err) + } + defer connection.Close() + connection.Connect() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + for connection.GetState() != connectivity.TransientFailure { + state := connection.GetState() + if !connection.WaitForStateChange(ctx, state) { + t.Fatal("expected a disconnected reverse transport") + } + } + store, err := storage.NewPebbleStore(storage.StoreOption{Path: t.TempDir()}) + if err != nil { + t.Fatal(err) + } + defer store.Close() + manager := workerManager{workersByName: map[string]*WorkerClient{}, idSeed: 1, store: store} + old := &WorkerClient{Name: "persisted-name", GrpcPeer: "127.0.0.1:6789", Connection: connection} + if _, err := manager.AddWorker(old); err != nil { + t.Fatal(err) + } + if !manager.IsAbandonedRegistration(old) || !manager.CanReplaceDisconnectedSession(old) { + t.Fatal("disconnected registration without a lifeline must be reclaimable") + } + fresh := &WorkerClient{Name: "new"} + manager.workersByName[fresh.Name] = fresh + if manager.IsAbandonedRegistration(fresh) || manager.CanReplaceDisconnectedSession(fresh) { + t.Fatal("an in-progress registration without a reverse connection is not abandoned") + } + active := &WorkerClient{Name: "active", Connection: connection, taskStreams: readyTaskStreams()} + manager.workersByName[active.Name] = active + if manager.IsAbandonedRegistration(active) { + t.Fatal("a peer with live task streams cannot be skipped merely for transient transport failure") + } + oldStream := active.taskStreams[LoadTaskStream] + replacementStream, done := context.WithCancel(context.Background()) + defer done() + manager.SetTaskStream(active, LoadTaskStream, replacementStream) + manager.ClearTaskStream(active, LoadTaskStream, oldStream) + if manager.ClaimOfflineSession(active) || manager.CanReplaceDisconnectedSession(active) { + t.Fatal("an obsolete stream EOF must not claim replacement live streams even during transient transport failure") + } + + if !manager.ClaimOfflineSession(old) { + t.Fatal("old abandoned registration must be claimed exactly once") + } + manager.RemoveWorkerSession(old) + replacement, err := manager.CreateWorker(old.GrpcPeer, "127.0.0.1", "test-version", "$") + if err != nil { + t.Fatal(err) + } + replacement.Connection = readyWorkerConnection(t) + if _, err := manager.AddWorker(replacement); err != nil { + t.Fatalf("same-address re-registration is blocked by a ghost: %v", err) + } + if replacement.Name != old.Name || manager.GetWorker(old.Name) != replacement { + t.Fatal("re-registration must preserve its stored identity with a new session") + } +} diff --git a/vermeer/apps/master/workers/worker_manager.go b/vermeer/apps/master/workers/worker_manager.go index 53773c3fc..52825a1b5 100644 --- a/vermeer/apps/master/workers/worker_manager.go +++ b/vermeer/apps/master/workers/worker_manager.go @@ -64,9 +64,11 @@ type WorkerClient struct { //LoadStream pb.Master_LoadGraphTaskServer `json:"-"` //ComputeStream pb.Master_ComputeTaskServer `json:"-"` //SuperStepStream pb.Master_SuperStepServer `json:"-"` - Connection *grpc.ClientConn `json:"-"` - Session pb.WorkerClient `json:"-"` - taskStreams [taskStreamCount]context.Context + Connection *grpc.ClientConn `json:"-"` + Session pb.WorkerClient `json:"-"` + taskStreams [taskStreamCount]context.Context + taskStreamsEnded [taskStreamCount]bool + offlineCleanup bool } func (wc *WorkerClient) Close() { @@ -233,11 +235,86 @@ func (wm *workerManager) GetAllWorkers() []*WorkerClient { return SortWorkersAsc(map2Workers(wm.workersByName)) } +// GetBootstrapWorkers retains newly registering peers, but excludes terminated sessions. +func (wm *workerManager) GetBootstrapWorkers() []*WorkerClient { + defer wm.Unlock(wm.Lock()) + peers := make([]*WorkerClient, 0, len(wm.workersByName)) + for _, worker := range wm.workersByName { + if !wm.sessionEndedLocked(worker) { + peers = append(peers, worker) + } + } + return SortWorkersAsc(peers) +} + +func (wm *workerManager) sessionEndedLocked(worker *WorkerClient) bool { + if worker.offlineCleanup || wm.abandonedRegistrationLocked(worker) || (worker.Connection != nil && worker.Connection.GetState() == connectivity.Shutdown) { + return true + } + for stream, ctx := range worker.taskStreams { + if worker.taskStreamsEnded[stream] || (ctx != nil && ctx.Err() != nil) { + return true + } + } + return false +} + +// CanReplaceDisconnectedSession requires a completed reverse dial and observed disconnection. +func (wm *workerManager) CanReplaceDisconnectedSession(worker *WorkerClient) bool { + defer wm.Unlock(wm.Lock()) + return worker != nil && wm.workersByName[worker.Name] == worker && !worker.offlineCleanup && worker.Connection != nil && + wm.sessionEndedLocked(worker) +} + +// A registration that never opened its lifeline cannot recover after its transport is lost. +func (wm *workerManager) IsAbandonedRegistration(worker *WorkerClient) bool { + defer wm.Unlock(wm.Lock()) + return worker != nil && wm.workersByName[worker.Name] == worker && wm.abandonedRegistrationLocked(worker) +} + +func (wm *workerManager) abandonedRegistrationLocked(worker *WorkerClient) bool { + if worker.Connection == nil { + return false + } + state := worker.Connection.GetState() + if state != connectivity.TransientFailure && state != connectivity.Shutdown { + return false + } + for stream, ctx := range worker.taskStreams { + if ctx != nil || worker.taskStreamsEnded[stream] { + return false + } + } + return true +} + +// ClaimOfflineSession gives only one old stream ownership of its session cleanup. +func (wm *workerManager) ClaimOfflineSession(worker *WorkerClient) bool { + defer wm.Unlock(wm.Lock()) + if worker == nil || wm.workersByName[worker.Name] != worker || worker.offlineCleanup { + return false + } + if !wm.sessionEndedLocked(worker) { + return false + } + worker.offlineCleanup = true + return true +} + +// RemoveWorkerSession cannot remove a new registration that reuses the same name. +func (wm *workerManager) RemoveWorkerSession(worker *WorkerClient) { + defer wm.Unlock(wm.Lock()) + if worker != nil && wm.workersByName[worker.Name] == worker { + delete(wm.workersByName, worker.Name) + common.PrometheusMetrics.WorkerCnt.WithLabelValues().Dec() + } +} + func (wm *workerManager) CheckWorkerAlive(name string) bool { defer wm.Unlock(wm.Lock()) worker, ok := wm.workersByName[name] - if !ok || worker.Connection == nil || worker.Connection.GetState() != connectivity.Ready { + if !ok || wm.sessionEndedLocked(worker) || worker.Connection == nil || worker.Connection.GetState() != connectivity.Ready { return false } @@ -255,6 +332,7 @@ func (wm *workerManager) SetTaskStream(worker *WorkerClient, stream TaskStream, defer wm.Unlock(wm.Lock()) if worker != nil && wm.workersByName[worker.Name] == worker { worker.taskStreams[stream] = ctx + worker.taskStreamsEnded[stream] = false } } @@ -262,6 +340,7 @@ func (wm *workerManager) ClearTaskStream(worker *WorkerClient, stream TaskStream defer wm.Unlock(wm.Lock()) if worker != nil && wm.workersByName[worker.Name] == worker && worker.taskStreams[stream] == ctx { worker.taskStreams[stream] = nil + worker.taskStreamsEnded[stream] = true } } @@ -271,7 +350,7 @@ func (wm *workerManager) IsWorkerReady(worker *WorkerClient) bool { } func (wm *workerManager) workerReadyLocked(worker *WorkerClient) bool { - if worker == nil || wm.workersByName[worker.Name] != worker || + if worker == nil || wm.workersByName[worker.Name] != worker || worker.offlineCleanup || worker.Connection == nil || worker.Connection.GetState() != connectivity.Ready { return false } diff --git a/vermeer/apps/master/workers/worker_util.go b/vermeer/apps/master/workers/worker_util.go index 6b0fa3ed9..eb507c2d4 100644 --- a/vermeer/apps/master/workers/worker_util.go +++ b/vermeer/apps/master/workers/worker_util.go @@ -82,7 +82,11 @@ func toMemoryLimitReq(limit *WorkerMemLimit) *pb.SetMemoryLimitReq { } -func doSendMemoryLimitReq(worker *WorkerClient, req *pb.SetMemoryLimitReq) (resp *pb.SetMemoryLimitResp, err error) { +func doSendMemoryLimitReq(worker *WorkerClient, req *pb.SetMemoryLimitReq) (*pb.SetMemoryLimitResp, error) { + return doSendMemoryLimitReqContext(context.Background(), worker, req) +} + +func doSendMemoryLimitReqContext(ctx context.Context, worker *WorkerClient, req *pb.SetMemoryLimitReq) (resp *pb.SetMemoryLimitResp, err error) { if worker == nil || req == nil { return nil, nil } @@ -98,7 +102,7 @@ func doSendMemoryLimitReq(worker *WorkerClient, req *pb.SetMemoryLimitReq) (resp } var response *pb.RuntimeActionResp - if response, err = worker.Session.RuntimeAction(context.Background(), request); err != nil { + if response, err = worker.Session.RuntimeAction(ctx, request); err != nil { return nil, err } diff --git a/vermeer/apps/master/workers/workers.go b/vermeer/apps/master/workers/workers.go index 963167040..dd2bcc438 100644 --- a/vermeer/apps/master/workers/workers.go +++ b/vermeer/apps/master/workers/workers.go @@ -18,6 +18,7 @@ under the License. package workers import ( + "context" "errors" "fmt" @@ -29,6 +30,10 @@ var WorkerManager = &workerManager{} var manager = WorkerManager func SendMemLimit(worker *WorkerClient, memLimit *WorkerMemLimit) error { + return SendMemLimitContext(context.Background(), worker, memLimit) +} + +func SendMemLimitContext(ctx context.Context, worker *WorkerClient, memLimit *WorkerMemLimit) error { if worker == nil { return errors.New("the argument `worker` cannot be nil") } @@ -36,7 +41,7 @@ func SendMemLimit(worker *WorkerClient, memLimit *WorkerMemLimit) error { return errors.New("the argument `memLimit` cannot be empty") } - if _, err := doSendMemoryLimitReq(worker, toMemoryLimitReq(memLimit)); err != nil { + if _, err := doSendMemoryLimitReqContext(ctx, worker, toMemoryLimitReq(memLimit)); err != nil { return fmt.Errorf("failed to send memory limit req to worker %s: %w", worker.Name, err) } logrus.Infof("sent memory limit '%s' via group '%s' to worker %s@%s", memLimit.toString(), worker.Group, worker.Name, worker.GrpcPeer) diff --git a/vermeer/apps/worker/service.go b/vermeer/apps/worker/service.go index 0f0ef7408..e8b599f2c 100644 --- a/vermeer/apps/worker/service.go +++ b/vermeer/apps/worker/service.go @@ -19,6 +19,7 @@ package worker import ( "context" + "fmt" "sync" "time" @@ -57,8 +58,19 @@ type Service struct { WorkerId int32 } -func (s *Service) Init() error { - var err error +func (s *Service) Init() (err error) { + defer func() { + if err != nil { + if s.masterConn != nil { + _ = s.masterConn.Close() + } + for _, peer := range PeerMgr.GetAllWorkers() { + if peer.peerConn != nil { + _ = peer.peerConn.Close() + } + } + } + }() //node, err := snowflake.NewNode(rand.Int63n(1023)) //if err != nil { // logrus.Errorf("new snowflake error: %s", err) @@ -77,8 +89,8 @@ func (s *Service) Init() error { grpc.MaxCallSendMsgSize(4*1024*1024*1024), grpc.MaxCallRecvMsgSize(4*1024*1024*1024)) - s.masterConn, err = grpc.Dial( - masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + s.masterConn, err = common.DialGRPCPeer(context.Background(), + masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { logrus.Errorf("connect master error: %s", err) return err @@ -97,28 +109,36 @@ func (s *Service) Init() error { logrus.Infof("Hello World, I am worker %s", s.WorkerName) PeerMgr.AddPeer(s.WorkerName, s.WorkerId, grpcPeer) md := metadata.New(map[string]string{"worker_name": s.WorkerName}) + // Keep registration tied to a live stream even if peer bootstrap fails. + // Compute/SuperStep stay unopened until the entire peer mesh is installed. + loadCtx := metadata.NewOutgoingContext(context.Background(), md) + s.LoadGraphHandler.grpcStream, err = s.MasterClient.LoadGraphTask(loadCtx) + if err != nil { + return fmt.Errorf("open registration lifeline: %w", err) + } for _, w := range resp.GetWorkers() { PeerMgr.AddPeer(w.Name, w.Id, w.GrpcPeer) peerClient := PeerMgr.GetPeer(w.Name) - peerClient.peerConn, err = grpc.Dial( - w.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + peerClient.peerConn, err = common.DialGRPCPeer(context.Background(), + w.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { - logrus.Fatalf("connect to peer %s error: %s", w.GrpcPeer, err) - return err + return fmt.Errorf("connect to peer %s: %w", w.GrpcPeer, err) } peerClient.peerSession = pb.NewWorkerClient(peerClient.peerConn) ctx, cancel2 := context.WithTimeout(context.Background(), 5*time.Second) resp, err := peerClient.peerSession.SayHelloPeer( ctx, &pb.HelloPeerReq{SourceName: s.WorkerName, TargetName: w.Name, WorkerPeer: grpcPeer}) if err != nil { - logrus.Fatalf("say hello peer %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } // install scatter ctx = metadata.NewOutgoingContext(context.Background(), md) stream, err := peerClient.peerSession.Scatter(ctx) if err != nil { - logrus.Fatalf("create scatter stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } peerClient.ScatterHandler.SetClient(stream) @@ -126,7 +146,8 @@ func (s *Service) Init() error { ctx = metadata.NewOutgoingContext(context.Background(), md) stream2, err := peerClient.peerSession.LoadAction(ctx) if err != nil { - logrus.Fatalf("create load action stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } peerClient.LoadActionHandler.SetClient(stream2) @@ -134,7 +155,8 @@ func (s *Service) Init() error { ctx = metadata.NewOutgoingContext(context.Background(), md) stream3, err := peerClient.peerSession.StepEnd(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } peerClient.StepEndHandler.SetClient(stream3) @@ -142,7 +164,8 @@ func (s *Service) Init() error { ctx = metadata.NewOutgoingContext(context.Background(), md) stream4, err := peerClient.peerSession.SettingAction(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", w.GrpcPeer, err) + cancel2() + return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } peerClient.SettingActionHandler.SetClient(stream4) @@ -151,14 +174,6 @@ func (s *Service) Init() error { } logrus.Infof("say hello master ok, worker id: %d", s.WorkerId) - ctx = metadata.NewOutgoingContext(context.Background(), md) - s.LoadGraphHandler.grpcStream, err = s.MasterClient.LoadGraphTask(ctx) - if err != nil { - logrus.Errorf("create load graph task stream error: %s", err) - return err - } - logrus.Infof("create load graph task stream ok") - ctx = metadata.NewOutgoingContext(context.Background(), md) s.ComputeTaskHandler.grpcStream, err = s.MasterClient.ComputeTask(ctx) if err != nil { @@ -232,7 +247,9 @@ func (s *Service) CheckMasterAlive() bool { if s.masterConn == nil || s.masterConn.GetState() != connectivity.Ready { return false } - return true + return s.LoadGraphHandler != nil && s.LoadGraphHandler.grpcStream != nil && s.LoadGraphHandler.grpcStream.Context().Err() == nil && + s.ComputeTaskHandler != nil && s.ComputeTaskHandler.grpcStream != nil && s.ComputeTaskHandler.grpcStream.Context().Err() == nil && + s.SuperStepHandler != nil && s.SuperStepHandler.grpcStream != nil && s.SuperStepHandler.grpcStream.Context().Err() == nil } func (s *Service) ReconnectMaster() { @@ -242,6 +259,20 @@ func (s *Service) ReconnectMaster() { logrus.Infof("master is alive:%v", s.masterConn.GetState()) return } + bootstrapped := false + defer func() { + if !bootstrapped { + if s.masterConn != nil { + _ = s.masterConn.Close() + s.masterConn = nil + } + for _, peer := range PeerMgr.GetAllWorkers() { + if peer.peerConn != nil { + _ = peer.peerConn.Close() + } + } + } + }() logrus.Infof("try to reconnect master") //初始化 SpaceMgr.Init() @@ -262,8 +293,8 @@ func (s *Service) ReconnectMaster() { grpc.MaxCallRecvMsgSize(4*1024*1024*1024), ) var err error - s.masterConn, err = grpc.Dial( - masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + s.masterConn, err = common.DialGRPCPeer(context.Background(), + masterPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { logrus.Errorf("connect master error: %s", err) return @@ -275,7 +306,6 @@ func (s *Service) ReconnectMaster() { ctx, &pb.HelloMasterReq{WorkerPeer: grpcPeer, Version: version.Version}) if err != nil { logrus.Infof("reconnect say hello master error:%v", err) - s.masterConn = nil return } s.WorkerId = resp.GetWorkerId() @@ -283,6 +313,13 @@ func (s *Service) ReconnectMaster() { logrus.Infof("Reconnect, I am worker %s", s.WorkerName) PeerMgr.AddPeer(s.WorkerName, s.WorkerId, grpcPeer) md := metadata.New(map[string]string{"worker_name": s.WorkerName}) + loadCtx := metadata.NewOutgoingContext(context.Background(), md) + tempLoadGraphGrpcStream, err := s.MasterClient.LoadGraphTask(loadCtx) + if err != nil { + logrus.Errorf("reconnect registration lifeline: %v", err) + return + } + s.LoadGraphHandler.grpcStream = tempLoadGraphGrpcStream for _, workerInfo := range resp.GetWorkers() { //对已建立grpc连接的peer忽略。建立未知peer的grpc连接，并启动handler peer := PeerMgr.GetPeer(workerInfo.GetName()) @@ -291,25 +328,28 @@ func (s *Service) ReconnectMaster() { } PeerMgr.AddPeer(workerInfo.Name, workerInfo.Id, workerInfo.GrpcPeer) peerClient := PeerMgr.GetPeer(workerInfo.Name) - peerClient.peerConn, err = grpc.Dial( - workerInfo.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions, grpc.WithBlock(), grpc.WithIdleTimeout(0)) + peerClient.peerConn, err = common.DialGRPCPeer(context.Background(), + workerInfo.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { - logrus.Fatalf("connect to peer %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("connect to peer %s error: %s", workerInfo.GrpcPeer, err) return } peerClient.peerSession = pb.NewWorkerClient(peerClient.peerConn) ctx, cancel2 := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel2() resp, err := peerClient.peerSession.SayHelloPeer( ctx, &pb.HelloPeerReq{SourceName: s.WorkerName, TargetName: workerInfo.Name, WorkerPeer: grpcPeer}) if err != nil { - logrus.Fatalf("say hello peer %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("say hello peer %s error: %s", workerInfo.GrpcPeer, err) + return } // install scatter ctx = metadata.NewOutgoingContext(context.Background(), md) stream, err := peerClient.peerSession.Scatter(ctx) if err != nil { - logrus.Fatalf("create scatter stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create scatter stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return } peerClient.ScatterHandler.SetClient(stream) @@ -317,7 +357,8 @@ func (s *Service) ReconnectMaster() { ctx = metadata.NewOutgoingContext(context.Background(), md) stream2, err := peerClient.peerSession.LoadAction(ctx) if err != nil { - logrus.Fatalf("create load action stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create load action stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return } peerClient.LoadActionHandler.SetClient(stream2) @@ -325,7 +366,8 @@ func (s *Service) ReconnectMaster() { ctx = metadata.NewOutgoingContext(context.Background(), md) stream3, err := peerClient.peerSession.StepEnd(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return } peerClient.StepEndHandler.SetClient(stream3) @@ -333,7 +375,8 @@ func (s *Service) ReconnectMaster() { ctx = metadata.NewOutgoingContext(context.Background(), md) stream4, err := peerClient.peerSession.SettingAction(ctx) if err != nil { - logrus.Fatalf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + logrus.Errorf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) + return } peerClient.SettingActionHandler.SetClient(stream4) @@ -353,15 +396,6 @@ func (s *Service) ReconnectMaster() { } } - ctx = metadata.NewOutgoingContext(context.Background(), md) - tempLoadGraphGrpcStream, err := s.MasterClient.LoadGraphTask(ctx) - if err != nil { - logrus.Errorf("reconnect create load graph task stream error: %s", err) - return - } - s.LoadGraphHandler.grpcStream = tempLoadGraphGrpcStream - logrus.Infof("reconnect create load graph task stream ok") - ctx = metadata.NewOutgoingContext(context.Background(), md) tempComputeGrpcStream, err := s.MasterClient.ComputeTask(ctx) if err != nil { @@ -380,4 +414,5 @@ func (s *Service) ReconnectMaster() { } s.SuperStepHandler.grpcStream = tempSuperStepGrpcStream logrus.Infof("reconnect create super step stream ok") + bootstrapped = true } diff --git a/vermeer/apps/worker/service_bootstrap_test.go b/vermeer/apps/worker/service_bootstrap_test.go new file mode 100644 index 000000000..022f17fa0 --- /dev/null +++ b/vermeer/apps/worker/service_bootstrap_test.go @@ -0,0 +1,102 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package worker + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" + "vermeer/apps/common" + pb "vermeer/apps/protos" +) + +type unavailableBootstrapMaster struct { + pb.UnimplementedMasterServer + peer string + lifelineClosed chan struct{} +} + +func (m *unavailableBootstrapMaster) SayHelloMaster(context.Context, *pb.HelloMasterReq) (*pb.HelloMasterResp, error) { + return &pb.HelloMasterResp{WorkerId: 1, WorkerName: "joining", Workers: []*pb.WorkerInfo{{Name: "stale", Id: 2, GrpcPeer: m.peer}}}, nil +} +func (m *unavailableBootstrapMaster) LoadGraphTask(stream pb.Master_LoadGraphTaskServer) error { + <-stream.Context().Done() + close(m.lifelineClosed) + return nil +} + +func TestWorkerBootstrapFailureReturnsAndClosesRegistrationLifeline(t *testing.T) { + previousMaster := common.GetConfig("master_peer") + previousPeer := common.GetConfig("grpc_peer") + previousMetrics := common.PrometheusMetrics.WorkerCnt + t.Cleanup(func() { + common.SetConfig("master_peer", previousMaster) + common.SetConfig("grpc_peer", previousPeer) + common.PrometheusMetrics.WorkerCnt = previousMetrics + }) + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_bootstrap_workers"}, nil) + t.Setenv("VERMEER_GRPC_DIAL_TIMEOUT", "400ms") + blackhole, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer blackhole.Close() + go func() { + conn, err := blackhole.Accept() + if err == nil { + defer conn.Close() + buffer := make([]byte, 4096) + for { + if _, err = conn.Read(buffer); err != nil { + return + } + } + } + }() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + master := &unavailableBootstrapMaster{peer: blackhole.Addr().String(), lifelineClosed: make(chan struct{})} + server := grpc.NewServer() + pb.RegisterMasterServer(server, master) + go server.Serve(listener) + defer server.Stop() + common.SetConfig("master_peer", listener.Addr().String()) + common.SetConfig("grpc_peer", "127.0.0.1:6789") + service := new(Service) + started := time.Now() + err = service.Init() + if !errors.Is(err, context.DeadlineExceeded) || time.Since(started) > 3*time.Second { + t.Fatalf("stale peer must fail bounded Init: %v", err) + } + if service.masterConn.GetState() != connectivity.Shutdown { + t.Fatal("failed bootstrap must close master transport") + } + select { + case <-master.lifelineClosed: + case <-time.After(time.Second): + t.Fatal("failed bootstrap must close its registration lifeline") + } +} From ff405ff427c1b5eb9b0bca88b2fed842f1598346 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 05:28:21 +0000 Subject: [PATCH 08/17] feat(vermeer): support short administrator credentials for native UI login --- .github/workflows/vermeer-ci.yml | 2 +- vermeer/README.md | 25 ++ vermeer/apps/auth/admin_password.go | 95 +++++++ vermeer/apps/auth/admin_password_test.go | 159 +++++++++++ vermeer/apps/auth/filter.go | 35 ++- vermeer/apps/master/master_main.go | 3 + vermeer/apps/master/services/http_login.go | 37 +-- .../services/http_login_password_test.go | 261 ++++++++++++++++++ .../master/services/http_login_v2_test.go | 34 +++ vermeer/apps/master/services/router.go | 4 +- vermeer/ui/ui/lib/functions.js | 2 +- vermeer/ui/ui/master.html | 5 +- 12 files changed, 628 insertions(+), 34 deletions(-) create mode 100644 vermeer/apps/auth/admin_password.go create mode 100644 vermeer/apps/auth/admin_password_test.go create mode 100644 vermeer/apps/master/services/http_login_password_test.go diff --git a/.github/workflows/vermeer-ci.yml b/.github/workflows/vermeer-ci.yml index f5d2d8be5..813c21efd 100644 --- a/.github/workflows/vermeer-ci.yml +++ b/.github/workflows/vermeer-ci.yml @@ -76,7 +76,7 @@ jobs: run: | go test -race ./apps/auth ./cmd/vermeer-token go test -race ./apps/common - go test -race ./apps/master/workers ./apps/master/services -run 'Test(WorkerAvailability|NativeLoginCookie)' + go test -race ./apps/master/workers ./apps/master/services -run 'Test(WorkerAvailability|Native)' go test -race ./apps/master/bl -run 'Test(WorkerAvailability|HugegraphLoadAuthorization)' go test -race ./apps/worker -run 'Test(LoadReceivers|RecvVertex|ComputeDispatchCancellation)' go test -race ./apps/structure -run 'Test(VertexStore|GraphPropagates|MemoryVertex|HugegraphSchema)' diff --git a/vermeer/README.md b/vermeer/README.md index b74b1ada8..b2a9d0d72 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -308,6 +308,31 @@ The generated token can be used in two ways: 1. **HTTP Header**: `Authorization: Bearer ` 2. **Login API**: `POST /api/v1/login` with body `{"token": ""}` (sets cookie) +### Native UI administrator password + +For a short administrator login, optionally configure Master with `VERMEER_ADMIN_PASSWORD_FILE=/path/to/admin-password`, +or `admin_password_file` in its INI file. The environment path takes precedence. This requires `auth=token` and a valid +file-backed v2 signing factor; legacy inline signing and `auth=none` reject the password configuration before listening. +An unconfigured password file preserves token-only login. + +Provide a protected `0600` password file, preferably a separately generated random 16-character alphanumeric credential. +Helm deployments generate and retain this credential separately from the internal signing factor and Hubble service token. +The file must be nonempty UTF-8 on one line, without CR/LF, and cannot contain only whitespace. All other bytes are preserved, +including leading or trailing spaces. A configured missing, unreadable or invalid file stops Master; it never disables authentication. + +Open `/ui/master.html` and paste this credential into **Admin password or token**. No user database or extra account is created: +successful password login issues an `admin` session in `$DEFAULT`, signed with v2 HMAC-SHA256 and valid for 24 hours. +Its cookie is HttpOnly and uses Secure for HTTPS, including requests forwarded with `X-Forwarded-Proto: https`. +The existing login request field remains `token`; it accepts either the short password or a valid opaque token. +Incorrect credentials return 401 without issuing a cookie. The short password is never accepted as an API Bearer token. +Hubble continues to use its independently generated long-lived service token without requiring user input. + +Password changes take effect after restarting Master. They change subsequent UI logins without rotating the signing factor +or Hubble token. Already issued UI sessions remain valid for their remaining 24-hour lifetime; rotate the signing factor +and regenerate dependent tokens when immediate session revocation is required. +With token authentication, Worker/Master metadata routes also require a valid session or Bearer token; +health probes, login and static login assets remain public. Explicit `auth=none` retains its existing behavior. + ### Worker Configuration (`worker.ini`) For a PD deployment using a shared secret, configure both Master and Worker with diff --git a/vermeer/apps/auth/admin_password.go b/vermeer/apps/auth/admin_password.go new file mode 100644 index 000000000..70c970b33 --- /dev/null +++ b/vermeer/apps/auth/admin_password.go @@ -0,0 +1,95 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package auth + +import ( + "crypto/subtle" + "encoding/json" + "errors" + "fmt" + "os" + "strings" + "unicode/utf8" + "vermeer/apps/common" + "vermeer/apps/structure" +) + +// AdminSessionMaxAge is the lifetime of a native UI password session in seconds. +const AdminSessionMaxAge = tokenMaxAge + +var adminPassword []byte + +func adminPasswordFile() string { + if file := os.Getenv("VERMEER_ADMIN_PASSWORD_FILE"); file != "" { + return file + } + file, _ := common.GetConfigDefault("admin_password_file", "").(string) + return file +} + +// ValidateAdminPasswordMode rejects a configured UI password when authentication is disabled. +func ValidateAdminPasswordMode(authType string) error { + if adminPasswordFile() != "" && authType != "token" { + return errors.New("admin password file requires file-backed v2 token authentication") + } + return nil +} + +func readAdminPasswordFile(file string) ([]byte, error) { + data, err := os.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("read admin password file: %w", err) + } + if len(data) == 0 || !utf8.Valid(data) || strings.ContainsAny(string(data), "\r\n") || + strings.TrimSpace(string(data)) == "" { + return nil, errors.New("admin password file must contain a nonempty single-line UTF-8 credential") + } + return data, nil +} + +// ExchangeAdminPassword authenticates the UI credential without exposing it as a Bearer token. +// The resulting session is an ordinary expiring v2 administrator token. +func ExchangeAdminPassword(credential string) (string, bool) { + if secureFactory == nil || len(adminPassword) == 0 || + subtle.ConstantTimeCompare([]byte(credential), adminPassword) != 1 { + return "", false + } + token := secureFactory.NewToken(structure.AdminName, structure.DefaultSpaceName, "native-ui") + secureFactory.Sign(token) + data, err := json.Marshal(token) + if err != nil { + return "", false + } + return ToBase58(string(data)), true +} + +func validSignature(token *Token) bool { + if secureFactory != nil { + return secureFactory.Verify(token) + } + return factory != nil && factory.Verify(token) +} + +// ValidLoginToken validates opaque token login before a session cookie is set. +func ValidLoginToken(token *Token) bool { + if token == nil || !validSignature(token) || tokenExpired(token) { + return false + } + _, err := structure.NewCredential(token.User, token.Space) + return err == nil +} diff --git a/vermeer/apps/auth/admin_password_test.go b/vermeer/apps/auth/admin_password_test.go new file mode 100644 index 000000000..ed90738e0 --- /dev/null +++ b/vermeer/apps/auth/admin_password_test.go @@ -0,0 +1,159 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package auth + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + "vermeer/apps/common" +) + +func isolateAdminPasswordConfig(t *testing.T) { + t.Helper() + previousFactory, previousSecure, previousPassword := factory, secureFactory, adminPassword + t.Cleanup(func() { factory, secureFactory, adminPassword = previousFactory, previousSecure, previousPassword }) + for _, key := range []string{"auth_token_factor", "auth_token_factor_file", "admin_password_file"} { + previous := common.GetConfigDefault(key, "") + common.SetConfig(key, "") + t.Cleanup(func() { common.SetConfig(key, previous) }) + } + for _, name := range []string{"VERMEER_AUTH_TOKEN_FACTOR_FILE", "VERMEER_ADMIN_PASSWORD_FILE"} { + t.Setenv(name, "") + } +} + +func writeAdminFixture(t *testing.T, name, data string) string { + t.Helper() + file := filepath.Join(t.TempDir(), name) + if err := os.WriteFile(file, []byte(data), 0600); err != nil { + t.Fatal(err) + } + return file +} + +func TestAdminPasswordFilePrecedenceAndRotation(t *testing.T) { + isolateAdminPasswordConfig(t) + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", writeAdminFixture(t, "factor", "synthetic-admin-ui-signing-factor")) + const configuredPassword = "ConfigPassword16" + const initialPassword = " First密碼\"\\Value " + configured := writeAdminFixture(t, "configured-password", configuredPassword) + override := writeAdminFixture(t, "environment-password", initialPassword) + common.SetConfig("admin_password_file", configured) + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", override) + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(configuredPassword); ok { + t.Fatal("environment password file must take precedence") + } + encoded, ok := ExchangeAdminPassword(initialPassword) + if !ok || encoded == initialPassword { + t.Fatal("password must exchange for a separate opaque session token") + } + session := ToTokenBase58(encoded) + now := time.Now().Unix() + if session == nil || session.User != "admin" || session.Space != "$DEFAULT" || + session.Time < now-1 || session.Time > now || !ValidLoginToken(session) { + t.Fatal("password login must issue a current v2 administrator session") + } + if !strings.HasPrefix(session.Digest, "v2:") || AdminSessionMaxAge != 24*60*60 { + t.Fatal("native UI sessions must use v2 signatures and the 24 hour token lifetime") + } + if ValidLoginToken(ToTokenBase58(initialPassword)) { + t.Fatal("short passwords must not authenticate as API tokens") + } + const rotatedPassword = "RotatedAdmin1234" + if err := os.WriteFile(override, []byte(rotatedPassword), 0600); err != nil { + t.Fatal(err) + } + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(initialPassword); ok { + t.Fatal("old password must stop working after a reload") + } + if _, ok := ExchangeAdminPassword(rotatedPassword); !ok { + t.Fatal("rotated password must authenticate") + } + if !ValidLoginToken(session) { + t.Fatal("password rotation alone must not rotate the token signing factor") + } + session.Time = time.Now().Unix() - 25*60*60 + secureFactory.Sign(session) + if ValidLoginToken(session) { + t.Fatal("expired UI sessions must be rejected") + } + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", "") + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(configuredPassword); !ok { + t.Fatal("INI password file must work without environment override") + } + common.SetConfig("admin_password_file", "") + if err := Init(); err != nil { + t.Fatal(err) + } + if _, ok := ExchangeAdminPassword(configuredPassword); ok { + t.Fatal("unconfigured deployments must remain token-only") + } +} + +func TestAdminPasswordRequiresV2AndFailsClosed(t *testing.T) { + isolateAdminPasswordConfig(t) + passwordFile := writeAdminFixture(t, "password", "SyntheticAdmin16") + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", passwordFile) + common.SetConfig("auth_token_factor", "legacy-inline-factor") + if err := Init(); err == nil || factory != nil || secureFactory != nil { + t.Fatal("admin password configuration must reject legacy inline signing") + } + for _, mode := range []string{"none", "", "invalid"} { + if err := ValidateAdminPasswordMode(mode); err == nil { + t.Fatal("password configuration must reject disabled authentication") + } + } + factorFile := writeAdminFixture(t, "factor", "synthetic-admin-password-factor") + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factorFile) + for _, test := range []struct{ name, contents string }{ + {"missing", ""}, {"empty", ""}, {"whitespace", " \t"}, {"newline", "SyntheticPrivateValue\n"}, + {"carriage-return", "SyntheticPrivateValue\r"}, {"invalid-utf8", "SyntheticPrivateValue\xff"}, + } { + t.Run(test.name, func(t *testing.T) { + file := filepath.Join(t.TempDir(), "password") + if test.name != "missing" { + if err := os.WriteFile(file, []byte(test.contents), 0600); err != nil { + t.Fatal(err) + } + } + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", file) + err := Init() + if err == nil || factory != nil || secureFactory != nil || len(adminPassword) != 0 { + t.Fatal("bad password files must not leave any active authentication factory") + } + if strings.Contains(err.Error(), "SyntheticPrivateValue") { + t.Fatal("errors must not echo password contents") + } + if _, ok := ExchangeAdminPassword("SyntheticPrivateValue"); ok { + t.Fatal("invalid configuration must never authenticate") + } + }) + } +} diff --git a/vermeer/apps/auth/filter.go b/vermeer/apps/auth/filter.go index eee77d036..d1acd7267 100644 --- a/vermeer/apps/auth/filter.go +++ b/vermeer/apps/auth/filter.go @@ -45,6 +45,8 @@ var secureFactory *SecureTokenFactory func Init() error { factory = nil secureFactory = nil + adminPassword = nil + passwordFile := adminPasswordFile() file := os.Getenv("VERMEER_AUTH_TOKEN_FACTOR_FILE") if file == "" { file, _ = common.GetConfigDefault("auth_token_factor_file", "").(string) @@ -55,8 +57,23 @@ func Init() error { factory = nil return err } - secureFactory, err = NewSecureTokenFactory(signingKey) - return err + configuredFactory, err := NewSecureTokenFactory(signingKey) + if err != nil { + return err + } + var configuredPassword []byte + if passwordFile != "" { + configuredPassword, err = readAdminPasswordFile(passwordFile) + if err != nil { + return err + } + } + secureFactory = configuredFactory + adminPassword = configuredPassword + return nil + } + if passwordFile != "" { + return errors.New("admin password file requires file-backed v2 token authentication") } factor, inlineConfigured := common.GetConfig("auth_token_factor").(string) if !inlineConfigured { @@ -80,7 +97,6 @@ func TokenFilter(ctx *gin.Context) { var credential *structure.Credential var err error var text string - var valid bool header := ctx.GetHeader(HeaderKey) if header == "" { if str, err := ctx.Cookie(CookieKey); err != nil { @@ -101,17 +117,12 @@ func TokenFilter(ctx *gin.Context) { goto LOGIN } - if secureFactory != nil { - valid = secureFactory.Verify(token) - } else if factory != nil { - valid = factory.Verify(token) - } - if !valid { + if !validSignature(token) { text = "Invalid Token" goto LOGIN } - if time.Now().Unix()-token.Time > tokenMaxAge { + if tokenExpired(token) { text = "Expired Token" goto LOGIN } @@ -211,3 +222,7 @@ func DecodeBase58(str string) string { return res } + +func tokenExpired(token *Token) bool { + return time.Now().Unix()-token.Time > tokenMaxAge +} diff --git a/vermeer/apps/master/master_main.go b/vermeer/apps/master/master_main.go index 160d5422a..534f7efb9 100644 --- a/vermeer/apps/master/master_main.go +++ b/vermeer/apps/master/master_main.go @@ -40,6 +40,9 @@ func Main() { httpPeer := common.GetConfig("http_peer").(string) debugMode := common.GetConfig("debug_mode").(string) authType := common.GetConfigDefault("auth", "none").(string) + if err := auth.ValidateAdminPasswordMode(authType); err != nil { + logrus.Fatalf("token authentication initialization failed: %s", err) + } services.InfoMaster.IpAddr = httpPeer services.InfoMaster.DebugMod = debugMode services.InfoMaster.Version = version.Version diff --git a/vermeer/apps/master/services/http_login.go b/vermeer/apps/master/services/http_login.go index b8888328e..ecb001b12 100644 --- a/vermeer/apps/master/services/http_login.go +++ b/vermeer/apps/master/services/http_login.go @@ -18,7 +18,8 @@ under the License. package services import ( - "fmt" + "net/http" + "strings" "vermeer/apps/auth" "vermeer/apps/common" @@ -28,7 +29,7 @@ import ( //const cookieTokenExpire = 1 * time.Minute type LoginRequest struct { - Token *string + Token *string `json:"token"` } type LoginHandler struct { @@ -37,33 +38,33 @@ type LoginHandler struct { func (l *LoginHandler) POST(ctx *gin.Context) { req := LoginRequest{} - err := ctx.BindJSON(&req) - if isBad(err != nil, ctx, func() string { return fmt.Sprintf("request body not correct: %s", err) }) { + if err := ctx.ShouldBindJSON(&req); err != nil { + isBad(true, ctx, func() string { return "invalid login request" }) return } - - if req.Token != nil { - doTokenLogin(ctx, &req) - return - } - - if isBad(true, ctx, func() string { return fmt.Sprintf("request body not correct: %s", err) }) { + if req.Token == nil { + isBad(true, ctx, func() string { return "admin credential is required" }) return } + doTokenLogin(ctx, &req) } func doTokenLogin(ctx *gin.Context, req *LoginRequest) { + if session, okPassword := auth.ExchangeAdminPassword(*req.Token); okPassword { + setTokenCookie(ctx, session, auth.AdminSessionMaxAge) + ok(ctx, "login success") + return + } token := auth.ToTokenBase58(*req.Token) - if isBad(token == nil, ctx, func() string { return "invalid token string format" }) { + if !auth.ValidLoginToken(token) { + ctx.JSON(http.StatusUnauthorized, common.BaseResp{ErrCode: 1, Message: "invalid admin credential"}) return } - - setTokenCookie(ctx, *req.Token) - + setTokenCookie(ctx, *req.Token, 0) ok(ctx, "login success") } -func setTokenCookie(ctx *gin.Context, token string) { - // expires: session - ctx.SetCookie(auth.CookieKey, token, 0, "/", "", false, true) +func setTokenCookie(ctx *gin.Context, token string, maxAge int) { + secure := ctx.Request.TLS != nil || strings.EqualFold(ctx.GetHeader("X-Forwarded-Proto"), "https") + ctx.SetCookie(auth.CookieKey, token, maxAge, "/", "", secure, true) } diff --git a/vermeer/apps/master/services/http_login_password_test.go b/vermeer/apps/master/services/http_login_password_test.go new file mode 100644 index 000000000..5e0031eea --- /dev/null +++ b/vermeer/apps/master/services/http_login_password_test.go @@ -0,0 +1,261 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package services + +import ( + "bytes" + "encoding/json" + "math" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + "vermeer/apps/auth" + "vermeer/apps/common" + + "github.com/gin-gonic/gin" + "github.com/prometheus/client_golang/prometheus" + "github.com/sirupsen/logrus" +) + +const nativePasswordFixture = "AdminFixture1234" +const nativeSigningFixture = "synthetic-native-password-signing-factor" + +func configureNativePassword(t *testing.T) (string, string) { + t.Helper() + for _, key := range []string{"auth_token_factor", "auth_token_factor_file", "admin_password_file"} { + previous := common.GetConfigDefault(key, "") + common.SetConfig(key, "") + t.Cleanup(func() { common.SetConfig(key, previous) }) + } + directory := t.TempDir() + factor := filepath.Join(directory, "factor") + password := filepath.Join(directory, "password") + if err := os.WriteFile(factor, []byte(nativeSigningFixture), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(password, []byte(nativePasswordFixture), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", factor) + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", password) + if err := auth.Init(); err != nil { + t.Fatal(err) + } + factory, err := auth.NewSecureTokenFactory(nativeSigningFixture) + if err != nil { + t.Fatal(err) + } + token := factory.NewToken("admin", "$DEFAULT", "hubble-service") + token.Time = math.MaxInt64 + factory.Sign(token) + encoded, err := json.Marshal(token) + if err != nil { + t.Fatal(err) + } + return password, auth.ToBase58(string(encoded)) +} + +func postNativeCredential(t *testing.T, handler http.Handler, + credential, forwardedProto string) *httptest.ResponseRecorder { + t.Helper() + body, err := json.Marshal(map[string]string{"token": credential}) + if err != nil { + t.Fatal(err) + } + request := httptest.NewRequest(http.MethodPost, "/api/v1/login", bytes.NewReader(body)) + request.Header.Set("Content-Type", "application/json") + if forwardedProto != "" { + request.Header.Set("X-Forwarded-Proto", forwardedProto) + } + response := httptest.NewRecorder() + handler.ServeHTTP(response, request) + return response +} + +func TestNativeAdminPasswordCookieAndServiceToken(t *testing.T) { + passwordFile, serviceToken := configureNativePassword(t) + previousMode, previousOutput := gin.Mode(), logrus.StandardLogger().Out + var logs bytes.Buffer + logrus.SetOutput(&logs) + gin.SetMode(gin.TestMode) + t.Cleanup(func() { gin.SetMode(previousMode); logrus.SetOutput(previousOutput) }) + router := gin.New() + router.POST("/api/v1/login", (&LoginHandler{}).POST) + router.GET("/protected", auth.TokenFilter, auth.AdminFilter, func(ctx *gin.Context) { ctx.Status(http.StatusOK) }) + check := func(cookie *http.Cookie, bearer string, want int) { + t.Helper() + request := httptest.NewRequest(http.MethodGet, "/protected", nil) + if cookie != nil { + request.AddCookie(cookie) + } + if bearer != "" { + request.Header.Set(auth.HeaderKey, "Bearer "+bearer) + } + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != want { + t.Fatalf("protected status = %d, want %d", response.Code, want) + } + } + check(nil, "", http.StatusUnauthorized) + wrong := postNativeCredential(t, router, "IncorrectAdmin16", "") + if wrong.Code != http.StatusUnauthorized || len(wrong.Result().Cookies()) != 0 { + t.Fatal("incorrect UI password must not create a cookie") + } + response := postNativeCredential(t, router, nativePasswordFixture, "") + cookies := response.Result().Cookies() + if response.Code != http.StatusOK || len(cookies) != 1 || !cookies[0].HttpOnly || + cookies[0].MaxAge != auth.AdminSessionMaxAge || cookies[0].Secure { + t.Fatal("internal HTTP login must create its HttpOnly 24-hour session cookie") + } + cookie := cookies[0] + token := auth.ToTokenBase58(cookie.Value) + now := time.Now().Unix() + if token == nil || token.User != "admin" || token.Space != "$DEFAULT" || token.Time < now-1 || token.Time > now || + !strings.HasPrefix(token.Digest, "v2:") || cookie.Value == nativePasswordFixture { + t.Fatal("short password must exchange for a separate current v2 admin session") + } + check(cookie, "", http.StatusOK) + check(nil, nativePasswordFixture, http.StatusUnauthorized) + check(nil, serviceToken, http.StatusOK) + secureResponse := postNativeCredential(t, router, nativePasswordFixture, "https") + if secureResponse.Code != http.StatusOK || !secureResponse.Result().Cookies()[0].Secure { + t.Fatal("HTTPS browser sessions must carry the Secure cookie attribute") + } + serviceLogin := postNativeCredential(t, router, serviceToken, "") + if serviceLogin.Code != http.StatusOK || len(serviceLogin.Result().Cookies()) != 1 { + t.Fatal("long opaque service-token login must remain supported") + } + const rotated = "RotatedFixture16" + if err := os.WriteFile(passwordFile, []byte(rotated), 0600); err != nil { + t.Fatal(err) + } + if err := auth.Init(); err != nil { + t.Fatal(err) + } + if postNativeCredential(t, router, nativePasswordFixture, "").Code != http.StatusUnauthorized { + t.Fatal("old password must stop authenticating after password reload") + } + if postNativeCredential(t, router, rotated, "").Code != http.StatusOK { + t.Fatal("rotated password must authenticate") + } + check(nil, serviceToken, http.StatusOK) + check(cookie, "", http.StatusOK) + malformed := httptest.NewRequest(http.MethodPost, "/api/v1/login", strings.NewReader(`{"token":"`+rotated)) + malformed.Header.Set("Content-Type", "application/json") + malformedResponse := httptest.NewRecorder() + router.ServeHTTP(malformedResponse, malformed) + if malformedResponse.Code != http.StatusBadRequest { + t.Fatal("malformed login request must be rejected") + } + for _, observed := range []string{response.Body.String(), wrong.Body.String(), + malformedResponse.Body.String(), logs.String()} { + for _, sensitive := range []string{nativePasswordFixture, rotated, cookie.Value, serviceToken} { + if strings.Contains(observed, sensitive) { + t.Fatal("responses and logs must not expose credentials") + } + } + } +} + +func nativeRouteSentinel(t *testing.T, filters ...gin.HandlerFunc) *common.Sentinel { + t.Helper() + previousMetrics := common.PrometheusMetrics + previousRegisterer, previousGatherer := prometheus.DefaultRegisterer, prometheus.DefaultGatherer + previousMode := gin.Mode() + registry := prometheus.NewRegistry() + prometheus.DefaultRegisterer, prometheus.DefaultGatherer = registry, registry + t.Cleanup(func() { + common.PrometheusMetrics = previousMetrics + prometheus.DefaultRegisterer, prometheus.DefaultGatherer = previousRegisterer, previousGatherer + gin.SetMode(previousMode) + }) + sentinel := &common.Sentinel{} + sentinel.Init("127.0.0.1:0", "release") + SetRouters(sentinel, filters...) + return sentinel +} + +func TestNativeStatusRoutesRequireConfiguredAuthentication(t *testing.T) { + _, serviceToken := configureNativePassword(t) + factory, err := auth.NewSecureTokenFactory(nativeSigningFixture) + if err != nil { + t.Fatal(err) + } + reader := factory.NewToken("space-user", "own-space", "status-test") + factory.Sign(reader) + readerJSON, err := json.Marshal(reader) + if err != nil { + t.Fatal(err) + } + readerToken := auth.ToBase58(string(readerJSON)) + sentinel := nativeRouteSentinel(t, auth.TokenFilter) + login := postNativeCredential(t, sentinel, nativePasswordFixture, "") + if login.Code != http.StatusOK { + t.Fatal("native login route must remain reachable") + } + cookie := login.Result().Cookies()[0] + for _, path := range []string{"/workers", "/api/v1/workers", "/master", "/api/v1/master"} { + for _, test := range []struct { + name, bearer string + cookie *http.Cookie + want int + }{ + {name: "anonymous", want: http.StatusUnauthorized}, + {name: "raw-password", bearer: nativePasswordFixture, want: http.StatusUnauthorized}, + {name: "service-token", bearer: serviceToken, want: http.StatusOK}, + {name: "space-token", bearer: readerToken, want: http.StatusOK}, + {name: "password-session", cookie: cookie, want: http.StatusOK}, + } { + t.Run(path+"/"+test.name, func(t *testing.T) { + request := httptest.NewRequest(http.MethodGet, path, nil) + if test.bearer != "" { + request.Header.Set(auth.HeaderKey, "Bearer "+test.bearer) + } + if test.cookie != nil { + request.AddCookie(test.cookie) + } + response := httptest.NewRecorder() + sentinel.ServeHTTP(response, request) + if response.Code != test.want { + t.Fatalf("status = %d, want %d", response.Code, test.want) + } + }) + } + } + health := httptest.NewRecorder() + sentinel.ServeHTTP(health, httptest.NewRequest(http.MethodGet, "/api/v1/healthcheck", nil)) + if health.Code != http.StatusOK { + t.Fatal("health probes must remain public") + } +} + +func TestNativeStatusRoutesRetainExplicitNoneMode(t *testing.T) { + sentinel := nativeRouteSentinel(t, auth.NoneAuthFilter) + for _, path := range []string{"/api/v1/workers", "/api/v1/master"} { + response := httptest.NewRecorder() + sentinel.ServeHTTP(response, httptest.NewRequest(http.MethodGet, path, nil)) + if response.Code != http.StatusOK { + t.Fatal("explicit none mode must retain its historical route behavior") + } + } +} diff --git a/vermeer/apps/master/services/http_login_v2_test.go b/vermeer/apps/master/services/http_login_v2_test.go index 755dc450b..fa0211812 100644 --- a/vermeer/apps/master/services/http_login_v2_test.go +++ b/vermeer/apps/master/services/http_login_v2_test.go @@ -71,6 +71,12 @@ func TestNativeLoginCookieUsesSecureDeploymentToken(t *testing.T) { login.Header.Set("Content-Type", "application/json") response := httptest.NewRecorder() router.ServeHTTP(response, login) + if want != http.StatusOK { + if response.Code != http.StatusUnauthorized || len(response.Result().Cookies()) != 0 { + t.Fatal("invalid token login must reject before issuing a cookie") + } + return + } if response.Code != http.StatusOK { t.Fatalf("native login status = %d", response.Code) } @@ -94,3 +100,31 @@ func TestNativeLoginCookieUsesSecureDeploymentToken(t *testing.T) { legacy.Sign(token) authenticate(token, http.StatusUnauthorized) } + +func TestNativeLegacyTokenLoginRemainsSupported(t *testing.T) { + for _, key := range []string{"auth_token_factor", "auth_token_factor_file", "admin_password_file"} { + previous := common.GetConfigDefault(key, "") + common.SetConfig(key, "") + t.Cleanup(func() { common.SetConfig(key, previous) }) + } + t.Setenv("VERMEER_AUTH_TOKEN_FACTOR_FILE", "") + t.Setenv("VERMEER_ADMIN_PASSWORD_FILE", "") + const factor = "synthetic-legacy-inline-factor" + common.SetConfig("auth_token_factor", factor) + if err := auth.Init(); err != nil { + t.Fatal(err) + } + factory := &auth.TokenFactory{Factor: factor} + token := factory.NewToken("admin", "$DEFAULT", "legacy-client") + factory.Sign(token) + encoded, err := json.Marshal(token) + if err != nil { + t.Fatal(err) + } + router := gin.New() + router.POST("/api/v1/login", (&LoginHandler{}).POST) + response := postNativeCredential(t, router, auth.ToBase58(string(encoded)), "") + if response.Code != http.StatusOK || len(response.Result().Cookies()) != 1 { + t.Fatal("valid historical inline-token login must remain supported") + } +} diff --git a/vermeer/apps/master/services/router.go b/vermeer/apps/master/services/router.go index 2c3887ec5..7474d1138 100644 --- a/vermeer/apps/master/services/router.go +++ b/vermeer/apps/master/services/router.go @@ -62,7 +62,7 @@ func SetRouters(sen *common.Sentinel, authFilters ...gin.HandlerFunc) { // /workers regVerAPI(sen, 1, "/workers", map[string]common.BaseHandler{ "": &WorkersHandler{}, - }) + }, authFilters...) regVerAPI(sen, 1, "/workers", map[string]common.BaseHandler{ "/availability": &WorkerAvailabilityHandler{}, }, authFilters...) @@ -70,7 +70,7 @@ func SetRouters(sen *common.Sentinel, authFilters ...gin.HandlerFunc) { // /master regVerAPI(sen, 1, "/master", map[string]common.BaseHandler{ "": &InfoMasterHandler{}, - }) + }, authFilters...) } diff --git a/vermeer/ui/ui/lib/functions.js b/vermeer/ui/ui/lib/functions.js index faf736189..bbcfe2575 100644 --- a/vermeer/ui/ui/lib/functions.js +++ b/vermeer/ui/ui/lib/functions.js @@ -20,7 +20,7 @@ login: function () { const token = $('#admin_token').val(); if (!token) { - showDelModal('empty token'); + showDelModal('Enter an admin password or token.'); return; } diff --git a/vermeer/ui/ui/master.html b/vermeer/ui/ui/master.html index 27cb10605..49dd3abc8 100644 --- a/vermeer/ui/ui/master.html +++ b/vermeer/ui/ui/master.html @@ -62,8 +62,9 @@
- - + +
From 7ca4dfec5cea07cc32eee4bfae9bd3ebd414a301 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 05:31:31 +0000 Subject: [PATCH 09/17] fix(vermeer): keep peer stream cleanup within its registration generation --- vermeer/apps/worker/grpc_handlers.go | 361 +++++++++++++------- vermeer/apps/worker/peer_generation_test.go | 269 +++++++++++++++ vermeer/apps/worker/peer_manager.go | 116 ++++++- vermeer/apps/worker/service.go | 75 ++-- 4 files changed, 663 insertions(+), 158 deletions(-) create mode 100644 vermeer/apps/worker/peer_generation_test.go diff --git a/vermeer/apps/worker/grpc_handlers.go b/vermeer/apps/worker/grpc_handlers.go index 87d38b05d..9bb3bb5e4 100644 --- a/vermeer/apps/worker/grpc_handlers.go +++ b/vermeer/apps/worker/grpc_handlers.go @@ -67,36 +67,49 @@ func (ph *PeerHandler) SayHelloPeer(ctx context.Context, req *pb.HelloPeerReq) ( return &pb.HelloPeerResp{}, nil } +func incomingPeer(ctx context.Context) (string, *PeerClient, error) { + md, _ := metadata.FromIncomingContext(ctx) + names := md.Get("worker_name") + if len(names) != 1 || names[0] == "" { + return "", nil, status.Error(codes.InvalidArgument, "worker_name metadata is required") + } + peer := PeerMgr.GetPeer(names[0]) + if peer == nil { + return "", nil, status.Error(codes.FailedPrecondition, "peer must register before opening streams") + } + return names[0], peer, nil +} + func (ph *PeerHandler) Scatter(stream pb.Worker_ScatterServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).ScatterHandler.SetServer(stream) - PeerMgr.GetPeer(name).ScatterHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.ScatterHandler.receiveServer(name, stream) } func (ph *PeerHandler) LoadAction(stream pb.Worker_LoadActionServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).LoadActionHandler.SetServer(stream) - PeerMgr.GetPeer(name).LoadActionHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.LoadActionHandler.receiveServer(name, stream) } func (ph *PeerHandler) StepEnd(stream pb.Worker_StepEndServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).StepEndHandler.SetServer(stream) - PeerMgr.GetPeer(name).StepEndHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.StepEndHandler.receiveServer(name, stream) } func (ph *PeerHandler) SettingAction(stream pb.Worker_SettingActionServer) error { - md, _ := metadata.FromIncomingContext(stream.Context()) - name := md.Get("worker_name")[0] - PeerMgr.GetPeer(name).SettingActionHandler.SetServer(stream) - PeerMgr.GetPeer(name).SettingActionHandler.RecvHandler(name) - return nil + name, peer, err := incomingPeer(stream.Context()) + if err != nil { + return err + } + return peer.SettingActionHandler.receiveServer(name, stream) } func (ph *PeerHandler) DeleteGraph(ctx context.Context, req *pb.DeleteGraphReq) (*pb.DeleteGraphResp, error) { @@ -404,36 +417,67 @@ const ( type LoadActionHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_LoadActionServer grpcClient pb.Worker_LoadActionClient locker sync.Mutex } -func (dh *LoadActionHandler) SetServer(stream pb.Worker_LoadActionServer) { - dh.mode = HandlerModeServer - dh.grpcServer = stream +func (dh *LoadActionHandler) SetServer(stream pb.Worker_LoadActionServer) uint64 { + return dh.setServer(stream, nil) } - -func (dh *LoadActionHandler) SetClient(client pb.Worker_LoadActionClient) { - dh.mode = HandlerModeClient - dh.grpcClient = client +func (dh *LoadActionHandler) setServer(stream pb.Worker_LoadActionServer, cancel context.CancelFunc) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerLoadStream, cancel) + dh.mode, dh.grpcServer, dh.grpcClient = HandlerModeServer, stream, nil + return dh.generation +} +func (dh *LoadActionHandler) SetClient(stream pb.Worker_LoadActionClient) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerLoadStream, nil) + dh.mode, dh.grpcClient, dh.grpcServer = HandlerModeClient, stream, nil + return dh.generation +} +func (dh *LoadActionHandler) IsClient() bool { + dh.locker.Lock() + defer dh.locker.Unlock() + return dh.mode == HandlerModeClient +} +func (dh *LoadActionHandler) receiveServer(name string, stream pb.Worker_LoadActionServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := dh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { dh.recv(name, dh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (dh *LoadActionHandler) RecvHandler(name string) { + dh.locker.Lock() + owner, generation, mode, server, client := dh.peer, dh.generation, dh.mode, dh.grpcServer, dh.grpcClient + dh.locker.Unlock() + dh.recv(name, owner, generation, mode, server, client) +} + +func (dh *LoadActionHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_LoadActionServer, client pb.Worker_LoadActionClient) { logrus.Infof("load action recv handler setup: %s", name) - dh.locker = sync.Mutex{} loadBl := LoadGraphBl{} - if dh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := dh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv load action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerLoadStream, generation) + return } //logrus.Infof("LoadActionHandler server recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.LoadAction_LoadVertex { @@ -467,17 +511,13 @@ func (dh *LoadActionHandler) RecvHandler(name string) { resp.Data) } } - } else if dh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := dh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv load action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerLoadStream, generation) + return } //logrus.Infof("LoadActionHandler client recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.LoadAction_LoadVertex { @@ -552,50 +592,77 @@ func (dh *LoadActionHandler) LoadAction(taskId int32, action pb.LoadAction, coun type ScatterHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_ScatterServer grpcClient pb.Worker_ScatterClient locker sync.Mutex } -func (sh *ScatterHandler) SetServer(stream pb.Worker_ScatterServer) { - sh.mode = HandlerModeServer - sh.grpcServer = stream +func (sh *ScatterHandler) SetServer(stream pb.Worker_ScatterServer) uint64 { + return sh.setServer(stream, nil) } - -func (sh *ScatterHandler) SetClient(client pb.Worker_ScatterClient) { - sh.mode = HandlerModeClient - sh.grpcClient = client +func (sh *ScatterHandler) setServer(stream pb.Worker_ScatterServer, cancel context.CancelFunc) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerScatterStream, cancel) + sh.mode, sh.grpcServer, sh.grpcClient = HandlerModeServer, stream, nil + return sh.generation +} +func (sh *ScatterHandler) SetClient(stream pb.Worker_ScatterClient) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerScatterStream, nil) + sh.mode, sh.grpcClient, sh.grpcServer = HandlerModeClient, stream, nil + return sh.generation +} +func (sh *ScatterHandler) IsClient() bool { + sh.locker.Lock() + defer sh.locker.Unlock() + return sh.mode == HandlerModeClient +} +func (sh *ScatterHandler) receiveServer(name string, stream pb.Worker_ScatterServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := sh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { sh.recv(name, sh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (sh *ScatterHandler) RecvHandler(name string) { + sh.locker.Lock() + owner, generation, mode, server, client := sh.peer, sh.generation, sh.mode, sh.grpcServer, sh.grpcClient + sh.locker.Unlock() + sh.recv(name, owner, generation, mode, server, client) +} + +func (sh *ScatterHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_ScatterServer, client pb.Worker_ScatterClient) { logrus.Infof("scatter recv handler setup: %s", name) - sh.locker = sync.Mutex{} - if sh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := sh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv scatter error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerScatterStream, generation) + return } cb := ComputeBl{} go cb.RecvScatter(resp.TaskId, resp.Data, resp.End, resp.SIdx) } - } else if sh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := sh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv scatter error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerScatterStream, generation) + return } cb := ComputeBl{} go cb.RecvScatter(resp.TaskId, resp.Data, resp.End, resp.SIdx) @@ -639,50 +706,77 @@ func (sh *ScatterHandler) SendScatter(taskId int32, step int32, count int32, end type StepEndHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_StepEndServer grpcClient pb.Worker_StepEndClient locker sync.Mutex } -func (sh *StepEndHandler) SetServer(stream pb.Worker_StepEndServer) { - sh.mode = HandlerModeServer - sh.grpcServer = stream +func (sh *StepEndHandler) SetServer(stream pb.Worker_StepEndServer) uint64 { + return sh.setServer(stream, nil) } - -func (sh *StepEndHandler) SetClient(client pb.Worker_StepEndClient) { - sh.mode = HandlerModeClient - sh.grpcClient = client +func (sh *StepEndHandler) setServer(stream pb.Worker_StepEndServer, cancel context.CancelFunc) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerStepStream, cancel) + sh.mode, sh.grpcServer, sh.grpcClient = HandlerModeServer, stream, nil + return sh.generation +} +func (sh *StepEndHandler) SetClient(stream pb.Worker_StepEndClient) uint64 { + sh.locker.Lock() + defer sh.locker.Unlock() + sh.generation = PeerMgr.bindPeerStream(sh.peer, peerStepStream, nil) + sh.mode, sh.grpcClient, sh.grpcServer = HandlerModeClient, stream, nil + return sh.generation +} +func (sh *StepEndHandler) IsClient() bool { + sh.locker.Lock() + defer sh.locker.Unlock() + return sh.mode == HandlerModeClient +} +func (sh *StepEndHandler) receiveServer(name string, stream pb.Worker_StepEndServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := sh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { sh.recv(name, sh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (sh *StepEndHandler) RecvHandler(name string) { + sh.locker.Lock() + owner, generation, mode, server, client := sh.peer, sh.generation, sh.mode, sh.grpcServer, sh.grpcClient + sh.locker.Unlock() + sh.recv(name, owner, generation, mode, server, client) +} + +func (sh *StepEndHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_StepEndServer, client pb.Worker_StepEndClient) { logrus.Infof("step end recv handler setup: %s", name) - sh.locker = sync.Mutex{} - if sh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := sh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv step end error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerStepStream, generation) + return } cb := ComputeBl{} go cb.StepEnd(resp.TaskId, resp.WorkerName) } - } else if sh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := sh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv step end error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerStepStream, generation) + return } cb := ComputeBl{} go cb.StepEnd(resp.TaskId, resp.WorkerName) @@ -716,36 +810,67 @@ func (sh *StepEndHandler) SendStepEnd(taskId int32) { type SettingActionHandler struct { mode byte + peer *PeerClient + generation uint64 grpcServer pb.Worker_SettingActionServer grpcClient pb.Worker_SettingActionClient locker sync.Mutex } -func (dh *SettingActionHandler) SetServer(stream pb.Worker_SettingActionServer) { - dh.mode = HandlerModeServer - dh.grpcServer = stream +func (dh *SettingActionHandler) SetServer(stream pb.Worker_SettingActionServer) uint64 { + return dh.setServer(stream, nil) } - -func (dh *SettingActionHandler) SetClient(client pb.Worker_SettingActionClient) { - dh.mode = HandlerModeClient - dh.grpcClient = client +func (dh *SettingActionHandler) setServer(stream pb.Worker_SettingActionServer, cancel context.CancelFunc) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerSettingStream, cancel) + dh.mode, dh.grpcServer, dh.grpcClient = HandlerModeServer, stream, nil + return dh.generation +} +func (dh *SettingActionHandler) SetClient(stream pb.Worker_SettingActionClient) uint64 { + dh.locker.Lock() + defer dh.locker.Unlock() + dh.generation = PeerMgr.bindPeerStream(dh.peer, peerSettingStream, nil) + dh.mode, dh.grpcClient, dh.grpcServer = HandlerModeClient, stream, nil + return dh.generation +} +func (dh *SettingActionHandler) IsClient() bool { + dh.locker.Lock() + defer dh.locker.Unlock() + return dh.mode == HandlerModeClient +} +func (dh *SettingActionHandler) receiveServer(name string, stream pb.Worker_SettingActionServer) error { + ctx, cancel := context.WithCancel(stream.Context()) + defer cancel() + generation := dh.setServer(stream, cancel) + if generation == 0 { + return status.Error(codes.FailedPrecondition, "peer registration was replaced") + } + done := make(chan struct{}) + go func() { dh.recv(name, dh.peer, generation, HandlerModeServer, stream, nil); close(done) }() + select { + case <-done: + case <-ctx.Done(): + } + return nil } - func (dh *SettingActionHandler) RecvHandler(name string) { + dh.locker.Lock() + owner, generation, mode, server, client := dh.peer, dh.generation, dh.mode, dh.grpcServer, dh.grpcClient + dh.locker.Unlock() + dh.recv(name, owner, generation, mode, server, client) +} + +func (dh *SettingActionHandler) recv(name string, owner *PeerClient, generation uint64, mode byte, server pb.Worker_SettingActionServer, client pb.Worker_SettingActionClient) { logrus.Infof("load action recv handler setup: %s", name) - dh.locker = sync.Mutex{} settingBl := SettingBl{} - if dh.mode == HandlerModeServer { + if mode == HandlerModeServer { for { - resp, err := dh.grpcServer.Recv() + resp, err := server.Recv() if err != nil { logrus.Errorf("recv setting action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerSettingStream, generation) + return } //logrus.Infof("LoadActionHandler server recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.SettingAction_SetOutEdges { @@ -764,17 +889,13 @@ func (dh *SettingActionHandler) RecvHandler(name string) { resp.Data) } } - } else if dh.mode == HandlerModeClient { + } else if mode == HandlerModeClient { for { - resp, err := dh.grpcClient.Recv() + resp, err := client.Recv() if err != nil { logrus.Errorf("recv setting action error: %s", err) - time.Sleep(2 * time.Second) - if !PeerMgr.CheckPeerAlive(name) { - PeerMgr.RemovePeer(name) - break - } - continue + PeerMgr.RemovePeerStream(owner, peerSettingStream, generation) + return } //logrus.Infof("LoadActionHandler client recv action: %v, end: %v", resp.Action, resp.End) if resp.Action == pb.SettingAction_SetOutEdges { diff --git a/vermeer/apps/worker/peer_generation_test.go b/vermeer/apps/worker/peer_generation_test.go new file mode 100644 index 000000000..cd68623da --- /dev/null +++ b/vermeer/apps/worker/peer_generation_test.go @@ -0,0 +1,269 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package worker + +import ( + "context" + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + "io" + "sync" + "testing" + "time" + "vermeer/apps/common" + pb "vermeer/apps/protos" +) + +type peerStreamFixture struct { + grpc.ServerStream + ctx context.Context + entered chan struct{} + release chan struct{} + enteredOnce sync.Once + releaseOnce sync.Once +} + +func (f *peerStreamFixture) Context() context.Context { return f.ctx } +func (f *peerStreamFixture) eof() error { + f.enteredOnce.Do(func() { close(f.entered) }) + <-f.release + return io.EOF +} +func (f *peerStreamFixture) finish() { f.releaseOnce.Do(func() { close(f.release) }) } +func newPeerStreamFixture(t *testing.T, ctx context.Context) *peerStreamFixture { + t.Helper() + f := &peerStreamFixture{ctx: ctx, entered: make(chan struct{}), release: make(chan struct{})} + t.Cleanup(f.finish) + return f +} +func peerTestMetrics(t *testing.T) { + t.Helper() + previous := common.PrometheusMetrics.WorkerCnt + common.PrometheusMetrics.WorkerCnt = prometheus.NewGaugeVec(prometheus.GaugeOpts{Name: "test_peer_stream_generations"}, nil) + PeerMgr.Init() + t.Cleanup(func() { PeerMgr.Init(); common.PrometheusMetrics.WorkerCnt = previous }) +} +func peerCount(t *testing.T) float64 { + t.Helper() + metric := new(dto.Metric) + if err := common.PrometheusMetrics.WorkerCnt.WithLabelValues().Write(metric); err != nil { + t.Fatal(err) + } + return metric.GetGauge().GetValue() +} +func awaitPeerEvent(t *testing.T, ch <-chan struct{}) { + t.Helper() + select { + case <-ch: + case <-time.After(time.Second): + t.Fatal("peer stream did not reach expected lifecycle event") + } +} + +type testScatterServer struct{ *peerStreamFixture } + +func (f *testScatterServer) Recv() (*pb.ScatterReq, error) { return nil, f.eof() } +func (f *testScatterServer) Send(*pb.ScatterResp) error { return nil } + +type testLoadActionServer struct{ *peerStreamFixture } + +func (f *testLoadActionServer) Recv() (*pb.LoadActionReq, error) { return nil, f.eof() } +func (f *testLoadActionServer) Send(*pb.LoadActionResp) error { return nil } + +type testStepEndServer struct{ *peerStreamFixture } + +func (f *testStepEndServer) Recv() (*pb.StepEndReq, error) { return nil, f.eof() } +func (f *testStepEndServer) Send(*pb.StepEndResp) error { return nil } + +type testSettingActionServer struct{ *peerStreamFixture } + +func (f *testSettingActionServer) Recv() (*pb.SettingActionReq, error) { return nil, f.eof() } +func (f *testSettingActionServer) Send(*pb.SettingActionResp) error { return nil } + +type testScatterClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testScatterClient) Context() context.Context { return f.fixture.ctx } +func (f *testScatterClient) Recv() (*pb.ScatterResp, error) { return nil, f.fixture.eof() } +func (f *testScatterClient) Send(*pb.ScatterReq) error { return nil } + +type testLoadActionClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testLoadActionClient) Context() context.Context { return f.fixture.ctx } +func (f *testLoadActionClient) Recv() (*pb.LoadActionResp, error) { return nil, f.fixture.eof() } +func (f *testLoadActionClient) Send(*pb.LoadActionReq) error { return nil } + +type testStepEndClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testStepEndClient) Context() context.Context { return f.fixture.ctx } +func (f *testStepEndClient) Recv() (*pb.StepEndResp, error) { return nil, f.fixture.eof() } +func (f *testStepEndClient) Send(*pb.StepEndReq) error { return nil } + +type testSettingActionClient struct { + grpc.ClientStream + fixture *peerStreamFixture +} + +func (f *testSettingActionClient) Context() context.Context { return f.fixture.ctx } +func (f *testSettingActionClient) Recv() (*pb.SettingActionResp, error) { return nil, f.fixture.eof() } +func (f *testSettingActionClient) Send(*pb.SettingActionReq) error { return nil } + +type peerStreamTestCase struct { + name string + bind func(*PeerClient, *peerStreamFixture) func() + rpc func(*peerStreamFixture) error +} + +func peerStreamTestCases() []peerStreamTestCase { + return []peerStreamTestCase{ + {"Scatter", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.ScatterHandler.SetServer(&testScatterServer{f}) + return func() { peer.ScatterHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).Scatter(&testScatterServer{f}) }}, + {"LoadAction", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.LoadActionHandler.SetServer(&testLoadActionServer{f}) + return func() { peer.LoadActionHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).LoadAction(&testLoadActionServer{f}) }}, + {"StepEnd", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.StepEndHandler.SetServer(&testStepEndServer{f}) + return func() { peer.StepEndHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).StepEnd(&testStepEndServer{f}) }}, + {"SettingAction", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.SettingActionHandler.SetServer(&testSettingActionServer{f}) + return func() { peer.SettingActionHandler.RecvHandler(peer.Name) } + }, func(f *peerStreamFixture) error { return new(PeerHandler).SettingAction(&testSettingActionServer{f}) }}, + } +} + +func peerClientTestCases() []peerStreamTestCase { + return []peerStreamTestCase{ + {"Scatter/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.ScatterHandler.SetClient(&testScatterClient{fixture: f}) + return func() { peer.ScatterHandler.RecvHandler(peer.Name) } + }, nil}, + {"LoadAction/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.LoadActionHandler.SetClient(&testLoadActionClient{fixture: f}) + return func() { peer.LoadActionHandler.RecvHandler(peer.Name) } + }, nil}, + {"StepEnd/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.StepEndHandler.SetClient(&testStepEndClient{fixture: f}) + return func() { peer.StepEndHandler.RecvHandler(peer.Name) } + }, nil}, + {"SettingAction/client", func(peer *PeerClient, f *peerStreamFixture) func() { + peer.SettingActionHandler.SetClient(&testSettingActionClient{fixture: f}) + return func() { peer.SettingActionHandler.RecvHandler(peer.Name) } + }, nil}, + } +} + +func TestPeerOldEOFDoesNotRemoveNewRegistrationOrStream(t *testing.T) { + for _, test := range append(peerStreamTestCases(), peerClientTestCases()...) { + for _, replacePeer := range []bool{false, true} { + suffix := "rebound-stream" + if replacePeer { + suffix = "new-hello" + } + t.Run(test.name+"/"+suffix, func(t *testing.T) { + peerTestMetrics(t) + old := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + first := newPeerStreamFixture(t, context.Background()) + receive := test.bind(old, first) + done := make(chan struct{}) + go func() { receive(); close(done) }() + awaitPeerEvent(t, first.entered) + current := old + if replacePeer { + current = PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + } + second := newPeerStreamFixture(t, context.Background()) + receiveNew := test.bind(current, second) + first.finish() + awaitPeerEvent(t, done) + if PeerMgr.GetPeer("peer") != current || peerCount(t) != 1 { + t.Fatal("old EOF removed replacement registration/stream or inflated peer metric") + } + second.finish() + receiveNew() + if PeerMgr.GetPeer("peer") != nil || peerCount(t) != 0 { + t.Fatal("current EOF must retire exactly its own session") + } + }) + } + } +} + +func TestPeerRPCRequiresRegistrationMetadata(t *testing.T) { + for _, test := range peerStreamTestCases() { + t.Run(test.name, func(t *testing.T) { + peerTestMetrics(t) + missing := newPeerStreamFixture(t, context.Background()) + if status.Code(test.rpc(missing)) != codes.InvalidArgument { + t.Fatal("missing worker metadata must not panic") + } + unknown := newPeerStreamFixture(t, metadata.NewIncomingContext(context.Background(), metadata.Pairs("worker_name", "missing"))) + if status.Code(test.rpc(unknown)) != codes.FailedPrecondition { + t.Fatal("unknown worker must register before streaming") + } + }) + } +} + +func TestReplacingPeerCancelsOldServerRPC(t *testing.T) { + peerTestMetrics(t) + PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + fixture := newPeerStreamFixture(t, metadata.NewIncomingContext(context.Background(), metadata.Pairs("worker_name", "peer"))) + done := make(chan struct{}) + go func() { _ = new(PeerHandler).Scatter(&testScatterServer{fixture}); close(done) }() + awaitPeerEvent(t, fixture.entered) + replacement := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + awaitPeerEvent(t, done) + if PeerMgr.GetPeer("peer") != replacement { + t.Fatal("retiring RPC must preserve replacement peer") + } + fixture.finish() +} + +func TestPeerConnectionPublicationCannotReviveOldHello(t *testing.T) { + peerTestMetrics(t) + old := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + current := PeerMgr.AddPeer("peer", 1, "127.0.0.1:6789") + connection, err := grpc.NewClient("passthrough:///unused", grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatal(err) + } + if PeerMgr.SetPeerConnection(old, connection) || connection.GetState() != connectivity.Shutdown { + t.Fatal("late connection must be rejected and closed") + } + if PeerMgr.GetPeer("peer") != current || peerCount(t) != 1 { + t.Fatal("connection race must not change current registration") + } +} diff --git a/vermeer/apps/worker/peer_manager.go b/vermeer/apps/worker/peer_manager.go index 7daa4aff0..805cdd6ab 100644 --- a/vermeer/apps/worker/peer_manager.go +++ b/vermeer/apps/worker/peer_manager.go @@ -18,6 +18,7 @@ under the License. package worker import ( + "context" "sync" "vermeer/apps/common" pb "vermeer/apps/protos" @@ -30,6 +31,16 @@ import ( var PeerMgr PeerManager +type peerStreamKind uint8 + +const ( + peerScatterStream peerStreamKind = iota + peerLoadStream + peerStepStream + peerSettingStream + peerStreamCount +) + type PeerClient struct { Id int32 Name string @@ -40,6 +51,8 @@ type PeerClient struct { StepEndHandler StepEndHandler SettingActionHandler SettingActionHandler peerConn *grpc.ClientConn + streamGenerations [peerStreamCount]uint64 + streamCancels [peerStreamCount]context.CancelFunc peerSession pb.WorkerClient } @@ -48,34 +61,109 @@ type PeerManager struct { locker sync.Mutex } +func closePeerResources(connection *grpc.ClientConn, cancels [peerStreamCount]context.CancelFunc) { + for _, cancel := range cancels { + if cancel != nil { + cancel() + } + } + if connection != nil { + _ = connection.Close() + } +} + func (pm *PeerManager) Init() { - pm.locker = sync.Mutex{} + pm.locker.Lock() + previous := pm.peersByName pm.peersByName = make(map[string]*PeerClient) + pm.locker.Unlock() + for _, peer := range previous { + closePeerResources(peer.peerConn, peer.streamCancels) + } } -func (pm *PeerManager) AddPeer(name string, id int32, grpcPeer string) { +func (pm *PeerManager) AddPeer(name string, id int32, grpcPeer string) *PeerClient { pm.locker.Lock() - defer pm.locker.Unlock() - peer := PeerClient{ - Id: id, - Name: name, - GrpcPeer: grpcPeer, - Self: name == ServiceWorker.WorkerName, + previous := pm.peersByName[name] + peer := &PeerClient{Id: id, Name: name, GrpcPeer: grpcPeer, Self: name == ServiceWorker.WorkerName} + peer.ScatterHandler.peer = peer + peer.LoadActionHandler.peer = peer + peer.StepEndHandler.peer = peer + peer.SettingActionHandler.peer = peer + if pm.peersByName == nil { + pm.peersByName = make(map[string]*PeerClient) + } + pm.peersByName[name] = peer + if previous == nil { + common.PrometheusMetrics.WorkerCnt.WithLabelValues().Inc() + } + var connection *grpc.ClientConn + var cancels [peerStreamCount]context.CancelFunc + if previous != nil { + connection, cancels = previous.peerConn, previous.streamCancels } - pm.peersByName[name] = &peer - common.PrometheusMetrics.WorkerCnt.WithLabelValues().Inc() + pm.locker.Unlock() + closePeerResources(connection, cancels) + return peer +} + +func (pm *PeerManager) SetPeerConnection(peer *PeerClient, connection *grpc.ClientConn) bool { + pm.locker.Lock() + if peer == nil || pm.peersByName[peer.Name] != peer { + pm.locker.Unlock() + _ = connection.Close() + return false + } + peer.peerConn, peer.peerSession = connection, pb.NewWorkerClient(connection) + pm.locker.Unlock() + return true +} + +func (pm *PeerManager) bindPeerStream(peer *PeerClient, kind peerStreamKind, cancel context.CancelFunc) uint64 { + pm.locker.Lock() + if peer == nil || pm.peersByName[peer.Name] != peer { + pm.locker.Unlock() + return 0 + } + previous := peer.streamCancels[kind] + peer.streamGenerations[kind]++ + generation := peer.streamGenerations[kind] + peer.streamCancels[kind] = cancel + pm.locker.Unlock() + if previous != nil { + previous() + } + return generation +} + +// RemovePeerStream ignores both old Hello registrations and rebound stream generations. +func (pm *PeerManager) RemovePeerStream(peer *PeerClient, kind peerStreamKind, generation uint64) bool { + pm.locker.Lock() + if peer == nil || pm.peersByName[peer.Name] != peer || peer.streamGenerations[kind] != generation { + pm.locker.Unlock() + return false + } + delete(pm.peersByName, peer.Name) + connection, cancels := peer.peerConn, peer.streamCancels + common.PrometheusMetrics.WorkerCnt.WithLabelValues().Dec() + pm.locker.Unlock() + closePeerResources(connection, cancels) + logrus.Infof("removed peer session: %s", peer.Name) + return true } func (pm *PeerManager) RemovePeer(name string) { pm.locker.Lock() - defer pm.locker.Unlock() - _, ok := pm.peersByName[name] - if !ok { + peer := pm.peersByName[name] + if peer == nil { + pm.locker.Unlock() return } - logrus.Infof("removed peer:%v", name) delete(pm.peersByName, name) + connection, cancels := peer.peerConn, peer.streamCancels common.PrometheusMetrics.WorkerCnt.WithLabelValues().Dec() + pm.locker.Unlock() + closePeerResources(connection, cancels) } func (pm *PeerManager) GetPeer(name string) *PeerClient { diff --git a/vermeer/apps/worker/service.go b/vermeer/apps/worker/service.go index e8b599f2c..16bf74faf 100644 --- a/vermeer/apps/worker/service.go +++ b/vermeer/apps/worker/service.go @@ -117,14 +117,15 @@ func (s *Service) Init() (err error) { return fmt.Errorf("open registration lifeline: %w", err) } for _, w := range resp.GetWorkers() { - PeerMgr.AddPeer(w.Name, w.Id, w.GrpcPeer) - peerClient := PeerMgr.GetPeer(w.Name) - peerClient.peerConn, err = common.DialGRPCPeer(context.Background(), + peerClient := PeerMgr.AddPeer(w.Name, w.Id, w.GrpcPeer) + peerConnection, err := common.DialGRPCPeer(context.Background(), w.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { return fmt.Errorf("connect to peer %s: %w", w.GrpcPeer, err) } - peerClient.peerSession = pb.NewWorkerClient(peerClient.peerConn) + if !PeerMgr.SetPeerConnection(peerClient, peerConnection) { + return fmt.Errorf("peer registration was replaced during bootstrap") + } ctx, cancel2 := context.WithTimeout(context.Background(), 5*time.Second) resp, err := peerClient.peerSession.SayHelloPeer( ctx, &pb.HelloPeerReq{SourceName: s.WorkerName, TargetName: w.Name, WorkerPeer: grpcPeer}) @@ -140,7 +141,10 @@ func (s *Service) Init() (err error) { cancel2() return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } - peerClient.ScatterHandler.SetClient(stream) + if peerClient.ScatterHandler.SetClient(stream) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") + } // install load action ctx = metadata.NewOutgoingContext(context.Background(), md) @@ -149,7 +153,10 @@ func (s *Service) Init() (err error) { cancel2() return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } - peerClient.LoadActionHandler.SetClient(stream2) + if peerClient.LoadActionHandler.SetClient(stream2) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") + } // install step end ctx = metadata.NewOutgoingContext(context.Background(), md) @@ -158,7 +165,10 @@ func (s *Service) Init() (err error) { cancel2() return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } - peerClient.StepEndHandler.SetClient(stream3) + if peerClient.StepEndHandler.SetClient(stream3) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") + } // install setting action ctx = metadata.NewOutgoingContext(context.Background(), md) @@ -167,7 +177,10 @@ func (s *Service) Init() (err error) { cancel2() return fmt.Errorf("bootstrap peer %s: %w", w.GrpcPeer, err) } - peerClient.SettingActionHandler.SetClient(stream4) + if peerClient.SettingActionHandler.SetClient(stream4) == 0 { + cancel2() + return fmt.Errorf("peer registration was replaced while binding streams") + } logrus.Infof("say hello peer %s, ok, %s", w.GrpcPeer, resp.GetStatus()) cancel2() @@ -220,16 +233,16 @@ func (s *Service) Run() { //与peer worker的连接 for _, w := range PeerMgr.GetAllWorkers() { - if w.ScatterHandler.mode == HandlerModeClient { + if w.ScatterHandler.IsClient() { go w.ScatterHandler.RecvHandler(w.Name) } - if w.LoadActionHandler.mode == HandlerModeClient { + if w.LoadActionHandler.IsClient() { go w.LoadActionHandler.RecvHandler(w.Name) } - if w.StepEndHandler.mode == HandlerModeClient { + if w.StepEndHandler.IsClient() { go w.StepEndHandler.RecvHandler(w.Name) } - if w.SettingActionHandler.mode == HandlerModeClient { + if w.SettingActionHandler.IsClient() { go w.SettingActionHandler.RecvHandler(w.Name) } } @@ -326,15 +339,17 @@ func (s *Service) ReconnectMaster() { if peer != nil { continue } - PeerMgr.AddPeer(workerInfo.Name, workerInfo.Id, workerInfo.GrpcPeer) - peerClient := PeerMgr.GetPeer(workerInfo.Name) - peerClient.peerConn, err = common.DialGRPCPeer(context.Background(), + peerClient := PeerMgr.AddPeer(workerInfo.Name, workerInfo.Id, workerInfo.GrpcPeer) + peerConnection, err := common.DialGRPCPeer(context.Background(), workerInfo.GrpcPeer, grpc.WithTransportCredentials(insecure.NewCredentials()), dialOptions) if err != nil { logrus.Errorf("connect to peer %s error: %s", workerInfo.GrpcPeer, err) return } - peerClient.peerSession = pb.NewWorkerClient(peerClient.peerConn) + if !PeerMgr.SetPeerConnection(peerClient, peerConnection) { + logrus.Error("peer registration was replaced during reconnect") + return + } ctx, cancel2 := context.WithTimeout(context.Background(), 5*time.Second) defer cancel2() resp, err := peerClient.peerSession.SayHelloPeer( @@ -351,7 +366,10 @@ func (s *Service) ReconnectMaster() { logrus.Errorf("create scatter stream peer: %s error: %s", workerInfo.GrpcPeer, err) return } - peerClient.ScatterHandler.SetClient(stream) + if peerClient.ScatterHandler.SetClient(stream) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return + } // install load action ctx = metadata.NewOutgoingContext(context.Background(), md) @@ -360,7 +378,10 @@ func (s *Service) ReconnectMaster() { logrus.Errorf("create load action stream peer: %s error: %s", workerInfo.GrpcPeer, err) return } - peerClient.LoadActionHandler.SetClient(stream2) + if peerClient.LoadActionHandler.SetClient(stream2) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return + } // install step end ctx = metadata.NewOutgoingContext(context.Background(), md) @@ -369,7 +390,10 @@ func (s *Service) ReconnectMaster() { logrus.Errorf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) return } - peerClient.StepEndHandler.SetClient(stream3) + if peerClient.StepEndHandler.SetClient(stream3) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return + } // install setting action ctx = metadata.NewOutgoingContext(context.Background(), md) @@ -378,20 +402,23 @@ func (s *Service) ReconnectMaster() { logrus.Errorf("create step end stream peer: %s error: %s", workerInfo.GrpcPeer, err) return } - peerClient.SettingActionHandler.SetClient(stream4) + if peerClient.SettingActionHandler.SetClient(stream4) == 0 { + logrus.Error("peer registration was replaced while binding streams") + return + } logrus.Infof("say hello peer %s, ok, %s", workerInfo.GrpcPeer, resp.GetStatus()) cancel2() - if peerClient.ScatterHandler.mode == HandlerModeClient { + if peerClient.ScatterHandler.IsClient() { go peerClient.ScatterHandler.RecvHandler(peerClient.Name) } - if peerClient.LoadActionHandler.mode == HandlerModeClient { + if peerClient.LoadActionHandler.IsClient() { go peerClient.LoadActionHandler.RecvHandler(peerClient.Name) } - if peerClient.StepEndHandler.mode == HandlerModeClient { + if peerClient.StepEndHandler.IsClient() { go peerClient.StepEndHandler.RecvHandler(peerClient.Name) } - if peerClient.SettingActionHandler.mode == HandlerModeClient { + if peerClient.SettingActionHandler.IsClient() { go peerClient.SettingActionHandler.RecvHandler(peerClient.Name) } } From 2678ff91b56ae9501fbcdb0e1cf937144e508241 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 05:35:45 +0000 Subject: [PATCH 10/17] test(vermeer): reuse bounded authenticated runtime image smoke --- .github/workflows/codeql-analysis.yml | 13 -- .github/workflows/vermeer-ci.yml | 63 +----- vermeer/README.md | 21 +- vermeer/scripts/test-runtime-image.sh | 20 ++ vermeer/scripts/test_runtime_image.py | 268 ++++++++++++++++++++++++++ 5 files changed, 311 insertions(+), 74 deletions(-) create mode 100755 vermeer/scripts/test-runtime-image.sh create mode 100755 vermeer/scripts/test_runtime_image.py diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index cf85eff6c..6051eb371 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -66,19 +66,6 @@ jobs: - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v3 - with: - output: ${{ runner.temp }}/codeql-results - - # Preserve paths for security findings even when the installation token - # cannot read the repository's code-scanning API. This does not filter results. - - name: Preserve Go CodeQL diagnostics - if: always() && matrix.language == 'go' - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: codeql-go-sarif-${{ github.run_attempt }} - path: ${{ runner.temp }}/codeql-results/go.sarif - if-no-files-found: ignore - retention-days: 1 # Disabled for now because dependency review is unsupported until the # repository dependency graph is enabled. diff --git a/.github/workflows/vermeer-ci.yml b/.github/workflows/vermeer-ci.yml index 813c21efd..11ba4a4c9 100644 --- a/.github/workflows/vermeer-ci.yml +++ b/.github/workflows/vermeer-ci.yml @@ -76,9 +76,9 @@ jobs: run: | go test -race ./apps/auth ./cmd/vermeer-token go test -race ./apps/common - go test -race ./apps/master/workers ./apps/master/services -run 'Test(WorkerAvailability|Native)' + go test -race ./apps/master/workers ./apps/master/services go test -race ./apps/master/bl -run 'Test(WorkerAvailability|HugegraphLoadAuthorization)' - go test -race ./apps/worker -run 'Test(LoadReceivers|RecvVertex|ComputeDispatchCancellation)' + go test -race ./apps/worker go test -race ./apps/structure -run 'Test(VertexStore|GraphPropagates|MemoryVertex|HugegraphSchema)' - name: Build @@ -118,60 +118,5 @@ jobs: docker run --rm --entrypoint test vermeer:ci-check -f /go/bin/release-docs/NOTICE docker run --rm --entrypoint test vermeer:ci-check -d /go/bin/release-docs/licenses - - name: Verify native token helper and invalid factor rejection - run: | - umask 077 - TOKEN_CHECK_DIR=$(mktemp -d) - trap 'rm -rf "${TOKEN_CHECK_DIR}"' EXIT - openssl rand -hex 32 > "${TOKEN_CHECK_DIR}/factor" - docker run --rm -v "${TOKEN_CHECK_DIR}:/probe" \ - --entrypoint /go/bin/vermeer-token vermeer:ci-check \ - -factor-file /probe/factor -output-file /probe/token \ - -user admin -space '$DEFAULT' -i -quiet \ - > "${TOKEN_CHECK_DIR}/stdout" 2> "${TOKEN_CHECK_DIR}/stderr" - test -s "${TOKEN_CHECK_DIR}/token" - test "$(stat -c '%a' "${TOKEN_CHECK_DIR}/token")" = 600 - test ! -s "${TOKEN_CHECK_DIR}/stdout" - test ! -s "${TOKEN_CHECK_DIR}/stderr" - if docker run --rm --entrypoint /go/bin/vermeer-token vermeer:ci-check \ - -factor synthetic-legacy-factor -quiet \ - > "${TOKEN_CHECK_DIR}/stdout" 2> "${TOKEN_CHECK_DIR}/stderr"; then - echo 'runtime token helper accepted a legacy inline factor' >&2 - exit 1 - fi - test ! -s "${TOKEN_CHECK_DIR}/stdout" - if docker run --rm -v "${TOKEN_CHECK_DIR}:/probe" \ - --entrypoint /go/bin/vermeer-token vermeer:ci-check \ - -factor-file /probe/missing -output-file /probe/rejected -quiet \ - > "${TOKEN_CHECK_DIR}/stdout" 2> "${TOKEN_CHECK_DIR}/stderr"; then - echo 'token generation accepted a missing factor file' >&2 - exit 1 - fi - test ! -e "${TOKEN_CHECK_DIR}/rejected" - test ! -s "${TOKEN_CHECK_DIR}/stdout" - if docker run --rm -e VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/missing \ - vermeer:ci-check -env=master -auth=token \ - > "${TOKEN_CHECK_DIR}/master-log" 2>&1; then - echo 'Master accepted a missing factor file' >&2 - exit 1 - fi - if grep -qF -f "${TOKEN_CHECK_DIR}/factor" "${TOKEN_CHECK_DIR}/master-log"; then - echo 'Master log exposed the factor' >&2 - exit 1 - fi - # Exercise the same non-root identity used by the Helm test hook. - # Keep its synthetic factor and token inside a private writable /tmp directory. - docker run --rm --user 100:101 --entrypoint /bin/sh vermeer:ci-check -ec ' - umask 077 - token_dir=$(mktemp -d /tmp/vermeer-token.XXXXXX) - trap '\''rm -rf "${token_dir}"'\'' EXIT - printf "%s" synthetic-nonroot-test-factor > "${token_dir}/factor" - test -x /go/bin/app - /go/bin/vermeer-token -factor-file "${token_dir}/factor" \ - -output-file "${token_dir}/token" -user admin -space '\''$DEFAULT'\'' -i -quiet \ - > "${token_dir}/stdout" 2> "${token_dir}/stderr" - test -s "${token_dir}/token" - test "$(stat -c %a "${token_dir}/token")" = 600 - test ! -s "${token_dir}/stdout" - test ! -s "${token_dir}/stderr" - ' + - name: Verify runtime credentials and native UI login + run: bash scripts/test-runtime-image.sh vermeer:ci-check diff --git a/vermeer/README.md b/vermeer/README.md index b2a9d0d72..275838e43 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -277,7 +277,8 @@ go run ./cmd/vermeer-token -user admin -space '$DEFAULT' \ -factor-file config/token-factor -output-file config/admin-token -i -quiet # Or run in Docker (no local Go required) -docker run --rm -v "$PWD/config:/tokens" --entrypoint /go/bin/vermeer-token \ +docker run --rm --user "$(id -u):$(id -g)" -v "$PWD/config:/tokens" \ + --entrypoint /go/bin/vermeer-token \ hugegraph/vermeer -user admin -space '$DEFAULT' \ -factor-file /tokens/token-factor -output-file /tokens/admin-token -i -quiet ``` @@ -320,7 +321,7 @@ Helm deployments generate and retain this credential separately from the interna The file must be nonempty UTF-8 on one line, without CR/LF, and cannot contain only whitespace. All other bytes are preserved, including leading or trailing spaces. A configured missing, unreadable or invalid file stops Master; it never disables authentication. -Open `/ui/master.html` and paste this credential into **Admin password or token**. No user database or extra account is created: +Open `/` and paste this credential into **Admin password or token**. No user database or extra account is created: successful password login issues an `admin` session in `$DEFAULT`, signed with v2 HMAC-SHA256 and valid for 24 hours. Its cookie is HttpOnly and uses Secure for HTTPS, including requests forwarded with `X-Forwarded-Proto: https`. The existing login request field remains `token`; it accepts either the short password or a valid opaque token. @@ -411,6 +412,10 @@ run_mode = worker worker_group = default ``` +For advanced connection troubleshooting, `grpc_dial_timeout` or `VERMEER_GRPC_DIAL_TIMEOUT` bounds Worker peer dialing. +It defaults to `5s` and must be a positive Go duration such as `500ms` or `10s`. An unreachable bootstrap peer fails +within this budget so the process supervisor or Kubernetes can retry; a healthy HTTP listener alone does not prove Worker availability. + ## Available Algorithms | Algorithm | Category | Description | @@ -713,6 +718,18 @@ Access the Web UI dashboard at `http://master-ip:6688/` for: - Check network latency between master and workers - Profile algorithm with built-in metrics (access via API) +## Runtime image validation + +Validate a built image with Docker and host Python 3 (standard library only), without PD or Store: + +```bash +bash scripts/test-runtime-image.sh hugegraph/vermeer:your-build +``` + +The check covers non-root helper execution, private v2 token files, invalid-configuration startup failures, +short-password UI login, cookie/API authentication and embedded login assets. It binds Master only to local loopback, +uses temporary test credentials, applies bounded timeouts and removes its containers and files on exit. + ## Links - [Project Homepage](https://hugegraph.apache.org/docs/quickstart/hugegraph-computer/) diff --git a/vermeer/scripts/test-runtime-image.sh b/vermeer/scripts/test-runtime-image.sh new file mode 100755 index 000000000..523dbdee4 --- /dev/null +++ b/vermeer/scripts/test-runtime-image.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +set -euo pipefail +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +exec python3 "${SCRIPT_DIR}/test_runtime_image.py" "$@" diff --git a/vermeer/scripts/test_runtime_image.py b/vermeer/scripts/test_runtime_image.py new file mode 100755 index 000000000..48a179a99 --- /dev/null +++ b/vermeer/scripts/test_runtime_image.py @@ -0,0 +1,268 @@ +#!/usr/bin/env python3 +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Check the real runtime image without PD/Store or externally exposed ports.""" + +import argparse +import hashlib +import hmac +import http.cookiejar +import http.cookies +import json +import os +from pathlib import Path +import secrets +import string +import subprocess +import sys +import tempfile +import time +import urllib.error +import urllib.request +import uuid + + +class SmokeFailure(Exception): + """A diagnostic that never includes credential values or raw service output.""" + + +def require(condition, message): + if not condition: + raise SmokeFailure(message) + + +def docker(arguments, timeout=30, success=True): + try: + result = subprocess.run( + ["docker", *arguments], capture_output=True, timeout=timeout, check=False + ) + except subprocess.TimeoutExpired as error: + raise SmokeFailure("Docker operation exceeded its bounded timeout") from error + if success and result.returncode != 0: + raise SmokeFailure("Docker operation failed; inspect the candidate image locally") + return result + + +def write_private(path, contents): + descriptor = os.open(path, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600) + with os.fdopen(descriptor, "wb") as stream: + stream.write(contents) + + +def decode_token(encoded, factor): + alphabet = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" + number = 0 + try: + for character in encoded: + number = number * 58 + alphabet.index(character) + raw = number.to_bytes((number.bit_length() + 7) // 8, "big") + token = json.loads(raw) + payload = [token["u"], token["s"], token["n"], token["c"], token["t"]] + source = b"vermeer-token-v2\x00" + json.dumps( + payload, ensure_ascii=False, separators=(",", ":") + ).encode() + expected = "v2:" + hmac.new(factor.encode(), source, hashlib.sha256).hexdigest() + require(hmac.compare_digest(token["d"], expected), "runtime token has an invalid v2 signature") + return token + except (ValueError, KeyError, TypeError) as error: + raise SmokeFailure("runtime token is not a valid opaque v2 credential") from error + + +def opener(cookie_jar=None): + # The only HTTP destination is the Docker port bound to local loopback. + handlers = [urllib.request.ProxyHandler({})] + if cookie_jar is not None: + handlers.append(urllib.request.HTTPCookieProcessor(cookie_jar)) + return urllib.request.build_opener(*handlers) + + +def request(client, base, path, body=None, bearer=None): + headers = {} + if body is not None: + headers["Content-Type"] = "application/json" + body = json.dumps(body).encode() + if bearer is not None: + headers["Authorization"] = "Bearer " + bearer + try: + response = client.open(urllib.request.Request(base + path, data=body, headers=headers), timeout=3) + except urllib.error.HTTPError as error: + response = error + with response: + return response.status, response.headers, response.read(2 * 1024 * 1024) + + +def check_image(image): + containers = [] + prefix = "vermeer-image-smoke-" + uuid.uuid4().hex[:12] + + def run(arguments, *, name, timeout=30, success=True): + container_name = prefix + "-" + name + containers.append(container_name) + try: + return docker(["run", "--name", container_name, "--rm", *arguments], timeout, success) + except SmokeFailure as error: + raise SmokeFailure("runtime image " + name + " check failed: " + str(error)) from error + + try: + if docker(["image", "inspect", image], success=False).returncode != 0: + docker(["pull", image], timeout=180) + modes = run( + ["--entrypoint", "stat", image, "-c", "%a", "/go/bin/app", "/go/bin/vermeer-token"], + name="modes", + ).stdout.splitlines() + require(modes == [b"755", b"755"], "runtime app and helper must both be executable mode 0755") + nonroot = run( + [ + "--user", "100:101", "--read-only", "--tmpfs", "/tmp:rw,nosuid,nodev,mode=1777", + "--entrypoint", "/bin/sh", image, "-ec", + """umask 077 +work=$(mktemp -d /tmp/vermeer-token.XXXXXX) +trap 'rm -rf "$work"' EXIT +printf '%s' synthetic-nonroot-runtime-factor > "$work/factor" +/go/bin/vermeer-token -factor-file "$work/factor" -output-file "$work/token" \ + -user admin -space '$DEFAULT' -client runtime-smoke -i -quiet > "$work/out" 2> "$work/err" +test ! -s "$work/out" +test ! -s "$work/err" +test "$(stat -c %a "$work/token")" = 600 +cat "$work/token" +""", + ], + name="nonroot", + ) + nonroot_token = decode_token(nonroot.stdout.decode().strip(), "synthetic-nonroot-runtime-factor") + require(nonroot_token["t"] == 2**63 - 1, "helper must support long-lived v2 service tokens") + rejected = run( + ["--entrypoint", "/go/bin/vermeer-token", image, "-factor", "synthetic-legacy-factor", "-quiet"], + name="legacy-flag", success=False, + ) + require(rejected.returncode != 0 and not rejected.stdout and + b"flag provided but not defined: -factor" in rejected.stderr, + "runtime helper must reject legacy inline factors") + missing = run( + ["--entrypoint", "/go/bin/vermeer-token", image, "-factor-file", "/missing-factor", "-quiet"], + name="missing-factor", success=False, + ) + require(missing.returncode != 0 and not missing.stdout and b"read token factor file" in missing.stderr, + "helper must fail closed for a missing factor file") + + with tempfile.TemporaryDirectory(prefix="vermeer-runtime-smoke-") as directory: + private = Path(directory) + factor = secrets.token_hex(32) + password = "".join(secrets.choice(string.ascii_letters + string.digits) for _ in range(16)) + write_private(private / "factor", factor.encode()) + write_private(private / "password", password.encode()) + write_private(private / "empty-password", b"") + mount = ["-v", f"{private}:/probe:ro"] + for label, environment, arguments in [ + ("master-factor", ["VERMEER_AUTH_TOKEN_FACTOR_FILE=/missing-factor"], ["-auth=token"]), + ("master-password", ["VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/factor", + "VERMEER_ADMIN_PASSWORD_FILE=/missing-password"], ["-auth=token"]), + ("master-empty", ["VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/factor", + "VERMEER_ADMIN_PASSWORD_FILE=/probe/empty-password"], ["-auth=token"]), + ("master-none", ["VERMEER_ADMIN_PASSWORD_FILE=/probe/password"], ["-auth=none"]), + ]: + environment_args = [item for value in environment for item in ("-e", value)] + failed = run([*mount, *environment_args, image, "-env=master", *arguments], + name=label, timeout=15, success=False) + observed = failed.stdout + failed.stderr + require(failed.returncode != 0 and b"token authentication initialization failed" in observed, + "Master must reject invalid deployment credentials before listening: " + label) + require(factor.encode() not in observed and password.encode() not in observed, + "startup errors must not expose credentials") + service = run( + [*mount, "--entrypoint", "/go/bin/vermeer-token", image, + "-factor-file", "/probe/factor", "-user", "admin", "-space", "$DEFAULT", + "-client", "runtime-smoke", "-i", "-quiet"], name="service-token", + ).stdout.decode().strip() + decode_token(service, factor) + master_name = prefix + "-master" + containers.append(master_name) + docker([ + "run", "--rm", "-d", "--name", master_name, + "-p", "127.0.0.1::6688", *mount, + "-e", "VERMEER_AUTH_TOKEN_FACTOR_FILE=/probe/factor", + "-e", "VERMEER_ADMIN_PASSWORD_FILE=/probe/password", + image, "-env=master", "-auth=token", + ]) + address = docker(["port", master_name, "6688/tcp"]).stdout.decode().strip() + require(address.startswith("127.0.0.1:"), "smoke Master must bind only local loopback") + base = "http://" + address + anonymous = opener() + deadline = time.monotonic() + 45 + while True: + try: + if request(anonymous, base, "/api/v1/healthcheck")[0] == 200: + break + except (OSError, urllib.error.URLError): + pass + require(time.monotonic() < deadline, "smoke Master did not become healthy within 45 seconds") + time.sleep(0.2) + status, _, html = request(anonymous, base, "/") + require(status == 200 and b"Admin password or token" in html and b'id="admin_token"' in html, + "production image must embed the short-credential login UI") + for path in ("/api/v1/master", "/api/v1/workers"): + require(request(anonymous, base, path)[0] == 401, "native metadata must reject anonymous requests") + require(request(anonymous, base, path, bearer=password)[0] == 401, + "short UI password must not act as an API Bearer token") + require(request(anonymous, base, path, bearer=service)[0] == 200, + "long-lived v2 service token must retain API access") + wrong = "x" * 16 if password != "x" * 16 else "y" * 16 + bad_status, bad_headers, bad_body = request(anonymous, base, "/api/v1/login", {"token": wrong}) + require(bad_status == 401 and not bad_headers.get("Set-Cookie"), + "incorrect administrator password must not issue a session") + session_client = opener(http.cookiejar.CookieJar()) + status, headers, body = request(session_client, base, "/api/v1/login", {"token": password}) + cookie = http.cookies.SimpleCookie() + cookie.load(headers.get("Set-Cookie", "")) + require(status == 200 and "vat" in cookie and cookie["vat"]["httponly"] and + cookie["vat"]["max-age"] == "86400", "password login must issue an HttpOnly 24-hour cookie") + cookie_value = cookie["vat"].value + session = decode_token(cookie_value, factor) + require(session["u"] == "admin" and session["s"] == "$DEFAULT" and + abs(time.time() - session["t"]) < 10, "UI session must carry the current admin identity") + for path in ("/api/v1/master", "/api/v1/workers"): + require(request(session_client, base, path)[0] == 200, "native UI session must authorize metadata") + require(request(anonymous, base, "/api/v1/login", {"token": service})[0] == 200, + "long opaque token login must remain supported") + observed_logs = docker(["logs", master_name]) + logs = observed_logs.stdout + observed_logs.stderr + for sensitive in (factor, password, service, cookie_value): + require(sensitive.encode() not in logs + body + bad_body, + "service output must not expose credentials or raw cookies") + finally: + if containers: + docker(["rm", "-f", *containers], timeout=15, success=False) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("image", help="locally built image tag or published image URL/digest") + arguments = parser.parse_args() + require(arguments.image and not arguments.image.startswith("-"), "an image reference is required") + check_image(arguments.image) + print("runtime image smoke passed: executable helpers, v2 tokens, fail-closed startup and authenticated UI") + + +if __name__ == "__main__": + try: + main() + except SmokeFailure as error: + print("runtime image smoke failed: " + str(error), file=sys.stderr) + sys.exit(1) + except Exception as error: + print("runtime image smoke failed with " + type(error).__name__, file=sys.stderr) + sys.exit(1) From e26e73a805520546a15d6776019b5549ec7b3981 Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 28 Oct 2026 14:27:08 +0800 Subject: [PATCH 11/17] fix(vermeer): reject weak file signing keys --- vermeer/apps/auth/factor_file.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/vermeer/apps/auth/factor_file.go b/vermeer/apps/auth/factor_file.go index 37dac33e2..4434ff03a 100644 --- a/vermeer/apps/auth/factor_file.go +++ b/vermeer/apps/auth/factor_file.go @@ -25,6 +25,8 @@ import ( "unicode/utf8" ) +const minTokenSigningKeyCharacters = 16 + // ReadTokenFactorFile is shared by the Master and the native token generator. // A single terminal newline is accepted; no other whitespace is changed. func ReadTokenFactorFile(file string) (string, error) { @@ -40,9 +42,10 @@ func ReadTokenFactorFile(file string) (string, error) { } func validateTokenSigningKey(factor string) error { - if !utf8.ValidString(factor) || utf8.RuneCountInString(factor) < 4 || + if !utf8.ValidString(factor) || + utf8.RuneCountInString(factor) < minTokenSigningKeyCharacters || strings.TrimSpace(factor) == "" || strings.ContainsAny(factor, "\r\n") { - return errors.New("token signing key must contain one UTF-8 line of at least four characters") + return errors.New("token signing key must contain one UTF-8 line of at least 16 characters") } return nil } From 354907ea0d505866dc5a0aab6a4f624235d4cfbb Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 28 Oct 2026 14:27:39 +0800 Subject: [PATCH 12/17] test(vermeer): cover signing key strength boundary --- vermeer/apps/auth/factor_file_test.go | 29 +++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/vermeer/apps/auth/factor_file_test.go b/vermeer/apps/auth/factor_file_test.go index 55278c684..d5d971ea3 100644 --- a/vermeer/apps/auth/factor_file_test.go +++ b/vermeer/apps/auth/factor_file_test.go @@ -9,10 +9,10 @@ You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. */ package auth @@ -58,8 +58,10 @@ func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { } for _, test := range []struct{ name, data string }{ {"missing", ""}, {"empty", ""}, {"invalid", "synthetic-secret\nsecond-line"}, - {"short-ascii", "abc"}, {"short-unicode", "密碼"}, {"invalid-utf8", "abcd\xff"}, - {"whitespace-only", " "}, + {"short-ascii", strings.Repeat("a", minTokenSigningKeyCharacters-1)}, + {"short-unicode", strings.Repeat("密", minTokenSigningKeyCharacters-1)}, + {"invalid-utf8", strings.Repeat("a", minTokenSigningKeyCharacters) + "\xff"}, + {"whitespace-only", strings.Repeat(" ", minTokenSigningKeyCharacters)}, } { t.Run(test.name, func(t *testing.T) { file := filepath.Join(t.TempDir(), "factor") @@ -83,3 +85,18 @@ func TestTokenFactorFilePrecedenceAndFailClosed(t *testing.T) { t.Fatal("unconfigured deployments must preserve inline factors") } } + +func TestTokenFactorFileAcceptsMinimumLength(t *testing.T) { + factor := strings.Repeat("x", minTokenSigningKeyCharacters) + file := filepath.Join(t.TempDir(), "factor") + if err := os.WriteFile(file, []byte(factor+"\n"), 0600); err != nil { + t.Fatal(err) + } + actual, err := ReadTokenFactorFile(file) + if err != nil { + t.Fatal(err) + } + if actual != factor { + t.Fatalf("unexpected factor after reading file: %q", actual) + } +} From 12e2873a590841b0cbf0b463713adf5e805d885c Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 28 Oct 2026 14:30:23 +0800 Subject: [PATCH 13/17] fix(vermeer): validate Master worker stream metadata --- vermeer/apps/master/worker_stream_metadata.go | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 vermeer/apps/master/worker_stream_metadata.go diff --git a/vermeer/apps/master/worker_stream_metadata.go b/vermeer/apps/master/worker_stream_metadata.go new file mode 100644 index 000000000..984a176d7 --- /dev/null +++ b/vermeer/apps/master/worker_stream_metadata.go @@ -0,0 +1,69 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package master + +import ( + "context" + "strings" + "vermeer/apps/master/workers" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +var registeredWorkerStreamMethods = map[string]struct{}{ + "/master.Master/LoadGraphTask": {}, + "/master.Master/ComputeTask": {}, + "/master.Master/SuperStep": {}, +} + +func validateWorkerStreamMetadata(ctx context.Context, fullMethod string) error { + if _, required := registeredWorkerStreamMethods[fullMethod]; !required { + return nil + } + + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return status.Error(codes.InvalidArgument, + "worker_name metadata is required") + } + names := md.Get("worker_name") + if len(names) != 1 || names[0] == "" || strings.TrimSpace(names[0]) != names[0] { + return status.Error(codes.InvalidArgument, + "worker_name metadata must contain exactly one non-empty value") + } + if workers.WorkerManager.GetWorker(names[0]) == nil { + return status.Error(codes.FailedPrecondition, + "worker must register before opening task streams") + } + return nil +} + +func validateWorkerStream( + srv any, + stream grpc.ServerStream, + info *grpc.StreamServerInfo, + handler grpc.StreamHandler, +) error { + if err := validateWorkerStreamMetadata(stream.Context(), info.FullMethod); err != nil { + return err + } + return handler(srv, stream) +} From 931a1336ebb408dd3bb52d9685fb68e8dffe7104 Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 28 Oct 2026 14:30:41 +0800 Subject: [PATCH 14/17] fix(vermeer): enforce worker registration on Master streams --- vermeer/apps/master/master_main.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vermeer/apps/master/master_main.go b/vermeer/apps/master/master_main.go index 534f7efb9..dd5f0a5e9 100644 --- a/vermeer/apps/master/master_main.go +++ b/vermeer/apps/master/master_main.go @@ -79,7 +79,9 @@ func Main() { } serverOptions := []grpc.ServerOption{ grpc.MaxRecvMsgSize(4 * 1024 * 1024 * 1024), - grpc.MaxSendMsgSize(4 * 1024 * 1024 * 1024)} + grpc.MaxSendMsgSize(4 * 1024 * 1024 * 1024), + grpc.StreamInterceptor(validateWorkerStream), + } // create grpc server grpcServer := grpc.NewServer(serverOptions...) From 65e2958699ebed25fa03093ed8cbf5c4164e8c3a Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 28 Oct 2026 14:30:54 +0800 Subject: [PATCH 15/17] test(vermeer): reject invalid Master stream identities --- .../master/worker_stream_metadata_test.go | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 vermeer/apps/master/worker_stream_metadata_test.go diff --git a/vermeer/apps/master/worker_stream_metadata_test.go b/vermeer/apps/master/worker_stream_metadata_test.go new file mode 100644 index 000000000..35fd1c765 --- /dev/null +++ b/vermeer/apps/master/worker_stream_metadata_test.go @@ -0,0 +1,72 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package master + +import ( + "context" + "testing" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +func TestValidateWorkerStreamMetadataRejectsInvalidIdentity(t *testing.T) { + methods := []string{ + "/master.Master/LoadGraphTask", + "/master.Master/ComputeTask", + "/master.Master/SuperStep", + } + invalid := []struct { + name string + ctx context.Context + }{ + {"missing", context.Background()}, + {"empty", metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", ""))}, + {"padded", metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", " worker "))}, + {"duplicate", metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", "one", "worker_name", "two"))}, + } + + for _, method := range methods { + for _, test := range invalid { + t.Run(method+"/"+test.name, func(t *testing.T) { + if code := status.Code(validateWorkerStreamMetadata(test.ctx, method)); + code != codes.InvalidArgument { + t.Fatalf("expected InvalidArgument, got %s", code) + } + }) + } + + unknown := metadata.NewIncomingContext(context.Background(), + metadata.Pairs("worker_name", "missing-worker-metadata-test")) + if code := status.Code(validateWorkerStreamMetadata(unknown, method)); + code != codes.FailedPrecondition { + t.Fatalf("%s: expected FailedPrecondition, got %s", method, code) + } + } +} + +func TestValidateWorkerStreamMetadataIgnoresOtherRPCs(t *testing.T) { + if err := validateWorkerStreamMetadata(context.Background(), + "/master.Master/FetchLoadPart"); err != nil { + t.Fatalf("unrelated RPC must not require stream metadata: %v", err) + } +} From ceaba5eab1ca6becde681c86173d0f231e5a1d01 Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 28 Oct 2026 14:35:10 +0800 Subject: [PATCH 16/17] docs(vermeer): document signing key strength --- vermeer/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vermeer/README.md b/vermeer/README.md index 275838e43..a2abb2754 100644 --- a/vermeer/README.md +++ b/vermeer/README.md @@ -259,7 +259,8 @@ openssl rand -hex 32 > config/token-factor Set `VERMEER_AUTH_TOKEN_FACTOR_FILE=/path/to/token-factor`, or set `auth_token_factor_file` in Master's INI file. The environment file takes precedence over the INI file, and either file takes precedence over `auth_token_factor`. -The file must contain one valid UTF-8 line with at least four characters and cannot contain only whitespace. +The file must contain one valid UTF-8 line with at least 16 characters and cannot contain only whitespace. +Use a randomly generated value; the example above provides 32 random bytes encoded as 64 hexadecimal characters. A terminal LF or CRLF is accepted; other spaces are preserved. An invalid configured file stops Master before it opens its listeners; it never falls back to an inline or default factor. @@ -744,4 +745,4 @@ See the main [Contributing Guide](../README.md#contributing) for how to contribu ## License -Vermeer is part of Apache HugeGraph-Computer, licensed under [Apache 2.0 License](https://github.com/apache/hugegraph-computer/blob/master/LICENSE). +Vermeer is part of Apache HugeGraph-Computer, licensed under [Apache 2.0 License](https://github.com/apache/hugegraph-computer/blob/master/LICENSE). \ No newline at end of file From 30b67d93e7d097374341cf46ee6a0703421512b1 Mon Sep 17 00:00:00 2001 From: orion Date: Wed, 28 Oct 2026 07:53:58 +0000 Subject: [PATCH 17/17] fix(vermeer): show login errors before generic unauthorized prompts --- vermeer/ui/ui/lib/functions.js | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/vermeer/ui/ui/lib/functions.js b/vermeer/ui/ui/lib/functions.js index bbcfe2575..20206119b 100644 --- a/vermeer/ui/ui/lib/functions.js +++ b/vermeer/ui/ui/lib/functions.js @@ -29,6 +29,10 @@ function (data) { console.log(data.message); location.reload(true); + }, + function (err) { + const message = err.responseJSON && err.responseJSON.message; + showDelModal(message || 'Login failed.'); } ); }, @@ -141,22 +145,22 @@ } }, error: function (err) { - if (err.status === 401) { - showDelModal('Login First!'); + if (error) { + if (caller) { + error.apply(caller, [err]); + } else { + error(err); + } return; - } else { - console.log('ajax request failed:', err); } - if (!error) { - let data = JSON.parse(err.responseText); - showDelModal(data.message); + if (err.status === 401) { + showDelModal('Login First!'); return; - } - if (caller) { - error.apply(caller, [err]); } else { - error(err); + console.log('ajax request failed:', err.status); } + let data = JSON.parse(err.responseText); + showDelModal(data.message); } }); }