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

[feature] Revisit TLS configurations #831

Open
FZambia opened this issue Jun 16, 2024 · 1 comment
Open

[feature] Revisit TLS configurations #831

FZambia opened this issue Jun 16, 2024 · 1 comment

Comments

@FZambia
Copy link
Member

FZambia commented Jun 16, 2024

Is your feature request related to a problem? Please describe.

Centrifugo has several places where it allows configuring TLS - in HTTP server, Redis connection, Kafka consumer, GRPC server and client. In the source code though we now use slightly different approach to configure TLS in various parts. I'd like to refactor the configuration a bit to use a single approach to naming and also tweak some TLS related option names. Because currently they are a bit confusing. For example, tls_cert option name should become tls_cert_pem_file. Another idea is to make TLS objects nested to detach TLS configuration objects from config key prefixes, like this:

{
  "tls": {
     "enabled": true,
     "cert_pem_file": "/tmp/centrifugo.crt",
     "key_pem_file": "/tmp/centrifugo.key"
  },
  "redis_tls": {
     "enabled": true,
     "cert_pem_file": "/tmp/redis.crt",
     "key_pem_file": "/tmp/redis.key",
     "root_ca_pem_file": "/tmp/redis_ca.pem"
  }
}

Revisited TLS configuration struct may look like this:

// TLSConfig is a common configuration for TLS.
type TLSConfig struct {
	// Enabled tells Centrifugo to enable TLS configuration.
	Enabled bool `mapstructure:"enabled" json:"enabled"`

	// Configure certificates to present to the other side of the connection.
	CertPem     string `mapstructure:"cert_pem" json:"cert_pem"`
	CertPemFile string `mapstructure:"cert_pem_file" json:"cert_pem_file"`
	KeyPem      string `mapstructure:"key_pem" json:"key_pem"`
	KeyPemFile  string `mapstructure:"key_pem_file" json:"key_pem_file"`

	// Configure the set of root certificate authorities that clients use when verifying
	// server certificates.
	RootCAPem     string `mapstructure:"root_ca_pem" json:"root_ca_pem"`
	RootCAPemFile string `mapstructure:"root_ca_pem_file" json:"root_ca_pem_file"`

	// Configure the set of root certificate authorities that servers use to verify
	// a client certificate.
	ClientCAPem     string `mapstructure:"client_ca_pem" json:"client_ca_pem"`
	ClientCAPemFile string `mapstructure:"client_ca_pem_file" json:"client_ca_pem_file"`

	InsecureSkipVerify bool   `mapstructure:"insecure_skip_verify" json:"insecure_skip_verify"`
	ServerName         string `mapstructure:"server_name" json:"server_name"`
}

Also, it seems tls options may be removed from command line flags of Centrifugo.

Finally, maybe we should natively support Base64 encoded PEM too. Like:

CertPemB64     string `mapstructure:"cert_pem_b64" json:"cert_pem_b64"`

Describe the solution you'd like.

What would the feature look like? How would it work? How would it change the API?

Look at all places where TLS may be configured and use revisited common configuration strategy. This should make configuration cleaner overall.

@swagftw
Copy link

swagftw commented Jun 28, 2024

For any one looking for insecureSkipVerify flag, this issue will add it.

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