Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
7 changes: 7 additions & 0 deletions k8s/cloud/base/api_deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
5 changes: 4 additions & 1 deletion k8s/cloud/base/proxy_nginx_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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}'";
Expand Down Expand Up @@ -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") }
Expand Down Expand Up @@ -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;
Copy link
Member Author

@ddelnano ddelnano Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a typo but something that's only used for development.

env PL_SCRIPT_MODIFICATION_DISABLED;
env ANALYTICS_ENABLED;
env ANNOUNCEMENT_ENABLED;
env ANNOUNCE_WIDGET_URL;
Expand Down
1 change: 1 addition & 0 deletions k8s/cloud/base/script_bundles_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ data:
"https://artifacts.px.dev/pxl_scripts/bundle.json"
]
SCRIPT_BUNDLE_DEV: "false"
PL_SCRIPT_MODIFICATION_DISABLED: "false"
1 change: 1 addition & 0 deletions k8s/cloud/dev/script_bundles_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ data:
"https://artifacts.px.dev/pxl_scripts/bundle.json"
]
SCRIPT_BUNDLE_DEV: "false"
PL_SCRIPT_MODIFICATION_DISABLED: "false"
1 change: 1 addition & 0 deletions k8s/cloud/prod/script_bundles_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ data:
"https://artifacts.px.dev/pxl_scripts/bundle.json"
]
SCRIPT_BUNDLE_DEV: "false"
PL_SCRIPT_MODIFICATION_DISABLED: "false"
1 change: 1 addition & 0 deletions k8s/cloud/public/base/script_bundles_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ data:
"https://artifacts.px.dev/pxl_scripts/bundle.json"
]
SCRIPT_BUNDLE_DEV: "false"
PL_SCRIPT_MODIFICATION_DISABLED: "false"
1 change: 1 addition & 0 deletions k8s/cloud/staging/script_bundles_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ data:
"https://artifacts.px.dev/pxl_scripts/bundle.json"
]
SCRIPT_BUNDLE_DEV: "false"
PL_SCRIPT_MODIFICATION_DISABLED: "false"
1 change: 1 addition & 0 deletions k8s/cloud/testing/script_bundles_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ data:
"https://artifacts.px.dev/pxl_scripts/bundle.json"
]
SCRIPT_BUNDLE_DEV: "false"
PL_SCRIPT_MODIFICATION_DISABLED: "false"
10 changes: 6 additions & 4 deletions src/cloud/api/api_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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.")
Expand Down
4 changes: 2 additions & 2 deletions src/cloud/api/apienv/scriptmgr_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}
Expand Down
4 changes: 4 additions & 0 deletions src/cloud/api/ptproxy/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
62 changes: 58 additions & 4 deletions src/cloud/api/ptproxy/vizier_pt_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand Down
Loading
Loading