From 81c2ad71b427db8ce0c8279e1ee7bb7472169309 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 2 Mar 2026 13:21:42 -0800 Subject: [PATCH 1/4] Add Azure AD certificate-based authentication for remote write (#2) * Initial plan * Add Azure AD certificate-based authentication support Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> * Update documentation for certificate-based authentication Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> * Address code review feedback - improve error messages and format detection Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> * Replace third-party go-pkcs12 with official golang.org/x/crypto/pkcs12 Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> * Extract certificate parsing to common util/certutil package Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> * Fix linting errors: import ordering and comment formatting Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> --- docs/configuration/configuration.md | 12 ++ go.mod | 2 +- storage/remote/azuread/azuread.go | 101 ++++++++- storage/remote/azuread/azuread_test.go | 39 +++- ...uread_bad_certificate_invalidclientid.yaml | 5 + ...uread_bad_certificate_missingclientid.yaml | 4 + .../azuread_bad_certificate_missingpath.yaml | 4 + ...uread_bad_certificate_missingtenantid.yaml | 4 + .../azuread_bad_certificate_oauth.yaml | 9 + .../testdata/azuread_good_certificate.yaml | 5 + .../azuread_good_certificate_pfx.yaml | 6 + .../azuread_good_certificate_with_key.yaml | 7 + util/certutil/certutil.go | 204 ++++++++++++++++++ util/certutil/certutil_test.go | 55 +++++ 14 files changed, 454 insertions(+), 3 deletions(-) create mode 100644 storage/remote/azuread/testdata/azuread_bad_certificate_invalidclientid.yaml create mode 100644 storage/remote/azuread/testdata/azuread_bad_certificate_missingclientid.yaml create mode 100644 storage/remote/azuread/testdata/azuread_bad_certificate_missingpath.yaml create mode 100644 storage/remote/azuread/testdata/azuread_bad_certificate_missingtenantid.yaml create mode 100644 storage/remote/azuread/testdata/azuread_bad_certificate_oauth.yaml create mode 100644 storage/remote/azuread/testdata/azuread_good_certificate.yaml create mode 100644 storage/remote/azuread/testdata/azuread_good_certificate_pfx.yaml create mode 100644 storage/remote/azuread/testdata/azuread_good_certificate_with_key.yaml create mode 100644 util/certutil/certutil.go create mode 100644 util/certutil/certutil_test.go diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 49b7774b5f..5e3e53c7a9 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -3416,6 +3416,18 @@ azuread: [ client_secret: ] [ tenant_id: ] ] + # Azure Certificate-based authentication. + [ certificate: + client_id: + tenant_id: + certificate_path: + # Optional path to private key file if separate from certificate + [ certificate_key_path: ] + # Optional password for password-protected certificate files (PFX/PKCS12) + [ certificate_password: ] + # Whether to send the certificate chain in the x5c header + [ send_certificate_chain: | default = false ] ] + # Azure SDK auth. # See https://learn.microsoft.com/en-us/azure/developer/go/azure-sdk-authentication [ sdk: diff --git a/go.mod b/go.mod index 89d468e874..96c97a9a47 100644 --- a/go.mod +++ b/go.mod @@ -239,7 +239,7 @@ require ( go.opentelemetry.io/collector/pipeline v1.51.0 // indirect go.opentelemetry.io/proto/otlp v1.9.0 // indirect go.uber.org/zap v1.27.1 // indirect - golang.org/x/crypto v0.47.0 // indirect + golang.org/x/crypto v0.47.0 golang.org/x/exp v0.0.0-20260112195511-716be5621a96 // indirect golang.org/x/mod v0.32.0 // indirect golang.org/x/net v0.49.0 // indirect diff --git a/storage/remote/azuread/azuread.go b/storage/remote/azuread/azuread.go index fe0c4f9e21..15267486ef 100644 --- a/storage/remote/azuread/azuread.go +++ b/storage/remote/azuread/azuread.go @@ -27,6 +27,8 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/google/uuid" "github.com/grafana/regexp" + + "github.com/prometheus/prometheus/util/certutil" ) // Clouds. @@ -87,6 +89,30 @@ type SDKConfig struct { TenantID string `yaml:"tenant_id,omitempty"` } +// CertificateConfig is used to store azure certificate-based authentication config values. +type CertificateConfig struct { + // ClientID is the clientId of the azure active directory application that is being used to authenticate. + ClientID string `yaml:"client_id,omitempty"` + + // TenantID is the tenantId of the azure active directory application that is being used to authenticate. + TenantID string `yaml:"tenant_id,omitempty"` + + // CertificatePath is the path to the certificate file (PEM or PFX format). + CertificatePath string `yaml:"certificate_path,omitempty"` + + // CertificateKeyPath is the path to the private key file (PEM format). + // This is optional and only needed if the certificate and key are in separate files. + CertificateKeyPath string `yaml:"certificate_key_path,omitempty"` + + // CertificatePassword is the password for the certificate file (for PFX files). + // This is optional and only needed if the certificate file is password-protected. + CertificatePassword string `yaml:"certificate_password,omitempty"` + + // SendCertificateChain controls whether to include x5c header in assertion to support + // subject name / issuer-based authentication. + SendCertificateChain bool `yaml:"send_certificate_chain,omitempty"` +} + // AzureADConfig is used to store the config values. type AzureADConfig struct { //nolint:revive // exported. // ManagedIdentity is the managed identity that is being used to authenticate. @@ -101,6 +127,9 @@ type AzureADConfig struct { //nolint:revive // exported. // SDK is the SDK config that is being used to authenticate. SDK *SDKConfig `yaml:"sdk,omitempty"` + // Certificate is the certificate config that is being used to authenticate. + Certificate *CertificateConfig `yaml:"certificate,omitempty"` + // Cloud is the Azure cloud in which the service is running. Example: AzurePublic/AzureGovernment/AzureChina. Cloud string `yaml:"cloud,omitempty"` @@ -150,9 +179,12 @@ func (c *AzureADConfig) Validate() error { if c.SDK != nil { authenticators++ } + if c.Certificate != nil { + authenticators++ + } if authenticators == 0 { - return errors.New("must provide an Azure Managed Identity, Azure Workload Identity, Azure OAuth or Azure SDK in the Azure AD config") + return errors.New("must provide an Azure Managed Identity, Azure Workload Identity, Azure OAuth, Azure Certificate or Azure SDK in the Azure AD config") } if authenticators > 1 { return errors.New("cannot provide multiple authentication methods in the Azure AD config") @@ -214,6 +246,25 @@ func (c *AzureADConfig) Validate() error { } } + if c.Certificate != nil { + if c.Certificate.ClientID == "" { + return errors.New("must provide an Azure Certificate client_id in the Azure AD config") + } + if c.Certificate.TenantID == "" { + return errors.New("must provide an Azure Certificate tenant_id in the Azure AD config") + } + if c.Certificate.CertificatePath == "" { + return errors.New("must provide an Azure Certificate certificate_path in the Azure AD config") + } + + if _, err := uuid.Parse(c.Certificate.ClientID); err != nil { + return errors.New("the provided Azure Certificate client_id is invalid") + } + if _, err := regexp.MatchString("^[0-9a-zA-Z-.]+$", c.Certificate.TenantID); err != nil { + return errors.New("the provided Azure Certificate tenant_id is invalid") + } + } + if c.Scope != "" { if matched, err := regexp.MatchString("^[\\w\\s:/.\\-]+$", c.Scope); err != nil || !matched { return errors.New("the provided scope contains invalid characters") @@ -324,6 +375,21 @@ func newTokenCredential(cfg *AzureADConfig) (azcore.TokenCredential, error) { } } + if cfg.Certificate != nil { + certificateConfig := &CertificateConfig{ + ClientID: cfg.Certificate.ClientID, + TenantID: cfg.Certificate.TenantID, + CertificatePath: cfg.Certificate.CertificatePath, + CertificateKeyPath: cfg.Certificate.CertificateKeyPath, + CertificatePassword: cfg.Certificate.CertificatePassword, + SendCertificateChain: cfg.Certificate.SendCertificateChain, + } + cred, err = newCertificateTokenCredential(clientOpts, certificateConfig) + if err != nil { + return nil, err + } + } + return cred, nil } @@ -366,6 +432,39 @@ func newSDKTokenCredential(clientOpts *azcore.ClientOptions, sdkConfig *SDKConfi return azidentity.NewDefaultAzureCredential(opts) } +// newCertificateTokenCredential returns new certificate-based token credential. +func newCertificateTokenCredential(clientOpts *azcore.ClientOptions, certConfig *CertificateConfig) (azcore.TokenCredential, error) { + // Use certutil to parse the certificate files + certData, err := certutil.ParseCertificateFiles( + certConfig.CertificatePath, + certConfig.CertificateKeyPath, + certConfig.CertificatePassword, + ) + if err != nil { + return nil, err + } + + if len(certData.Certificates) == 0 { + return nil, errors.New("no certificates found in certificate file") + } + if certData.PrivateKey == nil { + return nil, errors.New("no private key found") + } + + opts := &azidentity.ClientCertificateCredentialOptions{ + ClientOptions: *clientOpts, + SendCertificateChain: certConfig.SendCertificateChain, + } + + return azidentity.NewClientCertificateCredential( + certConfig.TenantID, + certConfig.ClientID, + certData.Certificates, + certData.PrivateKey, + opts, + ) +} + // newTokenProvider helps to fetch accessToken for different types of credential. This also takes care of // refreshing the accessToken before expiry. This accessToken is attached to the Authorization header while making requests. func newTokenProvider(cfg *AzureADConfig, cred azcore.TokenCredential) (*tokenProvider, error) { diff --git a/storage/remote/azuread/azuread_test.go b/storage/remote/azuread/azuread_test.go index 857ecdba8a..167e72d28b 100644 --- a/storage/remote/azuread/azuread_test.go +++ b/storage/remote/azuread/azuread_test.go @@ -156,7 +156,7 @@ func TestAzureAdConfig(t *testing.T) { // Missing managedidentity or oauth field. { filename: "testdata/azuread_bad_configmissing.yaml", - err: "must provide an Azure Managed Identity, Azure Workload Identity, Azure OAuth or Azure SDK in the Azure AD config", + err: "must provide an Azure Managed Identity, Azure Workload Identity, Azure OAuth, Azure Certificate or Azure SDK in the Azure AD config", }, // Invalid managedidentity client id. { @@ -231,6 +231,43 @@ func TestAzureAdConfig(t *testing.T) { { filename: "testdata/azuread_good_oauth_customscope.yaml", }, + // Valid certificate config. + { + filename: "testdata/azuread_good_certificate.yaml", + }, + // Valid certificate config with separate key file. + { + filename: "testdata/azuread_good_certificate_with_key.yaml", + }, + // Valid certificate config with PFX. + { + filename: "testdata/azuread_good_certificate_pfx.yaml", + }, + // Missing certificate client id. + { + filename: "testdata/azuread_bad_certificate_missingclientid.yaml", + err: "must provide an Azure Certificate client_id in the Azure AD config", + }, + // Missing certificate tenant id. + { + filename: "testdata/azuread_bad_certificate_missingtenantid.yaml", + err: "must provide an Azure Certificate tenant_id in the Azure AD config", + }, + // Missing certificate path. + { + filename: "testdata/azuread_bad_certificate_missingpath.yaml", + err: "must provide an Azure Certificate certificate_path in the Azure AD config", + }, + // Invalid certificate client id. + { + filename: "testdata/azuread_bad_certificate_invalidclientid.yaml", + err: "the provided Azure Certificate client_id is invalid", + }, + // Invalid config when both certificate and oauth is provided. + { + filename: "testdata/azuread_bad_certificate_oauth.yaml", + err: "cannot provide multiple authentication methods in the Azure AD config", + }, } for _, c := range cases { _, err := loadAzureAdConfig(c.filename) diff --git a/storage/remote/azuread/testdata/azuread_bad_certificate_invalidclientid.yaml b/storage/remote/azuread/testdata/azuread_bad_certificate_invalidclientid.yaml new file mode 100644 index 0000000000..b28581519f --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_bad_certificate_invalidclientid.yaml @@ -0,0 +1,5 @@ +cloud: AzurePublic +certificate: + client_id: invalid-client-id + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 + certificate_path: /path/to/certificate.pem diff --git a/storage/remote/azuread/testdata/azuread_bad_certificate_missingclientid.yaml b/storage/remote/azuread/testdata/azuread_bad_certificate_missingclientid.yaml new file mode 100644 index 0000000000..e75bb56f52 --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_bad_certificate_missingclientid.yaml @@ -0,0 +1,4 @@ +cloud: AzurePublic +certificate: + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 + certificate_path: /path/to/certificate.pem diff --git a/storage/remote/azuread/testdata/azuread_bad_certificate_missingpath.yaml b/storage/remote/azuread/testdata/azuread_bad_certificate_missingpath.yaml new file mode 100644 index 0000000000..99bb6a72cf --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_bad_certificate_missingpath.yaml @@ -0,0 +1,4 @@ +cloud: AzurePublic +certificate: + client_id: 00000000-0000-0000-0000-000000000000 + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 diff --git a/storage/remote/azuread/testdata/azuread_bad_certificate_missingtenantid.yaml b/storage/remote/azuread/testdata/azuread_bad_certificate_missingtenantid.yaml new file mode 100644 index 0000000000..9295bed2a1 --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_bad_certificate_missingtenantid.yaml @@ -0,0 +1,4 @@ +cloud: AzurePublic +certificate: + client_id: 00000000-0000-0000-0000-000000000000 + certificate_path: /path/to/certificate.pem diff --git a/storage/remote/azuread/testdata/azuread_bad_certificate_oauth.yaml b/storage/remote/azuread/testdata/azuread_bad_certificate_oauth.yaml new file mode 100644 index 0000000000..e14aa12663 --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_bad_certificate_oauth.yaml @@ -0,0 +1,9 @@ +cloud: AzurePublic +certificate: + client_id: 00000000-0000-0000-0000-000000000000 + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 + certificate_path: /path/to/certificate.pem +oauth: + client_id: 00000000-0000-0000-0000-000000000000 + client_secret: Cl1ent$ecret! + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 diff --git a/storage/remote/azuread/testdata/azuread_good_certificate.yaml b/storage/remote/azuread/testdata/azuread_good_certificate.yaml new file mode 100644 index 0000000000..a60a779a65 --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_good_certificate.yaml @@ -0,0 +1,5 @@ +cloud: AzurePublic +certificate: + client_id: 00000000-0000-0000-0000-000000000000 + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 + certificate_path: /path/to/certificate.pem diff --git a/storage/remote/azuread/testdata/azuread_good_certificate_pfx.yaml b/storage/remote/azuread/testdata/azuread_good_certificate_pfx.yaml new file mode 100644 index 0000000000..f90b61ac53 --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_good_certificate_pfx.yaml @@ -0,0 +1,6 @@ +cloud: AzurePublic +certificate: + client_id: 00000000-0000-0000-0000-000000000000 + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 + certificate_path: /path/to/certificate.pfx + certificate_password: P@ssw0rd! diff --git a/storage/remote/azuread/testdata/azuread_good_certificate_with_key.yaml b/storage/remote/azuread/testdata/azuread_good_certificate_with_key.yaml new file mode 100644 index 0000000000..7f7bbec98f --- /dev/null +++ b/storage/remote/azuread/testdata/azuread_good_certificate_with_key.yaml @@ -0,0 +1,7 @@ +cloud: AzurePublic +certificate: + client_id: 00000000-0000-0000-0000-000000000000 + tenant_id: 00000000-a12b-3cd4-e56f-000000000000 + certificate_path: /path/to/certificate.pem + certificate_key_path: /path/to/key.pem + send_certificate_chain: true diff --git a/util/certutil/certutil.go b/util/certutil/certutil.go new file mode 100644 index 0000000000..e428ff4424 --- /dev/null +++ b/util/certutil/certutil.go @@ -0,0 +1,204 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package certutil provides utilities for loading and parsing X.509 certificates +// and private keys from various formats (PEM, PKCS12/PFX). +package certutil + +import ( + "crypto/x509" + "encoding/pem" + "errors" + "os" + + "golang.org/x/crypto/pkcs12" +) + +// CertificateData represents parsed certificate data including certificates and private key. +type CertificateData struct { + // Certificates is a slice of X.509 certificates (leaf certificate first, followed by any intermediates) + Certificates []*x509.Certificate + // PrivateKey is the private key associated with the leaf certificate + PrivateKey any +} + +// ParseCertificateFiles loads and parses certificate(s) and private key from files. +// It supports both PEM and PKCS12/PFX formats. +// +// Parameters: +// - certPath: Path to the certificate file (required) +// - keyPath: Path to a separate private key file (optional, only used for PEM format) +// - password: Password for PKCS12/PFX files (optional, only used for PKCS12 format) +// +// Returns the parsed certificate data or an error. +func ParseCertificateFiles(certPath, keyPath, password string) (*CertificateData, error) { + // Read certificate file + certData, err := os.ReadFile(certPath) + if err != nil { + return nil, errors.New("failed to read certificate file " + certPath + ": " + err.Error()) + } + + // Detect format: check if PEM format first + if isPEMFormat(certData) { + return parsePEMData(certData, keyPath, certPath) + } + + // Must be PKCS12/PFX format + return parsePKCS12Data(certData, password, certPath) +} + +// isPEMFormat checks if data is in PEM format. +func isPEMFormat(data []byte) bool { + block, _ := pem.Decode(data) + return block != nil +} + +// parsePEMData parses PEM-encoded certificate and private key data. +func parsePEMData(certData []byte, keyPath, certPath string) (*CertificateData, error) { + var certs []*x509.Certificate + var privateKey any + + // Parse certificates and keys from certificate file + rest := certData + for { + var block *pem.Block + block, rest = pem.Decode(rest) + if block == nil { + break + } + + switch block.Type { + case "CERTIFICATE": + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse certificate from " + certPath + ": " + err.Error()) + } + certs = append(certs, cert) + case "PRIVATE KEY": + if privateKey == nil { // Only take the first private key + var err error + privateKey, err = x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse PKCS8 private key from " + certPath + ": " + err.Error()) + } + } + case "RSA PRIVATE KEY": + if privateKey == nil { // Only take the first private key + var err error + privateKey, err = x509.ParsePKCS1PrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse RSA private key from " + certPath + ": " + err.Error()) + } + } + case "EC PRIVATE KEY": + if privateKey == nil { // Only take the first private key + var err error + privateKey, err = x509.ParseECPrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse EC private key from " + certPath + ": " + err.Error()) + } + } + } + } + + // If no private key found in main file and separate key file is provided, read it + if privateKey == nil && keyPath != "" { + keyData, err := os.ReadFile(keyPath) + if err != nil { + return nil, errors.New("failed to read private key file " + keyPath + ": " + err.Error()) + } + + block, _ := pem.Decode(keyData) + if block == nil { + return nil, errors.New("failed to decode PEM private key from " + keyPath) + } + + switch block.Type { + case "PRIVATE KEY": + privateKey, err = x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse PKCS8 private key from " + keyPath + ": " + err.Error()) + } + case "RSA PRIVATE KEY": + privateKey, err = x509.ParsePKCS1PrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse RSA private key from " + keyPath + ": " + err.Error()) + } + case "EC PRIVATE KEY": + privateKey, err = x509.ParseECPrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse EC private key from " + keyPath + ": " + err.Error()) + } + default: + return nil, errors.New("unsupported private key type in " + keyPath + ": " + block.Type) + } + } + + return &CertificateData{ + Certificates: certs, + PrivateKey: privateKey, + }, nil +} + +// parsePKCS12Data parses PKCS12/PFX-encoded certificate and private key data. +func parsePKCS12Data(certData []byte, password, certPath string) (*CertificateData, error) { + // Convert PKCS12 to PEM blocks + pemBlocks, err := pkcs12.ToPEM(certData, password) + if err != nil { + return nil, errors.New("failed to parse PFX certificate " + certPath + ": " + err.Error()) + } + + var certs []*x509.Certificate + var privateKey any + + // Parse PEM blocks to extract certificates and private key + for _, block := range pemBlocks { + switch block.Type { + case "CERTIFICATE": + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse certificate from PFX " + certPath + ": " + err.Error()) + } + certs = append(certs, cert) + case "PRIVATE KEY": + // PKCS8 private key + if privateKey == nil { // Only take the first private key + privateKey, err = x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse PKCS8 private key from PFX " + certPath + ": " + err.Error()) + } + } + case "RSA PRIVATE KEY": + // PKCS1 RSA private key + if privateKey == nil { // Only take the first private key + privateKey, err = x509.ParsePKCS1PrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse RSA private key from PFX " + certPath + ": " + err.Error()) + } + } + case "EC PRIVATE KEY": + // EC private key + if privateKey == nil { // Only take the first private key + privateKey, err = x509.ParseECPrivateKey(block.Bytes) + if err != nil { + return nil, errors.New("failed to parse EC private key from PFX " + certPath + ": " + err.Error()) + } + } + } + } + + return &CertificateData{ + Certificates: certs, + PrivateKey: privateKey, + }, nil +} diff --git a/util/certutil/certutil_test.go b/util/certutil/certutil_test.go new file mode 100644 index 0000000000..59c7da465b --- /dev/null +++ b/util/certutil/certutil_test.go @@ -0,0 +1,55 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package certutil + +import ( + "testing" +) + +// TestIsPEMFormat tests the PEM format detection. +func TestIsPEMFormat(t *testing.T) { + tests := []struct { + name string + data []byte + expected bool + }{ + { + name: "valid PEM", + data: []byte(`-----BEGIN CERTIFICATE----- +MIIBkTCB+wIJAKHHCgVZU6pfMA0GCSqGSIb3DQEBCwUAMA0xCzAJBgNVBAYTAlVT +MB4XDTE5MDEwMTAwMDAwMFoXDTIwMDEwMTAwMDAwMFowDTELMAkGA1UEBhMCVVMw +-----END CERTIFICATE-----`), + expected: true, + }, + { + name: "not PEM", + data: []byte("random binary data"), + expected: false, + }, + { + name: "empty", + data: []byte(""), + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isPEMFormat(tt.data) + if result != tt.expected { + t.Errorf("isPEMFormat() = %v, want %v", result, tt.expected) + } + }) + } +} From bb8f8bdbca25047997ab333000c0d7279d4b3421 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Mon, 30 Mar 2026 14:10:49 -0700 Subject: [PATCH 2/4] Replace util/certutil with azidentity.ParseCertificates (#4) Address review feedback: use azidentity.ParseCertificates instead of the custom util/certutil package for certificate parsing. The azidentity package already handles PEM and PKCS#12 formats natively. - Remove util/certutil package entirely - Use azidentity.ParseCertificates in newCertificateTokenCredential - Revert golang.org/x/crypto to indirect dependency Agent-Logs-Url: https://github.com/bragi92/prometheus/sessions/efc3d6c5-9927-4d5b-8aa9-afe94b659c6e Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bragi92 <28612268+bragi92@users.noreply.github.com> --- go.mod | 2 +- storage/remote/azuread/azuread.go | 36 +++--- util/certutil/certutil.go | 204 ------------------------------ util/certutil/certutil_test.go | 55 -------- 4 files changed, 22 insertions(+), 275 deletions(-) delete mode 100644 util/certutil/certutil.go delete mode 100644 util/certutil/certutil_test.go diff --git a/go.mod b/go.mod index 96c97a9a47..89d468e874 100644 --- a/go.mod +++ b/go.mod @@ -239,7 +239,7 @@ require ( go.opentelemetry.io/collector/pipeline v1.51.0 // indirect go.opentelemetry.io/proto/otlp v1.9.0 // indirect go.uber.org/zap v1.27.1 // indirect - golang.org/x/crypto v0.47.0 + golang.org/x/crypto v0.47.0 // indirect golang.org/x/exp v0.0.0-20260112195511-716be5621a96 // indirect golang.org/x/mod v0.32.0 // indirect golang.org/x/net v0.49.0 // indirect diff --git a/storage/remote/azuread/azuread.go b/storage/remote/azuread/azuread.go index 15267486ef..ff8ff6c50f 100644 --- a/storage/remote/azuread/azuread.go +++ b/storage/remote/azuread/azuread.go @@ -17,6 +17,7 @@ import ( "context" "errors" "net/http" + "os" "strings" "sync" "time" @@ -27,8 +28,6 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/google/uuid" "github.com/grafana/regexp" - - "github.com/prometheus/prometheus/util/certutil" ) // Clouds. @@ -434,21 +433,28 @@ func newSDKTokenCredential(clientOpts *azcore.ClientOptions, sdkConfig *SDKConfi // newCertificateTokenCredential returns new certificate-based token credential. func newCertificateTokenCredential(clientOpts *azcore.ClientOptions, certConfig *CertificateConfig) (azcore.TokenCredential, error) { - // Use certutil to parse the certificate files - certData, err := certutil.ParseCertificateFiles( - certConfig.CertificatePath, - certConfig.CertificateKeyPath, - certConfig.CertificatePassword, - ) + certData, err := os.ReadFile(certConfig.CertificatePath) if err != nil { - return nil, err + return nil, errors.New("failed to read certificate file " + certConfig.CertificatePath + ": " + err.Error()) } - if len(certData.Certificates) == 0 { - return nil, errors.New("no certificates found in certificate file") + // If a separate key file is provided, append it to the cert data so ParseCertificates can find the private key. + if certConfig.CertificateKeyPath != "" { + keyData, err := os.ReadFile(certConfig.CertificateKeyPath) + if err != nil { + return nil, errors.New("failed to read private key file " + certConfig.CertificateKeyPath + ": " + err.Error()) + } + certData = append(append(certData, '\n'), keyData...) } - if certData.PrivateKey == nil { - return nil, errors.New("no private key found") + + var password []byte + if certConfig.CertificatePassword != "" { + password = []byte(certConfig.CertificatePassword) + } + + certs, key, err := azidentity.ParseCertificates(certData, password) + if err != nil { + return nil, errors.New("failed to parse certificate data: " + err.Error()) } opts := &azidentity.ClientCertificateCredentialOptions{ @@ -459,8 +465,8 @@ func newCertificateTokenCredential(clientOpts *azcore.ClientOptions, certConfig return azidentity.NewClientCertificateCredential( certConfig.TenantID, certConfig.ClientID, - certData.Certificates, - certData.PrivateKey, + certs, + key, opts, ) } diff --git a/util/certutil/certutil.go b/util/certutil/certutil.go deleted file mode 100644 index e428ff4424..0000000000 --- a/util/certutil/certutil.go +++ /dev/null @@ -1,204 +0,0 @@ -// Copyright The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package certutil provides utilities for loading and parsing X.509 certificates -// and private keys from various formats (PEM, PKCS12/PFX). -package certutil - -import ( - "crypto/x509" - "encoding/pem" - "errors" - "os" - - "golang.org/x/crypto/pkcs12" -) - -// CertificateData represents parsed certificate data including certificates and private key. -type CertificateData struct { - // Certificates is a slice of X.509 certificates (leaf certificate first, followed by any intermediates) - Certificates []*x509.Certificate - // PrivateKey is the private key associated with the leaf certificate - PrivateKey any -} - -// ParseCertificateFiles loads and parses certificate(s) and private key from files. -// It supports both PEM and PKCS12/PFX formats. -// -// Parameters: -// - certPath: Path to the certificate file (required) -// - keyPath: Path to a separate private key file (optional, only used for PEM format) -// - password: Password for PKCS12/PFX files (optional, only used for PKCS12 format) -// -// Returns the parsed certificate data or an error. -func ParseCertificateFiles(certPath, keyPath, password string) (*CertificateData, error) { - // Read certificate file - certData, err := os.ReadFile(certPath) - if err != nil { - return nil, errors.New("failed to read certificate file " + certPath + ": " + err.Error()) - } - - // Detect format: check if PEM format first - if isPEMFormat(certData) { - return parsePEMData(certData, keyPath, certPath) - } - - // Must be PKCS12/PFX format - return parsePKCS12Data(certData, password, certPath) -} - -// isPEMFormat checks if data is in PEM format. -func isPEMFormat(data []byte) bool { - block, _ := pem.Decode(data) - return block != nil -} - -// parsePEMData parses PEM-encoded certificate and private key data. -func parsePEMData(certData []byte, keyPath, certPath string) (*CertificateData, error) { - var certs []*x509.Certificate - var privateKey any - - // Parse certificates and keys from certificate file - rest := certData - for { - var block *pem.Block - block, rest = pem.Decode(rest) - if block == nil { - break - } - - switch block.Type { - case "CERTIFICATE": - cert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse certificate from " + certPath + ": " + err.Error()) - } - certs = append(certs, cert) - case "PRIVATE KEY": - if privateKey == nil { // Only take the first private key - var err error - privateKey, err = x509.ParsePKCS8PrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse PKCS8 private key from " + certPath + ": " + err.Error()) - } - } - case "RSA PRIVATE KEY": - if privateKey == nil { // Only take the first private key - var err error - privateKey, err = x509.ParsePKCS1PrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse RSA private key from " + certPath + ": " + err.Error()) - } - } - case "EC PRIVATE KEY": - if privateKey == nil { // Only take the first private key - var err error - privateKey, err = x509.ParseECPrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse EC private key from " + certPath + ": " + err.Error()) - } - } - } - } - - // If no private key found in main file and separate key file is provided, read it - if privateKey == nil && keyPath != "" { - keyData, err := os.ReadFile(keyPath) - if err != nil { - return nil, errors.New("failed to read private key file " + keyPath + ": " + err.Error()) - } - - block, _ := pem.Decode(keyData) - if block == nil { - return nil, errors.New("failed to decode PEM private key from " + keyPath) - } - - switch block.Type { - case "PRIVATE KEY": - privateKey, err = x509.ParsePKCS8PrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse PKCS8 private key from " + keyPath + ": " + err.Error()) - } - case "RSA PRIVATE KEY": - privateKey, err = x509.ParsePKCS1PrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse RSA private key from " + keyPath + ": " + err.Error()) - } - case "EC PRIVATE KEY": - privateKey, err = x509.ParseECPrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse EC private key from " + keyPath + ": " + err.Error()) - } - default: - return nil, errors.New("unsupported private key type in " + keyPath + ": " + block.Type) - } - } - - return &CertificateData{ - Certificates: certs, - PrivateKey: privateKey, - }, nil -} - -// parsePKCS12Data parses PKCS12/PFX-encoded certificate and private key data. -func parsePKCS12Data(certData []byte, password, certPath string) (*CertificateData, error) { - // Convert PKCS12 to PEM blocks - pemBlocks, err := pkcs12.ToPEM(certData, password) - if err != nil { - return nil, errors.New("failed to parse PFX certificate " + certPath + ": " + err.Error()) - } - - var certs []*x509.Certificate - var privateKey any - - // Parse PEM blocks to extract certificates and private key - for _, block := range pemBlocks { - switch block.Type { - case "CERTIFICATE": - cert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse certificate from PFX " + certPath + ": " + err.Error()) - } - certs = append(certs, cert) - case "PRIVATE KEY": - // PKCS8 private key - if privateKey == nil { // Only take the first private key - privateKey, err = x509.ParsePKCS8PrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse PKCS8 private key from PFX " + certPath + ": " + err.Error()) - } - } - case "RSA PRIVATE KEY": - // PKCS1 RSA private key - if privateKey == nil { // Only take the first private key - privateKey, err = x509.ParsePKCS1PrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse RSA private key from PFX " + certPath + ": " + err.Error()) - } - } - case "EC PRIVATE KEY": - // EC private key - if privateKey == nil { // Only take the first private key - privateKey, err = x509.ParseECPrivateKey(block.Bytes) - if err != nil { - return nil, errors.New("failed to parse EC private key from PFX " + certPath + ": " + err.Error()) - } - } - } - } - - return &CertificateData{ - Certificates: certs, - PrivateKey: privateKey, - }, nil -} diff --git a/util/certutil/certutil_test.go b/util/certutil/certutil_test.go deleted file mode 100644 index 59c7da465b..0000000000 --- a/util/certutil/certutil_test.go +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright The Prometheus Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package certutil - -import ( - "testing" -) - -// TestIsPEMFormat tests the PEM format detection. -func TestIsPEMFormat(t *testing.T) { - tests := []struct { - name string - data []byte - expected bool - }{ - { - name: "valid PEM", - data: []byte(`-----BEGIN CERTIFICATE----- -MIIBkTCB+wIJAKHHCgVZU6pfMA0GCSqGSIb3DQEBCwUAMA0xCzAJBgNVBAYTAlVT -MB4XDTE5MDEwMTAwMDAwMFoXDTIwMDEwMTAwMDAwMFowDTELMAkGA1UEBhMCVVMw ------END CERTIFICATE-----`), - expected: true, - }, - { - name: "not PEM", - data: []byte("random binary data"), - expected: false, - }, - { - name: "empty", - data: []byte(""), - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := isPEMFormat(tt.data) - if result != tt.expected { - t.Errorf("isPEMFormat() = %v, want %v", result, tt.expected) - } - }) - } -} From 933dd26f358544447950cd69b52436fd5538ade2 Mon Sep 17 00:00:00 2001 From: bragi92 Date: Mon, 1 Jun 2026 11:27:10 -0700 Subject: [PATCH 3/4] Update storage/remote/azuread/azuread.go Co-authored-by: Bartlomiej Plotka Signed-off-by: bragi92 --- storage/remote/azuread/azuread.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/remote/azuread/azuread.go b/storage/remote/azuread/azuread.go index ff8ff6c50f..052dffe7f8 100644 --- a/storage/remote/azuread/azuread.go +++ b/storage/remote/azuread/azuread.go @@ -105,7 +105,7 @@ type CertificateConfig struct { // CertificatePassword is the password for the certificate file (for PFX files). // This is optional and only needed if the certificate file is password-protected. - CertificatePassword string `yaml:"certificate_password,omitempty"` + CertificatePassword config_util.Secret `yaml:"certificate_password,omitempty"` // SendCertificateChain controls whether to include x5c header in assertion to support // subject name / issuer-based authentication. From f972cac551d8c7ebad66c25f30fdf17c504d050b Mon Sep 17 00:00:00 2001 From: "Kaveesh Dubey (from Dev Box)" Date: Mon, 1 Jun 2026 21:36:19 -0700 Subject: [PATCH 4/4] azuread: import config_util used by CertificatePassword Secret type The previous commit changed CertificatePassword from string to config_util.Secret but did not add the corresponding import. The CI build for this PR alone passed only because GitHub builds the merge of the PR with upstream main, which already imports config_util (introduced in upstream commit 5ccebcdb3 for ClientSecret). Add the import so the PR's azuread.go is self-consistent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Kaveesh Dubey (from Dev Box) --- storage/remote/azuread/azuread.go | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/remote/azuread/azuread.go b/storage/remote/azuread/azuread.go index 052dffe7f8..1131ab50ed 100644 --- a/storage/remote/azuread/azuread.go +++ b/storage/remote/azuread/azuread.go @@ -28,6 +28,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/google/uuid" "github.com/grafana/regexp" + config_util "github.com/prometheus/common/config" ) // Clouds.