Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows users to configure to include system certs pool in the request #7774

Closed
splunkericl opened this issue May 25, 2023 · 0 comments · Fixed by #9142
Closed

Allows users to configure to include system certs pool in the request #7774

splunkericl opened this issue May 25, 2023 · 0 comments · Fixed by #9142

Comments

@splunkericl
Copy link
Contributor

Is your feature request related to a problem? Please describe.
As a user of TLSSetting, I want an option to allow appending ca certs on top of the system certs pool. Today, if the user specify any CA file for TLSSetting it will only use that.

The background is the the application our exporter calls can have either cacert that is well known(e.g: Digi cert that is available in os) or private(e.g: Splunk CMP stack). We want the exporter to be able to communicate to both.

Describe the solution you'd like
Add a new config option in TLSSetting

type TLSSetting struct {
   UseSystemCACerts bool
}

when loading ca cert:

func (c TLSSetting) loadCert(caPath string) (*x509.CertPool, error) {
	caPEM, err := os.ReadFile(filepath.Clean(caPath))
	if err != nil {
		return nil, fmt.Errorf("failed to load CA %s: %w", caPath, err)
	}

	var certPool *x509.CertPool
	if c.UseSystemCACerts {
		certPool, _ = x509.SystemCertPool()
	}
	if certPool == nil {
		certPool = x509.NewCertPool()
	}
	if !certPool.AppendCertsFromPEM(caPEM) {
		return nil, fmt.Errorf("failed to parse CA %s", caPath)
	}
	return certPool, nil
}

Describe alternatives you've considered
An alternative is to always load system certs in the ca pool. However, this would increase HTTPs traffic payload for everyone

Additional context
N/A

dmitryax pushed a commit that referenced this issue Feb 29, 2024
**Description:**
Add `include_system_ca_certs_pool` to configtls, allowing to load system
certs and additional custom certs.

**Link to tracking Issue:**
Fixes #7774
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants