From 0fa0a56a5a0c490901d5216d4d154d0ec5355a8c Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 28 Apr 2026 01:29:41 -0700 Subject: [PATCH] filer(mysql): TLS hostname/SNI knobs + MariaDB upsert documentation (#9260) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(filer/mysql): set tls.Config per-instance via Connector instead of global registry Replace the use of `mysql.RegisterTLSConfig("mysql-tls", ...)` and the `&tls=mysql-tls` DSN suffix with a per-instance setup that assigns the `*tls.Config` directly to `mysql.Config.TLS` and opens the database via `mysql.NewConnector` + `sql.OpenDB`. The driver's TLS-config registry is process-wide; if a second `MysqlStore` were ever initialized with different TLS settings (e.g., a filer plus a separately configured store) the second registration would silently overwrite the first. The connector pattern keeps the TLS configuration attached to the connector and avoids that global side effect. Behavior is otherwise unchanged: TLS is enabled when `enable_tls=true`, the same `ca_crt`/`client_crt`/`client_key` knobs are honored, and the TLS minimum version remains 1.2. * filer(mysql): use system root CAs when ca_crt is empty Previously, enabling `enable_tls=true` without setting `ca_crt` returned an unhelpful empty-path read error. Many managed MySQL/MariaDB providers serve certificates that chain to a public CA already in the host's trust store, so requiring an explicit CA bundle adds friction with no security benefit. Leave `RootCAs` unset when `ca_crt` is empty so Go's `tls.Config` falls back to the system trust store, matching the standard behavior of `mysql --ssl`. Existing setups with `ca_crt` configured are unaffected. Also wraps the CA read/parse errors with the file path for easier diagnosis. * filer(mysql): fail loudly when client_crt / client_key are unreadable The previous implementation called `tls.LoadX509KeyPair` and silently discarded any error, falling back to a non-mTLS connection. A typo or permissions problem in `client_crt` / `client_key` therefore appeared as a confusing server-side handshake error rather than as a config error, because the server was expecting a client cert that the filer never sent. Treat the keypair as required when either path is set, and surface the underlying load error with both filenames so the misconfiguration is obvious. The default (both paths empty) is unchanged: no client cert is sent. * filer(mysql): add tls_insecure_skip_verify and tls_server_name knobs When the filer connects to a MySQL/MariaDB cluster whose server certificate's SAN does not match the connection address (common with internal load balancers, IP-only connection strings, or self-signed cluster certs), the TLS handshake fails with `x509: certificate is valid for X, not Y`. There was previously no way to fix this short of reissuing the cert. Expose two new optional knobs on `[mysql]`: - `tls_server_name` overrides the SNI / cert hostname used for verification — the standard fix when the cert SAN is correct but the connection address is not. - `tls_insecure_skip_verify` disables verification entirely as an escape hatch for testing or for clusters with no usable SAN. Both default to off, so existing configurations continue to verify the server certificate against the connection address as before. * docs(scaffold/filer.toml): document mysql TLS knobs and MariaDB upsert override - Document the new `tls_insecure_skip_verify` and `tls_server_name` options. - Update the `ca_crt` comment to reflect that it is optional and that the system trust store is used when the path is empty (matches the runtime behavior in mysql_store.go). - Reword the client cert comments to make the mTLS pairing requirement explicit (both `client_crt` and `client_key` must be set together). - Add a commented-out MariaDB / MySQL 5.7 alternative for `upsertQuery`, noting that the default (`AS new` row alias) requires MySQL 8.0.19+. * filer(mysql): drop redundant blank import of go-sql-driver/mysql The package was imported twice: once with the `mysql` alias (used for `mysql.MySQLError`, `mysql.Config`, `mysql.NewConnector`, etc.) and once as `_` to register the driver. The named import already triggers `init()` and registers the driver, so the blank import is dead weight. --- weed/command/scaffold/filer.toml | 13 +++- weed/filer/mysql/mysql_store.go | 101 ++++++++++++++++++------------- 2 files changed, 69 insertions(+), 45 deletions(-) diff --git a/weed/command/scaffold/filer.toml b/weed/command/scaffold/filer.toml index d65e97207..073acba83 100644 --- a/weed/command/scaffold/filer.toml +++ b/weed/command/scaffold/filer.toml @@ -58,9 +58,11 @@ enabled = false # [username[:password]@][protocol[(address)]]/dbname[?param1=value1&...¶mN=valueN] dsn = "root@tcp(localhost:3306)/seaweedfs?collation=utf8mb4_bin" enable_tls = false -ca_crt = "" # ca.crt dir when enable_tls set true -client_crt = "" # mysql client.crt dir when enable_tls set true -client_key = "" # mysql client.key dir when enable_tls set true +ca_crt = "" # path to CA cert (PEM) — optional; if empty, the system trust store is used +client_crt = "" # path to client cert (PEM) — only when server requires mTLS; must be set together with client_key +client_key = "" # path to client key (PEM) — only when server requires mTLS; must be set together with client_crt +tls_insecure_skip_verify = false # skip server cert verification (use only for testing or with self-signed certs) +tls_server_name = "" # override SNI / cert hostname; leave empty to use `hostname` above hostname = "localhost" port = 3306 username = "root" @@ -72,6 +74,11 @@ connection_max_lifetime_seconds = 300 interpolateParams = false # if insert/upsert failing, you can disable upsert or update query syntax to match your RDBMS syntax: enableUpsert = true +# Default uses the row-alias form (`AS new`) added in MySQL 8.0.19 and is the +# preferred syntax there. For MariaDB (any version) and MySQL 5.7, override +# with the form below — MariaDB does not support row aliases in +# INSERT ... ON DUPLICATE KEY UPDATE: +# upsertQuery = """INSERT INTO `%s` (`dirhash`,`name`,`directory`,`meta`) VALUES (?,?,?,?) ON DUPLICATE KEY UPDATE `meta` = VALUES(`meta`)""" upsertQuery = """INSERT INTO `%s` (`dirhash`,`name`,`directory`,`meta`) VALUES (?,?,?,?) AS `new` ON DUPLICATE KEY UPDATE `meta` = `new`.`meta`""" [mysql2] # or memsql, tidb diff --git a/weed/filer/mysql/mysql_store.go b/weed/filer/mysql/mysql_store.go index a4f8a3ce2..cb96dea1d 100644 --- a/weed/filer/mysql/mysql_store.go +++ b/weed/filer/mysql/mysql_store.go @@ -11,15 +11,13 @@ import ( "time" "github.com/go-sql-driver/mysql" - _ "github.com/go-sql-driver/mysql" "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/filer/abstract_sql" "github.com/seaweedfs/seaweedfs/weed/util" ) const ( - CONNECTION_URL_PATTERN = "%s:%s@tcp(%s:%d)/%s?collation=utf8mb4_bin" - CONNECTION_TLS_URL_PATTERN = "%s:%s@tcp(%s:%d)/%s?collation=utf8mb4_bin&tls=mysql-tls" + CONNECTION_URL_PATTERN = "%s:%s@tcp(%s:%d)/%s?collation=utf8mb4_bin" ) func init() { @@ -52,11 +50,14 @@ func (store *MysqlStore) Initialize(configuration util.Configuration, prefix str configuration.GetString(prefix+"ca_crt"), configuration.GetString(prefix+"client_crt"), configuration.GetString(prefix+"client_key"), + configuration.GetBool(prefix+"tls_insecure_skip_verify"), + configuration.GetString(prefix+"tls_server_name"), ) } func (store *MysqlStore) initialize(dsn string, upsertQuery string, enableUpsert bool, user, password, hostname string, port int, database string, maxIdle, maxOpen, - maxLifetimeSeconds int, interpolateParams bool, enableTls bool, caCrtDir string, clientCrtDir string, clientKeyDir string) (err error) { + maxLifetimeSeconds int, interpolateParams bool, enableTls bool, caCrtDir string, clientCrtDir string, clientKeyDir string, + tlsInsecureSkipVerify bool, tlsServerName string) (err error) { store.SupportBucketTable = false if !enableUpsert { @@ -81,38 +82,8 @@ func (store *MysqlStore) initialize(dsn string, upsertQuery string, enableUpsert return false } - if enableTls { - rootCertPool := x509.NewCertPool() - pem, err := os.ReadFile(caCrtDir) - if err != nil { - return err - } - if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { - return fmt.Errorf("failed to append root certificate") - } - - clientCert := make([]tls.Certificate, 0) - if cert, err := tls.LoadX509KeyPair(clientCrtDir, clientKeyDir); err == nil { - clientCert = append(clientCert, cert) - } - - tlsConfig := &tls.Config{ - RootCAs: rootCertPool, - Certificates: clientCert, - MinVersion: tls.VersionTLS12, - } - err = mysql.RegisterTLSConfig("mysql-tls", tlsConfig) - if err != nil { - return err - } - } - if dsn == "" { - pattern := CONNECTION_URL_PATTERN - if enableTls { - pattern = CONNECTION_TLS_URL_PATTERN - } - dsn = fmt.Sprintf(pattern, user, password, hostname, port, database) + dsn = fmt.Sprintf(CONNECTION_URL_PATTERN, user, password, hostname, port, database) if interpolateParams { dsn += "&interpolateParams=true" } @@ -122,21 +93,67 @@ func (store *MysqlStore) initialize(dsn string, upsertQuery string, enableUpsert return fmt.Errorf("can not parse DSN error:%w", err) } - var dbErr error - store.DB, dbErr = sql.Open("mysql", dsn) - if dbErr != nil { - store.DB.Close() - store.DB = nil - return fmt.Errorf("can not connect to %s error:%v", strings.ReplaceAll(dsn, cfg.Passwd, ""), err) + if enableTls { + tlsConfig := &tls.Config{ + MinVersion: tls.VersionTLS12, + InsecureSkipVerify: tlsInsecureSkipVerify, + ServerName: tlsServerName, + } + + // When ca_crt is empty, leave RootCAs nil so Go falls back to the + // system trust store. This is the common case for managed databases + // (RDS, Aiven, ...) whose certs chain to a public CA already on the host. + if caCrtDir != "" { + rootCertPool := x509.NewCertPool() + pem, err := os.ReadFile(caCrtDir) + if err != nil { + return fmt.Errorf("read ca_crt %s: %w", caCrtDir, err) + } + if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { + return fmt.Errorf("failed to append root certificate from %s", caCrtDir) + } + tlsConfig.RootCAs = rootCertPool + } + + // Only attempt to load a client keypair when at least one of the paths is + // set. If either is set, both must load successfully — silently skipping + // a typo'd path used to mask broken mTLS setups as confusing handshake + // failures. + if clientCrtDir != "" || clientKeyDir != "" { + cert, err := tls.LoadX509KeyPair(clientCrtDir, clientKeyDir) + if err != nil { + return fmt.Errorf("load mysql client keypair (crt=%s key=%s): %w", clientCrtDir, clientKeyDir, err) + } + tlsConfig.Certificates = []tls.Certificate{cert} + } + + // Set TLS directly on the parsed Config rather than registering a global + // "mysql-tls" entry — the global registry is process-wide and would be + // overwritten if a second MysqlStore is initialized with different TLS + // settings. + cfg.TLS = tlsConfig } + connector, err := mysql.NewConnector(cfg) + if err != nil { + return fmt.Errorf("can not create mysql connector for %s error:%w", maskedDSN(cfg), err) + } + + store.DB = sql.OpenDB(connector) store.DB.SetMaxIdleConns(maxIdle) store.DB.SetMaxOpenConns(maxOpen) store.DB.SetConnMaxLifetime(time.Duration(maxLifetimeSeconds) * time.Second) if err = store.DB.Ping(); err != nil { - return fmt.Errorf("connect to %s error:%v", strings.ReplaceAll(dsn, cfg.Passwd, ""), err) + return fmt.Errorf("connect to %s error:%v", maskedDSN(cfg), err) } return nil } + +func maskedDSN(cfg *mysql.Config) string { + if cfg.Passwd == "" { + return cfg.FormatDSN() + } + return strings.ReplaceAll(cfg.FormatDSN(), cfg.Passwd, "") +}