mirror of
https://gitee.com/milvus-io/milvus.git
synced 2026-02-02 01:06:41 +08:00
enhance: add security controls for /expr endpoin (#46753)
related: https://github.com/milvus-io/milvus/issues/46442 core changes: - Add config (default: false) to disable /expr endpoint by default - On Proxy nodes, require root user authentication via HTTP Basic Auth when enabled - On non-Proxy nodes, keep original auth parameter behavior for backward compatibility - Add HasRegistered() and AuthBypass to expr package for node type detection --------- Signed-off-by: shaoting-huang <shaoting.huang@zilliz.com>
This commit is contained in:
parent
2a647751b1
commit
7bcd3b1061
@ -975,6 +975,7 @@ common:
|
||||
defaultRootPassword: Milvus
|
||||
rootShouldBindRole: false # Whether the root user should bind a role when the authorization is enabled.
|
||||
enablePublicPrivilege: true # Whether to enable public privilege
|
||||
exprEnabled: false # Whether to enable the /expr endpoint for debugging. When enabled, only root user can access it via HTTP Basic Auth on Proxy nodes.
|
||||
rbac:
|
||||
overrideBuiltInPrivilegeGroups:
|
||||
enabled: false # Whether to override build-in privilege groups
|
||||
|
||||
@ -17,6 +17,7 @@
|
||||
package http
|
||||
|
||||
import (
|
||||
"context"
|
||||
"embed"
|
||||
"fmt"
|
||||
"net/http"
|
||||
@ -44,8 +45,18 @@ const (
|
||||
var (
|
||||
metricsServer *http.ServeMux
|
||||
server *http.Server
|
||||
|
||||
// passwordVerifyFunc is a callback function to verify user password.
|
||||
// This is set by the proxy package to avoid circular dependency.
|
||||
passwordVerifyFunc func(ctx context.Context, username, password string) bool
|
||||
)
|
||||
|
||||
// RegisterPasswordVerifyFunc registers a function to verify user password.
|
||||
// This should be called by the proxy package during initialization.
|
||||
func RegisterPasswordVerifyFunc(fn func(ctx context.Context, username, password string) bool) {
|
||||
passwordVerifyFunc = fn
|
||||
}
|
||||
|
||||
// Embedding all static files of webui folder to binary
|
||||
//
|
||||
//go:embed webui
|
||||
@ -87,8 +98,32 @@ func registerDefaults() {
|
||||
Register(&Handler{
|
||||
Path: ExprPath,
|
||||
Handler: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
// Check if expr endpoint is enabled
|
||||
if !paramtable.Get().CommonCfg.ExprEnabled.GetAsBool() {
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"msg": "expr endpoint is disabled. Set common.security.exprEnabled to true to enable it."}`))
|
||||
return
|
||||
}
|
||||
|
||||
code := req.URL.Query().Get("code")
|
||||
auth := req.URL.Query().Get("auth")
|
||||
var auth string
|
||||
|
||||
// Only Proxy nodes can access /expr endpoint
|
||||
if !expr.HasRegistered("proxy") || passwordVerifyFunc == nil {
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"msg": "/expr endpoint is only available on Proxy nodes"}`))
|
||||
return
|
||||
}
|
||||
|
||||
// On Proxy node: require root user authentication via HTTP Basic Auth
|
||||
if err := checkExprRootAuth(req); err != nil {
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
w.Write([]byte(fmt.Sprintf(`{"msg": "%s"}`, err.Error())))
|
||||
return
|
||||
}
|
||||
// Use bypass since we've already authenticated
|
||||
auth = expr.AuthBypass
|
||||
|
||||
output, err := expr.Exec(code, auth)
|
||||
if err != nil {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
@ -271,3 +306,42 @@ func getHTTPAddr() string {
|
||||
|
||||
return fmt.Sprintf(":%s", port)
|
||||
}
|
||||
|
||||
// checkExprRootAuth verifies that the request is from the root user.
|
||||
// It supports HTTP Basic Auth and Bearer token formats.
|
||||
func checkExprRootAuth(req *http.Request) error {
|
||||
// Try HTTP Basic Auth first
|
||||
username, password, ok := req.BasicAuth()
|
||||
if !ok {
|
||||
// Try Bearer token format: "user:password"
|
||||
auth := req.Header.Get("Authorization")
|
||||
auth = strings.TrimPrefix(auth, "Bearer ")
|
||||
parts := strings.SplitN(auth, ":", 2)
|
||||
if len(parts) == 2 {
|
||||
username, password = parts[0], parts[1]
|
||||
ok = true
|
||||
}
|
||||
}
|
||||
|
||||
if !ok || username == "" || password == "" {
|
||||
return fmt.Errorf("authentication required. Use HTTP Basic Auth with root credentials")
|
||||
}
|
||||
|
||||
// Only root user can access /expr
|
||||
if username != "root" {
|
||||
log.Warn("non-root user attempted to access /expr", zap.String("username", username))
|
||||
return fmt.Errorf("only root user can access /expr endpoint")
|
||||
}
|
||||
|
||||
// Verify root password
|
||||
if passwordVerifyFunc == nil {
|
||||
return fmt.Errorf("password verification not available")
|
||||
}
|
||||
if !passwordVerifyFunc(context.Background(), username, password) {
|
||||
log.Warn("invalid root password for /expr access")
|
||||
return fmt.Errorf("invalid root password")
|
||||
}
|
||||
|
||||
log.Info("root user authenticated for /expr access")
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -203,26 +203,96 @@ func (suite *HTTPServerTestSuite) TestPprofHandler() {
|
||||
func (suite *HTTPServerTestSuite) TestExprHandler() {
|
||||
expr.Init()
|
||||
expr.Register("foo", "hello")
|
||||
suite.Run("fail", func() {
|
||||
|
||||
suite.Run("disabled_by_default", func() {
|
||||
// By default, exprEnabled is false, should return 403
|
||||
paramtable.Get().Save("common.security.exprEnabled", "false")
|
||||
url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo&auth=by-dev"
|
||||
client := http.Client{}
|
||||
req, _ := http.NewRequest(http.MethodGet, url, nil)
|
||||
resp, err := client.Do(req)
|
||||
suite.Nil(err)
|
||||
defer resp.Body.Close()
|
||||
suite.Equal(http.StatusForbidden, resp.StatusCode)
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
suite.True(strings.Contains(string(body), "expr endpoint is disabled"))
|
||||
})
|
||||
|
||||
suite.Run("disabled_on_non_proxy_nodes", func() {
|
||||
// When enabled but not on Proxy node (no proxy registered, no passwordVerifyFunc),
|
||||
// it should return 403 Forbidden
|
||||
paramtable.Get().Save("common.security.exprEnabled", "true")
|
||||
|
||||
// Should be forbidden on non-Proxy nodes
|
||||
url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo&auth=by-dev"
|
||||
client := http.Client{}
|
||||
req, _ := http.NewRequest(http.MethodGet, url, nil)
|
||||
resp, err := client.Do(req)
|
||||
suite.Nil(err)
|
||||
defer resp.Body.Close()
|
||||
suite.Equal(http.StatusForbidden, resp.StatusCode)
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
suite.True(strings.Contains(string(body), "only available on Proxy nodes"))
|
||||
})
|
||||
|
||||
suite.Run("enabled_on_proxy_requires_root_auth", func() {
|
||||
// When enabled on Proxy node (proxy registered and passwordVerifyFunc set),
|
||||
// it should require root user authentication
|
||||
paramtable.Get().Save("common.security.exprEnabled", "true")
|
||||
expr.Register("proxy", "mock_proxy")
|
||||
|
||||
// Register a mock password verify function
|
||||
RegisterPasswordVerifyFunc(func(ctx context.Context, username, password string) bool {
|
||||
return username == "root" && password == "Milvus"
|
||||
})
|
||||
|
||||
// Without auth header - should fail with 401
|
||||
url := "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo"
|
||||
client := http.Client{}
|
||||
req, _ := http.NewRequest(http.MethodGet, url, nil)
|
||||
resp, err := client.Do(req)
|
||||
suite.Nil(err)
|
||||
defer resp.Body.Close()
|
||||
suite.Equal(http.StatusUnauthorized, resp.StatusCode)
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
suite.True(strings.Contains(string(body), "failed to execute"))
|
||||
})
|
||||
suite.Run("success", func() {
|
||||
url := "http://localhost:" + DefaultListenPort + ExprPath + "?auth=by-dev&code=foo"
|
||||
client := http.Client{}
|
||||
req, _ := http.NewRequest(http.MethodGet, url, nil)
|
||||
resp, err := client.Do(req)
|
||||
suite.True(strings.Contains(string(body), "authentication required"))
|
||||
|
||||
// With non-root user - should fail with 401
|
||||
url = "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo"
|
||||
req, _ = http.NewRequest(http.MethodGet, url, nil)
|
||||
req.SetBasicAuth("admin", "password")
|
||||
resp, err = client.Do(req)
|
||||
suite.Nil(err)
|
||||
defer resp.Body.Close()
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
suite.Equal(http.StatusUnauthorized, resp.StatusCode)
|
||||
body, _ = io.ReadAll(resp.Body)
|
||||
suite.True(strings.Contains(string(body), "only root user"))
|
||||
|
||||
// With root user but wrong password - should fail with 401
|
||||
url = "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo"
|
||||
req, _ = http.NewRequest(http.MethodGet, url, nil)
|
||||
req.SetBasicAuth("root", "wrong_password")
|
||||
resp, err = client.Do(req)
|
||||
suite.Nil(err)
|
||||
defer resp.Body.Close()
|
||||
suite.Equal(http.StatusUnauthorized, resp.StatusCode)
|
||||
body, _ = io.ReadAll(resp.Body)
|
||||
suite.True(strings.Contains(string(body), "invalid root password"))
|
||||
|
||||
// With correct root credentials - should succeed
|
||||
url = "http://localhost:" + DefaultListenPort + ExprPath + "?code=foo"
|
||||
req, _ = http.NewRequest(http.MethodGet, url, nil)
|
||||
req.SetBasicAuth("root", "Milvus")
|
||||
resp, err = client.Do(req)
|
||||
suite.Nil(err)
|
||||
defer resp.Body.Close()
|
||||
suite.Equal(http.StatusOK, resp.StatusCode)
|
||||
body, _ = io.ReadAll(resp.Body)
|
||||
suite.True(strings.Contains(string(body), "hello"))
|
||||
})
|
||||
|
||||
// Reset config
|
||||
paramtable.Get().Save("common.security.exprEnabled", "false")
|
||||
}
|
||||
|
||||
func TestHTTPServerSuite(t *testing.T) {
|
||||
|
||||
@ -30,6 +30,7 @@ import (
|
||||
"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v2/milvuspb"
|
||||
"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
|
||||
internalhttp "github.com/milvus-io/milvus/internal/http"
|
||||
"github.com/milvus-io/milvus/internal/proxy/privilege"
|
||||
"github.com/milvus-io/milvus/internal/types"
|
||||
"github.com/milvus-io/milvus/pkg/v2/common"
|
||||
@ -376,6 +377,9 @@ func InitMetaCache(ctx context.Context, mixCoord types.MixCoordClient) error {
|
||||
return err
|
||||
}
|
||||
|
||||
// Register password verify function for /expr endpoint authentication
|
||||
internalhttp.RegisterPasswordVerifyFunc(PasswordVerify)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@ -33,6 +33,12 @@ import (
|
||||
"github.com/milvus-io/milvus/pkg/v2/util/paramtable"
|
||||
)
|
||||
|
||||
const (
|
||||
// AuthBypass is a special auth value that skips the auth check.
|
||||
// This should only be used when authentication has already been verified externally.
|
||||
AuthBypass = "__bypass__"
|
||||
)
|
||||
|
||||
var (
|
||||
v *vm.VM
|
||||
env map[string]any
|
||||
@ -60,6 +66,16 @@ func Register(key string, value any) {
|
||||
}
|
||||
}
|
||||
|
||||
// HasRegistered checks if a key has been registered in the expr environment.
|
||||
// This is useful for determining which component is running (e.g., checking if "proxy" is registered).
|
||||
func HasRegistered(key string) bool {
|
||||
if env == nil {
|
||||
return false
|
||||
}
|
||||
_, ok := env[key]
|
||||
return ok
|
||||
}
|
||||
|
||||
func Exec(code, auth string) (res string, err error) {
|
||||
defer func() {
|
||||
if e := recover(); e != nil {
|
||||
@ -75,7 +91,8 @@ func Exec(code, auth string) (res string, err error) {
|
||||
if auth == "" {
|
||||
return "", errors.New("the expr auth is empty")
|
||||
}
|
||||
if authKey != auth {
|
||||
// Allow bypass when authentication has been verified externally (e.g., by HTTP handler)
|
||||
if auth != AuthBypass && authKey != auth {
|
||||
return "", errors.New("the expr auth is invalid")
|
||||
}
|
||||
program, err := expr.Compile(code, expr.Env(env), expr.WithContext("ctx"))
|
||||
|
||||
@ -100,4 +100,32 @@ func TestExec(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, fmt.Sprintf("%d", innerSize(mockMessage)), out)
|
||||
})
|
||||
|
||||
t.Run("auth bypass", func(t *testing.T) {
|
||||
// AuthBypass should allow execution without checking the auth key
|
||||
out, err := Exec("foo", AuthBypass)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, "hello", out)
|
||||
})
|
||||
}
|
||||
|
||||
func TestHasRegistered(t *testing.T) {
|
||||
// Before init, should return false
|
||||
env = nil
|
||||
assert.False(t, HasRegistered("foo"))
|
||||
|
||||
// After init
|
||||
Init()
|
||||
Register("testKey", "testValue")
|
||||
|
||||
// Registered key should return true
|
||||
assert.True(t, HasRegistered("testKey"))
|
||||
|
||||
// Non-registered key should return false
|
||||
assert.False(t, HasRegistered("nonExistentKey"))
|
||||
|
||||
// Check for "proxy" key (used to detect Proxy node)
|
||||
assert.False(t, HasRegistered("proxy"))
|
||||
Register("proxy", "mock_proxy")
|
||||
assert.True(t, HasRegistered("proxy"))
|
||||
}
|
||||
|
||||
@ -261,6 +261,7 @@ type commonConfig struct {
|
||||
DefaultRootPassword ParamItem `refreshable:"false"`
|
||||
RootShouldBindRole ParamItem `refreshable:"true"`
|
||||
EnablePublicPrivilege ParamItem `refreshable:"false"`
|
||||
ExprEnabled ParamItem `refreshable:"false"`
|
||||
|
||||
ClusterName ParamItem `refreshable:"false"`
|
||||
|
||||
@ -824,6 +825,15 @@ Large numeric passwords require double quotes to avoid yaml parsing precision is
|
||||
}
|
||||
p.EnablePublicPrivilege.Init(base.mgr)
|
||||
|
||||
p.ExprEnabled = ParamItem{
|
||||
Key: "common.security.exprEnabled",
|
||||
Version: "2.6.0",
|
||||
DefaultValue: "false",
|
||||
Doc: "Whether to enable the /expr endpoint for debugging. When enabled, only root user can access it via HTTP Basic Auth on Proxy nodes.",
|
||||
Export: true,
|
||||
}
|
||||
p.ExprEnabled.Init(base.mgr)
|
||||
|
||||
p.ClusterName = ParamItem{
|
||||
Key: "common.cluster.name",
|
||||
Version: "2.0.0",
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user