From 0bb425a4de6139d17c55ba0824770f3ae350ccc3 Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Mon, 16 Oct 2023 17:20:37 +0200 Subject: [PATCH 1/5] Add client certificate, require to verify client cert --- arbnode/dataposter/data_poster.go | 33 ++++++++++++---- arbnode/dataposter/dataposter_test.go | 31 ++++++++++++--- arbnode/dataposter/testdata/client.cnf | 52 ++++++++++++++++++++++++++ arbnode/dataposter/testdata/client.crt | 28 ++++++++++++++ arbnode/dataposter/testdata/client.key | 28 ++++++++++++++ 5 files changed, 159 insertions(+), 13 deletions(-) create mode 100644 arbnode/dataposter/testdata/client.cnf create mode 100644 arbnode/dataposter/testdata/client.crt create mode 100644 arbnode/dataposter/testdata/client.key diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 56e87e3b29..02a3548660 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -175,21 +175,32 @@ func NewDataPoster(ctx context.Context, opts *DataPosterOpts) (*DataPoster, erro } func rpcClient(ctx context.Context, opts *ExternalSignerCfg) (*rpc.Client, error) { - rootCrt, err := os.ReadFile(opts.RootCA) + clientCert, err := tls.LoadX509KeyPair(opts.ClientCert, opts.ClientPrivateKey) if err != nil { - return nil, fmt.Errorf("error reading external signer root CA: %w", err) + return nil, fmt.Errorf("error loading client certificate and private key: %w", err) } - pool := x509.NewCertPool() - pool.AppendCertsFromPEM(rootCrt) + + tlsCfg := &tls.Config{ + MinVersion: tls.VersionTLS12, + Certificates: []tls.Certificate{clientCert}, + } + if opts.RootCA != "" { + rootCrt, err := os.ReadFile(opts.RootCA) + if err != nil { + return nil, fmt.Errorf("error reading external signer root CA: %w", err) + } + rootCertPool := x509.NewCertPool() + rootCertPool.AppendCertsFromPEM(rootCrt) + tlsCfg.RootCAs = rootCertPool + } + return rpc.DialOptions( ctx, opts.URL, rpc.WithHTTPClient( &http.Client{ Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - RootCAs: pool, - }, + TLSClientConfig: tlsCfg, }, }, ), @@ -742,9 +753,13 @@ type ExternalSignerCfg struct { Address string `koanf:"address"` // API method name (e.g. eth_signTransaction). Method string `koanf:"method"` - // Path to the external signer root CA certificate. + // (Optional) Path to the external signer root CA certificate. // This allows us to use self-signed certificats on the external signer. RootCA string `koanf:"root-ca"` + // Client certificate for mtls. + ClientCert string `koanf:"client-cert"` + // Client certificate key for mtls. + ClientPrivateKey string `koanf:"client-private-key"` } type DangerousConfig struct { @@ -787,6 +802,8 @@ func addExternalSignerOptions(prefix string, f *pflag.FlagSet) { f.String(prefix+".address", DefaultDataPosterConfig.ExternalSigner.Address, "external signer address") f.String(prefix+".method", DefaultDataPosterConfig.ExternalSigner.Method, "external signer method") f.String(prefix+".root-ca", DefaultDataPosterConfig.ExternalSigner.RootCA, "external signer root CA") + f.String(prefix+".client-cert", DefaultDataPosterConfig.ExternalSigner.ClientCert, "rpc client cert") + f.String(prefix+".client-private-key", DefaultDataPosterConfig.ExternalSigner.ClientPrivateKey, "rpc client private key") } var DefaultDataPosterConfig = DataPosterConfig{ diff --git a/arbnode/dataposter/dataposter_test.go b/arbnode/dataposter/dataposter_test.go index 0b3b8cba5c..d21390bccd 100644 --- a/arbnode/dataposter/dataposter_test.go +++ b/arbnode/dataposter/dataposter_test.go @@ -2,6 +2,8 @@ package dataposter import ( "context" + "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "io" @@ -74,10 +76,12 @@ func TestExternalSigner(t *testing.T) { }() signer, addr, err := externalSigner(ctx, &ExternalSignerCfg{ - Address: srv.address.Hex(), - URL: "https://localhost:1234", - Method: "test_signTransaction", - RootCA: cert, + Address: srv.address.Hex(), + URL: "https://localhost:1234", + Method: "test_signTransaction", + RootCA: cert, + ClientCert: "./testdata/client.crt", + ClientPrivateKey: "./testdata/client.key", }) if err != nil { t.Fatalf("Error getting external signer: %v", err) @@ -129,7 +133,24 @@ func newServer(ctx context.Context, t *testing.T) (*http.Server, *server) { "test_signTransaction": s.signTransaction, } m := http.NewServeMux() - httpSrv := &http.Server{Addr: ":1234", Handler: m, ReadTimeout: 5 * time.Second} + + clientCert, err := os.ReadFile("./testdata/client.crt") + if err != nil { + panic(err) + } + pool := x509.NewCertPool() + pool.AppendCertsFromPEM(clientCert) + + httpSrv := &http.Server{ + Addr: ":1234", + Handler: m, + ReadTimeout: 5 * time.Second, + TLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: pool, + }, + } m.HandleFunc("/", s.mux) return httpSrv, s } diff --git a/arbnode/dataposter/testdata/client.cnf b/arbnode/dataposter/testdata/client.cnf new file mode 100644 index 0000000000..8c15cc3dbc --- /dev/null +++ b/arbnode/dataposter/testdata/client.cnf @@ -0,0 +1,52 @@ +[req] +default_bits = 2048 +default_keyfile = server-key.pem +distinguished_name = subject +req_extensions = req_ext +x509_extensions = x509_ext +string_mask = utf8only + +[subject] +countryName = CH +countryName_default = CH + +stateOrProvinceName = Zurich +stateOrProvinceName_default = ZH + +localityName = city +localityName_default = Zurich + +organizationName = Offchain Labs +organizationName_default = Offchain Labs + +commonName = offchainlabs.ch +commonName_default = localhost + +emailAddress = Email Address +emailAddress_default = notabigdeal@offchainlabs.ch + +[x509_ext] +subjectKeyIdentifier = hash +authorityKeyIdentifier = keyid,issuer + +basicConstraints = CA:FALSE +keyUsage = digitalSignature, keyEncipherment +subjectAltName = @alternate_names +nsComment = "OpenSSL Generated Certificate" + +[req_ext] +subjectKeyIdentifier = hash + +basicConstraints = CA:FALSE +keyUsage = digitalSignature, keyEncipherment +subjectAltName = @alternate_names +nsComment = "OpenSSL Generated Certificate" + +[alternate_names] +DNS.1 = localhost +DNS.2 = 127.0.0.1 + +[alternate_names] +DNS.1 = localhost +DNS.2 = 127.0.0.1 + diff --git a/arbnode/dataposter/testdata/client.crt b/arbnode/dataposter/testdata/client.crt new file mode 100644 index 0000000000..3d494be820 --- /dev/null +++ b/arbnode/dataposter/testdata/client.crt @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIE0jCCA7qgAwIBAgIUPaBB3/hHMpZfGB3VOw1+mHG4LnUwDQYJKoZIhvcNAQEL +BQAwgYMxCzAJBgNVBAYTAkNIMQswCQYDVQQIDAJaSDEPMA0GA1UEBwwGWnVyaWNo +MRYwFAYDVQQKDA1PZmZjaGFpbiBMYWJzMRIwEAYDVQQDDAlsb2NhbGhvc3QxKjAo +BgkqhkiG9w0BCQEWG25vdGFiaWdkZWFsQG9mZmNoYWlubGFicy5jaDAeFw0yMzEw +MTYxNDU2MjhaFw0yNDEwMTUxNDU2MjhaMIGDMQswCQYDVQQGEwJDSDELMAkGA1UE +CAwCWkgxDzANBgNVBAcMBlp1cmljaDEWMBQGA1UECgwNT2ZmY2hhaW4gTGFiczES +MBAGA1UEAwwJbG9jYWxob3N0MSowKAYJKoZIhvcNAQkBFhtub3RhYmlnZGVhbEBv +ZmZjaGFpbmxhYnMuY2gwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC1 +1asfUzv07QTVwlM4o3g51ilIFEApPkpdQej/GIItLEVRQW+GI9jYuEM07wdwMhSH +JPFNbZB3dmBuqDLx13hY03ufyeY+nab0/sO6x13kXChvIqgPRyJtkEAoYkMM3W0D +S6HeL/6DFoTQ2xAlZb/7i/9deuUwDL3MNVSjPCm9PjFzSOFgAQQud2uUT7aENGuG +Whw3oXz9gU/8gv3keLzcIa2PHyEW5M7jeGSYMjfW3wr0d+Z5mSNRc/U6kncKi06c +QrMKrgFfF7a5kHgxUL7bRCGgCMemXe7VfrW6oKT11JcLWDKhe+uo6bNXUptek55H +HfQi6x8cbM46/h3riZA3AgMBAAGjggE6MIIBNjAdBgNVHQ4EFgQUQD2BOems0+JQ +br234cW5noMmXRIwga0GA1UdIwSBpTCBoqGBiaSBhjCBgzELMAkGA1UEBhMCQ0gx +CzAJBgNVBAgMAlpIMQ8wDQYDVQQHDAZadXJpY2gxFjAUBgNVBAoMDU9mZmNoYWlu +IExhYnMxEjAQBgNVBAMMCWxvY2FsaG9zdDEqMCgGCSqGSIb3DQEJARYbbm90YWJp +Z2RlYWxAb2ZmY2hhaW5sYWJzLmNoghQ9oEHf+Ecyll8YHdU7DX6YcbgudTAJBgNV +HRMEAjAAMAsGA1UdDwQEAwIFoDAfBgNVHREEGDAWgglsb2NhbGhvc3SCCTEyNy4w +LjAuMTAsBglghkgBhvhCAQ0EHxYdT3BlblNTTCBHZW5lcmF0ZWQgQ2VydGlmaWNh +dGUwDQYJKoZIhvcNAQELBQADggEBAF4EVkOZZeMIvv0JViP7NsmIl2ke/935x6Hd +hQiLUw13XHYXzMa5/8Y5fnKjttBODpFoQlwjgI18vzuYzItYMBc2cabQJcpfG+Wq +M3m/wl1TC2XOuHj1E4RA/nU3tslntahtXG+vkks9RN+f9irHUhDRR6AGSnSB2Gi/ +B2OGmXn7S4Qge8+fGHAjN+tlu+tOoEWP6R3if/a9UIe5EGM8QTe4zw6lr+iPrOhC +M94pK5IEWn5IIGhr3zJIYkm/Dp+rFqhV1sqPOjjFLVCA7KJ3jVVVHlcm4Xa/+fyk +CIm7/VAmnbeUNlMbkXNOfQMeku8Iwsu80pvf3kjhU/PgO/5oojk= +-----END CERTIFICATE----- diff --git a/arbnode/dataposter/testdata/client.key b/arbnode/dataposter/testdata/client.key new file mode 100644 index 0000000000..b14941dd9f --- /dev/null +++ b/arbnode/dataposter/testdata/client.key @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQC11asfUzv07QTV +wlM4o3g51ilIFEApPkpdQej/GIItLEVRQW+GI9jYuEM07wdwMhSHJPFNbZB3dmBu +qDLx13hY03ufyeY+nab0/sO6x13kXChvIqgPRyJtkEAoYkMM3W0DS6HeL/6DFoTQ +2xAlZb/7i/9deuUwDL3MNVSjPCm9PjFzSOFgAQQud2uUT7aENGuGWhw3oXz9gU/8 +gv3keLzcIa2PHyEW5M7jeGSYMjfW3wr0d+Z5mSNRc/U6kncKi06cQrMKrgFfF7a5 +kHgxUL7bRCGgCMemXe7VfrW6oKT11JcLWDKhe+uo6bNXUptek55HHfQi6x8cbM46 +/h3riZA3AgMBAAECggEADUboCYMCpm+LqIhzNCtqswQD6QsiSwCmqs8nuKZGk9ue ++hmZj5IpgMJZLrgvWY4s+PGfgiRR/28QCBrVXkETiZ5zirQFN4tvLlKcSK4xZf29 +FBRUCiPxck36NhiqrBNOi1Mn8BKedl4cESkvSu1cvcmeOh100HPcHfLDVqHx3qsl +D/5yMkT2+zdhtLa+X3nkAa+3aibOvgtyfkV679e20CG6h89N9GBKkTXO8ioLZZVm +84ksnd4FcpTo7ebJJxElEB+ZA4akPHbF6ArUmcpqtGso5GtwqqO2ZlguSn2XQT0d +jqvOG4DwfSXk6SpE/dpWvU92fmxWAxZvGrZNgDyJ2QKBgQDyQ8NN4b80Yza/YXar +LWx8A6B0eMc1dXgt9m3UUI+titt45jEcaXhCX01FRFTznWGmWFtJmcWBoaQVPVel +IcDYQSxEuBUrCeI75ocv/IQtENaiX3TK7Nlz5RHfpQpfDVJq45lpiD38CGkYkAif +9pSzC8aup4W3WR0JJZ1AOHUZaQKBgQDAJNJnaSNzB+eDWTKCIN5V9X3QMkmjsuir +Nf2lBXHYARnlYWAbtYFG12wLJQMTNX5ewVQQrWtsdPkGPpCnPLelUTxMssrsXjej +JlLzYUfzRBqEXMI3AA9bVdiauxId2RTcp2F81SM1keCMcuHYxrzVkBSOC9u3wCnb +Whb6+feInwKBgQCbzgC5AcoaQwReqKvNAvWV/C8hONvFAbs8tBOGTBlbHsZvRnun +Lh1tciUbuwp3cmvuszxiZUakS/RexIitZrvDWIbD2y+h8kVRCL1Am0HWSdH/syxF +pXVkF5obHuVApCyxGZb8S+axRCdy6I7jcY3IaHZqtMpGVEVcMJilSKnmoQKBgQCC +tEmgaMfhhx34nqOaG4vDA4T7LEolnh1h4g9RwztnCZC5FZ1QHA79xqrLhfjqhzgY +cwChe6aYl5WSptq1uLrgLTuMnQ8m7QyB4h8JSkKse8ZiBctjqJnJssLutpSjUzk6 +xG2vgjk6RqpuP/PcB40K5cDlw7FJ9OFEQqthPMsi1wKBgQC0/vv5bY3DQ+wV6gUy +nFoSa/XNHaa8y7jmmlCnWJqs6DAAQQ3VW0tPX03GYL/NDcI+PwzYDHDkSB6Qa/o8 +VzVGK1/kr/+bveNvqmi0vNb54fMFLveGgsY4Cu1cffiw8m6nYJ/V4eCsHfpF1B5L +5HDnt5rFKt1Mi9WsUSRtxipxBA== +-----END PRIVATE KEY----- From 059a45e804657901d62a3ab908d1886a76ad0f22 Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Thu, 19 Oct 2023 10:40:51 +0200 Subject: [PATCH 2/5] revert testnode pin --- nitro-testnode | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nitro-testnode b/nitro-testnode index 7ad12c0f1b..aee6ceff9c 160000 --- a/nitro-testnode +++ b/nitro-testnode @@ -1 +1 @@ -Subproject commit 7ad12c0f1be75a72c7360d5258e0090f8225594e +Subproject commit aee6ceff9c9d3fb2749da55a7d7842f23d1bfc8e From b015ab8f06f8fc72c827af4388428949cec8a5c6 Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Thu, 19 Oct 2023 10:46:31 +0200 Subject: [PATCH 3/5] Make client certificate optional --- arbnode/dataposter/data_poster.go | 19 +++++++++++-------- arbnode/dataposter/dataposter_test.go | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 39fda3f2ac..687b26ba26 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -175,15 +175,18 @@ func NewDataPoster(ctx context.Context, opts *DataPosterOpts) (*DataPoster, erro } func rpcClient(ctx context.Context, opts *ExternalSignerCfg) (*rpc.Client, error) { - clientCert, err := tls.LoadX509KeyPair(opts.ClientCert, opts.ClientPrivateKey) - if err != nil { - return nil, fmt.Errorf("error loading client certificate and private key: %w", err) + tlsCfg := &tls.Config{ + MinVersion: tls.VersionTLS12, } - tlsCfg := &tls.Config{ - MinVersion: tls.VersionTLS12, - Certificates: []tls.Certificate{clientCert}, + if opts.ClientCert == "" || opts.ClientPrivateKey == "" { + clientCert, err := tls.LoadX509KeyPair(opts.ClientCert, opts.ClientPrivateKey) + if err != nil { + return nil, fmt.Errorf("error loading client certificate and private key: %w", err) + } + tlsCfg.Certificates = []tls.Certificate{clientCert} } + if opts.RootCA != "" { rootCrt, err := os.ReadFile(opts.RootCA) if err != nil { @@ -756,9 +759,9 @@ type ExternalSignerCfg struct { // (Optional) Path to the external signer root CA certificate. // This allows us to use self-signed certificats on the external signer. RootCA string `koanf:"root-ca"` - // Client certificate for mtls. + // (Optional) Client certificate for mtls. ClientCert string `koanf:"client-cert"` - // Client certificate key for mtls. + // (Optional) Client certificate key for mtls. ClientPrivateKey string `koanf:"client-private-key"` } diff --git a/arbnode/dataposter/dataposter_test.go b/arbnode/dataposter/dataposter_test.go index 4734295ae8..d4d72bbbf4 100644 --- a/arbnode/dataposter/dataposter_test.go +++ b/arbnode/dataposter/dataposter_test.go @@ -136,7 +136,7 @@ func newServer(ctx context.Context, t *testing.T) (*http.Server, *server) { clientCert, err := os.ReadFile("./testdata/client.crt") if err != nil { - panic(err) + t.Fatalf("Error reading client certificate: %v", err) } pool := x509.NewCertPool() pool.AppendCertsFromPEM(clientCert) From 1d524078dc2c41fed4f2bc3efd401e18f911047f Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Thu, 19 Oct 2023 10:55:46 +0200 Subject: [PATCH 4/5] Log when certificate is enabled, fix enabling --- arbnode/dataposter/data_poster.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arbnode/dataposter/data_poster.go b/arbnode/dataposter/data_poster.go index 687b26ba26..1a202171ec 100644 --- a/arbnode/dataposter/data_poster.go +++ b/arbnode/dataposter/data_poster.go @@ -179,7 +179,8 @@ func rpcClient(ctx context.Context, opts *ExternalSignerCfg) (*rpc.Client, error MinVersion: tls.VersionTLS12, } - if opts.ClientCert == "" || opts.ClientPrivateKey == "" { + if opts.ClientCert != "" && opts.ClientPrivateKey != "" { + log.Info("Client certificate for external signer is enabled") clientCert, err := tls.LoadX509KeyPair(opts.ClientCert, opts.ClientPrivateKey) if err != nil { return nil, fmt.Errorf("error loading client certificate and private key: %w", err) @@ -762,6 +763,7 @@ type ExternalSignerCfg struct { // (Optional) Client certificate for mtls. ClientCert string `koanf:"client-cert"` // (Optional) Client certificate key for mtls. + // This is required when client-cert is set. ClientPrivateKey string `koanf:"client-private-key"` } From 058db5408a5cfda478332d5b8eb4061cb1279a8b Mon Sep 17 00:00:00 2001 From: Nodar Ambroladze Date: Mon, 23 Oct 2023 13:23:31 +0200 Subject: [PATCH 5/5] Suppress golangci-lint error --- cmd/genericconf/pprof.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/genericconf/pprof.go b/cmd/genericconf/pprof.go index e55bfddd32..9fd3a6f2a4 100644 --- a/cmd/genericconf/pprof.go +++ b/cmd/genericconf/pprof.go @@ -17,8 +17,7 @@ func StartPprof(address string) { log.Info("Starting metrics server with pprof", "addr", fmt.Sprintf("http://%s/debug/metrics", address)) log.Info("Pprof endpoint", "addr", fmt.Sprintf("http://%s/debug/pprof", address)) go func() { - // #nosec G114 - if err := http.ListenAndServe(address, http.DefaultServeMux); err != nil { + if err := http.ListenAndServe(address, http.DefaultServeMux); /* #nosec G114 */ err != nil { log.Error("Failure in running pprof server", "err", err) } }()