[cloud] Provide ability to disable executing modified pxl scripts (#2062

)

Summary: [cloud] Provide ability to disable executing modified pxl
scripts

Certain security conscious users are hesitant to use Pixie because
without RBAC anyone with Pixie UI access can write arbitrary BPF code
(bpftrace integration), access or export arbitrary data (modifying pxl
scripts, writing export scripts). This change aims to address this
concern with a global setting to prevent the ability to execute modified
scripts. When an adhoc script is executed, the cloud will hash the
contents of the script and check it against the scripts known to the
scriptmgr service. If it is contained in the scriptmgr service, the
script will be allowed to execute.

Note: this does not prevent users from writing new export scripts. Since
the query broker can source its scripts from a configmap as of #1326,
this is deemed as an appropriate mitigation for cluster admins and I'll
follow up with UI support to reflect that a vizier is in "configmap
mode".

Relevant Issues: N/A

Type of change: /kind feature

Test Plan: The following checks were performed
- [x] New tests verify the scriptmgr and api service changes work
- [x] Skaffold'ed to a testing cluster and verified script modification
is blocked and unmodified scripts are allowed to run. In addition to
this, the code editor in the UI is made read only and shows an
explanation

<details><summary>Screenshots</summary>

![Screen Shot 2025-01-07 at 8 58 34
AM](https://github.com/user-attachments/assets/26c7cc23-08e2-4064-ab15-6172a2593391)
![Screen Shot 2025-01-07 at 8 58 37
AM](https://github.com/user-attachments/assets/8ddf05be-7f83-4935-af0a-44b424a8dc8a)
![Screen Shot 2025-01-07 at 8 58 59
AM](https://github.com/user-attachments/assets/b0033854-758d-4843-98ca-39120f8f8326)
</details>

Changelog Message: Pixie Cloud can now disable executing modified pxl
scripts via the `PL_SCRIPT_MODIFICATION_DISABLED` key in the
`pl-script-bundle-config` ConfigMap. See reference manifests for more
details.

---------

Signed-off-by: Dom Del Nano <[email protected]>

k8s/cloud/base/api_deployment.yaml

@@ -40,6 +40,8 @@ spec:
             name: pl-ory-service-config
         - configMapRef:
             name: pl-auth-connector-config
+        - configMapRef:
+            name: pl-script-bundles-config
         - configMapRef:
             name: pl-errors-config
             optional: true
@@ -59,6 +61,11 @@ spec:
             configMapKeyRef:
               name: pl-service-config
               key: PL_VZMGR_SERVICE
+        - name: PL_SCRIPTMGR_SERVICE
+          valueFrom:
+            configMapKeyRef:
+              name: pl-service-config
+              key: PL_SCRIPTMGR_SERVICE
         - name: PL_AUTH_SERVICE
           valueFrom:
             configMapKeyRef:

k8s/cloud/base/proxy_nginx_config.yaml

@@ -35,6 +35,7 @@ data:
     sub_filter '__CONFIG_DOMAIN_NAME__' "'${domain_name}'";
     sub_filter '__CONFIG_SCRIPT_BUNDLE_URLS__' "'${script_bundle_urls}'";
     sub_filter '__CONFIG_SCRIPT_BUNDLE_DEV__' "'${script_bundle_dev}'";
+    sub_filter '__CONFIG_SCRIPT_MODIFICATION_DISABLED__' "${script_modification_disabled}";
     sub_filter '__SEGMENT_UI_WRITE_KEY__' "'${segment_ui_write_key}'";
     sub_filter '__SEGMENT_ANALYTICS_JS_DOMAIN__' "'segment.${domain_name}'";
     sub_filter '__CONFIG_LD_CLIENT_ID__' "'${ld_client_id}'";
@@ -134,6 +135,7 @@ data:
     set_by_lua_block $segment_cli_write_key  { return os.getenv("PL_SEGMENT_CLI_WRITE_KEY") }
     set_by_lua_block $script_bundle_urls  { return os.getenv("SCRIPT_BUNDLE_URLS") }
     set_by_lua_block $script_bundle_dev  { return os.getenv("SCRIPT_BUNDLE_DEV") }
+    set_by_lua_block $script_modification_disabled  { return os.getenv("PL_SCRIPT_MODIFICATION_DISABLED") }
     set_by_lua_block $analytics_enabled  { return os.getenv("ANALYTICS_ENABLED") }
     set_by_lua_block $announcement_enabled  { return os.getenv("ANNOUNCEMENT_ENABLED") }
     set_by_lua_block $announce_widget_url  { return os.getenv("ANNOUNCE_WIDGET_URL") }
@@ -169,7 +171,8 @@ data:
     env PL_HYDRA_SERVICE;
     env PL_KRATOS_SERVICE;
     env SCRIPT_BUNDLE_URLS;
-    env SCRIPT_BUNDE_DEV;
+    env SCRIPT_BUNDLE_DEV;
+    env PL_SCRIPT_MODIFICATION_DISABLED;
     env ANALYTICS_ENABLED;
     env ANNOUNCEMENT_ENABLED;
     env ANNOUNCE_WIDGET_URL;

k8s/cloud/base/script_bundles_config.yaml

@@ -9,3 +9,4 @@ data:
     "https://artifacts.px.dev/pxl_scripts/bundle.json"
     ]
   SCRIPT_BUNDLE_DEV: "false"
+  PL_SCRIPT_MODIFICATION_DISABLED: "false"

k8s/cloud/dev/script_bundles_config.yaml

@@ -10,3 +10,4 @@ data:
     "https://artifacts.px.dev/pxl_scripts/bundle.json"
     ]
   SCRIPT_BUNDLE_DEV: "false"
+  PL_SCRIPT_MODIFICATION_DISABLED: "false"

k8s/cloud/prod/script_bundles_config.yaml

@@ -10,3 +10,4 @@ data:
     "https://artifacts.px.dev/pxl_scripts/bundle.json"
     ]
   SCRIPT_BUNDLE_DEV: "false"
+  PL_SCRIPT_MODIFICATION_DISABLED: "false"

k8s/cloud/public/base/script_bundles_config.yaml

@@ -9,3 +9,4 @@ data:
     "https://artifacts.px.dev/pxl_scripts/bundle.json"
     ]
   SCRIPT_BUNDLE_DEV: "false"
+  PL_SCRIPT_MODIFICATION_DISABLED: "false"

k8s/cloud/staging/script_bundles_config.yaml

@@ -10,3 +10,4 @@ data:
     "https://artifacts.px.dev/pxl_scripts/bundle.json"
     ]
   SCRIPT_BUNDLE_DEV: "false"
+  PL_SCRIPT_MODIFICATION_DISABLED: "false"

k8s/cloud/testing/script_bundles_config.yaml

@@ -10,3 +10,4 @@ data:
     "https://artifacts.px.dev/pxl_scripts/bundle.json"
     ]
   SCRIPT_BUNDLE_DEV: "false"
+  PL_SCRIPT_MODIFICATION_DISABLED: "false"

src/cloud/api/api_server.go

@@ -66,6 +66,7 @@ func init() {
 
 	pflag.String("auth_connector_name", "", "If any, the name of the auth connector to be used with Pixie")
 	pflag.String("auth_connector_callback_url", "", "If any, the callback URL for the auth connector")
+	pflag.Bool("script_modification_disabled", false, "If script modification should be disallowed to prevent arbitrary script execution")
 }
 
 func main() {
@@ -213,17 +214,18 @@ func main() {
 	authServer := &controllers.AuthServer{AuthClient: ac}
 	cloudpb.RegisterAuthServiceServer(s.GRPCServer(), authServer)
 
-	vpt := ptproxy.NewVizierPassThroughProxy(nc, vc)
-	vizierpb.RegisterVizierServiceServer(s.GRPCServer(), vpt)
-	vizierpb.RegisterVizierDebugServiceServer(s.GRPCServer(), vpt)
-
 	sm, err := apienv.NewScriptMgrServiceClient()
 	if err != nil {
 		log.WithError(err).Fatal("Failed to init scriptmgr client.")
 	}
 	sms := &controllers.ScriptMgrServer{ScriptMgr: sm}
 	cloudpb.RegisterScriptMgrServer(s.GRPCServer(), sms)
 
+	scriptModificationDisabled := viper.GetBool("script_modification_disabled")
+	vpt := ptproxy.NewVizierPassThroughProxy(nc, vc, sm, scriptModificationDisabled)
+	vizierpb.RegisterVizierServiceServer(s.GRPCServer(), vpt)
+	vizierpb.RegisterVizierDebugServiceServer(s.GRPCServer(), vpt)
+
 	mdIndexName := viper.GetString("md_index_name")
 	if mdIndexName == "" {
 		log.Fatal("Must specify a name for the elastic index.")

src/cloud/api/apienv/scriptmgr_client.go

@@ -28,7 +28,7 @@ import (
 )
 
 func init() {
-	pflag.String("scriptmgr_service", "scriptmgr-service.plc.svc.local:52000", "The profile service url (load balancer/list is ok)")
+	pflag.String("scriptmgr_service", "scriptmgr-service.plc.svc.local:52000", "The scriptmgr service url (load balancer/list is ok)")
 }
 
 // NewScriptMgrServiceClient creates a new scriptmgr RPC client stub.
@@ -38,7 +38,7 @@ func NewScriptMgrServiceClient() (scriptmgrpb.ScriptMgrServiceClient, error) {
 		return nil, err
 	}
 
-	authChannel, err := grpc.Dial(viper.GetString("scripts_service"), dialOpts...)
+	authChannel, err := grpc.Dial(viper.GetString("scriptmgr_service"), dialOpts...)
 	if err != nil {
 		return nil, err
 	}

src/cloud/api/ptproxy/BUILD.bazel

@@ -28,16 +28,19 @@ go_library(
     deps = [
         "//src/api/proto/uuidpb:uuid_pl_go_proto",
         "//src/api/proto/vizierpb:vizier_pl_go_proto",
+        "//src/cloud/scriptmgr/scriptmgrpb:service_pl_go_proto",
         "//src/cloud/shared/vzshard",
         "//src/shared/cvmsgspb:cvmsgs_pl_go_proto",
         "//src/shared/services/authcontext",
         "//src/shared/services/jwtpb:jwt_pl_go_proto",
+        "//src/shared/services/utils",
         "//src/utils",
         "@com_github_gofrs_uuid//:uuid",
         "@com_github_gogo_protobuf//proto",
         "@com_github_gogo_protobuf//types",
         "@com_github_nats_io_nats_go//:nats_go",
         "@com_github_sirupsen_logrus//:logrus",
+        "@com_github_spf13_viper//:viper",
         "@org_golang_google_grpc//:grpc",
         "@org_golang_google_grpc//codes",
         "@org_golang_google_grpc//metadata",
@@ -53,6 +56,7 @@ pl_go_test(
         ":ptproxy",
         "//src/api/proto/uuidpb:uuid_pl_go_proto",
         "//src/api/proto/vizierpb:vizier_pl_go_proto",
+        "//src/cloud/scriptmgr/scriptmgrpb:service_pl_go_proto",
         "//src/cloud/shared/vzshard",
         "//src/shared/cvmsgspb:cvmsgs_pl_go_proto",
         "//src/shared/services/env",

src/cloud/api/ptproxy/vizier_pt_proxy.go

@@ -20,32 +20,75 @@ package ptproxy
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
 
 	"github.com/nats-io/nats.go"
+	"github.com/spf13/viper"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 
 	"px.dev/pixie/src/api/proto/uuidpb"
 	"px.dev/pixie/src/api/proto/vizierpb"
+	"px.dev/pixie/src/cloud/scriptmgr/scriptmgrpb"
 	"px.dev/pixie/src/shared/cvmsgspb"
 	"px.dev/pixie/src/shared/services/authcontext"
 	"px.dev/pixie/src/shared/services/jwtpb"
+	jwtutils "px.dev/pixie/src/shared/services/utils"
 )
 
 type vzmgrClient interface {
 	GetVizierInfo(ctx context.Context, in *uuidpb.UUID, opts ...grpc.CallOption) (*cvmsgspb.VizierInfo, error)
 	GetVizierConnectionInfo(ctx context.Context, in *uuidpb.UUID, opts ...grpc.CallOption) (*cvmsgspb.VizierConnectionInfo, error)
 }
 
+type scriptmgrClient interface {
+	CheckScriptExists(ctx context.Context, req *scriptmgrpb.CheckScriptExistsReq, opts ...grpc.CallOption) (*scriptmgrpb.CheckScriptExistsResp, error)
+}
+
 // VizierPassThroughProxy implements the VizierAPI and allows proxying the data to the actual
 // vizier cluster.
 type VizierPassThroughProxy struct {
-	nc *nats.Conn
-	vc vzmgrClient
+	nc                         *nats.Conn
+	vc                         vzmgrClient
+	sm                         scriptmgrClient
+	scriptModificationDisabled bool
+}
+
+// getServiceCredentials returns JWT credentials for inter-service requests.
+func getServiceCredentials(signingKey string) (string, error) {
+	claims := jwtutils.GenerateJWTForService("cloud api", viper.GetString("domain_name"))
+	return jwtutils.SignJWTClaims(claims, signingKey)
 }
 
 // NewVizierPassThroughProxy creates a new passthrough proxy.
-func NewVizierPassThroughProxy(nc *nats.Conn, vc vzmgrClient) *VizierPassThroughProxy {
-	return &VizierPassThroughProxy{nc: nc, vc: vc}
+func NewVizierPassThroughProxy(nc *nats.Conn, vc vzmgrClient, sm scriptmgrClient, scriptModificationDisabled bool) *VizierPassThroughProxy {
+	return &VizierPassThroughProxy{nc: nc, vc: vc, sm: sm, scriptModificationDisabled: scriptModificationDisabled}
+}
+
+func (v *VizierPassThroughProxy) isScriptModified(ctx context.Context, script string) (bool, error) {
+	hash := sha256.New()
+	hash.Write([]byte(script))
+	hashStr := hex.EncodeToString(hash.Sum(nil))
+	req := &scriptmgrpb.CheckScriptExistsReq{Sha256Hash: hashStr}
+
+	serviceAuthToken, err := getServiceCredentials(viper.GetString("jwt_signing_key"))
+	ctx = metadata.AppendToOutgoingContext(ctx, "authorization",
+		fmt.Sprintf("bearer %s", serviceAuthToken))
+
+	if err != nil {
+		return false, err
+	}
+
+	resp, err := v.sm.CheckScriptExists(ctx, req)
+
+	if err != nil {
+		return false, err
+	}
+	return !resp.Exists, nil
 }
 
 // ExecuteScript is the GRPC stream method.
@@ -55,6 +98,17 @@ func (v *VizierPassThroughProxy) ExecuteScript(req *vizierpb.ExecuteScriptReques
 		return err
 	}
 	defer rp.Finish()
+	if v.scriptModificationDisabled {
+		modified, err := v.isScriptModified(srv.Context(), req.QueryStr)
+		if err != nil {
+			return err
+		}
+
+		if modified {
+			return status.Error(codes.InvalidArgument, "Script modification has been disabled")
+		}
+	}
+
 	vizReq := rp.prepareVizierRequest()
 	vizReq.Msg = &cvmsgspb.C2VAPIStreamRequest_ExecReq{ExecReq: req}
 	if err := rp.sendMessageToVizier(vizReq); err != nil {