From bb466fe8bacd957eb6dcc2ed022cc1d592c0f7e4 Mon Sep 17 00:00:00 2001 From: Matthew Sainsbury Date: Fri, 3 Jan 2025 11:43:54 -0800 Subject: [PATCH 1/4] config: add client certificate and client key functionality #6378 --- CHANGELOG.md | 3 +- config/v0.3.0/config.go | 37 +++++++++++++++-------- config/v0.3.0/config_test.go | 57 ++++++++++++++++++++++++++++++++++++ config/v0.3.0/log.go | 20 +++++-------- config/v0.3.0/log_test.go | 40 +++++++++++++++++++++++-- config/v0.3.0/metric.go | 20 +++++-------- config/v0.3.0/metric_test.go | 40 +++++++++++++++++++++++-- config/v0.3.0/trace.go | 20 +++++-------- config/v0.3.0/trace_test.go | 40 +++++++++++++++++++++++-- 9 files changed, 221 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f40d8501cc0..506a1b62086 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Use a `sync.Pool` for metric options in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6394) - Added support for configuring `Certificate` field when configuring OTLP exporters in `go.opentelemetry.io/contrib/config`. (#6376) - Added support for the `WithMetricAttributesFn` option to middlewares in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6542) +- Added support for configuring `ClientCertificate` and `ClientKey` field when exporting OTLP over gRPC in `go.opentelemetry.io/contrib/config`. (#6378) ### Changed @@ -61,7 +62,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Use `baggagecopy.NewLogProcessor` when configuring a Log Provider. - `NewLogProcessor` accepts a `Filter` function type that selects which baggage members are added to the log record. -### Changed +### Changed - Transform raw (`slog.KindAny`) attribute values to matching `log.Value` types. For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254) diff --git a/config/v0.3.0/config.go b/config/v0.3.0/config.go index b54c60479c2..b4097ac2552 100644 --- a/config/v0.3.0/config.go +++ b/config/v0.3.0/config.go @@ -8,6 +8,7 @@ import ( "crypto/tls" "crypto/x509" "errors" + "fmt" "os" "gopkg.in/yaml.v3" @@ -159,19 +160,29 @@ func toStringMap(pairs []NameStringValuePair) map[string]string { return output } -// createTLSConfig creates a tls.Config from a raw certificate bytes -// to verify a server certificate. -func createTLSConfig(certFile string) (*tls.Config, error) { - b, err := os.ReadFile(certFile) - if err != nil { - return nil, err +// createTLSConfig creates a tls.Config from certificate files. +func createTLSConfig(caCertFile *string, clientCertFile *string, clientKeyFile *string) (*tls.Config, error) { + tlsConfig := &tls.Config{} + if caCertFile != nil { + caText, err := os.ReadFile(*caCertFile) + if err != nil { + return nil, err + } + certPool := x509.NewCertPool() + if !certPool.AppendCertsFromPEM(caText) { + return nil, errors.New("could not create certificate authority chain from certificate") + } + tlsConfig.RootCAs = certPool } - cp := x509.NewCertPool() - if ok := cp.AppendCertsFromPEM(b); !ok { - return nil, errors.New("failed to append certificate to the cert pool") + if clientCertFile != nil { + if clientKeyFile == nil { + return nil, errors.New("client certificate was provided but no client key was provided") + } + clientCert, err := tls.LoadX509KeyPair(*clientCertFile, *clientKeyFile) + if err != nil { + return nil, fmt.Errorf("could not use client certificate: %w", err) + } + tlsConfig.Certificates = []tls.Certificate{clientCert} } - - return &tls.Config{ - RootCAs: cp, - }, nil + return tlsConfig, nil } diff --git a/config/v0.3.0/config_test.go b/config/v0.3.0/config_test.go index ebe45c95286..ccc57486a4a 100644 --- a/config/v0.3.0/config_test.go +++ b/config/v0.3.0/config_test.go @@ -5,6 +5,7 @@ package config import ( "context" + "crypto/tls" "encoding/json" "errors" "os" @@ -511,6 +512,62 @@ func TestSerializeJSON(t *testing.T) { } } +func TestCreateTLSConfig(t *testing.T) { + tests := []struct { + name string + caCertFile *string + clientCertFile *string + clientKeyFile *string + wantErr error + want func(*tls.Config, *testing.T) + }{ + { + name: "no-input", + want: func(result *tls.Config, t *testing.T) { + require.Nil(t, result.Certificates) + require.Nil(t, result.RootCAs) + }, + }, + { + name: "only-cacert-provided", + caCertFile: ptr(filepath.Join("..", "testdata", "ca.crt")), + want: func(result *tls.Config, t *testing.T) { + require.Nil(t, result.Certificates) + require.NotNil(t, result.RootCAs) + }, + }, + { + name: "nonexistent-cacert-file", + caCertFile: ptr("nowhere.crt"), + wantErr: errors.New("open nowhere.crt: no such file or directory"), + }, + { + name: "nonexistent-clientcert-file", + clientCertFile: ptr("nowhere.crt"), + clientKeyFile: ptr("nowhere.crt"), + wantErr: errors.New("could not use client certificate: open nowhere.crt: no such file or directory"), + }, + { + name: "bad-cacert-file", + caCertFile: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + wantErr: errors.New("could not create certificate authority chain from certificate"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := createTLSConfig(tt.caCertFile, tt.clientCertFile, tt.clientKeyFile) + + if tt.wantErr != nil { + require.Equal(t, tt.wantErr.Error(), err.Error()) + } else { + require.NoError(t, err) + tt.want(got, t) + } + }) + } +} + func ptr[T any](v T) *T { return &v } diff --git a/config/v0.3.0/log.go b/config/v0.3.0/log.go index 42b1b209207..bca5d235594 100644 --- a/config/v0.3.0/log.go +++ b/config/v0.3.0/log.go @@ -156,13 +156,11 @@ func otlpHTTPLogExporter(ctx context.Context, otlpConfig *OTLP) (sdklog.Exporter opts = append(opts, otlploghttp.WithHeaders(toStringMap(otlpConfig.Headers))) } - if otlpConfig.Certificate != nil { - creds, err := createTLSConfig(*otlpConfig.Certificate) - if err != nil { - return nil, fmt.Errorf("could not create client tls credentials: %w", err) - } - opts = append(opts, otlploghttp.WithTLSClientConfig(creds)) + tlsConfig, err := createTLSConfig(otlpConfig.Certificate, otlpConfig.ClientCertificate, otlpConfig.ClientKey) + if err != nil { + return nil, err } + opts = append(opts, otlploghttp.WithTLSClientConfig(tlsConfig)) return otlploghttp.New(ctx, opts...) } @@ -206,13 +204,11 @@ func otlpGRPCLogExporter(ctx context.Context, otlpConfig *OTLP) (sdklog.Exporter opts = append(opts, otlploggrpc.WithHeaders(toStringMap(otlpConfig.Headers))) } - if otlpConfig.Certificate != nil { - creds, err := credentials.NewClientTLSFromFile(*otlpConfig.Certificate, "") - if err != nil { - return nil, fmt.Errorf("could not create client tls credentials: %w", err) - } - opts = append(opts, otlploggrpc.WithTLSCredentials(creds)) + tlsConfig, err := createTLSConfig(otlpConfig.Certificate, otlpConfig.ClientCertificate, otlpConfig.ClientKey) + if err != nil { + return nil, err } + opts = append(opts, otlploggrpc.WithTLSCredentials(credentials.NewTLS(tlsConfig))) return otlploggrpc.New(ctx, opts...) } diff --git a/config/v0.3.0/log_test.go b/config/v0.3.0/log_test.go index 7d96fee647d..1135a0111b3 100644 --- a/config/v0.3.0/log_test.go +++ b/config/v0.3.0/log_test.go @@ -255,7 +255,25 @@ func TestLogProcessor(t *testing.T) { }, }, }, - wantErr: fmt.Errorf("could not create client tls credentials: %w", errors.New("credentials: failed to append certificates")), + wantErr: fmt.Errorf("could not create certificate authority chain from certificate"), + }, + { + name: "batch/otlp-grpc-bad-client-certificate", + processor: LogRecordProcessor{ + Batch: &BatchLogRecordProcessor{ + Exporter: LogRecordExporter{ + OTLP: &OTLP{ + Protocol: ptr("grpc"), + Endpoint: ptr("localhost:4317"), + Compression: ptr("gzip"), + Timeout: ptr(1000), + ClientCertificate: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + ClientKey: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + }, + }, + }, + }, + wantErr: fmt.Errorf("could not use client certificate: %w", errors.New("tls: failed to find any PEM data in certificate input")), }, { name: "batch/otlp-grpc-exporter-no-scheme", @@ -381,7 +399,25 @@ func TestLogProcessor(t *testing.T) { }, }, }, - wantErr: fmt.Errorf("could not create client tls credentials: %w", errors.New("failed to append certificate to the cert pool")), + wantErr: fmt.Errorf("could not create certificate authority chain from certificate"), + }, + { + name: "batch/otlp-http-bad-client-certificate", + processor: LogRecordProcessor{ + Batch: &BatchLogRecordProcessor{ + Exporter: LogRecordExporter{ + OTLP: &OTLP{ + Protocol: ptr("http/protobuf"), + Endpoint: ptr("localhost:4317"), + Compression: ptr("gzip"), + Timeout: ptr(1000), + ClientCertificate: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + ClientKey: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + }, + }, + }, + }, + wantErr: fmt.Errorf("could not use client certificate: %w", errors.New("tls: failed to find any PEM data in certificate input")), }, { name: "batch/otlp-http-exporter-with-path", diff --git a/config/v0.3.0/metric.go b/config/v0.3.0/metric.go index e88c02349c9..b7d68106b43 100644 --- a/config/v0.3.0/metric.go +++ b/config/v0.3.0/metric.go @@ -182,13 +182,11 @@ func otlpHTTPMetricExporter(ctx context.Context, otlpConfig *OTLPMetric) (sdkmet } } - if otlpConfig.Certificate != nil { - creds, err := createTLSConfig(*otlpConfig.Certificate) - if err != nil { - return nil, fmt.Errorf("could not create client tls credentials: %w", err) - } - opts = append(opts, otlpmetrichttp.WithTLSClientConfig(creds)) + tlsConfig, err := createTLSConfig(otlpConfig.Certificate, otlpConfig.ClientCertificate, otlpConfig.ClientKey) + if err != nil { + return nil, err } + opts = append(opts, otlpmetrichttp.WithTLSClientConfig(tlsConfig)) return otlpmetrichttp.New(ctx, opts...) } @@ -245,13 +243,11 @@ func otlpGRPCMetricExporter(ctx context.Context, otlpConfig *OTLPMetric) (sdkmet } } - if otlpConfig.Certificate != nil { - creds, err := credentials.NewClientTLSFromFile(*otlpConfig.Certificate, "") - if err != nil { - return nil, fmt.Errorf("could not create client tls credentials: %w", err) - } - opts = append(opts, otlpmetricgrpc.WithTLSCredentials(creds)) + tlsConfig, err := createTLSConfig(otlpConfig.Certificate, otlpConfig.ClientCertificate, otlpConfig.ClientKey) + if err != nil { + return nil, err } + opts = append(opts, otlpmetricgrpc.WithTLSCredentials(credentials.NewTLS(tlsConfig))) return otlpmetricgrpc.New(ctx, opts...) } diff --git a/config/v0.3.0/metric_test.go b/config/v0.3.0/metric_test.go index aa01665831f..c6a4151c2c1 100644 --- a/config/v0.3.0/metric_test.go +++ b/config/v0.3.0/metric_test.go @@ -248,7 +248,25 @@ func TestReader(t *testing.T) { }, }, }, - wantErr: fmt.Errorf("could not create client tls credentials: %w", errors.New("credentials: failed to append certificates")), + wantErr: errors.New("could not create certificate authority chain from certificate"), + }, + { + name: "periodic/otlp-grpc-bad-client-certificate", + reader: MetricReader{ + Periodic: &PeriodicMetricReader{ + Exporter: PushMetricExporter{ + OTLP: &OTLPMetric{ + Protocol: ptr("grpc"), + Endpoint: ptr("localhost:4317"), + Compression: ptr("gzip"), + Timeout: ptr(1000), + ClientCertificate: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + ClientKey: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + }, + }, + }, + }, + wantErr: fmt.Errorf("could not use client certificate: %w", errors.New("tls: failed to find any PEM data in certificate input")), }, { name: "periodic/otlp-grpc-exporter-no-endpoint", @@ -475,7 +493,25 @@ func TestReader(t *testing.T) { }, }, }, - wantErr: fmt.Errorf("could not create client tls credentials: %w", errors.New("failed to append certificate to the cert pool")), + wantErr: errors.New("could not create certificate authority chain from certificate"), + }, + { + name: "periodic/otlp-http-bad-client-certificate", + reader: MetricReader{ + Periodic: &PeriodicMetricReader{ + Exporter: PushMetricExporter{ + OTLP: &OTLPMetric{ + Protocol: ptr("http/protobuf"), + Endpoint: ptr("localhost:4317"), + Compression: ptr("gzip"), + Timeout: ptr(1000), + ClientCertificate: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + ClientKey: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + }, + }, + }, + }, + wantErr: fmt.Errorf("could not use client certificate: %w", errors.New("tls: failed to find any PEM data in certificate input")), }, { name: "periodic/otlp-http-exporter-with-path", diff --git a/config/v0.3.0/trace.go b/config/v0.3.0/trace.go index 10d2473912f..498686e15f5 100644 --- a/config/v0.3.0/trace.go +++ b/config/v0.3.0/trace.go @@ -129,13 +129,11 @@ func otlpGRPCSpanExporter(ctx context.Context, otlpConfig *OTLP) (sdktrace.SpanE opts = append(opts, otlptracegrpc.WithHeaders(toStringMap(otlpConfig.Headers))) } - if otlpConfig.Certificate != nil { - creds, err := credentials.NewClientTLSFromFile(*otlpConfig.Certificate, "") - if err != nil { - return nil, fmt.Errorf("could not create client tls credentials: %w", err) - } - opts = append(opts, otlptracegrpc.WithTLSCredentials(creds)) + tlsConfig, err := createTLSConfig(otlpConfig.Certificate, otlpConfig.ClientCertificate, otlpConfig.ClientKey) + if err != nil { + return nil, err } + opts = append(opts, otlptracegrpc.WithTLSCredentials(credentials.NewTLS(tlsConfig))) return otlptracegrpc.New(ctx, opts...) } @@ -174,13 +172,11 @@ func otlpHTTPSpanExporter(ctx context.Context, otlpConfig *OTLP) (sdktrace.SpanE opts = append(opts, otlptracehttp.WithHeaders(toStringMap(otlpConfig.Headers))) } - if otlpConfig.Certificate != nil { - creds, err := createTLSConfig(*otlpConfig.Certificate) - if err != nil { - return nil, fmt.Errorf("could not create client tls credentials: %w", err) - } - opts = append(opts, otlptracehttp.WithTLSClientConfig(creds)) + tlsConfig, err := createTLSConfig(otlpConfig.Certificate, otlpConfig.ClientCertificate, otlpConfig.ClientKey) + if err != nil { + return nil, err } + opts = append(opts, otlptracehttp.WithTLSClientConfig(tlsConfig)) return otlptracehttp.New(ctx, opts...) } diff --git a/config/v0.3.0/trace_test.go b/config/v0.3.0/trace_test.go index 8e0ef27a479..d5a282da113 100644 --- a/config/v0.3.0/trace_test.go +++ b/config/v0.3.0/trace_test.go @@ -295,7 +295,25 @@ func TestSpanProcessor(t *testing.T) { }, }, }, - wantErr: fmt.Errorf("could not create client tls credentials: %w", errors.New("credentials: failed to append certificates")), + wantErr: errors.New("could not create certificate authority chain from certificate"), + }, + { + name: "batch/otlp-grpc-bad-client-certificate", + processor: SpanProcessor{ + Batch: &BatchSpanProcessor{ + Exporter: SpanExporter{ + OTLP: &OTLP{ + Protocol: ptr("grpc"), + Endpoint: ptr("localhost:4317"), + Compression: ptr("gzip"), + Timeout: ptr(1000), + ClientCertificate: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + ClientKey: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + }, + }, + }, + }, + wantErr: fmt.Errorf("could not use client certificate: %w", errors.New("tls: failed to find any PEM data in certificate input")), }, { name: "batch/otlp-grpc-exporter-no-scheme", @@ -421,7 +439,25 @@ func TestSpanProcessor(t *testing.T) { }, }, }, - wantErr: fmt.Errorf("could not create client tls credentials: %w", errors.New("failed to append certificate to the cert pool")), + wantErr: errors.New("could not create certificate authority chain from certificate"), + }, + { + name: "batch/otlp-http-bad-client-certificate", + processor: SpanProcessor{ + Batch: &BatchSpanProcessor{ + Exporter: SpanExporter{ + OTLP: &OTLP{ + Protocol: ptr("http/protobuf"), + Endpoint: ptr("localhost:4317"), + Compression: ptr("gzip"), + Timeout: ptr(1000), + ClientCertificate: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + ClientKey: ptr(filepath.Join("..", "testdata", "bad_cert.crt")), + }, + }, + }, + }, + wantErr: fmt.Errorf("could not use client certificate: %w", errors.New("tls: failed to find any PEM data in certificate input")), }, { name: "batch/otlp-http-exporter-with-path", From 3f4cdb7373d6865f2df88044f2a1e2d2e2bf9e7a Mon Sep 17 00:00:00 2001 From: Matthew Sainsbury Date: Wed, 8 Jan 2025 11:56:44 -0800 Subject: [PATCH 2/4] avoid changing CHANGELOG trailing whitespace --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 506a1b62086..2ec87502644 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,7 +62,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Use `baggagecopy.NewLogProcessor` when configuring a Log Provider. - `NewLogProcessor` accepts a `Filter` function type that selects which baggage members are added to the log record. -### Changed +### Changed - Transform raw (`slog.KindAny`) attribute values to matching `log.Value` types. For example, `[]string{"foo", "bar"}` attribute value is now transformed to `log.SliceValue(log.StringValue("foo"), log.StringValue("bar"))` instead of `log.String("[foo bar"])`. (#6254) From a2c8d0604d5655dd4f8a261bebbbed1d9465ba66 Mon Sep 17 00:00:00 2001 From: Matthew Sainsbury Date: Thu, 9 Jan 2025 09:22:38 -0800 Subject: [PATCH 3/4] fix changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e72699dad0..fd0b6f92792 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - Generate server metrics with semantic conventions v1.26 in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` when `OTEL_SEMCONV_STABILITY_OPT_IN` is set to `http/dup`. (#6411) +- Added support for configuring `ClientCertificate` and `ClientKey` field when exporting OTLP over gRPC in `go.opentelemetry.io/contrib/config`. (#6378) + ### Fixed @@ -31,7 +33,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Use a `sync.Pool` for metric options in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6394) - Added support for configuring `Certificate` field when configuring OTLP exporters in `go.opentelemetry.io/contrib/config`. (#6376) - Added support for the `WithMetricAttributesFn` option to middlewares in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#6542) -- Added support for configuring `ClientCertificate` and `ClientKey` field when exporting OTLP over gRPC in `go.opentelemetry.io/contrib/config`. (#6378) ### Changed From bfe0cdd168185d66c4c395f91a8339543dd7b3af Mon Sep 17 00:00:00 2001 From: Matthew Sainsbury Date: Thu, 9 Jan 2025 09:33:29 -0800 Subject: [PATCH 4/4] Update CHANGELOG.md Co-authored-by: Damien Mathieu <42@dmathieu.com> --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd0b6f92792..a7ead7e66d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Generate server metrics with semantic conventions v1.26 in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp` when `OTEL_SEMCONV_STABILITY_OPT_IN` is set to `http/dup`. (#6411) - Added support for configuring `ClientCertificate` and `ClientKey` field when exporting OTLP over gRPC in `go.opentelemetry.io/contrib/config`. (#6378) - ### Fixed - Fix error logged by Jaeger remote sampler on empty or unset `OTEL_TRACES_SAMPLER_ARG` environment variable (#6511)