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

[Proposal] Add in memory certificate support to TLSSetting #7538

Closed
erikbaranowski opened this issue Apr 13, 2023 · 2 comments
Closed

[Proposal] Add in memory certificate support to TLSSetting #7538

erikbaranowski opened this issue Apr 13, 2023 · 2 comments

Comments

@erikbaranowski
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently, the OpenTelemetry Collector supports TLS settings read from filepaths. This requires that the certificates exist on a file system and does not work with in memory certificates.

Describe the solution you'd like
Update the API for TLSSettings in configtls.go to fully support in memory certificates. This in memory data will be validated so that it doesn't exist in addition to a file path for the same data. For example, CertFile and CertPem cannot both be set but you can do CertFile with KeyPem. In addition, the existing file reload functionality doesn't take action for any in memory certificate.

Proposed API

// TLSSetting exposes the common client and server TLS configurations.
// Note: Since there isn't anything specific to a server connection. Components
// with server connections should use TLSSetting.
type TLSSetting struct {
	// Path to the CA cert. For a client this verifies the server certificate.
	// For a server this verifies client certificates. If empty uses system root CA.
	// (optional)
	CAFile string `mapstructure:"ca_file"`

	// In memory PEM encoded cert. (optional)
	CAPem []byte `mapstructure:"ca_pem"`

	// Path to the TLS cert to use for TLS required connections. (optional)
	CertFile string `mapstructure:"cert_file"`

	// In memory PEM encoded TLS cert to use for TLS required connections. (optional)
	CertPem []byte `mapstructure:"cert_pem"`

	// Path to the TLS key to use for TLS required connections. (optional)
	KeyFile string `mapstructure:"key_file"`

	// In memory PEM encoded TLS key to use for TLS required connections. (optional)
	KeyPem []byte `mapstructure:"key_pem"`

	// MinVersion sets the minimum TLS version that is acceptable.
	// If not set, TLS 1.2 will be used. (optional)
	MinVersion string `mapstructure:"min_version"`

	// MaxVersion sets the maximum TLS version that is acceptable.
	// If not set, refer to crypto/tls for defaults. (optional)
	MaxVersion string `mapstructure:"max_version"`

	// ReloadInterval specifies the duration after which the certificate will be reloaded
	// If not set, it will never be reloaded (optional)
	ReloadInterval time.Duration `mapstructure:"reload_interval"`
}

Additional context
If this proposal is accepted, I will submit a PR with the proposed changes. If you'd like to review a prototype, this proposal has been implemented on a fork here.

@atoulme
Copy link
Contributor

atoulme commented Apr 17, 2023

This is a duplicate of #7313. Please review and consider closing in favor of continuing the work under the other issue.

@erikbaranowski
Copy link
Contributor Author

Thank you for the redirect, I will close and comment there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants