Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ type TelemetryTLS struct {
Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"`
InsecureSkipVerify *bool `json:"insecure_skip_verify,omitempty" yaml:"insecure_skip_verify,omitempty"`
CAFile string `json:"ca_file,omitempty" yaml:"ca_file,omitempty"`
CertFile string `json:"cert_file,omitempty" yaml:"cert_file,omitempty"`
KeyFile string `json:"key_file,omitempty" yaml:"key_file,omitempty"`
}

// TelemetryBatch holds batch export settings.
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/settings_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,8 @@ type V1TelemetryTLSConfig struct {
Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty" koanf:"enabled"`
InsecureSkipVerify *bool `json:"insecure_skip_verify,omitempty" yaml:"insecure_skip_verify,omitempty" koanf:"insecure_skip_verify"`
CAFile string `json:"ca_file,omitempty" yaml:"ca_file,omitempty" koanf:"ca_file"`
CertFile string `json:"cert_file,omitempty" yaml:"cert_file,omitempty" koanf:"cert_file"`
KeyFile string `json:"key_file,omitempty" yaml:"key_file,omitempty" koanf:"key_file"`
}

// V1TelemetryBatchConfig holds batch export settings.
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/telemetry_convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ func ConvertV1TelemetryToAPI(v1 *V1TelemetryConfig) *api.TelemetryConfig {
Enabled: v1.Cloud.TLS.Enabled,
InsecureSkipVerify: v1.Cloud.TLS.InsecureSkipVerify,
CAFile: v1.Cloud.TLS.CAFile,
CertFile: v1.Cloud.TLS.CertFile,
KeyFile: v1.Cloud.TLS.KeyFile,
}
}
if v1.Cloud.Batch != nil {
Expand Down
10 changes: 10 additions & 0 deletions pkg/config/telemetry_convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ func TestConvertV1TelemetryToAPI_Full(t *testing.T) {
insecure := false
tlsEnabled := true
caFile := "/etc/ssl/certs/custom-root.pem"
certFile := "/etc/ssl/certs/client.pem"
keyFile := "/etc/ssl/private/client-key.pem"
hubEnabled := true
localEnabled := true
console := false
Expand All @@ -53,6 +55,8 @@ func TestConvertV1TelemetryToAPI_Full(t *testing.T) {
Enabled: &tlsEnabled,
InsecureSkipVerify: &insecure,
CAFile: caFile,
CertFile: certFile,
KeyFile: keyFile,
},
Batch: &V1TelemetryBatchConfig{
MaxSize: 512,
Expand Down Expand Up @@ -116,6 +120,12 @@ func TestConvertV1TelemetryToAPI_Full(t *testing.T) {
if result.Cloud.TLS.CAFile != caFile {
t.Errorf("Cloud.TLS.CAFile = %q, want %q", result.Cloud.TLS.CAFile, caFile)
}
if result.Cloud.TLS.CertFile != certFile {
t.Errorf("Cloud.TLS.CertFile = %q, want %q", result.Cloud.TLS.CertFile, certFile)
}
if result.Cloud.TLS.KeyFile != keyFile {
t.Errorf("Cloud.TLS.KeyFile = %q, want %q", result.Cloud.TLS.KeyFile, keyFile)
}
if result.Cloud.Batch == nil {
t.Fatal("Cloud.Batch is nil")
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/config/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,12 @@ func mergeTelemetryConfig(base, override *api.TelemetryConfig) *api.TelemetryCon
if override.Cloud.TLS.CAFile != "" {
result.Cloud.TLS.CAFile = override.Cloud.TLS.CAFile
}
if override.Cloud.TLS.CertFile != "" {
result.Cloud.TLS.CertFile = override.Cloud.TLS.CertFile
}
if override.Cloud.TLS.KeyFile != "" {
result.Cloud.TLS.KeyFile = override.Cloud.TLS.KeyFile
}
}
if override.Cloud.Batch != nil {
if result.Cloud.Batch == nil {
Expand Down
4 changes: 4 additions & 0 deletions pkg/config/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1076,6 +1076,8 @@ func TestMergeScionConfigTelemetry(t *testing.T) {
Enabled: boolP(true),
InsecureSkipVerify: boolP(false),
CAFile: "/etc/ssl/certs/base-root.pem",
CertFile: "/etc/ssl/certs/base-client.pem",
KeyFile: "/etc/ssl/private/base-client-key.pem",
},
Batch: &api.TelemetryBatch{
MaxSize: 512,
Expand Down Expand Up @@ -1111,6 +1113,8 @@ func TestMergeScionConfigTelemetry(t *testing.T) {
TLS: &api.TelemetryTLS{
InsecureSkipVerify: boolP(true),
CAFile: "/etc/ssl/certs/override-root.pem",
CertFile: "/etc/ssl/certs/override-client.pem",
KeyFile: "/etc/ssl/private/override-client-key.pem",
},
Batch: &api.TelemetryBatch{
MaxSize: 256,
Expand Down
10 changes: 10 additions & 0 deletions pkg/sciontool/telemetry/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ const (
EnvInsecure = "SCION_OTEL_INSECURE"
// EnvCAFile is the path to a PEM-encoded CA bundle for OTLP TLS.
EnvCAFile = "SCION_OTEL_CA_FILE"
// EnvCertFile is the path to a PEM-encoded client certificate for OTLP mTLS.
EnvCertFile = "SCION_OTEL_CERT_FILE"
// EnvKeyFile is the path to a PEM-encoded client private key for OTLP mTLS.
EnvKeyFile = "SCION_OTEL_KEY_FILE"
// EnvGRPCPort is the local gRPC receiver port.
EnvGRPCPort = "SCION_OTEL_GRPC_PORT"
// EnvHTTPPort is the local HTTP receiver port.
Expand Down Expand Up @@ -81,6 +85,10 @@ type Config struct {
Insecure bool
// CAFile is the path to a PEM-encoded CA bundle for OTLP TLS.
CAFile string
// CertFile is the path to a PEM-encoded client certificate for OTLP mTLS.
CertFile string
// KeyFile is the path to a PEM-encoded client private key for OTLP mTLS.
KeyFile string
// GRPCPort is the local gRPC receiver port.
GRPCPort int
// HTTPPort is the local HTTP receiver port.
Expand Down Expand Up @@ -122,6 +130,8 @@ func LoadConfig() *Config {
Protocol: getEnvOrDefault(EnvProtocol, DefaultProtocol),
Insecure: parseBoolEnv(EnvInsecure, false),
CAFile: os.Getenv(EnvCAFile),
CertFile: os.Getenv(EnvCertFile),
KeyFile: os.Getenv(EnvKeyFile),
GRPCPort: parseIntEnv(EnvGRPCPort, DefaultGRPCPort),
HTTPPort: parseIntEnv(EnvHTTPPort, DefaultHTTPPort),
ProjectID: os.Getenv(EnvProjectID),
Expand Down
24 changes: 24 additions & 0 deletions pkg/sciontool/telemetry/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ func TestLoadConfig_Defaults(t *testing.T) {
if cfg.CAFile != "" {
t.Errorf("Expected CAFile to be empty by default, got %q", cfg.CAFile)
}
if cfg.CertFile != "" {
t.Errorf("Expected CertFile to be empty by default, got %q", cfg.CertFile)
}
if cfg.KeyFile != "" {
t.Errorf("Expected KeyFile to be empty by default, got %q", cfg.KeyFile)
}
if cfg.MetricsDebug {
t.Error("Expected MetricsDebug to be false by default")
}
Expand All @@ -57,6 +63,12 @@ func TestLoadConfig_EnvOverrides(t *testing.T) {
if err := os.Setenv(EnvCAFile, "/etc/ssl/certs/custom-root.pem"); err != nil {
t.Fatalf("failed to set %s: %v", EnvCAFile, err)
}
if err := os.Setenv(EnvCertFile, "/etc/ssl/certs/client.pem"); err != nil {
t.Fatalf("failed to set %s: %v", EnvCertFile, err)
}
if err := os.Setenv(EnvKeyFile, "/etc/ssl/private/client-key.pem"); err != nil {
t.Fatalf("failed to set %s: %v", EnvKeyFile, err)
}
os.Setenv(EnvGRPCPort, "14317")
os.Setenv(EnvHTTPPort, "14318")
os.Setenv(EnvProjectID, "my-project")
Expand Down Expand Up @@ -85,6 +97,12 @@ func TestLoadConfig_EnvOverrides(t *testing.T) {
if cfg.CAFile != "/etc/ssl/certs/custom-root.pem" {
t.Errorf("Expected CAFile to be '/etc/ssl/certs/custom-root.pem', got %q", cfg.CAFile)
}
if cfg.CertFile != "/etc/ssl/certs/client.pem" {
t.Errorf("Expected CertFile to be '/etc/ssl/certs/client.pem', got %q", cfg.CertFile)
}
if cfg.KeyFile != "/etc/ssl/private/client-key.pem" {
t.Errorf("Expected KeyFile to be '/etc/ssl/private/client-key.pem', got %q", cfg.KeyFile)
}
if cfg.GRPCPort != 14317 {
t.Errorf("Expected GRPCPort to be 14317, got %d", cfg.GRPCPort)
}
Expand Down Expand Up @@ -568,6 +586,12 @@ func clearTelemetryEnv() {
if err := os.Unsetenv(EnvCAFile); err != nil {
panic(err)
}
if err := os.Unsetenv(EnvCertFile); err != nil {
panic(err)
}
if err := os.Unsetenv(EnvKeyFile); err != nil {
panic(err)
}
os.Unsetenv(EnvGRPCPort)
os.Unsetenv(EnvHTTPPort)
os.Unsetenv(EnvFilterExclude)
Expand Down
24 changes: 0 additions & 24 deletions pkg/sciontool/telemetry/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ package telemetry

import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"os"

Expand All @@ -26,27 +23,6 @@ import (
"google.golang.org/grpc/credentials/oauth"
)

var errOTLPCACertsNotFound = errors.New("parsing OTLP CA file: no certificates found")

func loadOTLPTLSConfig(caFile string) (*tls.Config, error) {
tlsConfig := &tls.Config{}
if caFile == "" {
return tlsConfig, nil
}

pemBytes, err := os.ReadFile(caFile)
if err != nil {
return nil, fmt.Errorf("reading OTLP CA file: %w", err)
}

roots := x509.NewCertPool()
if !roots.AppendCertsFromPEM(pemBytes) {
return nil, errOTLPCACertsNotFound
}
tlsConfig.RootCAs = roots
return tlsConfig, nil
}

// loadGCPDialOptions loads GCP credentials from a service account key file
// and returns gRPC dial options for per-RPC authentication. Returns (nil, nil)
// if credFile is empty. The credentials are scoped for Cloud Trace, Logging,
Expand Down
61 changes: 58 additions & 3 deletions pkg/sciontool/telemetry/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"crypto/x509/pkix"
"encoding/json"
"encoding/pem"
"errors"
"math/big"
"os"
"path/filepath"
Expand Down Expand Up @@ -95,7 +96,7 @@ func TestLoadGCPDialOptions_ValidKey(t *testing.T) {
}

func TestLoadOTLPTLSConfig_EmptyPath(t *testing.T) {
tlsConfig, err := loadOTLPTLSConfig("")
tlsConfig, err := LoadOTLPTLSConfig("", "", "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -108,7 +109,7 @@ func TestLoadOTLPTLSConfig_EmptyPath(t *testing.T) {
}

func TestLoadOTLPTLSConfig_InvalidPath(t *testing.T) {
_, err := loadOTLPTLSConfig("/nonexistent/path/root.pem")
_, err := LoadOTLPTLSConfig("/nonexistent/path/root.pem", "", "")
if err == nil {
t.Fatal("expected error for missing CA file")
}
Expand All @@ -122,7 +123,7 @@ func TestLoadOTLPTLSConfig_ValidCAFile(t *testing.T) {
t.Fatalf("failed to write CA file: %v", err)
}

tlsConfig, err := loadOTLPTLSConfig(caPath)
tlsConfig, err := LoadOTLPTLSConfig(caPath, "", "")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
Expand All @@ -142,6 +143,34 @@ func TestLoadOTLPTLSConfig_ValidCAFile(t *testing.T) {
}
}

func TestLoadOTLPTLSConfig_MissingClientKeyPair(t *testing.T) {
_, err := LoadOTLPTLSConfig("", "/tmp/client.pem", "")
if !errors.Is(err, errOTLPMissingClientKeyPair) {
t.Fatalf("expected errOTLPMissingClientKeyPair, got %v", err)
}
}

func TestLoadOTLPTLSConfig_ClientKeyPair(t *testing.T) {
tmpDir := t.TempDir()
certPEM, keyPEM := generateTestClientKeyPairPEM(t)
certPath := filepath.Join(tmpDir, "client.pem")
keyPath := filepath.Join(tmpDir, "client-key.pem")
if err := os.WriteFile(certPath, certPEM, 0600); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(keyPath, keyPEM, 0600); err != nil {
t.Fatal(err)
}

tlsConfig, err := LoadOTLPTLSConfig("", certPath, keyPath)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(tlsConfig.Certificates) != 1 {
t.Fatalf("expected one client certificate, got %d", len(tlsConfig.Certificates))
}
}

func generateTestCertificatePEM(t *testing.T) []byte {
t.Helper()

Expand Down Expand Up @@ -172,3 +201,29 @@ func generateTestCertificatePEM(t *testing.T) []byte {
Bytes: derBytes,
})
}

func generateTestClientKeyPairPEM(t *testing.T) ([]byte, []byte) {
t.Helper()

privKey, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
t.Fatalf("failed to generate RSA key: %v", err)
}

tmpl := &x509.Certificate{
SerialNumber: big.NewInt(2),
Subject: pkix.Name{CommonName: "scion-test-client"},
NotBefore: time.Now().Add(-time.Hour),
NotAfter: time.Now().Add(time.Hour),
KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment,
}

derBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &privKey.PublicKey, privKey)
if err != nil {
t.Fatalf("failed to create test client certificate: %v", err)
}

certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: derBytes})
keyPEM := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(privKey)})
return certPEM, keyPEM
}
39 changes: 38 additions & 1 deletion pkg/sciontool/telemetry/otlp_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ package telemetry
import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"os"

"go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploggrpc"
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc"
Expand All @@ -18,6 +21,40 @@ import (
"google.golang.org/grpc/credentials/insecure"
)

var errOTLPCACertsNotFound = errors.New("parsing OTLP CA file: no certificates found")
var errOTLPMissingClientKeyPair = errors.New("OTLP client certificate and key must be provided together")

func LoadOTLPTLSConfig(caFile, certFile, keyFile string) (*tls.Config, error) {
tlsConfig := &tls.Config{}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

For better security, it is recommended to explicitly set the minimum TLS version to 1.2. This ensures that the client does not negotiate older, less secure versions of TLS, which is a best practice for modern OTLP communication.

	tlsConfig := &tls.Config{
		MinVersion: tls.VersionTLS12,
	}


if caFile != "" {
pemBytes, err := os.ReadFile(caFile)
if err != nil {
return nil, fmt.Errorf("reading OTLP CA file: %w", err)
}

roots := x509.NewCertPool()
if !roots.AppendCertsFromPEM(pemBytes) {
return nil, errOTLPCACertsNotFound
}
tlsConfig.RootCAs = roots
}

if certFile == "" && keyFile == "" {
return tlsConfig, nil
}
if certFile == "" || keyFile == "" {
return nil, errOTLPMissingClientKeyPair
}

clientCert, err := tls.LoadX509KeyPair(certFile, keyFile)
if err != nil {
return nil, fmt.Errorf("loading OTLP client certificate: %w", err)
}
tlsConfig.Certificates = []tls.Certificate{clientCert}
return tlsConfig, nil
}

func loadSecureGCPDialOptions(ctx context.Context, config *Config) ([]grpc.DialOption, error) {
if config.GCPCredentialsFile == "" || config.Insecure {
return nil, nil
Expand All @@ -32,7 +69,7 @@ func loadSecureGCPDialOptions(ctx context.Context, config *Config) ([]grpc.DialO
}

func loadSecureOTLPTLSConfig(config *Config) (*tls.Config, error) {
tlsConfig, err := loadOTLPTLSConfig(config.CAFile)
tlsConfig, err := LoadOTLPTLSConfig(config.CAFile, config.CertFile, config.KeyFile)
if err != nil {
return nil, fmt.Errorf("failed to load OTLP TLS config: %w", err)
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/util/logging/otel.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ type OTelConfig struct {
Protocol string
// Insecure skips TLS verification.
Insecure bool
// CAFile is the path to a PEM-encoded CA bundle for OTLP TLS.
CAFile string
// CertFile is the path to a PEM-encoded client certificate for OTLP mTLS.
CertFile string
// KeyFile is the path to a PEM-encoded client private key for OTLP mTLS.
KeyFile string
// ProjectID is the GCP project ID.
ProjectID string
}
Expand Down
Loading