From 6ab866de392c7060169a7da855d0a83f09e358d6 Mon Sep 17 00:00:00 2001 From: Jakub Nowak Date: Wed, 21 Oct 2026 13:54:25 +0200 Subject: [PATCH] fix(client): verify TLS certificates on Workflow Registry RPC connections seth dials RPC URLs through NewLoggingTransport, which always sets InsecureSkipVerify. Every onchain command (workflow deploy/activate/ pause/delete, account link-key/unlink-key, secrets) therefore talked to the RPC node without certificate checks. Dial the RPC ourselves with go-ethereum's default transport and pass it to seth via WithEthClient. Restore the client URL and revert tracer that seth only sets up when it dials the URL itself. DEVSVCS-6732 Co-Authored-By: Claude Opus 5.5 --- cmd/client/eth_client.go | 79 ++++++++++++++++++++++++++---- cmd/client/eth_client_tls_test.go | 80 +++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+), 9 deletions(-) create mode 100644 cmd/client/eth_client_tls_test.go diff --git a/cmd/client/eth_client.go b/cmd/client/eth_client.go index 04be9d97..48f67b62 100644 --- a/cmd/client/eth_client.go +++ b/cmd/client/eth_client.go @@ -2,13 +2,16 @@ package client import ( "context" + "errors" "fmt" + "net/http" "os" "strconv" "time" "github.com/BurntSushi/toml" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/ethclient/simulated" "github.com/ethereum/go-ethereum/rpc" "github.com/rs/zerolog" @@ -131,13 +134,13 @@ func NewSethClient( func NewSethClientWithSimulated( configFile string, - rpc string, + rpcURL string, privateKeys []string, chainId uint64, backend *simulated.Backend, ) (*seth.Client, error) { var sethClientBuilder *seth.ClientBuilder - var err error + var rpcHeaders http.Header // if a config file is provided, we will use it to create the client if configFile != "" { sethConfig, readErr := readSethConfigFromFile(configFile) @@ -146,8 +149,12 @@ func NewSethClientWithSimulated( } sethClientBuilder = seth.NewClientBuilderWithConfig(sethConfig). - UseNetworkWithChainId(chainId). - WithRpcUrl(rpc) + UseNetworkWithChainId(chainId) + rpcHeaders = sethConfig.RPCHeaders + // The RPC connection is passed in through WithEthClient below, and seth rejects configs that set both. + if sethConfig.Network != nil { + sethConfig.Network.URLs = nil + } } else { // if full flexibility is not needed we create a client with reasonable defaults // if you need to further tweak them, please refer to https://github.com/smartcontractkit/chainlink-testing-framework/blob/main/seth/README.md @@ -156,11 +163,23 @@ func NewSethClientWithSimulated( // Auto priority will use what the blockchain node suggests // estimationBlocks and attemptCount can be disregarded for Auto priority WithGasPriceEstimations(false, 20, seth.Priority_Auto, 1) - if rpc != "" { - sethClientBuilder.WithRpcUrl(rpc) - } else { - sethClientBuilder.WithEthClient(backend.Client()) + } + + // Never let seth dial the RPC URL itself: its transport sets InsecureSkipVerify, so TLS certificates + // would not be checked. We dial with go-ethereum's default transport instead, which verifies them. + var rpcClient *rpc.Client + switch { + case rpcURL != "": + var dialErr error + rpcClient, dialErr = dialRPC(rpcURL, rpcHeaders) + if dialErr != nil { + return nil, dialErr } + sethClientBuilder.WithEthClient(ethclient.NewClient(rpcClient)) + case backend != nil: + sethClientBuilder.WithEthClient(backend.Client()) + default: + return nil, errors.New("no RPC URL provided") } // if private key is provided, we will use it to sign transactions @@ -172,8 +191,50 @@ func NewSethClientWithSimulated( } sethClient, err := sethClientBuilder.Build() + if err != nil { + if rpcClient != nil { + rpcClient.Close() + } + return nil, err + } + + if rpcURL != "" { + if err := restoreRPCURL(sethClient, rpcURL); err != nil { + rpcClient.Close() + return nil, err + } + } + + return sethClient, nil +} + +// dialRPC connects to the RPC node using go-ethereum's default HTTP transport, which verifies TLS certificates. +func dialRPC(rpcURL string, headers http.Header) (*rpc.Client, error) { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + rpcClient, err := rpc.DialOptions(ctx, rpcURL, rpc.WithHeaders(headers)) + if err != nil { + return nil, fmt.Errorf("failed to connect RPC client to '%s': %w", crpc.RedactURL(rpcURL), err) + } + return rpcClient, nil +} + +// restoreRPCURL puts back what seth only sets up when it dials the URL itself: the client URL and the tracer +// used to decode reverted transactions. The tracer opens its own connection with go-ethereum's default +// transport, so certificates are verified there too. +func restoreRPCURL(sethClient *seth.Client, rpcURL string) error { + sethClient.URL = rpcURL + sethClient.Cfg.Network.URLs = []string{rpcURL} - return sethClient, err + if sethClient.Cfg.TracingLevel == seth.TracingLevel_None || sethClient.Tracer != nil { + return nil + } + tracer, err := seth.NewTracer(sethClient.ContractStore, sethClient.ABIFinder, sethClient.Cfg, sethClient.ContractAddressToNameMap, sethClient.Addresses) + if err != nil { + return fmt.Errorf("failed to create tracer: %w", err) + } + sethClient.Tracer = tracer + return nil } func readSethConfigFromFile(configPath string) (*seth.Config, error) { diff --git a/cmd/client/eth_client_tls_test.go b/cmd/client/eth_client_tls_test.go new file mode 100644 index 00000000..88d74b48 --- /dev/null +++ b/cmd/client/eth_client_tls_test.go @@ -0,0 +1,80 @@ +package client_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/cre-cli/cmd/client" +) + +const testChainID = 1337 + +// newJSONRPCServer starts a TLS server with a self-signed certificate that answers eth_chainId and eth_blockNumber. +func newJSONRPCServer(t *testing.T, requests *atomic.Int32) *httptest.Server { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + var req struct { + ID json.RawMessage `json:"id"` + Method string `json:"method"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + var result string + switch req.Method { + case "eth_chainId": + result = "0x539" + case "eth_blockNumber": + result = "0x10" + default: + http.Error(w, "unexpected method "+req.Method, http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"jsonrpc": "2.0", "id": req.ID, "result": result}) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestNewSethClientRejectsUntrustedCertificate(t *testing.T) { + t.Parallel() + + var requests atomic.Int32 + srv := newJSONRPCServer(t, &requests) + + sethClient, err := client.NewSethClient("", srv.URL, nil, testChainID) + if err == nil { + _, err = sethClient.Client.BlockNumber(context.Background()) + } + require.Error(t, err) + require.Contains(t, err.Error(), "certificate") + require.Zero(t, requests.Load(), "no RPC request may reach a server with an untrusted certificate") +} + +// Not parallel: swaps http.DefaultTransport so the test server's certificate is trusted. +func TestNewSethClientTrustedCertificate(t *testing.T) { + var requests atomic.Int32 + srv := newJSONRPCServer(t, &requests) + + original := http.DefaultTransport + http.DefaultTransport = srv.Client().Transport + t.Cleanup(func() { http.DefaultTransport = original }) + + sethClient, err := client.NewSethClient("", srv.URL, nil, testChainID) + require.NoError(t, err) + + block, err := sethClient.Client.BlockNumber(context.Background()) + require.NoError(t, err) + require.Equal(t, uint64(16), block) + require.Equal(t, srv.URL, sethClient.URL) + require.NotNil(t, sethClient.Tracer, "tracer is needed to decode reverted transactions") +}