MDEV-39857: Add multi-certificate SSL support via --ssl-cert-add/--ssl-key-add - #5178
MDEV-39857: Add multi-certificate SSL support via --ssl-cert-add/--ssl-key-add#5178rmetrich wants to merge 14 commits into
Conversation
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces dual RSA and ECDSA certificate support by adding the --ssl-alt-cert and --ssl-alt-key configuration options. It updates the SSL initialization logic to load these alternate credentials into the same SSL context and includes integration tests to verify that clients can connect using either RSA or ECDSA ciphers. The feedback suggests adding validation to ensure that if either the alternate certificate or key is provided, both must be specified, preventing the configuration from being silently ignored.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
gkodinov
added
the
External Contribution
rmetrich
changed the title
rmetrich
changed the title
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Thank you for your contribution! This is a preliminary review.
I believe this needs to be designed well first. So please consider adding a Jira about it, as mentioned by the community contributions guidelines.
Also, please keep 1 commit per "feature". I believe in your case that should be 1 in total: makes it easier to merge later.
Now on the substance of the issue:
First of all: Great analysis! This is something we've missed for so many years. So, again, thanks for bringing this up!
Now on the execution: You are saying that OpenSSL (currently) supports 3 certificate chains (so far). And it's picking which one to use based on the cipher.
now, you're adding --ssl-alt-cert and --ssl-alt-key. Singular. This makes 2 cert/key pairs. What about the 3d? Or future ones?
Now, one thing you're not saying is that these certs need to come from the same cert chain. https://linux.die.net/man/3/ssl_ctx_use_certificate_file says that
SSL_CTX_use_certificate_file() loads the first certificate stored in file into ctx.
The first!
So you might end up needing --ssl-alt-ca etc.
And, finally, you have exactly the same issue on the client. Do you want to support multiple certificate chains on it too?
This is where some forethought and a good design would have been handy!
I have no strong preference here (it's the preliminary review too). But I'll pitch in some thoughts:
- make all tools take multiple --ssl-cert --ssl-key options. It solves the "multiple" design problem. But then you may need to somehow match the keys to the certs. Define that. I'd use positional matching: first cert matches with the first key etc. Not really needed by openssl, but it'd be nice to define for future use.
- keep all tools in lock-step: people by now are used to having the same options on the client and on the server.
- Consider switching to SSL_CTX_use_certificate_chain_file() when processing these files so you could keep the cert chain out of the --ssl-ca list.
- Consider exposing the current cert selected and/or the list of certs/keys available as status vars for observability. This will help with writing more meaningful tests too.
- Consider the effects on other SSL libraries mariadb supports. Will it work on e.g. wolfSSL? How exactly? Ditto GnuTLS. You might need to have a status variable for this alone. E.g. so that automated configs somehow know if this is supported or not.
Looking forward to the Jira with a good design that has all of these questions answered.
|
Regarding 2 certificates only, it's true OpenSSL can load up to 3, but I believe it's totally overkill for now, because Regarding |
|
What do you mean by file a JIRA btw? it's the first time I contribute. I have an internal JIRA for Red Hat people but I guess it's a different one you need :-) |
https://jira.mariadb.org is the place to keep all features and bug reports. |
|
gkodinov
changed the title
Well, in design IMHO there's one or many. But that's just me. Anyway, I said my peace. But to do a best-effort preliminary review. So I am not here to debate the choices. All I asked is to document these decisions intо the design (jira). |
42561cf to
94babad
Compare
|
@rmetrich, first of all, thank you very much for turning my wish into this pull request! @gkodinov, I am not a fan of making this functionality more complicated as needed, just because it could be done. I don't want to sound harsh, but from my point of view MariaDB upstream unfortunately did not modernize the TLS part for a long time, thus any contributed improvement to MariaDB is already great – even it might not cover fancy/other use-cases. Personally, I do not see any benefit in Our practical scenario in some web hosting environments is that MariaDB should simply be able to offer certificates on RSA-based and ECDSA-based keys to satisfy whatever the MariaDB/MySQL client needs (just like Apache HTTP Server, HAProxy, NGINX and others do). So far I only tested commit 82564ca, which works. However I personally like the repeatable |
rmetrich
changed the title
10217e2 to
5108eef
Compare
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Let's leave the whole discussion about the format for the final review.
I was just expressing my own personal opinion. But I'm not the one to decide.
For now, let's focus on the failing buildbot tests and maybe squashing the commits to one per logical "task".
|
I plan to merge later once all is fixed. |
Alright. Then it's just the failing tests. |
|
| Scenario | Intent | What happens |
|---|---|---|
my.cnf: cert+key, cmdline: --ssl-key=other |
Override primary key | Counted as 2nd key → mismatch error |
my.cnf: cert+key, cmdline: --ssl-cert=X --ssl-key=Y |
Add 2nd pair | Works correctly (both counts match) |
my.cnf: cert+key, cmdline: --ssl-cert=X --ssl-key=Y --ssl-cert=Z --ssl-key=W |
Override primary + add 2nd pair | 1st cmdline pair overrides, 2nd adds — but code can't tell |
Why --ssl-alt-cert/--ssl-alt-key didn't have this problem
With separate option names, overriding --ssl-cert on the command line never interferes with --ssl-alt-cert. The namespaces are independent, so override vs. addition is unambiguous.
Possible fixes
-
Keep repeatable
--ssl-cert/--ssl-key: treat a second key/cert as an override (not addition) when alt counts are balanced. This works for the common case but makes ordering validation less strict and adds subtle edge cases. -
Revert to
--ssl-alt-cert/--ssl-alt-key: clean separation, no ambiguity.--ssl-alt-certcan itself be repeatable to support N certs (e.g. ECDSA + EdDSA). Still the issue would remain for--ssl-alt-cert/`--ssl-alt-key in case one alt cert is set in system cfgfile and one on command line or user cfgfile.
Looking for guidance on the preferred direction.
This comment was marked as resolved.
This comment was marked as resolved.
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Very close to perfect. One important IMHO suggestion on the test structuring. The rest are just possible code improvements.
Please stand by for the final review.
And thank you both for working with me! I really appreciate both the contribution and the review of it!
|
Moved back to Draft to fix the new issues due to WolfSSL ... |
|
The failing build doesn't seem related to this work. It's failing in timeout after 900 seconds. |
gkodinov
left a comment
gkodinov
left a comment
There was a problem hiding this comment.
Please find some optional suggestions to improve the diff below.
…-key-add Add --ssl-cert-add and --ssl-key-add server options to load additional certificate/key pairs into the same SSL_CTX. OpenSSL natively supports one certificate per key type (RSA, ECDSA, EdDSA) and automatically selects the matching certificate during the TLS handshake. Following the --plugin-load / --plugin-load-add pattern: - --ssl-cert / --ssl-key override the primary cert/key and reset any previously added alt certs/keys - --ssl-cert-add / --ssl-key-add append additional certs/keys. If no primary exists, the first add becomes the primary Each additional certificate is loaded via SSL_CTX_use_certificate_chain_file() so intermediate CA chains are included. Up to 5 certificates total are supported. New status variables: - Ssl_server_cert_type (session): key type used for this connection - Ssl_server_cert_types (global): all loaded cert key types WolfSSL: multiple certificates are not supported. The server rejects startup with an error if --ssl-cert-add/--ssl-key-add are specified. Cert type enumeration falls back to reporting only the primary type. Example configurations: [mysqld] ssl-cert=/path/to/server-rsa.crt ssl-key=/path/to/server-rsa.key ssl-cert-add=/path/to/server-ecdsa.crt ssl-key-add=/path/to/server-ecdsa.key or equivalently (first add becomes primary): [mysqld] ssl-cert-add=/path/to/server-rsa.crt ssl-key-add=/path/to/server-rsa.key ssl-cert-add=/path/to/server-ecdsa.crt ssl-key-add=/path/to/server-ecdsa.key Pre-generated ECDSA and EdDSA test certificates added to std_data/. Test coverage: 4 tests in ssl_multi_cert (3 cert types, cipher selection, TLS 1.3) and 15 tests in ssl_multi_cert_errors (invalid files, count mismatches, reset behavior, FLUSH SSL, order independence, first-add-becomes-primary for both cert and key sides).
Each --ssl-cert-add must be positionally matched with a corresponding --ssl-key-add. The loading loop pairs cert[0] with key[0], cert[1] with key[1], etc. and verifies each pair with SSL_CTX_check_private_key().
Refactor new_VioSSLAcceptorFd() and new_VioSSLFd() to take cert/key arrays instead of separate primary + alt parameters. The caller builds combined arrays with the primary at index 0 and additional certs at subsequent indices.
Split ssl_multi_cert into two tests with .opt files: - ssl_multi_cert: TLS 1.2 with 3 cert types, cipher-based selection - ssl_multi_cert_tlsv13: TLS 1.3 with EdDSA verification Each test uses a -master.opt file for server parameters instead of restarting mid-test. This makes tests faster (24ms + 19ms vs ~4300ms), parallelizable by MTR, and easier to debug.
WolfSSL does not support multiple certificates per SSL_CTX. When tests use .opt files to pass --ssl-cert-add at server startup, the server aborts before the test's WolfSSL skip guard can run. Work around this by not registering --ssl-cert-add/--ssl-key-add options on WolfSSL builds (!defined(HAVE_WOLFSSL) guard on option registration). The .opt files use the --loose- prefix so the options are silently ignored when unknown. This workaround can be removed once WolfSSL supports multiple certificate types per SSL_CTX (see MDEV-36656).
Also remove the useless "name" variable.
|
@vuvova @FaramosCZ Could you please review this. |
6 participants
MariaDB currently supports only a single SSL certificate, configured via
--ssl-certand--ssl-key. Services like httpd, nginx and haproxy can serve both RSA and ECDSA certificates simultaneously, selecting the appropriate one based on the cipher suite negotiated with the client.This patch adds
--ssl-cert-addand--ssl-key-addserver options to load additional certificate/key pairs into the same SSL_CTX. OpenSSL natively supports one certificate per key type (RSA, ECDSA, EdDSA) and automatically selects the matching certificate during the TLS handshake.Following the
--plugin-load/--plugin-load-addpattern:--ssl-cert/--ssl-key— override the primary cert/key and reset any previously added alt certs/keys--ssl-cert-add/--ssl-key-add— append additional certs/keys. If no primary exists, the first add becomes the primaryEach additional certificate is loaded via
SSL_CTX_use_certificate_chain_file()so intermediate CA chains are included. Up to 5 certificates total are supported.Example configurations:
or equivalently (first add becomes primary):
New status variables
Ssl_server_cert_typeSsl_server_cert_typesWolfSSL
Multiple certificates are not supported with WolfSSL — the server rejects startup with an error if
--ssl-cert-add/--ssl-key-addare specified. The cert type enumeration forSsl_server_cert_typesfalls back to reporting only the primary cert type. Tests are skipped on WolfSSL builds. Code compiles cleanly with both OpenSSL and bundled WolfSSL.Tests
ssl_multi_cert(4 tests): 3 cert types (RSA + ECDSA + EdDSA), cipher-based selection, TLS 1.3 with EdDSAssl_multi_cert_errors(15 tests):--ssl-cert-addinvalid file--ssl-key-addinvalid file--ssl-cert-addwithout--ssl-key-add--ssl-key-addwithout--ssl-cert-addSsl_server_cert_typesFLUSH SSL--ssl-certoverride +--ssl-cert-add--ssl-cert-addthen--ssl-cert--ssl-cert-addalongside my.cnf default--ssl-cert=(empty) +--ssl-cert-add--ssl-keyoverride resets alt keys--ssl-key=(empty) +--ssl-key-add