From e5d384f1274c9a332ae3bd57b82d4c42e3055d89 Mon Sep 17 00:00:00 2001 From: Ethen Pociask Date: Fri, 25 Oct 2024 21:36:42 -0400 Subject: [PATCH 1/6] fix: Hide other sensitive cfg values --- cmd/server/entrypoint.go | 6 ++---- e2e/setup.go | 1 - store/precomputed_key/redis/redis.go | 9 ++++----- store/precomputed_key/s3/s3.go | 14 +++++++------- 4 files changed, 13 insertions(+), 17 deletions(-) diff --git a/cmd/server/entrypoint.go b/cmd/server/entrypoint.go index d269faa6..b709a824 100644 --- a/cmd/server/entrypoint.go +++ b/cmd/server/entrypoint.go @@ -73,17 +73,15 @@ func StartProxySvr(cliCtx *cli.Context) error { } // TODO: we should probably just change EdaClientConfig struct definition in eigenda-client -// to have a `json:"-"` tag on the SignerPrivateKeyHex field, to prevent the privateKey from being marshaled at all func prettyPrintConfig(cliCtx *cli.Context, log log.Logger) error { // we read a new config which we modify to hide private info in order to log the rest cfg := server.ReadCLIConfig(cliCtx) - cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "HIDDEN" - cfg.EigenDAConfig.VerifierConfig.RPCURL = "HIDDEN" - configJSON, err := json.MarshalIndent(cfg, "", " ") if err != nil { return fmt.Errorf("failed to marshal config: %w", err) } + cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "" // marshaling defined in client config + cfg.EigenDAConfig.EdaClientConfig.RPC = "" // hiding as RPC providers typically use sensitive API keys within log.Info(fmt.Sprintf("Initializing EigenDA proxy server with config: %v", string(configJSON))) return nil } diff --git a/e2e/setup.go b/e2e/setup.go index ecd4fb2d..7ddd5041 100644 --- a/e2e/setup.go +++ b/e2e/setup.go @@ -133,7 +133,6 @@ func createRedisConfig(eigendaCfg server.Config) server.CLIConfig { Password: "", DB: 0, Eviction: 10 * time.Minute, - Profile: true, } return server.CLIConfig{ EigenDAConfig: eigendaCfg, diff --git a/store/precomputed_key/redis/redis.go b/store/precomputed_key/redis/redis.go index 2888e776..04f163d3 100644 --- a/store/precomputed_key/redis/redis.go +++ b/store/precomputed_key/redis/redis.go @@ -12,11 +12,10 @@ import ( // Config ... user configurable type Config struct { - Endpoint string - Password string - DB int - Eviction time.Duration - Profile bool + Endpoint string `json:"endpoint"` + Password string `json:"-"` + DB int `json:"database"` + Eviction time.Duration `json:"eviction"` } // Store ... Redis storage backend implementation diff --git a/store/precomputed_key/s3/s3.go b/store/precomputed_key/s3/s3.go index 54a438c0..fb39178c 100644 --- a/store/precomputed_key/s3/s3.go +++ b/store/precomputed_key/s3/s3.go @@ -38,13 +38,13 @@ var _ common.PrecomputedKeyStore = (*Store)(nil) type CredentialType string type Config struct { - CredentialType CredentialType - Endpoint string - EnableTLS bool - AccessKeyID string - AccessKeySecret string - Bucket string - Path string + CredentialType CredentialType `json:"credential_type"` + Endpoint string `json:"endpoint"` + EnableTLS bool `json:"enable_tls"` + AccessKeyID string `json:"access_key_id"` + AccessKeySecret string `json:"-"` + Bucket string `json:"bucket"` + Path string `json:"path"` } // Store ... S3 store From e3b993309cc592c67d8e90ff7a0abbe457ac2bc2 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 18 Nov 2024 16:21:56 +0400 Subject: [PATCH 2/6] fix: prettyPrintConfig hide values before marshalling --- cmd/server/entrypoint.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/server/entrypoint.go b/cmd/server/entrypoint.go index b709a824..98786aeb 100644 --- a/cmd/server/entrypoint.go +++ b/cmd/server/entrypoint.go @@ -76,12 +76,13 @@ func StartProxySvr(cliCtx *cli.Context) error { func prettyPrintConfig(cliCtx *cli.Context, log log.Logger) error { // we read a new config which we modify to hide private info in order to log the rest cfg := server.ReadCLIConfig(cliCtx) + cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "*****" // marshaling defined in client config + cfg.EigenDAConfig.EdaClientConfig.RPC = "*****" // hiding as RPC providers typically use sensitive API keys within + configJSON, err := json.MarshalIndent(cfg, "", " ") if err != nil { return fmt.Errorf("failed to marshal config: %w", err) } - cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "" // marshaling defined in client config - cfg.EigenDAConfig.EdaClientConfig.RPC = "" // hiding as RPC providers typically use sensitive API keys within log.Info(fmt.Sprintf("Initializing EigenDA proxy server with config: %v", string(configJSON))) return nil } From f188b1dbf93187a01877246da87968139963fa3a Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 18 Nov 2024 16:32:30 +0400 Subject: [PATCH 3/6] docs: add comment for why we are hiding password when marshalling --- store/precomputed_key/redis/redis.go | 2 +- store/precomputed_key/s3/s3.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/store/precomputed_key/redis/redis.go b/store/precomputed_key/redis/redis.go index 04f163d3..dc3db5b6 100644 --- a/store/precomputed_key/redis/redis.go +++ b/store/precomputed_key/redis/redis.go @@ -13,7 +13,7 @@ import ( // Config ... user configurable type Config struct { Endpoint string `json:"endpoint"` - Password string `json:"-"` + Password string `json:"-"` // hide password when marshaling (only used to print config on startup) DB int `json:"database"` Eviction time.Duration `json:"eviction"` } diff --git a/store/precomputed_key/s3/s3.go b/store/precomputed_key/s3/s3.go index fb39178c..a6749280 100644 --- a/store/precomputed_key/s3/s3.go +++ b/store/precomputed_key/s3/s3.go @@ -42,7 +42,7 @@ type Config struct { Endpoint string `json:"endpoint"` EnableTLS bool `json:"enable_tls"` AccessKeyID string `json:"access_key_id"` - AccessKeySecret string `json:"-"` + AccessKeySecret string `json:"-"` // hide key when marshaling (only used to print config on startup) Bucket string `json:"bucket"` Path string `json:"path"` } From 6c3b02c1c453ff0b0f3720cd0f92572507197df3 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 18 Nov 2024 17:15:34 +0400 Subject: [PATCH 4/6] fix(makefile): run-memstore-server command missing new mandatory flags --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 39648a68..2de9c461 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ docker-build: @docker build -t ghcr.io/layr-labs/eigenda-proxy:dev . run-memstore-server: - ./bin/eigenda-proxy --memstore.enabled + ./bin/eigenda-proxy --memstore.enabled --eigenda.cert-verification-disabled --eigenda.eth-rpc http://localhost:8545 --eigenda.svc-manager-addr 0x123 disperse-test-blob: curl -X POST -d my-blob-content http://127.0.0.1:3100/put/ From 47ae06d77385ea870b27f1e418ce1a0ef8d2ed61 Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 18 Nov 2024 17:16:27 +0400 Subject: [PATCH 5/6] fix: prettyPrintConfig was hiding wrong field. Change RPC->EthRpcUrl --- cmd/server/entrypoint.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/server/entrypoint.go b/cmd/server/entrypoint.go index 98786aeb..09bfdf6f 100644 --- a/cmd/server/entrypoint.go +++ b/cmd/server/entrypoint.go @@ -76,13 +76,17 @@ func StartProxySvr(cliCtx *cli.Context) error { func prettyPrintConfig(cliCtx *cli.Context, log log.Logger) error { // we read a new config which we modify to hide private info in order to log the rest cfg := server.ReadCLIConfig(cliCtx) - cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "*****" // marshaling defined in client config - cfg.EigenDAConfig.EdaClientConfig.RPC = "*****" // hiding as RPC providers typically use sensitive API keys within + if cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex != "" { + cfg.EigenDAConfig.EdaClientConfig.SignerPrivateKeyHex = "*****" // marshaling defined in client config + } + if cfg.EigenDAConfig.EdaClientConfig.EthRpcUrl != "" { + cfg.EigenDAConfig.EdaClientConfig.EthRpcUrl = "*****" // hiding as RPC providers typically use sensitive API keys within + } configJSON, err := json.MarshalIndent(cfg, "", " ") if err != nil { return fmt.Errorf("failed to marshal config: %w", err) } - log.Info(fmt.Sprintf("Initializing EigenDA proxy server with config: %v", string(configJSON))) + log.Info(fmt.Sprintf("Initializing EigenDA proxy server with config (\"*****\" fields are hidden): %v", string(configJSON))) return nil } From 4a1404b42979775ef2b90c09cee1ba2a674a154d Mon Sep 17 00:00:00 2001 From: Samuel Laferriere Date: Mon, 18 Nov 2024 17:17:07 +0400 Subject: [PATCH 6/6] refactor: way redis/s3 hides config details (use custom marshalling function) --- store/precomputed_key/redis/redis.go | 22 ++++++++++++++++++---- store/precomputed_key/s3/s3.go | 28 +++++++++++++++++++++------- verify/verifier.go | 12 ++++++++++++ 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/store/precomputed_key/redis/redis.go b/store/precomputed_key/redis/redis.go index dc3db5b6..4b30853e 100644 --- a/store/precomputed_key/redis/redis.go +++ b/store/precomputed_key/redis/redis.go @@ -2,6 +2,7 @@ package redis import ( "context" + "encoding/json" "errors" "fmt" "time" @@ -12,10 +13,23 @@ import ( // Config ... user configurable type Config struct { - Endpoint string `json:"endpoint"` - Password string `json:"-"` // hide password when marshaling (only used to print config on startup) - DB int `json:"database"` - Eviction time.Duration `json:"eviction"` + Endpoint string + Password string + DB int + Eviction time.Duration +} + +// Custom MarshalJSON function to control what gets included in the JSON output. +// TODO: Probably best would be to separate config from secrets everywhere. +// Then we could just log the config and not worry about secrets. +func (c Config) MarshalJSON() ([]byte, error) { + type Alias Config // Use an alias to avoid recursion with MarshalJSON + aux := (Alias)(c) + // Conditionally include a masked password if it is set + if aux.Password != "" { + aux.Password = "*****" + } + return json.Marshal(aux) } // Store ... Redis storage backend implementation diff --git a/store/precomputed_key/s3/s3.go b/store/precomputed_key/s3/s3.go index a6749280..fc468192 100644 --- a/store/precomputed_key/s3/s3.go +++ b/store/precomputed_key/s3/s3.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/hex" + "encoding/json" "errors" "fmt" "io" @@ -38,13 +39,26 @@ var _ common.PrecomputedKeyStore = (*Store)(nil) type CredentialType string type Config struct { - CredentialType CredentialType `json:"credential_type"` - Endpoint string `json:"endpoint"` - EnableTLS bool `json:"enable_tls"` - AccessKeyID string `json:"access_key_id"` - AccessKeySecret string `json:"-"` // hide key when marshaling (only used to print config on startup) - Bucket string `json:"bucket"` - Path string `json:"path"` + CredentialType CredentialType + Endpoint string + EnableTLS bool + AccessKeyID string + AccessKeySecret string + Bucket string + Path string +} + +// Custom MarshalJSON function to control what gets included in the JSON output +// TODO: Probably best would be to separate config from secrets everywhere. +// Then we could just log the config and not worry about secrets. +func (c Config) MarshalJSON() ([]byte, error) { + type Alias Config // Use an alias to avoid recursion with MarshalJSON + aux := (Alias)(c) + // Conditionally include a masked password if it is set + if aux.AccessKeySecret != "" { + aux.AccessKeySecret = "*****" + } + return json.Marshal(aux) } // Store ... S3 store diff --git a/verify/verifier.go b/verify/verifier.go index b825194d..a79c9f84 100644 --- a/verify/verifier.go +++ b/verify/verifier.go @@ -2,6 +2,7 @@ package verify import ( "context" + "encoding/json" "fmt" "math/big" @@ -28,6 +29,17 @@ type Config struct { WaitForFinalization bool } +// Custom MarshalJSON function to control what gets included in the JSON output +func (c Config) MarshalJSON() ([]byte, error) { + type Alias Config // Use an alias to avoid recursion with MarshalJSON + aux := (Alias)(c) + // Conditionally include a masked password if it is set + if aux.RPCURL != "" { + aux.RPCURL = "*****" + } + return json.Marshal(aux) +} + // TODO: right now verification and confirmation depth are tightly coupled. we should decouple them type Verifier struct { // kzgVerifier is needed to commit blobs to the memstore