From 1148d684d7ec280ebc58502f0754434dadcaf848 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Thu, 2 Feb 2023 18:18:51 -0500 Subject: [PATCH 01/25] add TLS support & tests Adds the first pass at TLS helpers and integration with tests. https works now, grpc does not and needs more work --- data_for_test.go | 61 ++++++++++++++- main_test.go | 55 +++++++++---- otelcli/config.go | 4 + tls_for_test.go | 196 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 299 insertions(+), 17 deletions(-) create mode 100644 tls_for_test.go diff --git a/data_for_test.go b/data_for_test.go index f4864bd..8569292 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -27,7 +27,8 @@ type FixtureConfig struct { // TODO: maybe move this up to the suite? IsLongTest bool // either grpcProtocol or httpProtocol, defaults to grpc - ServerProtocol serverProtocol + ServerProtocol serverProtocol + ServerTLSEnabled bool // for timeout tests we need to start the server to generate the endpoint // but do not want it to answer when otel-cli calls, this does that StopServerBeforeExec bool @@ -135,6 +136,64 @@ var suites = []FixtureSuite{ Spans: 1, }, }, + // TLS connections + /* + { + Name: "minimum configuration (tls, recording, grpc)", + Config: FixtureConfig{ + ServerProtocol: grpcProtocol, + CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--protocol", "grpc", "--no-tls-verify"}, + TestTimeoutMs: 1000, + ServerTLSEnabled: true, + Env: map[string]string{ + "SSL_CERT_FILE": "{{cert}}", + }, + }, + Expect: Results{ + // otel-cli should NOT set insecure when it auto-detects localhost + Config: otelcli.DefaultConfig(). + WithEndpoint("{{endpoint}}"), + Diagnostics: otelcli.Diagnostics{ + IsRecording: true, + NumArgs: 6, + DetectedLocalhost: true, + ParsedTimeoutMs: 1000, + }, + Spans: 1, + Env: map[string]string{ + "SSL_CERT_FILE": "{{cert}}", + }, + }, + }, + */ + { + Name: "minimum configuration (tls, recording, https)", + Config: FixtureConfig{ + ServerProtocol: httpProtocol, + CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--no-tls-verify"}, + TestTimeoutMs: 2000, + ServerTLSEnabled: true, + Env: map[string]string{ + "SSL_CERT_FILE": "{{cert}}", + }, + }, + Expect: Results{ + // otel-cli should NOT set insecure when it auto-detects localhost + Config: otelcli.DefaultConfig(). + WithEndpoint("https://{{endpoint}}"). + WithNoTlsVerify(true), + Diagnostics: otelcli.Diagnostics{ + IsRecording: true, + NumArgs: 4, + DetectedLocalhost: true, + ParsedTimeoutMs: 1000, + }, + Env: map[string]string{ + "SSL_CERT_FILE": "{{cert}}", + }, + Spans: 1, + }, + }, }, // ensure things fail when they're supposed to fail { diff --git a/main_test.go b/main_test.go index 3825c20..a2742ec 100644 --- a/main_test.go +++ b/main_test.go @@ -5,6 +5,7 @@ package main_test // TODO: stop using fixture.Name to track foreground/background import ( + "crypto/tls" "encoding/json" "log" "net" @@ -24,6 +25,8 @@ import ( const minimumPath = `/bin:/usr/bin` const defaultTestTimeout = time.Second +var tlsTest tlsHelpers + func TestMain(m *testing.M) { // wipe out this process's envvars right away to avoid pollution & leakage os.Clearenv() @@ -38,6 +41,8 @@ func TestOtelCli(t *testing.T) { t.Fatalf("otel-cli must be built and present as ./otel-cli for this test suite to work (try: go build)") } + tlsTest = generateTLSData(t) + var fixtureCount int for _, suite := range suites { fixtureCount += len(suite) @@ -176,7 +181,7 @@ func checkProcess(t *testing.T, fixture Fixture, results Results) { // checkOutput looks that otel-cli output stored in the results and compares against // the fixture expectation (with {{endpoint}} replaced). func checkOutput(t *testing.T, fixture Fixture, endpoint string, results Results) { - wantOutput := strings.ReplaceAll(fixture.Expect.CliOutput, "{{endpoint}}", endpoint) + wantOutput := injectVars(fixture.Expect.CliOutput, endpoint) gotOutput := results.CliOutput if fixture.Expect.CliOutputRe != nil { gotOutput = fixture.Expect.CliOutputRe.ReplaceAllString(gotOutput, "") @@ -193,7 +198,7 @@ func checkOutput(t *testing.T, fixture Fixture, endpoint string, results Results // fixture data, substituting {{endpoint}} into fixture data as needed. func checkStatusData(t *testing.T, fixture Fixture, endpoint string, results Results) { // check the env - injectEndpoint(endpoint, fixture.Expect.Env) + injectMapVars(endpoint, fixture.Expect.Env) if diff := cmp.Diff(fixture.Expect.Env, results.Env); diff != "" { t.Errorf("env data did not match fixture in %q (-want +got):\n%s", fixture.Name, diff) } @@ -201,7 +206,7 @@ func checkStatusData(t *testing.T, fixture Fixture, endpoint string, results Res // check diagnostics, use string maps so the diff output is easy to compare to json wantDiag := fixture.Expect.Diagnostics.ToStringMap() gotDiag := results.Diagnostics.ToStringMap() - injectEndpoint(endpoint, wantDiag) + injectMapVars(endpoint, wantDiag) // there's almost always going to be cli_args in practice, so if the fixture has // an empty string, just ignore that key if wantDiag["cli_args"] == "" { @@ -222,7 +227,7 @@ func checkStatusData(t *testing.T, fixture Fixture, endpoint string, results Res wantConf[k] = gotConf[k] } } - injectEndpoint(endpoint, wantConf) + injectMapVars(endpoint, wantConf) if diff := cmp.Diff(wantConf, gotConf); diff != "" { t.Errorf("[%s] config data did not match fixture (-want +got):\n%s", fixture.Name, diff) } @@ -246,7 +251,7 @@ var spanRegexChecks = map[string]*regexp.Regexp{ func checkSpanData(t *testing.T, fixture Fixture, endpoint string, span otlpserver.CliEvent, events otlpserver.CliEventList) { // check the expected span data against what was received by the OTLP server gotSpan := span.ToStringMap() - injectEndpoint(endpoint, gotSpan) + injectMapVars(endpoint, gotSpan) // remove keys that aren't supported for comparison (for now) delete(gotSpan, "is_populated") delete(gotSpan, "library") @@ -285,7 +290,7 @@ func checkSpanData(t *testing.T, fixture Fixture, endpoint string, span otlpserv } // do a diff on a generated map that sets values to * when the * check succeeded - injectEndpoint(endpoint, wantSpan) + injectMapVars(endpoint, wantSpan) if diff := cmp.Diff(wantSpan, gotSpan); diff != "" { t.Errorf("[%s] otel span info did not match fixture (-want +got):\n%s", fixture.Name, diff) } @@ -346,7 +351,13 @@ func runOtelCli(t *testing.T, fixture Fixture) (string, Results, otlpserver.CliE }() // port :0 means randomly assigned port, which we copy into {{endpoint}} - listener, err := net.Listen("tcp", "localhost:0") + var listener net.Listener + var err error + if fixture.Config.ServerTLSEnabled { + listener, err = tls.Listen("tcp", "localhost:0", tlsTest.serverTLSConf) + } else { + listener, err = net.Listen("tcp", "localhost:0") + } endpoint := listener.Addr().String() if err != nil { // t.Fatalf is not allowed since we run this in a goroutine @@ -374,7 +385,7 @@ func runOtelCli(t *testing.T, fixture Fixture) (string, Results, otlpserver.CliE args := []string{} if len(fixture.Config.CliArgs) > 0 { for _, v := range fixture.Config.CliArgs { - args = append(args, strings.ReplaceAll(v, "{{endpoint}}", endpoint)) + args = append(args, injectVars(v, endpoint)) } } statusCmd := exec.Command("./otel-cli", args...) @@ -412,7 +423,7 @@ func runOtelCli(t *testing.T, fixture Fixture) (string, Results, otlpserver.CliE // only try to parse status json if it was a status command // TODO: support variations on otel-cli where status isn't the first arg - if len(cliOut) > 0 && len(args) > 0 && args[0] == "status" && !fixture.Expect.CommandFailed { + if len(args) > 0 && args[0] == "status" && !fixture.Expect.CommandFailed { err = json.Unmarshal(cliOut, &results) if err != nil { t.Errorf("[%s] parsing otel-cli status output failed: %s", fixture.Name, err) @@ -464,7 +475,7 @@ func mkEnviron(endpoint string, env map[string]string) []string { mapped := make([]string, len(env)+1) var i int for k, v := range env { - mapped[i] = k + "=" + strings.ReplaceAll(v, "{{endpoint}}", endpoint) + mapped[i] = k + "=" + injectVars(v, endpoint) i++ } @@ -475,12 +486,24 @@ func mkEnviron(endpoint string, env map[string]string) []string { return mapped } -// injectEndpoint iterates over the map and updates the values, replacing all instances -// of {{endpoint}} with the provided endpoint. 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. -func injectEndpoint(endpoint string, target map[string]string) { +// injectMapVars iterates over the map and updates the values, replacing all instances +// of {{endpoint}}, {{cert}}, {{client_key}}, and {{client_key_cert}} with test values. +func injectMapVars(endpoint string, target map[string]string) { for k, v := range target { - target[k] = strings.ReplaceAll(v, "{{endpoint}}", endpoint) + target[k] = injectVars(v, endpoint) } } + +// injectVars replaces all instances of {{endpoint}}, {{cert}}, {{client_key}}, and +// {{client_key_cert}} 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) string { + out := strings.ReplaceAll(in, "{{endpoint}}", endpoint) + out = strings.ReplaceAll(out, "{{cert}}", tlsTest.caFile) + out = strings.ReplaceAll(out, "{{client_key}}", tlsTest.clientPrivKeyFile) + out = strings.ReplaceAll(out, "{{client_key_cert}}", tlsTest.clientFile) + return out +} diff --git a/otelcli/config.go b/otelcli/config.go index 17e65d7..fd54de0 100644 --- a/otelcli/config.go +++ b/otelcli/config.go @@ -66,6 +66,10 @@ type Config struct { Blocking bool `json:"otlp_blocking" env:"OTEL_EXPORTER_OTLP_BLOCKING"` NoTlsVerify bool `json:"no_tls_verify" env:"OTEL_CLI_NO_TLS_VERIFY"` + Certificate string `json:"certificate_file" env:"OTEL_EXPORTER_OTLP_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE"` + ClientKey string `json:"client_key_file" env:"OTEL_EXPORTER_OTLP_CLIENT_KEY,OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY"` + ClientCert string `json:"client_key_certificate_file" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTFICATE"` + ServiceName string `json:"service_name" env:"OTEL_CLI_SERVICE_NAME"` SpanName string `json:"span_name" env:"OTEL_CLI_SPAN_NAME"` Kind string `json:"span_kind" env:"OTEL_CLI_TRACE_KIND"` diff --git a/tls_for_test.go b/tls_for_test.go new file mode 100644 index 0000000..14413ff --- /dev/null +++ b/tls_for_test.go @@ -0,0 +1,196 @@ +package main_test + +import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net" + "os" + "testing" + "time" +) + +type tlsHelpers struct { + pool string + ca *x509.Certificate + caPrivKey *ecdsa.PrivateKey + caPEM *bytes.Buffer + caPrivKeyPEM *bytes.Buffer + caFile string + caPrivKeyFile string + serverCert *x509.Certificate + serverPrivKey *ecdsa.PrivateKey + serverPEM *bytes.Buffer + serverFile string + serverPrivKeyPEM *bytes.Buffer + serverPrivKeyFile string + serverCertPair tls.Certificate + certpool *x509.CertPool + serverTLSConf *tls.Config + clientCert *x509.Certificate + clientPrivKey *ecdsa.PrivateKey + clientPEM *bytes.Buffer + clientFile string + clientPrivKeyPEM *bytes.Buffer + clientPrivKeyFile string + clientTLSConf *tls.Config +} + +func generateTLSData(t *testing.T) tlsHelpers { + var err error + var out tlsHelpers + + // ------------- CA ------------- + + out.ca = &x509.Certificate{ + SerialNumber: big.NewInt(4317), + Subject: pkix.Name{ + Organization: []string{"otel-cli testing, inc"}, + Country: []string{"Open Source"}, + Province: []string{"Go"}, + Locality: []string{"OpenTelemetry"}, + StreetAddress: []string{"github.com/equinix-labs/otel-cli"}, + PostalCode: []string{"4317"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + } + + // create a private key + out.caPrivKey, err = ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + if err != nil { + t.Fatalf("error generating ca private key: %s", err) + } + + // create a cert on the CA with the ^^ private key + caBytes, err := x509.CreateCertificate(rand.Reader, out.ca, out.ca, &out.caPrivKey.PublicKey, out.caPrivKey) + if err != nil { + t.Fatalf("error generating ca cert: %s", err) + } + + // get the PEM encoding that the tests will use + out.caPEM = new(bytes.Buffer) + pem.Encode(out.caPEM, &pem.Block{Type: "CERTIFICATE", Bytes: caBytes}) + out.caFile = pemToTempFile(t, out.caPEM) + + out.caPrivKeyPEM = new(bytes.Buffer) + caPrivKeyBytes, err := x509.MarshalECPrivateKey(out.caPrivKey) + if err != nil { + t.Fatalf("error marshaling server cert: %s", err) + } + pem.Encode(out.caPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: caPrivKeyBytes}) + out.caPrivKeyFile = pemToTempFile(t, out.caPrivKeyPEM) + + out.certpool = x509.NewCertPool() + out.certpool.AppendCertsFromPEM(out.caPEM.Bytes()) + + data := new(bytes.Buffer) + pem.Encode(data, &pem.Block{Type: "EC PRIVATE KEY", Bytes: caPrivKeyBytes}) + + // ------------- server ------------- + + out.serverCert = &x509.Certificate{ + SerialNumber: big.NewInt(4318), + Subject: out.ca.Subject, + SubjectKeyId: []byte{1, 2, 3, 4, 6}, + IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, + DNSNames: []string{"localhost"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature, + } + + out.serverPrivKey, err = ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + if err != nil { + t.Fatalf("error generating server private key: %s", err) + } + + serverBytes, err := x509.CreateCertificate(rand.Reader, out.serverCert, out.ca, &out.serverPrivKey.PublicKey, out.caPrivKey) + if err != nil { + t.Fatalf("error generating server cert: %s", err) + } + + out.serverPEM = new(bytes.Buffer) + pem.Encode(out.serverPEM, &pem.Block{Type: "CERTIFICATE", Bytes: serverBytes}) + out.serverFile = pemToTempFile(t, out.serverPEM) + + out.serverPrivKeyPEM = new(bytes.Buffer) + serverPrivKeyBytes, err := x509.MarshalECPrivateKey(out.serverPrivKey) + if err != nil { + t.Fatalf("error marshaling server cert: %s", err) + } + pem.Encode(out.serverPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: serverPrivKeyBytes}) + out.serverPrivKeyFile = pemToTempFile(t, out.serverPrivKeyPEM) + + out.serverCertPair, err = tls.X509KeyPair(out.serverPEM.Bytes(), out.serverPrivKeyPEM.Bytes()) + if err != nil { + t.Fatalf("error generating server cert pair: %s", err) + } + + out.serverTLSConf = &tls.Config{ + RootCAs: out.certpool, + Certificates: []tls.Certificate{out.serverCertPair}, + } + + // ------------- client ------------- + + out.clientCert = &x509.Certificate{ + SerialNumber: big.NewInt(4319), + Subject: out.ca.Subject, + SubjectKeyId: []byte{1, 2, 3, 4, 6}, + IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, + DNSNames: []string{"localhost"}, + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageDigitalSignature, + } + + out.clientPrivKey, err = ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + if err != nil { + t.Fatalf("error generating client private key: %s", err) + } + + clientBytes, err := x509.CreateCertificate(rand.Reader, out.clientCert, out.ca, &out.clientPrivKey.PublicKey, out.caPrivKey) + if err != nil { + t.Fatalf("error generating client cert: %s", err) + } + + out.clientPEM = new(bytes.Buffer) + pem.Encode(out.clientPEM, &pem.Block{Type: "CERTIFICATE", Bytes: clientBytes}) + out.clientFile = pemToTempFile(t, out.clientPEM) + + out.clientPrivKeyPEM = new(bytes.Buffer) + clientPrivKeyBytes, err := x509.MarshalECPrivateKey(out.clientPrivKey) + if err != nil { + t.Fatalf("error marshaling client cert: %s", err) + } + pem.Encode(out.clientPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: clientPrivKeyBytes}) + out.clientPrivKeyFile = pemToTempFile(t, out.clientPrivKeyPEM) + + out.clientTLSConf = &tls.Config{ + RootCAs: out.certpool, + } + + return out +} + +func pemToTempFile(t *testing.T, buf *bytes.Buffer) string { + tmp, err := os.CreateTemp(os.TempDir(), "otel-cli-test-pem") + if err != nil { + t.Fatalf("error creating temp file: %s", err) + } + tmp.Write(buf.Bytes()) + tmp.Close() + return tmp.Name() +} From 25bba11fba0d458e82ced6188892d7796663c2f0 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Thu, 2 Feb 2023 18:28:52 -0500 Subject: [PATCH 02/25] add tempfile cleanup --- main_test.go | 1 + tls_for_test.go | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/main_test.go b/main_test.go index a2742ec..81be563 100644 --- a/main_test.go +++ b/main_test.go @@ -42,6 +42,7 @@ func TestOtelCli(t *testing.T) { } tlsTest = generateTLSData(t) + defer tlsTest.cleanup() var fixtureCount int for _, suite := range suites { diff --git a/tls_for_test.go b/tls_for_test.go index 14413ff..71edb54 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -185,6 +185,15 @@ func generateTLSData(t *testing.T) tlsHelpers { return out } +func (t tlsHelpers) cleanup() { + os.Remove(t.caFile) + os.Remove(t.caPrivKeyFile) + os.Remove(t.clientFile) + os.Remove(t.clientPrivKeyFile) + os.Remove(t.serverFile) + os.Remove(t.serverPrivKeyFile) +} + func pemToTempFile(t *testing.T, buf *bytes.Buffer) string { tmp, err := os.CreateTemp(os.TempDir(), "otel-cli-test-pem") if err != nil { From c9609f066fdd4cfab431be2ac623cc74d87913ac Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Fri, 3 Feb 2023 10:22:53 -0500 Subject: [PATCH 03/25] pass tls data around instead of using a global --- main_test.go | 67 ++++++++++++++++++++++++++-------------------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/main_test.go b/main_test.go index 81be563..4ef1ac4 100644 --- a/main_test.go +++ b/main_test.go @@ -25,8 +25,6 @@ import ( const minimumPath = `/bin:/usr/bin` const defaultTestTimeout = time.Second -var tlsTest tlsHelpers - func TestMain(m *testing.M) { // wipe out this process's envvars right away to avoid pollution & leakage os.Clearenv() @@ -41,9 +39,6 @@ func TestOtelCli(t *testing.T) { t.Fatalf("otel-cli must be built and present as ./otel-cli for this test suite to work (try: go build)") } - tlsTest = generateTLSData(t) - defer tlsTest.cleanup() - var fixtureCount int for _, suite := range suites { fixtureCount += len(suite) @@ -73,6 +68,10 @@ func TestOtelCli(t *testing.T) { t.Fatal("no test fixtures loaded!") } + // generates a CA, client, and server certs to use in tests + tlsData := generateTLSData(t) + defer tlsData.cleanup() + for _, suite := range suites { // a fixture can be backgrounded after starting it up for e.g. otel-cli span background // a second fixture with the same description later in the list will "foreground" it @@ -111,7 +110,7 @@ func TestOtelCli(t *testing.T) { fixtureWait := make(chan struct{}) fixtureDone := make(chan struct{}) - go runFixture(t, fixture, fixtureWait, fixtureDone) + go runFixture(t, fixture, fixtureWait, fixtureDone, tlsData) if fixture.Config.Background { // save off the channels for flow control @@ -129,16 +128,16 @@ func TestOtelCli(t *testing.T) { // runFixture runs the OTLP server & command, waits for signal, checks // results, then signals it's done. -func runFixture(t *testing.T, fixture Fixture, wait, done chan struct{}) { +func runFixture(t *testing.T, fixture Fixture, wait, done chan struct{}, tlsData tlsHelpers) { // sets up an OTLP server, runs otel-cli, packages data up in these return vals - endpoint, results, span, events := runOtelCli(t, fixture) + endpoint, results, span, events := runOtelCli(t, fixture, tlsData) <-wait - checkAll(t, fixture, endpoint, results, span, events) + checkAll(t, fixture, endpoint, results, span, events, tlsData) done <- struct{}{} } // checkAll gathers up all the check* funcs below into one function. -func checkAll(t *testing.T, fixture Fixture, endpoint string, results Results, span otlpserver.CliEvent, events otlpserver.CliEventList) { +func checkAll(t *testing.T, fixture Fixture, endpoint string, results Results, span otlpserver.CliEvent, events otlpserver.CliEventList, tlsData tlsHelpers) { // check timeout and process status expectations checkProcess(t, fixture, results) @@ -146,15 +145,15 @@ func checkAll(t *testing.T, fixture Fixture, endpoint string, results Results, s checkSpanCount(t, fixture, results) // compares the data in each recorded span against expectations in the fixture - checkSpanData(t, fixture, endpoint, span, events) + checkSpanData(t, fixture, endpoint, span, events, tlsData) // many of the basic plumbing tests use status so it has its own set of checks // but these shouldn't run for testing the other subcommands if len(fixture.Config.CliArgs) > 0 && fixture.Config.CliArgs[0] == "status" && !fixture.Expect.CommandFailed { - checkStatusData(t, fixture, endpoint, results) + checkStatusData(t, fixture, endpoint, results, tlsData) } else { // checking the text output only makes sense for non-status paths - checkOutput(t, fixture, endpoint, results) + checkOutput(t, fixture, endpoint, results, tlsData) } } @@ -181,8 +180,8 @@ func checkProcess(t *testing.T, fixture Fixture, results Results) { // checkOutput looks that otel-cli output stored in the results and compares against // the fixture expectation (with {{endpoint}} replaced). -func checkOutput(t *testing.T, fixture Fixture, endpoint string, results Results) { - wantOutput := injectVars(fixture.Expect.CliOutput, endpoint) +func checkOutput(t *testing.T, fixture Fixture, endpoint string, results Results, tlsData tlsHelpers) { + wantOutput := injectVars(fixture.Expect.CliOutput, endpoint, tlsData) gotOutput := results.CliOutput if fixture.Expect.CliOutputRe != nil { gotOutput = fixture.Expect.CliOutputRe.ReplaceAllString(gotOutput, "") @@ -197,9 +196,9 @@ func checkOutput(t *testing.T, fixture Fixture, endpoint string, results Results // checkStatusData compares the sections of otel-cli status output against // fixture data, substituting {{endpoint}} into fixture data as needed. -func checkStatusData(t *testing.T, fixture Fixture, endpoint string, results Results) { +func checkStatusData(t *testing.T, fixture Fixture, endpoint string, results Results, tlsData tlsHelpers) { // check the env - injectMapVars(endpoint, fixture.Expect.Env) + injectMapVars(endpoint, fixture.Expect.Env, tlsData) if diff := cmp.Diff(fixture.Expect.Env, results.Env); diff != "" { t.Errorf("env data did not match fixture in %q (-want +got):\n%s", fixture.Name, diff) } @@ -207,7 +206,7 @@ func checkStatusData(t *testing.T, fixture Fixture, endpoint string, results Res // check diagnostics, use string maps so the diff output is easy to compare to json wantDiag := fixture.Expect.Diagnostics.ToStringMap() gotDiag := results.Diagnostics.ToStringMap() - injectMapVars(endpoint, wantDiag) + injectMapVars(endpoint, wantDiag, tlsData) // there's almost always going to be cli_args in practice, so if the fixture has // an empty string, just ignore that key if wantDiag["cli_args"] == "" { @@ -228,7 +227,7 @@ func checkStatusData(t *testing.T, fixture Fixture, endpoint string, results Res wantConf[k] = gotConf[k] } } - injectMapVars(endpoint, wantConf) + injectMapVars(endpoint, wantConf, tlsData) if diff := cmp.Diff(wantConf, gotConf); diff != "" { t.Errorf("[%s] config data did not match fixture (-want +got):\n%s", fixture.Name, diff) } @@ -249,10 +248,10 @@ var spanRegexChecks = map[string]*regexp.Regexp{ // checkSpanData compares the data in spans received from otel-cli against the // fixture data. -func checkSpanData(t *testing.T, fixture Fixture, endpoint string, span otlpserver.CliEvent, events otlpserver.CliEventList) { +func checkSpanData(t *testing.T, fixture Fixture, endpoint string, span otlpserver.CliEvent, events otlpserver.CliEventList, tlsData tlsHelpers) { // check the expected span data against what was received by the OTLP server gotSpan := span.ToStringMap() - injectMapVars(endpoint, gotSpan) + injectMapVars(endpoint, gotSpan, tlsData) // remove keys that aren't supported for comparison (for now) delete(gotSpan, "is_populated") delete(gotSpan, "library") @@ -291,7 +290,7 @@ func checkSpanData(t *testing.T, fixture Fixture, endpoint string, span otlpserv } // do a diff on a generated map that sets values to * when the * check succeeded - injectMapVars(endpoint, wantSpan) + injectMapVars(endpoint, wantSpan, tlsData) if diff := cmp.Diff(wantSpan, gotSpan); diff != "" { t.Errorf("[%s] otel span info did not match fixture (-want +got):\n%s", fixture.Name, diff) } @@ -299,7 +298,7 @@ func checkSpanData(t *testing.T, fixture Fixture, endpoint string, span otlpserv // runOtelCli runs the a server and otel-cli together and captures their // output as data to return for further testing. -func runOtelCli(t *testing.T, fixture Fixture) (string, Results, otlpserver.CliEvent, otlpserver.CliEventList) { +func runOtelCli(t *testing.T, fixture Fixture, tlsData tlsHelpers) (string, Results, otlpserver.CliEvent, otlpserver.CliEventList) { started := time.Now() results := Results{ @@ -355,7 +354,7 @@ func runOtelCli(t *testing.T, fixture Fixture) (string, Results, otlpserver.CliE var listener net.Listener var err error if fixture.Config.ServerTLSEnabled { - listener, err = tls.Listen("tcp", "localhost:0", tlsTest.serverTLSConf) + listener, err = tls.Listen("tcp", "localhost:0", tlsData.serverTLSConf) } else { listener, err = net.Listen("tcp", "localhost:0") } @@ -386,11 +385,11 @@ func runOtelCli(t *testing.T, fixture Fixture) (string, Results, otlpserver.CliE args := []string{} if len(fixture.Config.CliArgs) > 0 { for _, v := range fixture.Config.CliArgs { - args = append(args, injectVars(v, endpoint)) + args = append(args, injectVars(v, endpoint, tlsData)) } } statusCmd := exec.Command("./otel-cli", args...) - statusCmd.Env = mkEnviron(endpoint, fixture.Config.Env) + statusCmd.Env = mkEnviron(endpoint, fixture.Config.Env, tlsData) cancelProcessTimeout := make(chan struct{}, 1) go func() { @@ -472,11 +471,11 @@ gather: } // mkEnviron converts a string map to a list of k=v strings and tacks on PATH. -func mkEnviron(endpoint string, env map[string]string) []string { +func mkEnviron(endpoint string, env map[string]string, tlsData tlsHelpers) []string { mapped := make([]string, len(env)+1) var i int for k, v := range env { - mapped[i] = k + "=" + injectVars(v, endpoint) + mapped[i] = k + "=" + injectVars(v, endpoint, tlsData) i++ } @@ -489,9 +488,9 @@ func mkEnviron(endpoint string, env map[string]string) []string { // injectMapVars iterates over the map and updates the values, replacing all instances // of {{endpoint}}, {{cert}}, {{client_key}}, and {{client_key_cert}} with test values. -func injectMapVars(endpoint string, target map[string]string) { +func injectMapVars(endpoint string, target map[string]string, tlsData tlsHelpers) { for k, v := range target { - target[k] = injectVars(v, endpoint) + target[k] = injectVars(v, endpoint, tlsData) } } @@ -501,10 +500,10 @@ func injectMapVars(endpoint string, target map[string]string) { // 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) string { +func injectVars(in, endpoint string, tlsData tlsHelpers) string { out := strings.ReplaceAll(in, "{{endpoint}}", endpoint) - out = strings.ReplaceAll(out, "{{cert}}", tlsTest.caFile) - out = strings.ReplaceAll(out, "{{client_key}}", tlsTest.clientPrivKeyFile) - out = strings.ReplaceAll(out, "{{client_key_cert}}", tlsTest.clientFile) + out = strings.ReplaceAll(out, "{{cert}}", tlsData.caFile) + out = strings.ReplaceAll(out, "{{client_key}}", tlsData.clientPrivKeyFile) + out = strings.ReplaceAll(out, "{{client_key_cert}}", tlsData.clientFile) return out } From 79b00e0ffd261a293adfd1d72c9f9672caf33b86 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Fri, 3 Feb 2023 10:23:12 -0500 Subject: [PATCH 04/25] remove unused field, touch up key id --- tls_for_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tls_for_test.go b/tls_for_test.go index 71edb54..1723400 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -17,7 +17,6 @@ import ( ) type tlsHelpers struct { - pool string ca *x509.Certificate caPrivKey *ecdsa.PrivateKey caPEM *bytes.Buffer @@ -147,7 +146,7 @@ func generateTLSData(t *testing.T) tlsHelpers { out.clientCert = &x509.Certificate{ SerialNumber: big.NewInt(4319), Subject: out.ca.Subject, - SubjectKeyId: []byte{1, 2, 3, 4, 6}, + SubjectKeyId: []byte{1, 2, 3, 4, 7}, IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, DNSNames: []string{"localhost"}, NotBefore: time.Now(), From fdd3d7688812a0e8d32908e9f3c58952df0a240d Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Fri, 3 Feb 2023 17:07:35 -0500 Subject: [PATCH 05/25] ignore pem files --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 206fefd..1110554 100644 --- a/.gitignore +++ b/.gitignore @@ -16,3 +16,5 @@ otel-cli # vendor/ dist/ + +*.pem From 8d312911897761c706ae9ca3cb2ff7a6da4ecdd0 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Fri, 3 Feb 2023 17:10:45 -0500 Subject: [PATCH 06/25] get TLS test working for gRPC, add CLI flags There's still more work to do. --no-tls-verify does not seem to work as expected and that needs figuring out and tested. Also need to add tests for client cert authentication. But this all works and the certs generated seem valid. Added diagnistic for no-tls-verify. Added setting a cacert in the root bundle in otel-cli. Need to double-check this is the right behavior to ship. Cacert config needs to be added to HTTPs as well if ^^ turns up as the behavior being correct. --- data_for_test.go | 64 ++++++++++++++++++------------------- otelcli/diagnostics.go | 17 +++++----- otelcli/plumbing.go | 24 ++++++++++---- otelcli/root.go | 4 +++ tls_for_test.go | 72 ++++++++++++++++++++++++++---------------- 5 files changed, 105 insertions(+), 76 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index 4e61ff5..1ff439e 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -137,40 +137,39 @@ var suites = []FixtureSuite{ }, }, // TLS connections - /* - { - Name: "minimum configuration (tls, recording, grpc)", - Config: FixtureConfig{ - ServerProtocol: grpcProtocol, - CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--protocol", "grpc", "--no-tls-verify"}, - TestTimeoutMs: 1000, - ServerTLSEnabled: true, - Env: map[string]string{ - "SSL_CERT_FILE": "{{cert}}", - }, - }, - Expect: Results{ - // otel-cli should NOT set insecure when it auto-detects localhost - Config: otelcli.DefaultConfig(). - WithEndpoint("{{endpoint}}"), - Diagnostics: otelcli.Diagnostics{ - IsRecording: true, - NumArgs: 6, - DetectedLocalhost: true, - ParsedTimeoutMs: 1000, - }, - Spans: 1, - Env: map[string]string{ - "SSL_CERT_FILE": "{{cert}}", - }, - }, - }, - */ + { + Name: "minimum configuration (tls, recording, grpc)", + Config: FixtureConfig{ + ServerProtocol: grpcProtocol, + CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--protocol", "grpc"}, + TestTimeoutMs: 1000, + ServerTLSEnabled: true, + Env: map[string]string{ + "SSL_CERT_FILE": "{{cert}}", + }, + }, + Expect: Results{ + // otel-cli should NOT set insecure when it auto-detects localhost + Config: otelcli.DefaultConfig(). + WithEndpoint("https://{{endpoint}}"). + WithProtocol("grpc"), + Diagnostics: otelcli.Diagnostics{ + IsRecording: true, + NumArgs: 5, + DetectedLocalhost: true, + ParsedTimeoutMs: 1000, + }, + Spans: 1, + Env: map[string]string{ + "SSL_CERT_FILE": "{{cert}}", + }, + }, + }, { Name: "minimum configuration (tls, recording, https)", Config: FixtureConfig{ ServerProtocol: httpProtocol, - CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--no-tls-verify"}, + CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}"}, TestTimeoutMs: 2000, ServerTLSEnabled: true, Env: map[string]string{ @@ -180,11 +179,10 @@ var suites = []FixtureSuite{ Expect: Results{ // otel-cli should NOT set insecure when it auto-detects localhost Config: otelcli.DefaultConfig(). - WithEndpoint("https://{{endpoint}}"). - WithNoTlsVerify(true), + WithEndpoint("https://{{endpoint}}"), Diagnostics: otelcli.Diagnostics{ IsRecording: true, - NumArgs: 4, + NumArgs: 3, DetectedLocalhost: true, ParsedTimeoutMs: 1000, }, diff --git a/otelcli/diagnostics.go b/otelcli/diagnostics.go index 4c4cf5d..d7766a3 100644 --- a/otelcli/diagnostics.go +++ b/otelcli/diagnostics.go @@ -12,14 +12,15 @@ var diagnostics Diagnostics // diagnosing issues with otel-cli. The only user-facing feature that should be // using these is otel-cli status. type Diagnostics struct { - CliArgs []string `json:"cli_args"` - IsRecording bool `json:"is_recording"` - ConfigFileLoaded bool `json:"config_file_loaded"` - NumArgs int `json:"number_of_args"` - DetectedLocalhost bool `json:"detected_localhost"` - ParsedTimeoutMs int64 `json:"parsed_timeout_ms"` - OtelError string `json:"otel_error"` - ExecExitCode int `json:"exec_exit_code"` + CliArgs []string `json:"cli_args"` + IsRecording bool `json:"is_recording"` + ConfigFileLoaded bool `json:"config_file_loaded"` + NumArgs int `json:"number_of_args"` + DetectedLocalhost bool `json:"detected_localhost"` + InsecureSkipVerify bool `json:"insecure_skip_verify"` + ParsedTimeoutMs int64 `json:"parsed_timeout_ms"` + OtelError string `json:"otel_error"` + ExecExitCode int `json:"exec_exit_code"` } // Handle complies with the otel error handler interface to capture errors diff --git a/otelcli/plumbing.go b/otelcli/plumbing.go index 7ac0e44..a6e9578 100644 --- a/otelcli/plumbing.go +++ b/otelcli/plumbing.go @@ -3,8 +3,10 @@ package otelcli import ( "context" "crypto/tls" + "crypto/x509" "net" "net/url" + "os" "regexp" "strings" @@ -128,11 +130,20 @@ func grpcOptions() []otlpgrpc.Option { if config.Insecure || (isLoopbackAddr(config.Endpoint) && !strings.HasPrefix(config.Endpoint, "https")) { grpcOpts = append(grpcOpts, otlpgrpc.WithInsecure()) } else if !isInsecureSchema(config.Endpoint) { - var tlsConfig *tls.Config + tlsConfig := &tls.Config{} if config.NoTlsVerify { - tlsConfig = &tls.Config{ - InsecureSkipVerify: true, + diagnostics.InsecureSkipVerify = true + tlsConfig.InsecureSkipVerify = true + } + if config.Certificate != "" { + data, err := os.ReadFile(config.Certificate) + if err != nil { + softFail("uanble to load certificate: %s", err) } + + certpool := x509.NewCertPool() + certpool.AppendCertsFromPEM(data) + tlsConfig.RootCAs = certpool } grpcOpts = append(grpcOpts, otlpgrpc.WithDialOption(grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))) } @@ -193,11 +204,10 @@ func httpOptions() []otlphttp.Option { if config.Insecure || (isLoopbackAddr(config.Endpoint) && !strings.HasPrefix(config.Endpoint, "https")) { httpOpts = append(httpOpts, otlphttp.WithInsecure()) } else if !isInsecureSchema(config.Endpoint) { - var tlsConfig *tls.Config + tlsConfig := &tls.Config{} if config.NoTlsVerify { - tlsConfig = &tls.Config{ - InsecureSkipVerify: true, - } + diagnostics.InsecureSkipVerify = true + tlsConfig.InsecureSkipVerify = true } httpOpts = append(httpOpts, otlphttp.WithTLSClientConfig(tlsConfig)) } diff --git a/otelcli/root.go b/otelcli/root.go index 2ee39de..6612eec 100644 --- a/otelcli/root.go +++ b/otelcli/root.go @@ -67,6 +67,10 @@ func addClientParams(cmd *cobra.Command) { cmd.Flags().StringToStringVar(&config.Headers, "otlp-headers", defaults.Headers, "a comma-sparated list of key=value headers to send on OTLP connection") cmd.Flags().BoolVar(&config.Blocking, "otlp-blocking", defaults.Blocking, "block on connecting to the OTLP server before proceeding") + cmd.Flags().StringVar(&config.Certificate, "cacert", defaults.Certificate, "a file containing the server's CA certificate") + cmd.Flags().StringVar(&config.ClientCert, "client-cert", defaults.ClientCert, "a file containing the client certificate") + cmd.Flags().StringVar(&config.ClientKey, "client-key", defaults.ClientKey, "a file containing the client certificate key") + // 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") cmd.Flags().StringVar(&config.TraceparentCarrierFile, "tp-carrier", defaults.TraceparentCarrierFile, "a file for reading and WRITING traceparent across invocations") diff --git a/tls_for_test.go b/tls_for_test.go index 1723400..35914d4 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -45,23 +45,34 @@ func generateTLSData(t *testing.T) tlsHelpers { var err error var out tlsHelpers + // this gets reused for each cert, with CommonName overwritten for each + subject := pkix.Name{ + CommonName: "otel-cli certificate authority", + Organization: []string{"otel-cli testing, inc"}, + Country: []string{"Open Source"}, + Province: []string{"Go"}, + Locality: []string{"OpenTelemetry"}, + StreetAddress: []string{"github.com/equinix-labs/otel-cli"}, + PostalCode: []string{"4317"}, + } + + expire := time.Now().Add(time.Hour * 1000) + // ------------- CA ------------- out.ca = &x509.Certificate{ - SerialNumber: big.NewInt(4317), - Subject: pkix.Name{ - Organization: []string{"otel-cli testing, inc"}, - Country: []string{"Open Source"}, - Province: []string{"Go"}, - Locality: []string{"OpenTelemetry"}, - StreetAddress: []string{"github.com/equinix-labs/otel-cli"}, - PostalCode: []string{"4317"}, + SerialNumber: big.NewInt(4317), + Subject: subject, + NotBefore: time.Now(), + NotAfter: expire, + IsCA: true, + BasicConstraintsValid: true, + ExtKeyUsage: []x509.ExtKeyUsage{ + x509.ExtKeyUsageClientAuth, + x509.ExtKeyUsageServerAuth, + x509.ExtKeyUsageOCSPSigning, }, - NotBefore: time.Now(), - NotAfter: time.Now().Add(time.Hour), - IsCA: true, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign | x509.KeyUsageCRLSign, } // create a private key @@ -79,7 +90,7 @@ func generateTLSData(t *testing.T) tlsHelpers { // get the PEM encoding that the tests will use out.caPEM = new(bytes.Buffer) pem.Encode(out.caPEM, &pem.Block{Type: "CERTIFICATE", Bytes: caBytes}) - out.caFile = pemToTempFile(t, out.caPEM) + out.caFile = pemToTempFile(t, "ca-cert", out.caPEM) out.caPrivKeyPEM = new(bytes.Buffer) caPrivKeyBytes, err := x509.MarshalECPrivateKey(out.caPrivKey) @@ -87,7 +98,7 @@ func generateTLSData(t *testing.T) tlsHelpers { t.Fatalf("error marshaling server cert: %s", err) } pem.Encode(out.caPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: caPrivKeyBytes}) - out.caPrivKeyFile = pemToTempFile(t, out.caPrivKeyPEM) + out.caPrivKeyFile = pemToTempFile(t, "ca-privkey", out.caPrivKeyPEM) out.certpool = x509.NewCertPool() out.certpool.AppendCertsFromPEM(out.caPEM.Bytes()) @@ -97,16 +108,20 @@ func generateTLSData(t *testing.T) tlsHelpers { // ------------- server ------------- + subject.CommonName = "server" out.serverCert = &x509.Certificate{ SerialNumber: big.NewInt(4318), - Subject: out.ca.Subject, + Subject: subject, SubjectKeyId: []byte{1, 2, 3, 4, 6}, IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, DNSNames: []string{"localhost"}, NotBefore: time.Now(), - NotAfter: time.Now().Add(time.Hour), - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, - KeyUsage: x509.KeyUsageDigitalSignature, + NotAfter: expire, + ExtKeyUsage: []x509.ExtKeyUsage{ + x509.ExtKeyUsageClientAuth, + x509.ExtKeyUsageServerAuth, + }, + KeyUsage: x509.KeyUsageKeyAgreement | x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, } out.serverPrivKey, err = ecdsa.GenerateKey(elliptic.P521(), rand.Reader) @@ -121,7 +136,7 @@ func generateTLSData(t *testing.T) tlsHelpers { out.serverPEM = new(bytes.Buffer) pem.Encode(out.serverPEM, &pem.Block{Type: "CERTIFICATE", Bytes: serverBytes}) - out.serverFile = pemToTempFile(t, out.serverPEM) + out.serverFile = pemToTempFile(t, "server-cert", out.serverPEM) out.serverPrivKeyPEM = new(bytes.Buffer) serverPrivKeyBytes, err := x509.MarshalECPrivateKey(out.serverPrivKey) @@ -129,7 +144,7 @@ func generateTLSData(t *testing.T) tlsHelpers { t.Fatalf("error marshaling server cert: %s", err) } pem.Encode(out.serverPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: serverPrivKeyBytes}) - out.serverPrivKeyFile = pemToTempFile(t, out.serverPrivKeyPEM) + out.serverPrivKeyFile = pemToTempFile(t, "server-privkey", out.serverPrivKeyPEM) out.serverCertPair, err = tls.X509KeyPair(out.serverPEM.Bytes(), out.serverPrivKeyPEM.Bytes()) if err != nil { @@ -143,15 +158,16 @@ func generateTLSData(t *testing.T) tlsHelpers { // ------------- client ------------- + subject.CommonName = "client" out.clientCert = &x509.Certificate{ SerialNumber: big.NewInt(4319), - Subject: out.ca.Subject, + Subject: subject, SubjectKeyId: []byte{1, 2, 3, 4, 7}, IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, DNSNames: []string{"localhost"}, NotBefore: time.Now(), - NotAfter: time.Now().Add(time.Hour), - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + NotAfter: expire, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, KeyUsage: x509.KeyUsageDigitalSignature, } @@ -167,7 +183,7 @@ func generateTLSData(t *testing.T) tlsHelpers { out.clientPEM = new(bytes.Buffer) pem.Encode(out.clientPEM, &pem.Block{Type: "CERTIFICATE", Bytes: clientBytes}) - out.clientFile = pemToTempFile(t, out.clientPEM) + out.clientFile = pemToTempFile(t, "client-cert", out.clientPEM) out.clientPrivKeyPEM = new(bytes.Buffer) clientPrivKeyBytes, err := x509.MarshalECPrivateKey(out.clientPrivKey) @@ -175,7 +191,7 @@ func generateTLSData(t *testing.T) tlsHelpers { t.Fatalf("error marshaling client cert: %s", err) } pem.Encode(out.clientPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: clientPrivKeyBytes}) - out.clientPrivKeyFile = pemToTempFile(t, out.clientPrivKeyPEM) + out.clientPrivKeyFile = pemToTempFile(t, "client-privkey", out.clientPrivKeyPEM) out.clientTLSConf = &tls.Config{ RootCAs: out.certpool, @@ -193,8 +209,8 @@ func (t tlsHelpers) cleanup() { os.Remove(t.serverPrivKeyFile) } -func pemToTempFile(t *testing.T, buf *bytes.Buffer) string { - tmp, err := os.CreateTemp(os.TempDir(), "otel-cli-test-pem") +func pemToTempFile(t *testing.T, tmpl string, buf *bytes.Buffer) string { + tmp, err := os.CreateTemp(os.TempDir(), "otel-cli-test-"+tmpl+"-pem") if err != nil { t.Fatalf("error creating temp file: %s", err) } From 9a368ea0123236f8b9083e895cf009763a5f3643 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Tue, 14 Feb 2023 18:28:30 -0500 Subject: [PATCH 07/25] remove struct fields that don't need to be there On the first pass of this code I wasn't sure what all I would need, so I put all the intermediate steps of the CA and certs in the struct. Now that some tests are running, it's clear some things don't need to be there and the code looks cleaner this way. --- tls_for_test.go | 83 +++++++++++++++++++++---------------------------- 1 file changed, 35 insertions(+), 48 deletions(-) diff --git a/tls_for_test.go b/tls_for_test.go index 35914d4..c33e1d1 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -17,27 +17,14 @@ import ( ) type tlsHelpers struct { - ca *x509.Certificate - caPrivKey *ecdsa.PrivateKey - caPEM *bytes.Buffer - caPrivKeyPEM *bytes.Buffer caFile string caPrivKeyFile string - serverCert *x509.Certificate - serverPrivKey *ecdsa.PrivateKey - serverPEM *bytes.Buffer serverFile string - serverPrivKeyPEM *bytes.Buffer serverPrivKeyFile string - serverCertPair tls.Certificate - certpool *x509.CertPool - serverTLSConf *tls.Config - clientCert *x509.Certificate - clientPrivKey *ecdsa.PrivateKey - clientPEM *bytes.Buffer clientFile string - clientPrivKeyPEM *bytes.Buffer clientPrivKeyFile string + serverTLSConf *tls.Config + certpool *x509.CertPool clientTLSConf *tls.Config } @@ -60,7 +47,7 @@ func generateTLSData(t *testing.T) tlsHelpers { // ------------- CA ------------- - out.ca = &x509.Certificate{ + ca := &x509.Certificate{ SerialNumber: big.NewInt(4317), Subject: subject, NotBefore: time.Now(), @@ -76,32 +63,32 @@ func generateTLSData(t *testing.T) tlsHelpers { } // create a private key - out.caPrivKey, err = ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + caPrivKey, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) if err != nil { t.Fatalf("error generating ca private key: %s", err) } // create a cert on the CA with the ^^ private key - caBytes, err := x509.CreateCertificate(rand.Reader, out.ca, out.ca, &out.caPrivKey.PublicKey, out.caPrivKey) + caBytes, err := x509.CreateCertificate(rand.Reader, ca, ca, &caPrivKey.PublicKey, caPrivKey) if err != nil { t.Fatalf("error generating ca cert: %s", err) } // get the PEM encoding that the tests will use - out.caPEM = new(bytes.Buffer) - pem.Encode(out.caPEM, &pem.Block{Type: "CERTIFICATE", Bytes: caBytes}) - out.caFile = pemToTempFile(t, "ca-cert", out.caPEM) + caPEM := new(bytes.Buffer) + pem.Encode(caPEM, &pem.Block{Type: "CERTIFICATE", Bytes: caBytes}) + out.caFile = pemToTempFile(t, "ca-cert", caPEM) - out.caPrivKeyPEM = new(bytes.Buffer) - caPrivKeyBytes, err := x509.MarshalECPrivateKey(out.caPrivKey) + caPrivKeyPEM := new(bytes.Buffer) + caPrivKeyBytes, err := x509.MarshalECPrivateKey(caPrivKey) if err != nil { t.Fatalf("error marshaling server cert: %s", err) } - pem.Encode(out.caPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: caPrivKeyBytes}) - out.caPrivKeyFile = pemToTempFile(t, "ca-privkey", out.caPrivKeyPEM) + pem.Encode(caPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: caPrivKeyBytes}) + out.caPrivKeyFile = pemToTempFile(t, "ca-privkey", caPrivKeyPEM) out.certpool = x509.NewCertPool() - out.certpool.AppendCertsFromPEM(out.caPEM.Bytes()) + out.certpool.AppendCertsFromPEM(caPEM.Bytes()) data := new(bytes.Buffer) pem.Encode(data, &pem.Block{Type: "EC PRIVATE KEY", Bytes: caPrivKeyBytes}) @@ -109,7 +96,7 @@ func generateTLSData(t *testing.T) tlsHelpers { // ------------- server ------------- subject.CommonName = "server" - out.serverCert = &x509.Certificate{ + serverCert := &x509.Certificate{ SerialNumber: big.NewInt(4318), Subject: subject, SubjectKeyId: []byte{1, 2, 3, 4, 6}, @@ -124,42 +111,42 @@ func generateTLSData(t *testing.T) tlsHelpers { KeyUsage: x509.KeyUsageKeyAgreement | x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, } - out.serverPrivKey, err = ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + serverPrivKey, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) if err != nil { t.Fatalf("error generating server private key: %s", err) } - serverBytes, err := x509.CreateCertificate(rand.Reader, out.serverCert, out.ca, &out.serverPrivKey.PublicKey, out.caPrivKey) + serverBytes, err := x509.CreateCertificate(rand.Reader, serverCert, ca, &serverPrivKey.PublicKey, caPrivKey) if err != nil { t.Fatalf("error generating server cert: %s", err) } - out.serverPEM = new(bytes.Buffer) - pem.Encode(out.serverPEM, &pem.Block{Type: "CERTIFICATE", Bytes: serverBytes}) - out.serverFile = pemToTempFile(t, "server-cert", out.serverPEM) + serverPEM := new(bytes.Buffer) + pem.Encode(serverPEM, &pem.Block{Type: "CERTIFICATE", Bytes: serverBytes}) + out.serverFile = pemToTempFile(t, "server-cert", serverPEM) - out.serverPrivKeyPEM = new(bytes.Buffer) - serverPrivKeyBytes, err := x509.MarshalECPrivateKey(out.serverPrivKey) + serverPrivKeyPEM := new(bytes.Buffer) + serverPrivKeyBytes, err := x509.MarshalECPrivateKey(serverPrivKey) if err != nil { t.Fatalf("error marshaling server cert: %s", err) } - pem.Encode(out.serverPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: serverPrivKeyBytes}) - out.serverPrivKeyFile = pemToTempFile(t, "server-privkey", out.serverPrivKeyPEM) + pem.Encode(serverPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: serverPrivKeyBytes}) + out.serverPrivKeyFile = pemToTempFile(t, "server-privkey", serverPrivKeyPEM) - out.serverCertPair, err = tls.X509KeyPair(out.serverPEM.Bytes(), out.serverPrivKeyPEM.Bytes()) + serverCertPair, err := tls.X509KeyPair(serverPEM.Bytes(), serverPrivKeyPEM.Bytes()) if err != nil { t.Fatalf("error generating server cert pair: %s", err) } out.serverTLSConf = &tls.Config{ RootCAs: out.certpool, - Certificates: []tls.Certificate{out.serverCertPair}, + Certificates: []tls.Certificate{serverCertPair}, } // ------------- client ------------- subject.CommonName = "client" - out.clientCert = &x509.Certificate{ + clientCert := &x509.Certificate{ SerialNumber: big.NewInt(4319), Subject: subject, SubjectKeyId: []byte{1, 2, 3, 4, 7}, @@ -171,27 +158,27 @@ func generateTLSData(t *testing.T) tlsHelpers { KeyUsage: x509.KeyUsageDigitalSignature, } - out.clientPrivKey, err = ecdsa.GenerateKey(elliptic.P521(), rand.Reader) + clientPrivKey, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) if err != nil { t.Fatalf("error generating client private key: %s", err) } - clientBytes, err := x509.CreateCertificate(rand.Reader, out.clientCert, out.ca, &out.clientPrivKey.PublicKey, out.caPrivKey) + clientBytes, err := x509.CreateCertificate(rand.Reader, clientCert, ca, &clientPrivKey.PublicKey, caPrivKey) if err != nil { t.Fatalf("error generating client cert: %s", err) } - out.clientPEM = new(bytes.Buffer) - pem.Encode(out.clientPEM, &pem.Block{Type: "CERTIFICATE", Bytes: clientBytes}) - out.clientFile = pemToTempFile(t, "client-cert", out.clientPEM) + clientPEM := new(bytes.Buffer) + pem.Encode(clientPEM, &pem.Block{Type: "CERTIFICATE", Bytes: clientBytes}) + out.clientFile = pemToTempFile(t, "client-cert", clientPEM) - out.clientPrivKeyPEM = new(bytes.Buffer) - clientPrivKeyBytes, err := x509.MarshalECPrivateKey(out.clientPrivKey) + clientPrivKeyPEM := new(bytes.Buffer) + clientPrivKeyBytes, err := x509.MarshalECPrivateKey(clientPrivKey) if err != nil { t.Fatalf("error marshaling client cert: %s", err) } - pem.Encode(out.clientPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: clientPrivKeyBytes}) - out.clientPrivKeyFile = pemToTempFile(t, "client-privkey", out.clientPrivKeyPEM) + pem.Encode(clientPrivKeyPEM, &pem.Block{Type: "EC PRIVATE KEY", Bytes: clientPrivKeyBytes}) + out.clientPrivKeyFile = pemToTempFile(t, "client-privkey", clientPrivKeyPEM) out.clientTLSConf = &tls.Config{ RootCAs: out.certpool, From ce27cb8c5f030f3df6aa17ce1941faeec424d1d9 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Tue, 14 Feb 2023 18:30:05 -0500 Subject: [PATCH 08/25] reorder fields --- tls_for_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tls_for_test.go b/tls_for_test.go index c33e1d1..d589cfa 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -24,8 +24,8 @@ type tlsHelpers struct { clientFile string clientPrivKeyFile string serverTLSConf *tls.Config - certpool *x509.CertPool clientTLSConf *tls.Config + certpool *x509.CertPool } func generateTLSData(t *testing.T) tlsHelpers { From 8cefb51e3ec7ba925efcc2fc1f0bc7c51598ac95 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 10:34:56 -0500 Subject: [PATCH 09/25] fix spelling error in envvar name --- otelcli/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otelcli/config.go b/otelcli/config.go index fd54de0..07c9f59 100644 --- a/otelcli/config.go +++ b/otelcli/config.go @@ -68,7 +68,7 @@ type Config struct { Certificate string `json:"certificate_file" env:"OTEL_EXPORTER_OTLP_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE"` ClientKey string `json:"client_key_file" env:"OTEL_EXPORTER_OTLP_CLIENT_KEY,OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY"` - ClientCert string `json:"client_key_certificate_file" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTFICATE"` + ClientCert string `json:"client_key_certificate_file" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE"` ServiceName string `json:"service_name" env:"OTEL_CLI_SERVICE_NAME"` SpanName string `json:"span_name" env:"OTEL_CLI_SPAN_NAME"` From bbf573ad89d3618d589ebe23aa13d51321369373 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 10:36:03 -0500 Subject: [PATCH 10/25] refactor TLS config to a func to reduce duplication They were out of sync and it was silly to copy the code in this case. :thinking: and now it can be tested independently. --- otelcli/plumbing.go | 49 +++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/otelcli/plumbing.go b/otelcli/plumbing.go index a6e9578..1331070 100644 --- a/otelcli/plumbing.go +++ b/otelcli/plumbing.go @@ -100,6 +100,31 @@ func initTracer() (context.Context, func()) { } } +// tlsConfig evaluates otel-cli configuration and returns a tls.Config +// that can be used by grpc or https. +func tlsConfig() *tls.Config { + tlsConfig := &tls.Config{} + + if config.NoTlsVerify { + diagnostics.InsecureSkipVerify = true + tlsConfig.InsecureSkipVerify = true + } + + // TODO: is this the right thing to do? Need to make sure ... + if config.Certificate != "" { + data, err := os.ReadFile(config.Certificate) + if err != nil { + softFail("uanble to load certificate: %s", err) + } + + certpool := x509.NewCertPool() + certpool.AppendCertsFromPEM(data) + //tlsConfig.RootCAs = certpool + } + + return tlsConfig +} + // grpcOptions convets config settings to an otlpgrpc.Option list. func grpcOptions() []otlpgrpc.Option { grpcOpts := []otlpgrpc.Option{} @@ -130,22 +155,7 @@ func grpcOptions() []otlpgrpc.Option { if config.Insecure || (isLoopbackAddr(config.Endpoint) && !strings.HasPrefix(config.Endpoint, "https")) { grpcOpts = append(grpcOpts, otlpgrpc.WithInsecure()) } else if !isInsecureSchema(config.Endpoint) { - tlsConfig := &tls.Config{} - if config.NoTlsVerify { - diagnostics.InsecureSkipVerify = true - tlsConfig.InsecureSkipVerify = true - } - if config.Certificate != "" { - data, err := os.ReadFile(config.Certificate) - if err != nil { - softFail("uanble to load certificate: %s", err) - } - - certpool := x509.NewCertPool() - certpool.AppendCertsFromPEM(data) - tlsConfig.RootCAs = certpool - } - grpcOpts = append(grpcOpts, otlpgrpc.WithDialOption(grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)))) + grpcOpts = append(grpcOpts, otlpgrpc.WithTLSCredentials(credentials.NewTLS(tlsConfig()))) } // support for OTLP headers, e.g. for authenticating to SaaS OTLP endpoints @@ -204,12 +214,7 @@ func httpOptions() []otlphttp.Option { if config.Insecure || (isLoopbackAddr(config.Endpoint) && !strings.HasPrefix(config.Endpoint, "https")) { httpOpts = append(httpOpts, otlphttp.WithInsecure()) } else if !isInsecureSchema(config.Endpoint) { - tlsConfig := &tls.Config{} - if config.NoTlsVerify { - diagnostics.InsecureSkipVerify = true - tlsConfig.InsecureSkipVerify = true - } - httpOpts = append(httpOpts, otlphttp.WithTLSClientConfig(tlsConfig)) + httpOpts = append(httpOpts, otlphttp.WithTLSClientConfig(tlsConfig())) } // support for OTLP headers, e.g. for authenticating to SaaS OTLP endpoints From 6218120d847ffe6d5a59281d7352f9097e59d412 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 10:37:06 -0500 Subject: [PATCH 11/25] fix help text --- otelcli/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otelcli/root.go b/otelcli/root.go index 6612eec..a52a941 100644 --- a/otelcli/root.go +++ b/otelcli/root.go @@ -77,7 +77,7 @@ func addClientParams(cmd *cobra.Command) { cmd.Flags().BoolVar(&config.TraceparentIgnoreEnv, "tp-ignore-env", defaults.TraceparentIgnoreEnv, "ignore the TRACEPARENT envvar even if it's set") cmd.Flags().BoolVar(&config.TraceparentPrint, "tp-print", defaults.TraceparentPrint, "print the trace id, span id, and the w3c-formatted traceparent representation of the new span") cmd.Flags().BoolVarP(&config.TraceparentPrintExport, "tp-export", "p", defaults.TraceparentPrintExport, "same as --tp-print but it puts an 'export ' in front so it's more convinenient to source in scripts") - cmd.Flags().BoolVar(&config.NoTlsVerify, "no-tls-verify", defaults.NoTlsVerify, "enable it when TLS is enabled and you want to ignore the certificate validation. This is common when you are testing and usign self-signed certificates.") + cmd.Flags().BoolVar(&config.NoTlsVerify, "no-tls-verify", defaults.NoTlsVerify, "enable it when TLS is enabled and you want to ignore the certificate validation. This is common when you are testing with self-signed certificates.") } func addSpanParams(cmd *cobra.Command) { From c76a5f08cebc8f8e74a8596360a896ee2b1b2fc2 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 10:37:57 -0500 Subject: [PATCH 12/25] remove unused fields, inappropriate RootCA, and add ServerName SubjectKeyId shouldn't have been there in the first place, I copied it and didn't trim it back. The IPAddresses doesn't seem to have an impact on removal so removing it too. The TLS config setups for server and client were both wrong. They shouldn't have had RootCAs set. Setting ServerName seems to have helped to get HTTP TLS working (but grpc still doesn't). Also setting ClientCAs on the server for when we start testing client cert auth. --- tls_for_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tls_for_test.go b/tls_for_test.go index d589cfa..bf2b572 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -99,7 +99,6 @@ func generateTLSData(t *testing.T) tlsHelpers { serverCert := &x509.Certificate{ SerialNumber: big.NewInt(4318), Subject: subject, - SubjectKeyId: []byte{1, 2, 3, 4, 6}, IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, DNSNames: []string{"localhost"}, NotBefore: time.Now(), @@ -139,7 +138,8 @@ func generateTLSData(t *testing.T) tlsHelpers { } out.serverTLSConf = &tls.Config{ - RootCAs: out.certpool, + ServerName: "localhost", + ClientCAs: out.certpool, Certificates: []tls.Certificate{serverCertPair}, } @@ -149,8 +149,6 @@ func generateTLSData(t *testing.T) tlsHelpers { clientCert := &x509.Certificate{ SerialNumber: big.NewInt(4319), Subject: subject, - SubjectKeyId: []byte{1, 2, 3, 4, 7}, - IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, DNSNames: []string{"localhost"}, NotBefore: time.Now(), NotAfter: expire, @@ -181,7 +179,7 @@ func generateTLSData(t *testing.T) tlsHelpers { out.clientPrivKeyFile = pemToTempFile(t, "client-privkey", clientPrivKeyPEM) out.clientTLSConf = &tls.Config{ - RootCAs: out.certpool, + ServerName: "localhost", } return out From fd98ba6b661105cf239ce4e498299e934bba806e Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 11:28:57 -0500 Subject: [PATCH 13/25] add CA to RootCAs in server TLS config See comment for explanation of bewilderment. --- tls_for_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tls_for_test.go b/tls_for_test.go index bf2b572..fc5010c 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -137,7 +137,12 @@ func generateTLSData(t *testing.T) tlsHelpers { t.Fatalf("error generating server cert pair: %s", err) } + // In theory, the server shouldn't need this CA in the RootCAs pool to accept client + // connections. Without it, grpc refuses the client connection with invalid CA. + // No amount of client config changes would work. The opentelemetry collector also sets + // RootCAs by default so it seems safe to copy that behavior here. out.serverTLSConf = &tls.Config{ + RootCAs: out.certpool, ServerName: "localhost", ClientCAs: out.certpool, Certificates: []tls.Certificate{serverCertPair}, From 3ce2d521e75fa50e38c746327f1ade17d4d94db5 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 11:29:32 -0500 Subject: [PATCH 14/25] switch from envvars for root pool to --no-tls-verify The way the tests passed before wasn't ideal but was good enough to call the approach to testing viable. Now that the TLS configs are sorted out, the tests pass under --no-tls-verify and further commits will add tests back for envvar and client auth. --- data_for_test.go | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index 1ff439e..f4d6014 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -138,57 +138,54 @@ var suites = []FixtureSuite{ }, // TLS connections { - Name: "minimum configuration (tls, recording, grpc)", + Name: "minimum configuration (tls, no-verify, recording, grpc)", Config: FixtureConfig{ - ServerProtocol: grpcProtocol, - CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--protocol", "grpc"}, + ServerProtocol: grpcProtocol, + CliArgs: []string{ + "status", + "--endpoint", "https://{{endpoint}}", + "--protocol", "grpc", + "--verbose", "--fail", "--no-tls-verify", + }, TestTimeoutMs: 1000, ServerTLSEnabled: true, - Env: map[string]string{ - "SSL_CERT_FILE": "{{cert}}", - }, }, Expect: Results{ // otel-cli should NOT set insecure when it auto-detects localhost Config: otelcli.DefaultConfig(). WithEndpoint("https://{{endpoint}}"). - WithProtocol("grpc"), + WithProtocol("grpc"). + WithVerbose(true). + WithNoTlsVerify(true), Diagnostics: otelcli.Diagnostics{ - IsRecording: true, - NumArgs: 5, - DetectedLocalhost: true, - ParsedTimeoutMs: 1000, + IsRecording: true, + NumArgs: 8, + DetectedLocalhost: true, + InsecureSkipVerify: true, + ParsedTimeoutMs: 1000, }, Spans: 1, - Env: map[string]string{ - "SSL_CERT_FILE": "{{cert}}", - }, }, }, { - Name: "minimum configuration (tls, recording, https)", + Name: "minimum configuration (tls, no-verify, recording, https)", Config: FixtureConfig{ ServerProtocol: httpProtocol, - CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}"}, + CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--no-tls-verify"}, TestTimeoutMs: 2000, ServerTLSEnabled: true, - Env: map[string]string{ - "SSL_CERT_FILE": "{{cert}}", - }, }, Expect: Results{ // otel-cli should NOT set insecure when it auto-detects localhost Config: otelcli.DefaultConfig(). + WithNoTlsVerify(true). WithEndpoint("https://{{endpoint}}"), Diagnostics: otelcli.Diagnostics{ IsRecording: true, - NumArgs: 3, + NumArgs: 4, DetectedLocalhost: true, ParsedTimeoutMs: 1000, }, - Env: map[string]string{ - "SSL_CERT_FILE": "{{cert}}", - }, Spans: 1, }, }, From 06f977abb9ae456cc86af2bca078b7012a79784c Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 11:31:45 -0500 Subject: [PATCH 15/25] expire all certs after an hour --- tls_for_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tls_for_test.go b/tls_for_test.go index fc5010c..49fb268 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -43,7 +43,7 @@ func generateTLSData(t *testing.T) tlsHelpers { PostalCode: []string{"4317"}, } - expire := time.Now().Add(time.Hour * 1000) + expire := time.Now().Add(time.Hour) // ------------- CA ------------- From 7c4c790747838979ac021248524fc4be5f7b2df2 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 11:33:38 -0500 Subject: [PATCH 16/25] add a comment to tls_for_test.go with a security warning --- tls_for_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tls_for_test.go b/tls_for_test.go index 49fb268..7ed3144 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -1,5 +1,14 @@ package main_test +/* + * This file implements a certificate authority and certs for testing otel-cli's + * TLS settings. + * + * Do NOT copy this code for production systems. It makes a few compromises to + * optimize for testing and ephemeral certs that are totally inappropriate for + * use in settings where security matters. + */ + import ( "bytes" "crypto/ecdsa" From c7ee1b0a4cc07519c10bcf4b7d3a8f7e1ab2f75c Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 11:44:31 -0500 Subject: [PATCH 17/25] clean up TLS setting names Renamed config struct member to CACert. Made config file names match the opentelemetry collector yaml, for better or worse. Added TLS settings to the example config. --- example-config.json | 4 ++++ otelcli/config.go | 7 ++++--- otelcli/plumbing.go | 4 ++-- otelcli/root.go | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/example-config.json b/example-config.json index 5ff9a0d..8c5ff6b 100644 --- a/example-config.json +++ b/example-config.json @@ -5,8 +5,12 @@ "header1" : "header1-value" }, "otlp_blocking" : false, + "insecure" : true, "no_tls_verify" : true, + "ca_file": "/dev/null", + "cert_file": "/dev/null", + "key_file": "/dev/null", "service_name" : "configured_in_config_file", diff --git a/otelcli/config.go b/otelcli/config.go index 07c9f59..78e8a20 100644 --- a/otelcli/config.go +++ b/otelcli/config.go @@ -66,9 +66,10 @@ type Config struct { Blocking bool `json:"otlp_blocking" env:"OTEL_EXPORTER_OTLP_BLOCKING"` NoTlsVerify bool `json:"no_tls_verify" env:"OTEL_CLI_NO_TLS_VERIFY"` - Certificate string `json:"certificate_file" env:"OTEL_EXPORTER_OTLP_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE"` - ClientKey string `json:"client_key_file" env:"OTEL_EXPORTER_OTLP_CLIENT_KEY,OTEL_EXPORTER_OTLP_TRACES_CLIENT_KEY"` - ClientCert string `json:"client_key_certificate_file" env:"OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE,OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE"` + // 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"` ServiceName string `json:"service_name" env:"OTEL_CLI_SERVICE_NAME"` SpanName string `json:"span_name" env:"OTEL_CLI_SPAN_NAME"` diff --git a/otelcli/plumbing.go b/otelcli/plumbing.go index 1331070..2563aab 100644 --- a/otelcli/plumbing.go +++ b/otelcli/plumbing.go @@ -111,8 +111,8 @@ func tlsConfig() *tls.Config { } // TODO: is this the right thing to do? Need to make sure ... - if config.Certificate != "" { - data, err := os.ReadFile(config.Certificate) + if config.CACert != "" { + data, err := os.ReadFile(config.CACert) if err != nil { softFail("uanble to load certificate: %s", err) } diff --git a/otelcli/root.go b/otelcli/root.go index a52a941..7344e32 100644 --- a/otelcli/root.go +++ b/otelcli/root.go @@ -67,7 +67,7 @@ func addClientParams(cmd *cobra.Command) { cmd.Flags().StringToStringVar(&config.Headers, "otlp-headers", defaults.Headers, "a comma-sparated list of key=value headers to send on OTLP connection") cmd.Flags().BoolVar(&config.Blocking, "otlp-blocking", defaults.Blocking, "block on connecting to the OTLP server before proceeding") - cmd.Flags().StringVar(&config.Certificate, "cacert", defaults.Certificate, "a file containing the server's CA certificate") + cmd.Flags().StringVar(&config.CACert, "ca-cert", defaults.CACert, "a file containing the certificate authority bundle") cmd.Flags().StringVar(&config.ClientCert, "client-cert", defaults.ClientCert, "a file containing the client certificate") cmd.Flags().StringVar(&config.ClientKey, "client-key", defaults.ClientKey, "a file containing the client certificate key") From 18028a9ee67b712d46e826c00991934812e182e6 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 12:02:47 -0500 Subject: [PATCH 18/25] rationalize client cert testing templates --- main_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/main_test.go b/main_test.go index 4ef1ac4..0dad0e8 100644 --- a/main_test.go +++ b/main_test.go @@ -487,23 +487,23 @@ 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}}, {{cert}}, {{client_key}}, and {{client_key_cert}} with test values. +// of {{endpoint}}, {{cacert}}, {{client_cert}}, and {{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}}, {{cert}}, {{client_key}}, and -// {{client_key_cert}} with test values. +// injectVars replaces all instances of {{endpoint}}, {{cacert}}, {{client_cert}}, and +// {{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, "{{cert}}", tlsData.caFile) + 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, "{{client_key_cert}}", tlsData.clientFile) return out } From 7c53a3960da349d898b244744fc362c7965f2c13 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 12:25:29 -0500 Subject: [PATCH 19/25] implement client cert auth tests for grpc & HTTP --- data_for_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++++++-- main_test.go | 7 ++++- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index f4d6014..a1877f6 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -27,8 +27,11 @@ type FixtureConfig struct { // TODO: maybe move this up to the suite? IsLongTest bool // either grpcProtocol or httpProtocol, defaults to grpc - ServerProtocol serverProtocol + ServerProtocol serverProtocol + // sets up the server with the test CA, requiring TLS ServerTLSEnabled bool + // tells the server to require client certificate authentication + ServerTLSAuthEnabled bool // for timeout tests we need to start the server to generate the endpoint // but do not want it to answer when otel-cli calls, this does that StopServerBeforeExec bool @@ -136,7 +139,9 @@ var suites = []FixtureSuite{ Spans: 1, }, }, - // TLS connections + }, + // TLS connections + { { Name: "minimum configuration (tls, no-verify, recording, grpc)", Config: FixtureConfig{ @@ -189,6 +194,67 @@ var suites = []FixtureSuite{ Spans: 1, }, }, + { + Name: "minimum configuration (tls, client cert auth, recording, grpc)", + Config: FixtureConfig{ + ServerProtocol: grpcProtocol, + CliArgs: []string{ + "status", + "--endpoint", "https://{{endpoint}}", + "--protocol", "grpc", + "--verbose", "--fail", + "--ca-cert", "{{cacert}}", + "--client-cert", "{{client_cert}}", + "--client-key", "{{client_key}}", + }, + TestTimeoutMs: 1000, + ServerTLSEnabled: true, + ServerTLSAuthEnabled: true, + }, + Expect: Results{ + Config: otelcli.DefaultConfig(). + WithEndpoint("https://{{endpoint}}"). + WithProtocol("grpc"). + WithVerbose(true), + Diagnostics: otelcli.Diagnostics{ + IsRecording: true, + NumArgs: 13, + DetectedLocalhost: true, + InsecureSkipVerify: true, + ParsedTimeoutMs: 1000, + }, + Spans: 1, + }, + }, + { + Name: "minimum configuration (tls, client cert auth, recording, https)", + Config: FixtureConfig{ + ServerProtocol: httpProtocol, + CliArgs: []string{ + "status", + "--endpoint", "https://{{endpoint}}", + "--verbose", "--fail", + "--ca-cert", "{{cacert}}", + "--client-cert", "{{client_cert}}", + "--client-key", "{{client_key}}", + }, + TestTimeoutMs: 2000, + ServerTLSEnabled: true, + ServerTLSAuthEnabled: true, + }, + Expect: Results{ + Config: otelcli.DefaultConfig(). + WithEndpoint("https://{{endpoint}}"). + WithVerbose(true), + Diagnostics: otelcli.Diagnostics{ + IsRecording: true, + NumArgs: 11, + DetectedLocalhost: true, + ParsedTimeoutMs: 1000, + }, + Spans: 1, + }, + }, }, // ensure things fail when they're supposed to fail { diff --git a/main_test.go b/main_test.go index 0dad0e8..822d995 100644 --- a/main_test.go +++ b/main_test.go @@ -354,7 +354,11 @@ func runOtelCli(t *testing.T, fixture Fixture, tlsData tlsHelpers) (string, Resu var listener net.Listener var err error if fixture.Config.ServerTLSEnabled { - listener, err = tls.Listen("tcp", "localhost:0", tlsData.serverTLSConf) + tlsConf := *tlsData.serverTLSConf + if fixture.Config.ServerTLSAuthEnabled { + tlsConf.ClientAuth = tls.RequireAndVerifyClientCert + } + listener, err = tls.Listen("tcp", "localhost:0", &tlsConf) } else { listener, err = net.Listen("tcp", "localhost:0") } @@ -427,6 +431,7 @@ func runOtelCli(t *testing.T, fixture Fixture, tlsData tlsHelpers) (string, Resu err = json.Unmarshal(cliOut, &results) if err != nil { t.Errorf("[%s] parsing otel-cli status output failed: %s", fixture.Name, err) + t.Logf("[%s] output received: %q", fixture.Name, cliOut) return endpoint, results, retSpan, retEvents } From ae46149e1d8f4be4eedece4175a5cf8d7a6baffb Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 12:26:07 -0500 Subject: [PATCH 20/25] implement client cert authentication --- otelcli/plumbing.go | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/otelcli/plumbing.go b/otelcli/plumbing.go index 2563aab..06734bb 100644 --- a/otelcli/plumbing.go +++ b/otelcli/plumbing.go @@ -110,16 +110,38 @@ func tlsConfig() *tls.Config { tlsConfig.InsecureSkipVerify = true } - // TODO: is this the right thing to do? Need to make sure ... + // 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 err != nil { - softFail("uanble to load certificate: %s", err) + softFail("uanble to load CA certificate: %s", err) } certpool := x509.NewCertPool() certpool.AppendCertsFromPEM(data) - //tlsConfig.RootCAs = certpool + tlsConfig.RootCAs = certpool + } + + // client certificate authentication + if config.ClientCert != "" && config.ClientKey != "" { + clientPEM, err := os.ReadFile(config.ClientCert) + if err != nil { + softFail("Unable to read client certificate file %s: %s", config.ClientCert, err) + } + clientKeyPEM, err := os.ReadFile(config.ClientKey) + if err != nil { + softFail("Unable to read client key file %s: %s", config.ClientKey, err) + } + certPair, err := tls.X509KeyPair(clientPEM, clientKeyPEM) + if err != nil { + softFail("error loading client cert pair: %s", err) + } + tlsConfig.Certificates = []tls.Certificate{certPair} + } else if config.ClientCert != "" { + softFail("client cert and key must be specified together") + } else if config.ClientKey != "" { + softFail("client cert and key must be specified together") } return tlsConfig From 673856274b1c9ea54c984b4133089bce30005017 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 12:28:07 -0500 Subject: [PATCH 21/25] fix up error messages --- otelcli/plumbing.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/otelcli/plumbing.go b/otelcli/plumbing.go index 06734bb..7ecd9f2 100644 --- a/otelcli/plumbing.go +++ b/otelcli/plumbing.go @@ -115,7 +115,7 @@ func tlsConfig() *tls.Config { if config.CACert != "" { data, err := os.ReadFile(config.CACert) if err != nil { - softFail("uanble to load CA certificate: %s", err) + softFail("failed to load CA certificate: %s", err) } certpool := x509.NewCertPool() @@ -127,15 +127,15 @@ func tlsConfig() *tls.Config { if config.ClientCert != "" && config.ClientKey != "" { clientPEM, err := os.ReadFile(config.ClientCert) if err != nil { - softFail("Unable to read client certificate file %s: %s", config.ClientCert, err) + softFail("failed to read client certificate file %s: %s", config.ClientCert, err) } clientKeyPEM, err := os.ReadFile(config.ClientKey) if err != nil { - softFail("Unable to read client key file %s: %s", config.ClientKey, err) + softFail("failed to read client key file %s: %s", config.ClientKey, err) } certPair, err := tls.X509KeyPair(clientPEM, clientKeyPEM) if err != nil { - softFail("error loading client cert pair: %s", err) + softFail("failed to parse client cert pair: %s", err) } tlsConfig.Certificates = []tls.Certificate{certPair} } else if config.ClientCert != "" { From d01483496c45c19ef3bf94a88fca86b07e0884a0 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 13:17:05 -0500 Subject: [PATCH 22/25] strip out unneeded TLS settings In the first pass I built up the CA and certs according to my outdated knowledge of building CAs with openssl and examples. Since Go doesn't really care much about a lot of the cert metadata I went through and stripped out as many superfluous fields as I could and keep tests passing. Less code is good :) --- tls_for_test.go | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/tls_for_test.go b/tls_for_test.go index 7ed3144..a8fceb9 100644 --- a/tls_for_test.go +++ b/tls_for_test.go @@ -16,7 +16,6 @@ import ( "crypto/rand" "crypto/tls" "crypto/x509" - "crypto/x509/pkix" "encoding/pem" "math/big" "net" @@ -41,34 +40,16 @@ func generateTLSData(t *testing.T) tlsHelpers { var err error var out tlsHelpers - // this gets reused for each cert, with CommonName overwritten for each - subject := pkix.Name{ - CommonName: "otel-cli certificate authority", - Organization: []string{"otel-cli testing, inc"}, - Country: []string{"Open Source"}, - Province: []string{"Go"}, - Locality: []string{"OpenTelemetry"}, - StreetAddress: []string{"github.com/equinix-labs/otel-cli"}, - PostalCode: []string{"4317"}, - } - expire := time.Now().Add(time.Hour) // ------------- CA ------------- ca := &x509.Certificate{ SerialNumber: big.NewInt(4317), - Subject: subject, NotBefore: time.Now(), NotAfter: expire, IsCA: true, BasicConstraintsValid: true, - ExtKeyUsage: []x509.ExtKeyUsage{ - x509.ExtKeyUsageClientAuth, - x509.ExtKeyUsageServerAuth, - x509.ExtKeyUsageOCSPSigning, - }, - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign | x509.KeyUsageCRLSign, } // create a private key @@ -104,19 +85,11 @@ func generateTLSData(t *testing.T) tlsHelpers { // ------------- server ------------- - subject.CommonName = "server" serverCert := &x509.Certificate{ SerialNumber: big.NewInt(4318), - Subject: subject, IPAddresses: []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback}, - DNSNames: []string{"localhost"}, NotBefore: time.Now(), NotAfter: expire, - ExtKeyUsage: []x509.ExtKeyUsage{ - x509.ExtKeyUsageClientAuth, - x509.ExtKeyUsageServerAuth, - }, - KeyUsage: x509.KeyUsageKeyAgreement | x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, } serverPrivKey, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) @@ -151,23 +124,16 @@ func generateTLSData(t *testing.T) tlsHelpers { // No amount of client config changes would work. The opentelemetry collector also sets // RootCAs by default so it seems safe to copy that behavior here. out.serverTLSConf = &tls.Config{ - RootCAs: out.certpool, - ServerName: "localhost", ClientCAs: out.certpool, Certificates: []tls.Certificate{serverCertPair}, } // ------------- client ------------- - subject.CommonName = "client" clientCert := &x509.Certificate{ SerialNumber: big.NewInt(4319), - Subject: subject, - DNSNames: []string{"localhost"}, NotBefore: time.Now(), NotAfter: expire, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, - KeyUsage: x509.KeyUsageDigitalSignature, } clientPrivKey, err := ecdsa.GenerateKey(elliptic.P521(), rand.Reader) From 26bed9138a6da40bd43300acc7f4e7703885d835 Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 16:08:07 -0500 Subject: [PATCH 23/25] go mod tidy --- go.sum | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/go.sum b/go.sum index 3bbff99..cbc48f7 100644 --- a/go.sum +++ b/go.sum @@ -215,32 +215,18 @@ go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8= go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.3/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= -go.opentelemetry.io/otel v1.12.0 h1:IgfC7kqQrRccIKuB7Cl+SRUmsKbEwSGPr0Eu+/ht1SQ= -go.opentelemetry.io/otel v1.12.0/go.mod h1:geaoz0L0r1BEOR81k7/n9W4TCXYCJ7bPO7K374jQHG0= go.opentelemetry.io/otel v1.13.0 h1:1ZAKnNQKwBBxFtww/GwxNUyTf0AxkZzrukO8MeXqe4Y= go.opentelemetry.io/otel v1.13.0/go.mod h1:FH3RtdZCzRkJYFTCsAKDy9l/XYjMdNv6QrkFFB8DvVg= -go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.12.0 h1:UfDENi+LTcLjQ/JhaXimjlIgn7wWjwbEMmdREm2Gyng= -go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.12.0/go.mod h1:rqbht/LlhVBgn5+k3M5QK96K5Xb0DvXpMJ5SFQpY6uw= go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.13.0 h1:pa05sNT/P8OsIQ8mPZKTIyiBuzS/xDGLVx+DCt0y6Vs= go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.13.0/go.mod h1:rqbht/LlhVBgn5+k3M5QK96K5Xb0DvXpMJ5SFQpY6uw= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.12.0 h1:ZVqtSAxrR4+ofzayuww0/EKamCjjnwnXTMRZzMudJoU= -go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.12.0/go.mod h1:IlaGLENJkAl9+Xoo3J0unkdOwtL+rmqZ3ryMjUtYA94= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.13.0 h1:Any/nVxaoMq1T2w0W85d6w5COlLuCCgOYKQhJJWEMwQ= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.13.0/go.mod h1:46vAP6RWfNn7EKov73l5KBFlNxz8kYlxR1woU+bJ4ZY= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.12.0 h1:+tsVdWosoqDfX6cdHAeacZozjQS94ySBd+aUXFwnNKA= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.12.0/go.mod h1:jSqjV+Knu1Jyvh+l3fx7V210Ev3HHgNQAi8YqpXaQP8= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.13.0 h1:Wz7UQn7/eIqZVDJbuNEM6PmqeA71cWXrWcXekP5HZgU= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.13.0/go.mod h1:OhH1xvgA5jZW2M/S4PcvtDlFE1VULRRBsibBrKuJQGI= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.12.0 h1:L23MzcHDznr05xOM1Ng1F98L0nVd7hm/S7y2jW9IRB4= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.12.0/go.mod h1:C+onYX2j5QH653b3wGJwowYr8jLMjBJw35QcaCQQK0U= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.13.0 h1:Ntu7izEOIRHEgQNjbGc7j3eNtYMAiZfElJJ4JiiRDH4= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.13.0/go.mod h1:wZ9SAjm2sjw3vStBhlCfMZWZusyOQrwrHOFo00jyMC4= -go.opentelemetry.io/otel/sdk v1.12.0 h1:8npliVYV7qc0t1FKdpU08eMnOjgPFMnriPhn0HH4q3o= -go.opentelemetry.io/otel/sdk v1.12.0/go.mod h1:WYcvtgquYvgODEvxOry5owO2y9MyciW7JqMz6cpXShE= go.opentelemetry.io/otel/sdk v1.13.0 h1:BHib5g8MvdqS65yo2vV1s6Le42Hm6rrw08qU6yz5JaM= go.opentelemetry.io/otel/sdk v1.13.0/go.mod h1:YLKPx5+6Vx/o1TCUYYs+bpymtkmazOMT6zoRrC7AQ7I= -go.opentelemetry.io/otel/trace v1.12.0 h1:p28in++7Kd0r2d8gSt931O57fdjUyWxkVbESuILAeUc= -go.opentelemetry.io/otel/trace v1.12.0/go.mod h1:pHlgBynn6s25qJ2szD+Bv+iwKJttjHSI3lUAyf0GNuQ= go.opentelemetry.io/otel/trace v1.13.0 h1:CBgRZ6ntv+Amuj1jDsMhZtlAPT6gbyIRdaIzFhfBSdY= go.opentelemetry.io/otel/trace v1.13.0/go.mod h1:muCvmmO9KKpvuXSf3KKAXXB2ygNYHQ+ZfI5X08d3tds= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= @@ -309,8 +295,6 @@ golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81R golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= -golang.org/x/net v0.4.0 h1:Q5QPcMlvfxFTAPV0+07Xz/MpK9NTXu2VDUuy0FeMfaU= -golang.org/x/net v0.4.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE= golang.org/x/net v0.5.0 h1:GyT4nK/YDHSqa1c4753ouYCDajOYKTja9Xb/OHtgvSw= golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -359,16 +343,12 @@ golang.org/x/sys v0.0.0-20210510120138-977fb7262007/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211013075003-97ac67df715c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220319134239-a9b59b0215f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= -golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18= golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.3.0 h1:qoo4akIqOcDME5bhc/NgxUdovd6BSS2uMsVjB56q1xI= -golang.org/x/term v0.3.0/go.mod h1:q750SLmJuPmVoN1blW3UFBPREJfb1KmY3vwxfr+nFDA= golang.org/x/term v0.4.0 h1:O7UWfv5+A2qiuulQk30kVinPoMtoIPeVaKLEgLpVkvg= golang.org/x/term v0.4.0/go.mod h1:9P2UbLfCdcvo3p/nzKvsmas4TnlujnuoV9hGgYzW1lQ= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -480,8 +460,6 @@ google.golang.org/genproto v0.0.0-20200729003335-053ba62fc06f/go.mod h1:FWY/as6D google.golang.org/genproto v0.0.0-20200804131852-c06518451d9c/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20200825200019-8632dd797987/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20211118181313-81c1377c94b1/go.mod h1:5CzLGKJ67TSI2B9POpiiyGha0AjJvZIUgRMt1dSmuhc= -google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6 h1:a2S6M0+660BgMNl++4JPlcAO/CjkqYItDEZwkoDQK7c= -google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6/go.mod h1:rZS5c/ZVYMaOGBfO68GWtjOw/eLaZM1X6iVtgjZ+EWg= google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f h1:BWUVssLB0HVOSY78gIdvk1dTVYtT1y8SBWtPYuTJ/6w= google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f/go.mod h1:RGgjbofJ8xD9Sq1VVhDM1Vok1vRONV+rg+CjzG4SZKM= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= @@ -500,8 +478,6 @@ google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTp google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= google.golang.org/grpc v1.40.0/go.mod h1:ogyxbiOoUXAkP+4+xa6PZSE9DZgIHtSpzjDTB9KAK34= google.golang.org/grpc v1.42.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= -google.golang.org/grpc v1.52.0 h1:kd48UiU7EHsV4rnLyOJRuP/Il/UHE7gdDAQ+SZI7nZk= -google.golang.org/grpc v1.52.0/go.mod h1:pu6fVzoFb+NBYNAvQL08ic+lvB2IojljRYuun5vorUY= google.golang.org/grpc v1.53.0 h1:LAv2ds7cmFV/XTS3XG1NneeENYrXGmorPxsBbptIjNc= google.golang.org/grpc v1.53.0/go.mod h1:OnIrk0ipVdj4N5d9IUoFUx72/VlD7+jUsHwZgwSMQpw= google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8= From d8c8691662b9d5dd7f597ee7b578716a79016b5b Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 18:27:14 -0500 Subject: [PATCH 24/25] rename TLS options to have a --tls prefix, clean up help text Per code review, I'm convinced that all the tls options should start with --tls for clarity so they've been renamed. --no-tls-verify has been renamed to --tls-no-verify. The old option is still supported but will be removed before 1.0. --- data_for_test.go | 14 +++++++------- otelcli/root.go | 10 ++++++---- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index dcacf39..63c0fcd 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -176,7 +176,7 @@ var suites = []FixtureSuite{ Name: "minimum configuration (tls, no-verify, recording, https)", Config: FixtureConfig{ ServerProtocol: httpProtocol, - CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--no-tls-verify"}, + CliArgs: []string{"status", "--endpoint", "https://{{endpoint}}", "--tls-no-verify"}, TestTimeoutMs: 2000, ServerTLSEnabled: true, }, @@ -203,9 +203,9 @@ var suites = []FixtureSuite{ "--endpoint", "https://{{endpoint}}", "--protocol", "grpc", "--verbose", "--fail", - "--ca-cert", "{{cacert}}", - "--client-cert", "{{client_cert}}", - "--client-key", "{{client_key}}", + "--tls-ca-cert", "{{cacert}}", + "--tls-client-cert", "{{client_cert}}", + "--tls-client-key", "{{client_key}}", }, TestTimeoutMs: 1000, ServerTLSEnabled: true, @@ -234,9 +234,9 @@ var suites = []FixtureSuite{ "status", "--endpoint", "https://{{endpoint}}", "--verbose", "--fail", - "--ca-cert", "{{cacert}}", - "--client-cert", "{{client_cert}}", - "--client-key", "{{client_key}}", + "--tls-ca-cert", "{{cacert}}", + "--tls-client-cert", "{{client_cert}}", + "--tls-client-key", "{{client_key}}", }, TestTimeoutMs: 2000, ServerTLSEnabled: true, diff --git a/otelcli/root.go b/otelcli/root.go index 7344e32..a554fc4 100644 --- a/otelcli/root.go +++ b/otelcli/root.go @@ -67,9 +67,12 @@ func addClientParams(cmd *cobra.Command) { cmd.Flags().StringToStringVar(&config.Headers, "otlp-headers", defaults.Headers, "a comma-sparated list of key=value headers to send on OTLP connection") cmd.Flags().BoolVar(&config.Blocking, "otlp-blocking", defaults.Blocking, "block on connecting to the OTLP server before proceeding") - cmd.Flags().StringVar(&config.CACert, "ca-cert", defaults.CACert, "a file containing the certificate authority bundle") - cmd.Flags().StringVar(&config.ClientCert, "client-cert", defaults.ClientCert, "a file containing the client certificate") - cmd.Flags().StringVar(&config.ClientKey, "client-key", defaults.ClientKey, "a file containing the client certificate key") + 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") + // --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") // 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") @@ -77,7 +80,6 @@ func addClientParams(cmd *cobra.Command) { cmd.Flags().BoolVar(&config.TraceparentIgnoreEnv, "tp-ignore-env", defaults.TraceparentIgnoreEnv, "ignore the TRACEPARENT envvar even if it's set") cmd.Flags().BoolVar(&config.TraceparentPrint, "tp-print", defaults.TraceparentPrint, "print the trace id, span id, and the w3c-formatted traceparent representation of the new span") cmd.Flags().BoolVarP(&config.TraceparentPrintExport, "tp-export", "p", defaults.TraceparentPrintExport, "same as --tp-print but it puts an 'export ' in front so it's more convinenient to source in scripts") - cmd.Flags().BoolVar(&config.NoTlsVerify, "no-tls-verify", defaults.NoTlsVerify, "enable it when TLS is enabled and you want to ignore the certificate validation. This is common when you are testing with self-signed certificates.") } func addSpanParams(cmd *cobra.Command) { From 4453a51f32e9315d8d8ad32c415020a0fc94882e Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Wed, 15 Feb 2023 18:33:00 -0500 Subject: [PATCH 25/25] fix --insecure help text No idea how this was so very wrong. Fixed now :) --- otelcli/root.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/otelcli/root.go b/otelcli/root.go index a554fc4..1d40983 100644 --- a/otelcli/root.go +++ b/otelcli/root.go @@ -63,10 +63,10 @@ func addCommonParams(cmd *cobra.Command) { func addClientParams(cmd *cobra.Command) { config.Headers = make(map[string]string) // OTEL_EXPORTER standard env and variable params - cmd.Flags().BoolVar(&config.Insecure, "insecure", defaults.Insecure, "refuse to connect if TLS is unavailable (true by default when endpoint is localhost)") cmd.Flags().StringToStringVar(&config.Headers, "otlp-headers", defaults.Headers, "a comma-sparated list of key=value headers to send on OTLP connection") 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")