Fix mutual TLS handling and improve documentation

Co-authored-by: otoolep <536312+otoolep@users.noreply.github.com>

cmd/rqlited/config_flags.go

@@ -37,7 +37,7 @@ type Config struct {
 	HTTPx509Key string
 	// Enable mutual TLS for HTTPS
 	HTTPVerifyClient bool
-	// Path to X.509 CA certificate for node-to-node encryption
+	// Path to X.509 CA certificate for node-to-node communication (used exclusively for certificate validation)
 	NodeX509CACert string
 	// Path to X.509 certificate for node-to-node mutual authentication and encryption
 	NodeX509Cert string
@@ -45,7 +45,7 @@ type Config struct {
 	NodeX509Key string
 	// Skip verification of any node-node certificate
 	NoNodeVerify bool
-	// Enable mutual TLS for node-to-node communication
+	// Enable mutual TLS for node-to-node communication (requires CA cert for client verification)
 	NodeVerifyClient bool
 	// Hostname to verify on certificate returned by a node
 	NodeVerifyServerName string
@@ -147,11 +147,11 @@ func Forge(arguments []string) (*flag.FlagSet, *Config, error) {
 	fs.StringVar(&config.HTTPx509Cert, "http-cert", "", "Path to HTTPS X.509 certificate")
 	fs.StringVar(&config.HTTPx509Key, "http-key", "", "Path to HTTPS X.509 private key")
 	fs.BoolVar(&config.HTTPVerifyClient, "http-verify-client", false, "Enable mutual TLS for HTTPS")
-	fs.StringVar(&config.NodeX509CACert, "node-ca-cert", "", "Path to X.509 CA certificate for node-to-node encryption")
+	fs.StringVar(&config.NodeX509CACert, "node-ca-cert", "", "Path to X.509 CA certificate for node-to-node communication (used exclusively for certificate validation)")
 	fs.StringVar(&config.NodeX509Cert, "node-cert", "", "Path to X.509 certificate for node-to-node mutual authentication and encryption")
 	fs.StringVar(&config.NodeX509Key, "node-key", "", "Path to X.509 private key for node-to-node mutual authentication and encryption")
 	fs.BoolVar(&config.NoNodeVerify, "node-no-verify", false, "Skip verification of any node-node certificate")
-	fs.BoolVar(&config.NodeVerifyClient, "node-verify-client", false, "Enable mutual TLS for node-to-node communication")
+	fs.BoolVar(&config.NodeVerifyClient, "node-verify-client", false, "Enable mutual TLS for node-to-node communication (requires CA cert for client verification)")
 	fs.StringVar(&config.NodeVerifyServerName, "node-verify-server-name", "", "Hostname to verify on certificate returned by a node")
 	fs.StringVar(&config.NodeID, "node-id", "", "Unique ID for node. If not set, set to advertised Raft address")
 	fs.StringVar(&config.RaftAddr, "raft-addr", "localhost:4002", "Raft communication bind address")

cmd/rqlited/main.go

@@ -414,7 +414,11 @@ func startNodeMux(cfg *Config, ln net.Listener) (*tcp.Mux, error) {
 		b.WriteString(fmt.Sprintf("enabling node-to-node encryption with cert: %s, key: %s",
 			cfg.NodeX509Cert, cfg.NodeX509Key))
 		if cfg.NodeX509CACert != "" {
-			b.WriteString(fmt.Sprintf(", CA cert %s", cfg.NodeX509CACert))
+			if cfg.NodeVerifyClient {
+				b.WriteString(fmt.Sprintf(", CA cert %s (for client verification)", cfg.NodeX509CACert))
+			} else {
+				b.WriteString(fmt.Sprintf(", CA cert %s (for server verification)", cfg.NodeX509CACert))
+			}
 		}
 		if cfg.NodeVerifyClient {
 			b.WriteString(", mutual TLS enabled")

internal/rtls/config.go

@@ -25,10 +25,13 @@ const (
 // CreateClientConfig creates a new tls.Config for use by a client. The certFile and keyFile
 // parameters are the paths to the client's certificate and key files, which will be used to
 // authenticate the client to the server if mutual TLS is active. The caCertFile parameter
-// is the path to the CA certificate file, which the client will use to verify any certificate
-// presented by the server. serverName can also be set, informing the client which hostname
-// should appear in the returned certificate. If noverify is true, the client will not verify
-// the server's certificate.
+// is the path to the CA certificate file, which the client will use to verify the server's
+// certificate during TLS handshake. serverName can also be set, informing the client which
+// hostname should appear in the server's certificate. If noverify is true, the client will
+// not verify the server's certificate.
+//
+// Note: The caCertFile is used for validating the server's certificate during TLS handshake.
+// This is separate from mutual TLS, where the server validates the client's certificate.
 func CreateClientConfig(certFile, keyFile, caCertFile, serverName string, noverify bool) (*tls.Config, error) {
 	var err error
 
@@ -51,9 +54,11 @@ func CreateClientConfig(certFile, keyFile, caCertFile, serverName string, noveri
 // CreateClientConfigWithFunc creates a new tls.Config for use by a client. The certFunc
 // parameter is a function that returns the client's certificate and key. The caCertFile
 // parameter is the path to the CA certificate file, which the client will use to verify
-// any certificate presented by the server. serverName can also be set, informing the client
-// which hostname should appear in the returned certificate. If noverify is true, the client
-// will not verify the server's certificate.
+// the server's certificate during TLS handshake. serverName can also be set, informing the
+// client which hostname should appear in the server's certificate. If noverify is true,
+// the client will not verify the server's certificate.
+//
+// Note: The caCertFile is used for validating the server's certificate during TLS handshake.
 func CreateClientConfigWithFunc(certFunc func() (*tls.Certificate, error), caCertFile, serverName string, noverify bool) (*tls.Config, error) {
 	config := createBaseTLSConfig(serverName, noverify)
 	if certFunc != nil {
@@ -73,8 +78,12 @@ func CreateClientConfigWithFunc(certFunc func() (*tls.Certificate, error), caCer
 // parameters are the paths to the server's certificate and key files, which will be used to
 // authenticate the server to the client. The caCertFile parameter is the path to the CA
 // certificate file, which the server will use to verify any certificate presented by the
-// client. If mtls is MTLSStateEnabled, the server will require the client to present a
-// valid certificate.
+// client during mutual TLS authentication. If mtls is MTLSStateEnabled, the server will
+// require clients to present a valid certificate that can be verified using the CA certificate.
+//
+// Note: The caCertFile is used exclusively for validating client certificates during mutual TLS.
+// It is not used for server certificate validation - that is handled by clients using their
+// own CA certificate configuration.
 func CreateServerConfig(certFile, keyFile, caCertFile string, mtls MTLSState) (*tls.Config, error) {
 	var err error
 
@@ -96,8 +105,11 @@ func CreateServerConfig(certFile, keyFile, caCertFile string, mtls MTLSState) (*
 // CreateServerConfigWithFunc creates a new tls.Config for use by a server. The certFunc
 // parameter is a function that returns the server's certificate and key. The caCertFile
 // parameter is the path to the CA certificate file, which the server will use to verify
-// any certificate presented by the client. If mtls is MTLSStateEnabled, the server will
-// require the client to present a valid certificate.
+// any certificate presented by the client during mutual TLS authentication. If mtls is
+// MTLSStateEnabled, the server will require clients to present a valid certificate that
+// can be verified using the CA certificate.
+//
+// Note: The caCertFile is used exclusively for validating client certificates during mutual TLS.
 func CreateServerConfigWithFunc(certFunc func() (*tls.Certificate, error), caCertFile string, mtls MTLSState) (*tls.Config, error) {
 	config := createBaseTLSConfig(NoServerName, false)
 	config.GetCertificate = func(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {

tcp/mux.go

@@ -110,11 +110,14 @@ func NewMux(ln net.Listener, adv net.Addr) (*Mux, error) {
 // then the server will not verify the client's certificate. If mutual is true,
 // then the server will require the client to present a trusted certificate.
 func NewTLSMux(ln net.Listener, adv net.Addr, cert, key, caCert string, insecure, mutual bool) (*Mux, error) {
-	return newTLSMux(ln, adv, cert, key, caCert, false)
+	return newTLSMux(ln, adv, cert, key, caCert, mutual)
 }
 
 // NewMutualTLSMux returns a new instance of Mux for ln, and encrypts all traffic
-// using TLS. The server will also verify the client's certificate.
+// using TLS with mutual authentication enabled. The server will require the client
+// to present a certificate that can be validated using the provided CA certificate.
+// The caCert parameter specifies the CA certificate used to validate client certificates
+// for mutual TLS authentication. If adv is nil, then the addr of ln is used.
 func NewMutualTLSMux(ln net.Listener, adv net.Addr, cert, key, caCert string) (*Mux, error) {
 	return newTLSMux(ln, adv, cert, key, caCert, true)
 }

tcp/mux_test.go

@@ -3,6 +3,7 @@ package tcp
 import (
 	"bytes"
 	"crypto/tls"
+	"crypto/x509/pkix"
 	"io"
 	"log"
 	"net"
@@ -13,6 +14,7 @@ import (
 	"testing/quick"
 	"time"
 
+	"github.com/rqlite/rqlite/v8/internal/rtls"
 	"github.com/rqlite/rqlite/v8/testdata/x509"
 )
 
@@ -263,3 +265,83 @@ func mustRename(new, old string) {
 		panic(err)
 	}
 }
+
+func mustWriteTempFile(t *testing.T, b []byte) string {
+	f, err := os.CreateTemp(t.TempDir(), "rqlite-test")
+	if err != nil {
+		panic("failed to create temp file")
+	}
+	defer f.Close()
+	if _, err := f.Write(b); err != nil {
+		panic("failed to write to temp file")
+	}
+	return f.Name()
+}
+
+// Test_NewTLSMux_MutualTLS_Enabled verifies that the mutual parameter is properly
+// passed through and that mutual TLS is actually enabled when requested.
+func Test_NewTLSMux_MutualTLS_Enabled(t *testing.T) {
+	ln := mustTCPListener("127.0.0.1:0")
+	defer ln.Close()
+
+	// Create cert and key files using the x509 helper
+	certFile := x509.CertExampleDotComFile("")
+	defer os.Remove(certFile)
+	keyFile := x509.KeyExampleDotComFile("")
+	defer os.Remove(keyFile)
+
+	// Generate a CA cert for testing
+	caCertPEM, _, err := rtls.GenerateCert(pkix.Name{CommonName: "rqlite-ca"}, 365*24*time.Hour, 2048, nil, nil)
+	if err != nil {
+		t.Fatalf("failed to generate CA cert: %v", err)
+	}
+	caCertFile := mustWriteTempFile(t, caCertPEM)
+	defer os.Remove(caCertFile)
+
+	// Test with mutual=true
+	mux, err := NewTLSMux(ln, nil, certFile, keyFile, caCertFile, false, true)
+	if err != nil {
+		t.Fatalf("failed to create TLS mux with mutual=true: %s", err.Error())
+	}
+	defer mux.Close()
+
+	// Check that the TLS config has mutual TLS enabled
+	if mux.tlsConfig.ClientAuth != tls.RequireAndVerifyClientCert {
+		t.Fatalf("expected ClientAuth to be RequireAndVerifyClientCert with mutual=true, got %v", mux.tlsConfig.ClientAuth)
+	}
+	if mux.tlsConfig.ClientCAs == nil {
+		t.Fatalf("expected ClientCAs to be set with mutual=true and caCert provided")
+	}
+}
+
+// Test_NewTLSMux_MutualTLS_Disabled verifies that mutual TLS is disabled when not requested.
+func Test_NewTLSMux_MutualTLS_Disabled(t *testing.T) {
+	ln := mustTCPListener("127.0.0.1:0")
+	defer ln.Close()
+
+	// Create cert and key files using the x509 helper
+	certFile := x509.CertExampleDotComFile("")
+	defer os.Remove(certFile)
+	keyFile := x509.KeyExampleDotComFile("")
+	defer os.Remove(keyFile)
+
+	// Generate a CA cert for testing
+	caCertPEM, _, err := rtls.GenerateCert(pkix.Name{CommonName: "rqlite-ca"}, 365*24*time.Hour, 2048, nil, nil)
+	if err != nil {
+		t.Fatalf("failed to generate CA cert: %v", err)
+	}
+	caCertFile := mustWriteTempFile(t, caCertPEM)
+	defer os.Remove(caCertFile)
+
+	// Test with mutual=false
+	mux, err := NewTLSMux(ln, nil, certFile, keyFile, caCertFile, false, false)
+	if err != nil {
+		t.Fatalf("failed to create TLS mux with mutual=false: %s", err.Error())
+	}
+	defer mux.Close()
+
+	// Check that the TLS config has mutual TLS disabled
+	if mux.tlsConfig.ClientAuth != tls.NoClientCert {
+		t.Fatalf("expected ClientAuth to be NoClientCert with mutual=false, got %v", mux.tlsConfig.ClientAuth)
+	}
+}