From 3a574a4ae8c19481107b98a9ac1515fe39ac12e7 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 22 Feb 2023 00:27:59 -0500 Subject: [PATCH 1/3] rename some TLS keys and variables to be consistent Renames NoTlsVerify to TlsNoVerify, adds Tls prefix to client cert vars & config keys. Adds missing config methods. Update tests to match new keys & methods. Fills in some gaps from first adding the TLS support. Adds comments indicating --no-tls-verify and its related envvar are deprecated. --- data_for_test.go | 16 +++++++++--- example-config.json | 8 +++--- otelcli/config.go | 58 +++++++++++++++++++++++++++++------------- otelcli/config_test.go | 19 ++++++++++++-- otelcli/plumbing.go | 20 +++++++-------- otelcli/root.go | 10 ++++---- 6 files changed, 90 insertions(+), 41 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index 63c0fcd..3269306 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -150,6 +150,7 @@ var suites = []FixtureSuite{ "status", "--endpoint", "https://{{endpoint}}", "--protocol", "grpc", + // TODO: switch to --tls-no-verify before 1.0, for now keep testing it "--verbose", "--fail", "--no-tls-verify", }, TestTimeoutMs: 1000, @@ -161,7 +162,7 @@ var suites = []FixtureSuite{ WithEndpoint("https://{{endpoint}}"). WithProtocol("grpc"). WithVerbose(true). - WithNoTlsVerify(true), + WithTlsNoVerify(true), Diagnostics: otelcli.Diagnostics{ IsRecording: true, NumArgs: 8, @@ -183,7 +184,7 @@ var suites = []FixtureSuite{ Expect: Results{ // otel-cli should NOT set insecure when it auto-detects localhost Config: otelcli.DefaultConfig(). - WithNoTlsVerify(true). + WithTlsNoVerify(true). WithEndpoint("https://{{endpoint}}"), Diagnostics: otelcli.Diagnostics{ IsRecording: true, @@ -215,6 +216,9 @@ var suites = []FixtureSuite{ Config: otelcli.DefaultConfig(). WithEndpoint("https://{{endpoint}}"). WithProtocol("grpc"). + WithTlsCACert("{{cacert}}"). + WithTlsClientKey("{{client_key}}"). + WithTlsClientCert("{{client_cert}}"). WithVerbose(true), Diagnostics: otelcli.Diagnostics{ IsRecording: true, @@ -245,6 +249,9 @@ var suites = []FixtureSuite{ Expect: Results{ Config: otelcli.DefaultConfig(). WithEndpoint("https://{{endpoint}}"). + WithTlsCACert("{{cacert}}"). + WithTlsClientKey("{{client_key}}"). + WithTlsClientCert("{{client_cert}}"). WithVerbose(true), Diagnostics: otelcli.Diagnostics{ IsRecording: true, @@ -352,7 +359,10 @@ var suites = []FixtureSuite{ WithHeaders(map[string]string{"header1": "header1-value"}). WithInsecure(true). WithBlocking(false). - WithNoTlsVerify(true). + WithTlsNoVerify(true). + WithTlsCACert("/dev/null"). + WithTlsClientKey("/dev/null"). + WithTlsClientCert("/dev/null"). WithServiceName("configured_in_config_file"). WithSpanName("config_file_span"). WithKind("server"). diff --git a/example-config.json b/example-config.json index 8c5ff6b..214436b 100644 --- a/example-config.json +++ b/example-config.json @@ -7,10 +7,10 @@ "otlp_blocking" : false, "insecure" : true, - "no_tls_verify" : true, - "ca_file": "/dev/null", - "cert_file": "/dev/null", - "key_file": "/dev/null", + "tls_no_verify" : true, + "tls_ca_cert": "/dev/null", + "tls_client_key": "/dev/null", + "tls_client_cert": "/dev/null", "service_name" : "configured_in_config_file", diff --git a/otelcli/config.go b/otelcli/config.go index a9b7cd0..15a1403 100644 --- a/otelcli/config.go +++ b/otelcli/config.go @@ -29,7 +29,10 @@ func DefaultConfig() Config { Headers: map[string]string{}, Insecure: false, Blocking: false, - NoTlsVerify: false, + TlsNoVerify: false, + TlsCACert: "", + TlsClientKey: "", + TlsClientCert: "", ServiceName: "otel-cli", SpanName: "todo-generate-default-span-names", Kind: "client", @@ -59,18 +62,18 @@ func DefaultConfig() Config { // This is used as a singleton as "config" and accessed from many other files. // Data structure is public so that it can serialize to json easily. type Config struct { - Endpoint string `json:"endpoint" env:"OTEL_EXPORTER_OTLP_ENDPOINT,OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"` - Protocol string `json:"protocol" env:"OTEL_EXPORTER_OTLP_PROTOCOL,OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"` - Timeout string `json:"timeout" env:"OTEL_EXPORTER_OTLP_TIMEOUT,OTEL_EXPORTER_OTLP_TRACES_TIMEOUT"` - Headers map[string]string `json:"otlp_headers" env:"OTEL_EXPORTER_OTLP_HEADERS"` // TODO: needs json marshaler hook to mask tokens - Insecure bool `json:"insecure" env:"OTEL_EXPORTER_OTLP_INSECURE"` - Blocking bool `json:"otlp_blocking" env:"OTEL_EXPORTER_OTLP_BLOCKING"` - NoTlsVerify bool `json:"no_tls_verify" env:"OTEL_CLI_NO_TLS_VERIFY"` - - // json keys match the otel collector yaml - CACert string `json:"ca_file" env:"OTEL_EXPORTER_OTLP_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE"` - ClientKey string `json:"cert_file" env:"OTEL_EXPORTER_OTLP_CLIENT_KEY,OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY"` - ClientCert string `json:"key_file" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE"` + Endpoint string `json:"endpoint" env:"OTEL_EXPORTER_OTLP_ENDPOINT,OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"` + Protocol string `json:"protocol" env:"OTEL_EXPORTER_OTLP_PROTOCOL,OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"` + Timeout string `json:"timeout" env:"OTEL_EXPORTER_OTLP_TIMEOUT,OTEL_EXPORTER_OTLP_TRACES_TIMEOUT"` + Headers map[string]string `json:"otlp_headers" env:"OTEL_EXPORTER_OTLP_HEADERS"` // TODO: needs json marshaler hook to mask tokens + Insecure bool `json:"insecure" env:"OTEL_EXPORTER_OTLP_INSECURE"` + Blocking bool `json:"otlp_blocking" env:"OTEL_EXPORTER_OTLP_BLOCKING"` + + TlsCACert string `json:"tls_ca_cert" env:"OTEL_EXPORTER_OTLP_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE"` + TlsClientKey string `json:"tls_client_key" env:"OTEL_EXPORTER_OTLP_CLIENT_KEY,OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY"` + TlsClientCert string `json:"tls_client_cert" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE"` + // OTEL_CLI_NO_TLS_VERIFY is deprecated and will be removed for 1.0 + TlsNoVerify bool `json:"tls_no_verify" env:"OTEL_CLI_TLS_NO_VERIFY,OTEL_CLI_NO_TLS_VERIFY"` ServiceName string `json:"service_name" env:"OTEL_CLI_SERVICE_NAME,OTEL_SERVICE_NAME"` SpanName string `json:"span_name" env:"OTEL_CLI_SPAN_NAME"` @@ -188,7 +191,10 @@ func (c Config) ToStringMap() map[string]string { "headers": flattenStringMap(c.Headers, "{}"), "insecure": strconv.FormatBool(c.Insecure), "blocking": strconv.FormatBool(c.Blocking), - "no_tls_verify": strconv.FormatBool(c.NoTlsVerify), + "tls_no_verify": strconv.FormatBool(c.TlsNoVerify), + "tls_ca_cert": c.TlsCACert, + "tls_client_key": c.TlsClientKey, + "tls_client_cert": c.TlsClientCert, "service_name": c.ServiceName, "span_name": c.SpanName, "span_kind": c.Kind, @@ -249,9 +255,27 @@ func (c Config) WithBlocking(with bool) Config { return c } -// WithNoTlsVerify returns the config with NoTlsVerify set to the provided value. -func (c Config) WithNoTlsVerify(with bool) Config { - c.NoTlsVerify = with +// WithTlsNoVerify returns the config with NoTlsVerify set to the provided value. +func (c Config) WithTlsNoVerify(with bool) Config { + c.TlsNoVerify = with + return c +} + +// WithTlsCACert returns the config with TlsCACert set to the provided value. +func (c Config) WithTlsCACert(with string) Config { + c.TlsCACert = with + return c +} + +// WithTlsClientKey returns the config with NoTlsClientKey set to the provided value. +func (c Config) WithTlsClientKey(with string) Config { + c.TlsClientKey = with + return c +} + +// WithTlsClientCert returns the config with NoTlsClientCert set to the provided value. +func (c Config) WithTlsClientCert(with string) Config { + c.TlsClientCert = with return c } diff --git a/otelcli/config_test.go b/otelcli/config_test.go index 9eadba3..18be2a8 100644 --- a/otelcli/config_test.go +++ b/otelcli/config_test.go @@ -52,8 +52,23 @@ func TestWithBlocking(t *testing.T) { t.Fail() } } -func TestWithNoTlsVerify(t *testing.T) { - if DefaultConfig().WithNoTlsVerify(true).NoTlsVerify != true { +func TestWithTlsNoVerify(t *testing.T) { + if DefaultConfig().WithTlsNoVerify(true).TlsNoVerify != true { + t.Fail() + } +} +func TestWithTlsCACert(t *testing.T) { + if DefaultConfig().WithTlsCACert("/a/b/c").TlsCACert != "/a/b/c" { + t.Fail() + } +} +func TestWithTlsClientKey(t *testing.T) { + if DefaultConfig().WithTlsClientKey("/c/b/a").TlsClientKey != "/c/b/a" { + t.Fail() + } +} +func TestWithTlsClientCert(t *testing.T) { + if DefaultConfig().WithTlsClientCert("/b/c/a").TlsClientCert != "/b/c/a" { t.Fail() } } diff --git a/otelcli/plumbing.go b/otelcli/plumbing.go index 7ecd9f2..324bec6 100644 --- a/otelcli/plumbing.go +++ b/otelcli/plumbing.go @@ -105,15 +105,15 @@ func initTracer() (context.Context, func()) { func tlsConfig() *tls.Config { tlsConfig := &tls.Config{} - if config.NoTlsVerify { + if config.TlsNoVerify { diagnostics.InsecureSkipVerify = true tlsConfig.InsecureSkipVerify = true } // puts the provided CA certificate into the root pool // when not provided, Go TLS will automatically load the system CA pool - if config.CACert != "" { - data, err := os.ReadFile(config.CACert) + if config.TlsCACert != "" { + data, err := os.ReadFile(config.TlsCACert) if err != nil { softFail("failed to load CA certificate: %s", err) } @@ -124,23 +124,23 @@ func tlsConfig() *tls.Config { } // client certificate authentication - if config.ClientCert != "" && config.ClientKey != "" { - clientPEM, err := os.ReadFile(config.ClientCert) + if config.TlsClientCert != "" && config.TlsClientKey != "" { + clientPEM, err := os.ReadFile(config.TlsClientCert) if err != nil { - softFail("failed to read client certificate file %s: %s", config.ClientCert, err) + softFail("failed to read client certificate file %s: %s", config.TlsClientCert, err) } - clientKeyPEM, err := os.ReadFile(config.ClientKey) + clientKeyPEM, err := os.ReadFile(config.TlsClientKey) if err != nil { - softFail("failed to read client key file %s: %s", config.ClientKey, err) + softFail("failed to read client key file %s: %s", config.TlsClientKey, err) } certPair, err := tls.X509KeyPair(clientPEM, clientKeyPEM) if err != nil { softFail("failed to parse client cert pair: %s", err) } tlsConfig.Certificates = []tls.Certificate{certPair} - } else if config.ClientCert != "" { + } else if config.TlsClientCert != "" { softFail("client cert and key must be specified together") - } else if config.ClientKey != "" { + } else if config.TlsClientKey != "" { softFail("client cert and key must be specified together") } diff --git a/otelcli/root.go b/otelcli/root.go index 1d40983..fbeb35e 100644 --- a/otelcli/root.go +++ b/otelcli/root.go @@ -67,12 +67,12 @@ func addClientParams(cmd *cobra.Command) { cmd.Flags().BoolVar(&config.Blocking, "otlp-blocking", defaults.Blocking, "block on connecting to the OTLP server before proceeding") cmd.Flags().BoolVar(&config.Insecure, "insecure", defaults.Insecure, "allow connecting to cleartext endpoints") - cmd.Flags().StringVar(&config.CACert, "tls-ca-cert", defaults.CACert, "a file containing the certificate authority bundle") - cmd.Flags().StringVar(&config.ClientCert, "tls-client-cert", defaults.ClientCert, "a file containing the client certificate") - cmd.Flags().StringVar(&config.ClientKey, "tls-client-key", defaults.ClientKey, "a file containing the client certificate key") - cmd.Flags().BoolVar(&config.NoTlsVerify, "tls-no-verify", defaults.NoTlsVerify, "insecure! disables verification of the server certificate and name, mostly for self-signed CAs") + cmd.Flags().StringVar(&config.TlsCACert, "tls-ca-cert", defaults.TlsCACert, "a file containing the certificate authority bundle") + cmd.Flags().StringVar(&config.TlsClientCert, "tls-client-cert", defaults.TlsClientCert, "a file containing the client certificate") + cmd.Flags().StringVar(&config.TlsClientKey, "tls-client-key", defaults.TlsClientKey, "a file containing the client certificate key") + cmd.Flags().BoolVar(&config.TlsNoVerify, "tls-no-verify", defaults.TlsNoVerify, "insecure! disables verification of the server certificate and name, mostly for self-signed CAs") // --no-tls-verify is deprecated, will remove before 1.0 - cmd.Flags().BoolVar(&config.NoTlsVerify, "no-tls-verify", defaults.NoTlsVerify, "(deprecated) same as --tls-no-verify") + cmd.Flags().BoolVar(&config.TlsNoVerify, "no-tls-verify", defaults.TlsNoVerify, "(deprecated) same as --tls-no-verify") // OTEL_CLI trace propagation options cmd.Flags().BoolVar(&config.TraceparentRequired, "tp-required", defaults.TraceparentRequired, "when set to true, fail and log if a traceparent can't be picked up from TRACEPARENT ennvar or a carrier file") From 829edbcd9efa8e18550e214a9f9c7eae539a3889 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 22 Feb 2023 00:34:01 -0500 Subject: [PATCH 2/3] rename test template variables to match config keys & flags --- data_for_test.go | 24 ++++++++++++------------ main_test.go | 13 +++++++------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index 3269306..1944ab7 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -204,9 +204,9 @@ var suites = []FixtureSuite{ "--endpoint", "https://{{endpoint}}", "--protocol", "grpc", "--verbose", "--fail", - "--tls-ca-cert", "{{cacert}}", - "--tls-client-cert", "{{client_cert}}", - "--tls-client-key", "{{client_key}}", + "--tls-ca-cert", "{{tls_ca_cert}}", + "--tls-client-cert", "{{tls_client_cert}}", + "--tls-client-key", "{{tls_client_key}}", }, TestTimeoutMs: 1000, ServerTLSEnabled: true, @@ -216,9 +216,9 @@ var suites = []FixtureSuite{ Config: otelcli.DefaultConfig(). WithEndpoint("https://{{endpoint}}"). WithProtocol("grpc"). - WithTlsCACert("{{cacert}}"). - WithTlsClientKey("{{client_key}}"). - WithTlsClientCert("{{client_cert}}"). + WithTlsCACert("{{tls_ca_cert}}"). + WithTlsClientKey("{{tls_client_key}}"). + WithTlsClientCert("{{tls_client_cert}}"). WithVerbose(true), Diagnostics: otelcli.Diagnostics{ IsRecording: true, @@ -238,9 +238,9 @@ var suites = []FixtureSuite{ "status", "--endpoint", "https://{{endpoint}}", "--verbose", "--fail", - "--tls-ca-cert", "{{cacert}}", - "--tls-client-cert", "{{client_cert}}", - "--tls-client-key", "{{client_key}}", + "--tls-ca-cert", "{{tls_ca_cert}}", + "--tls-client-cert", "{{tls_client_cert}}", + "--tls-client-key", "{{tls_client_key}}", }, TestTimeoutMs: 2000, ServerTLSEnabled: true, @@ -249,9 +249,9 @@ var suites = []FixtureSuite{ Expect: Results{ Config: otelcli.DefaultConfig(). WithEndpoint("https://{{endpoint}}"). - WithTlsCACert("{{cacert}}"). - WithTlsClientKey("{{client_key}}"). - WithTlsClientCert("{{client_cert}}"). + WithTlsCACert("{{tls_ca_cert}}"). + WithTlsClientKey("{{tls_client_key}}"). + WithTlsClientCert("{{tls_client_cert}}"). WithVerbose(true), Diagnostics: otelcli.Diagnostics{ IsRecording: true, diff --git a/main_test.go b/main_test.go index 822d995..4298aad 100644 --- a/main_test.go +++ b/main_test.go @@ -492,23 +492,24 @@ func mkEnviron(endpoint string, env map[string]string, tlsData tlsHelpers) []str } // injectMapVars iterates over the map and updates the values, replacing all instances -// of {{endpoint}}, {{cacert}}, {{client_cert}}, and {{client_key}} with test values. +// of {{endpoint}}, {{tls_ca_cert}}, {{tls_client_cert}}, and {{tls_client_key}} with +// test values. func injectMapVars(endpoint string, target map[string]string, tlsData tlsHelpers) { for k, v := range target { target[k] = injectVars(v, endpoint, tlsData) } } -// injectVars replaces all instances of {{endpoint}}, {{cacert}}, {{client_cert}}, and -// {{client_key}} with test values. +// injectVars replaces all instances of {{endpoint}}, {{tls_ca_cert}}, +// {{tls_client_cert}}, and {{tls_client_key}} with test values. // This is needed because the otlpserver is configured to listen on :0 which has it // grab a random port. Once that's generated we need to inject it into all the values // so the test comparisons work as expected. Similarly for TLS testing, a temp CA and // certs are created and need to be injected. func injectVars(in, endpoint string, tlsData tlsHelpers) string { out := strings.ReplaceAll(in, "{{endpoint}}", endpoint) - out = strings.ReplaceAll(out, "{{cacert}}", tlsData.caFile) - out = strings.ReplaceAll(out, "{{client_cert}}", tlsData.clientFile) - out = strings.ReplaceAll(out, "{{client_key}}", tlsData.clientPrivKeyFile) + out = strings.ReplaceAll(out, "{{tls_ca_cert}}", tlsData.caFile) + out = strings.ReplaceAll(out, "{{tls_client_cert}}", tlsData.clientFile) + out = strings.ReplaceAll(out, "{{tls_client_key}}", tlsData.clientPrivKeyFile) return out } From 1b4785135c593b4e973ca593affa601059c090f9 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 22 Feb 2023 01:02:55 -0500 Subject: [PATCH 3/3] add TLS settings to README.md --- README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a89ea21..6070779 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,10 @@ then config file, then environment variables. | --tp-ignore-env | OTEL_CLI_IGNORE_ENV | traceparent_ignore_env | false | | --tp-print | OTEL_CLI_PRINT_TRACEPARENT | traceparent_print | false | | --tp-export | OTEL_CLI_EXPORT_TRACEPARENT | traceparent_print_export | false | -| --no-tls-verify | OTEL_CLI_NO_TLS_VERIFY | no_tls_verify | false | +| --tls-no-verify | OTEL_CLI_TLS_NO_VERIFY | tls_no_verify | false | +| --tls-ca-cert | OTEL_EXPORTER_OTLP_CERTIFICATE | tls_ca_cert | | +| --tls-client-key | OTEL_EXPORTER_OTLP_CLIENT_KEY | tls_client_key | | +| --tls-client-cert | OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE | tls_client_cert | | [Valid timeout units](https://pkg.go.dev/time#ParseDuration) are "ns", "us"/"µs", "ms", "s", "m", "h".