Copilot commented on code in PR #13199:
URL: https://github.com/apache/trafficserver/pull/13199#discussion_r3300378725
##########
src/iocore/net/SSLConfig.cc:
##########
@@ -437,7 +440,36 @@ SSLConfigParams::initialize(ConfigContext ctx)
set_paths_helper(serverCertRelativePath, nullptr, &serverCertPathOnly,
nullptr);
}
- configFilePath =
ats_stringdup(RecConfigReadConfigPath("proxy.config.ssl.server.multicert.filename"));
+ // Resolve the multicert config path. Prefer the configured path; if the
+ // user is on the default (ssl_multicert.yaml) and it is absent while a
+ // legacy ssl_multicert.config exists alongside, fall back to the legacy
+ // file for backward compatibility.
+ {
+ char rec_buf[PATH_NAME_MAX] = {};
+ RecGetRecordString("proxy.config.ssl.server.multicert.filename", rec_buf,
PATH_NAME_MAX);
+ const bool record_default = (rec_buf[0] == '\0' || strcmp(rec_buf,
ts::filename::SSL_MULTICERT_YAML) == 0);
+
+ ats_scoped_str yaml_path(RecConfigReadConfigPath(nullptr,
ts::filename::SSL_MULTICERT_YAML));
+ ats_scoped_str legacy_path(RecConfigReadConfigPath(nullptr,
ts::filename::SSL_MULTICERT));
+
+ const bool yaml_exists = yaml_path &&
swoc::file::exists(swoc::file::path(yaml_path.get()));
+ const bool legacy_exists = legacy_path &&
swoc::file::exists(swoc::file::path(legacy_path.get()));
+
+ if (record_default && !yaml_exists && legacy_exists) {
+ Note("%s not found, falling back to %s",
ts::filename::SSL_MULTICERT_YAML, ts::filename::SSL_MULTICERT);
+ configFilePath = ats_strdup(legacy_path.get());
+ } else {
+ configFilePath =
ats_stringdup(RecConfigReadConfigPath("proxy.config.ssl.server.multicert.filename"));
+ if (record_default && yaml_exists && legacy_exists) {
+ Note("%s exists alongside %s; the legacy file is ignored. "
+ "To resolve, either: (a) migrate %s to %s (e.g. 'traffic_ctl
config convert ssl_multicert') and remove %s, "
+ "or (b) remove %s to fall back to %s.",
+ ts::filename::SSL_MULTICERT, ts::filename::SSL_MULTICERT_YAML,
ts::filename::SSL_MULTICERT,
+ ts::filename::SSL_MULTICERT_YAML, ts::filename::SSL_MULTICERT,
ts::filename::SSL_MULTICERT_YAML,
+ ts::filename::SSL_MULTICERT);
Review Comment:
The operator guidance in this `Note()` is now inconsistent with the docs: it
suggests `traffic_ctl config convert ssl_multicert` without the required
input/output arguments, which is not directly actionable. Please update the
example to include the expected arguments (e.g. `traffic_ctl config convert
ssl_multicert ssl_multicert.config ssl_multicert.yaml` or `... -` for stdout),
matching the documentation in `doc/admin-guide/files/ssl_multicert.yaml.en.rst`.
##########
src/traffic_server/traffic_server.cc:
##########
@@ -2536,7 +2536,22 @@ init_ssl_ctx_callback(void *ctx, bool server)
void
load_ssl_file_callback(const char *ssl_file)
{
- FileManager::instance().configFileChild(ts::filename::SSL_MULTICERT,
ssl_file);
+ // Parent lookup must match the actually-loaded multicert file, not just the
+ // YAML default name — when legacy fallback is in effect, configFilePath ends
+ // in ssl_multicert.config and the YAML-keyed parent binding would miss.
+ const char *parent = ts::filename::SSL_MULTICERT_YAML;
+ SSLConfig::scoped_config params;
+
+ if (params && params->configFilePath != nullptr) {
+ std::string_view fp{params->configFilePath};
+
+ if (auto slash = fp.find_last_of('/'); slash != std::string_view::npos) {
+ parent = params->configFilePath + slash + 1;
+ } else {
+ parent = params->configFilePath;
+ }
Review Comment:
`FileManager` bindings use the *relative* config filename (as stored in the
record, which may include subdirectories like `ssl/custom_multicert.yaml`).
Here the parent key is derived by stripping everything up to the last `/` from
`configFilePath`, which collapses `ssl/custom_multicert.yaml` to
`custom_multicert.yaml` and will fail to find the parent binding. As a result,
`configFileChild()` won’t register cert/key children under the multicert parent
when the record contains a path component. Consider deriving the parent key as
the path *relative to* `RecConfigReadConfigDir()` (preserving any
subdirectories), and only fall back to basename if it can’t be relativized.
##########
doc/admin-guide/files/ssl_multicert.config.en.rst:
##########
@@ -0,0 +1,236 @@
+.. Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+.. include:: ../../common.defs
+
+====================
+ssl_multicert.config
+====================
+
+.. configfile:: ssl_multicert.config
+
+.. note::
+
+ :file:`ssl_multicert.config` is the legacy line-based format. The
+ YAML-based :file:`ssl_multicert.yaml` is preferred. |TS| loads
+ :file:`ssl_multicert.config` only when :file:`ssl_multicert.yaml`
+ is absent. Use ``traffic_ctl config convert ssl_multicert
+ ssl_multicert.config ssl_multicert.yaml`` to migrate.
+
+The :file:`ssl_multicert.config` file lets you configure Traffic
+Server to use multiple SSL server certificates to terminate the SSL
+sessions. If you have a Traffic Server system with more than one
+IP address assigned to it, then you can assign a different SSL
+certificate to be served when a client requests a particular IP
+address or host name.
+
+At configuration time, certificates are parsed to extract the
+certificate subject and all the DNS `subject alternative names
+<http://en.wikipedia.org/wiki/SubjectAltName>`_. A certificate
+will be presented for connections requesting any of the hostnames
+found in the certificate. Wildcard names are supported, but only
+of the form `*.domain.com`, ie. where `*` is the leftmost domain
+component.
+
+Changes to :file:`ssl_multicert.config` can be applied to a running
+Traffic Server using :option:`traffic_ctl config reload`.
+
+Format
+======
+
+Each :file:`ssl_multicert.config` line consists of a sequence of
+`key=value` fields that specify how Traffic Server should use a
+particular SSL certificate.
+
+ssl_cert_name=FILENAME[,FILENAME ...]
+ The name of the file containing the TLS certificate. *FILENAME*
+ is located relative to the directory specified by the
+ :ts:cv:`proxy.config.ssl.server.cert.path` configuration variable.
+ It may also include the intermediate CA certificates, sorted from
+ leaf to root. At a minimum, the file must include a leaf
+ certificate.
+
+ When running with OpenSSL 1.0.2 or later, this directive can be
+ used to configure the intermediate CA chain on a per-certificate
+ basis. Multiple chain files are separated by comma character.
+ For example, it is possible to configure an ECDSA certificate
+ chain and an RSA certificate chain and serve them simultaneously,
+ allowing OpenSSL to determine which certificate would be used
+ when the TLS session cipher suites are negotiated. Note that the
+ leaf certs in `FILENAME1` and `FILENAME2` must have the same
+ subjects and alternate names. The first certificate is used
+ to match the client's SNI request.
+
+ You can also configure multiple leaf certificates in the same chain
+ with OpenSSL 1.0.1.
+
+ This is the only field that is required to be present.
+
+dest_ip=ADDRESS (optional)
+ The IP (v4 or v6) address that the certificate should be presented
+ on. This is now only used as a fallback in the case that the TLS
+ ServerNameIndication extension is not supported. If *ADDRESS* is
+ `*`, the corresponding certificate will be used as the global
+ default fallback if no other match can be made. The address may
+ contain a port specifier, in which case the corresponding certificate
+ will only match for connections accepted on the specified port.
+ IPv6 addresses must be enclosed by square brackets if they have
+ a port, eg, [::1]:80. Care should be taken to make each ADDRESS unique.
+
+ssl_key_name=FILENAME (optional)
+ The name of the file containing the private key for this certificate.
+ If the key is contained in the certificate file, this field can
+ be omitted, otherwise *FILENAME* is resolved relative to the
+ :ts:cv:`proxy.config.ssl.server.private_key.path` configuration variable.
+
+ssl_ca_name=FILENAME (optional)
+ If the certificate is issued by an authority that is not in the
+ system CA bundle, additional certificates may be needed to validate
+ the certificate chain. *FILENAME* is resolved relative to the
+ :ts:cv:`proxy.config.ssl.CA.cert.path` configuration variable.
+
+ssl_ocsp_name=FILENAME (optional)
+ The name of the file containing the prefetched OCSP stapling response
+ for this certificate. This field can be omitted to let trafficserver
+ fetch OCSP responses dynamically. Otherwise, when included, the
administrator is
+ responsible for updating the file's content. *FILENAME* is resolved
+ relative to the :ts:cv:`proxy.config.ssl.ocsp.response.path`
+ configuration variable.
+
+ssl_ticket_enabled=1|0 (optional)
+ Enable RFC 5077 stateless TLS session tickets. To support this,
+ OpenSSL should be upgraded to version 0.9.8f or higher. This
+ option must be set to `0` to disable session ticket support.
+
+ssl_ticket_number=INTEGER (optional)
+ Specifies the number of TLSv1.3 session tickets that are issued.
+ This defaults to 2 (the OpenSSL default)
+
+ssl_key_dialog=builtin|"exec:/path/to/program [args]" (optional)
+ Method used to provide a pass phrase for encrypted private keys. If the
+ pass phrase is incorrect, SSL negotiation for this dest_ip will fail for
+ clients who attempt to connect.
+ Two options are supported: builtin and exec:
+
+ ``builtin`` - Requests pass phrase via stdin/stdout. User will be
+ provided the ssl_cert_name and be prompted for the pass phrase.
+ Useful for debugging.
+
+ ``exec:`` - Executes program /path/to/program and passes args, if
+ specified, to the program and reads the output from stdout for
+ the pass phrase. If args are provided then the entire exec: string
+ must be quoted with "" (see examples). Arguments with white space
+ are supported by single quoting ('). The intent is that this
+ program runs a security check to ensure that the system is not
+ compromised by an attacker before providing the pass phrase.
+
+Certificate Selection
+=====================
+
+Traffic Server attempts two certificate selections during SSL
+connection setup. An initial selection is made when a TCP connection
+is accepted. This selection examines the IP address and port that
+the client is connecting to and chooses the best certificate from
+the those that have a ``dest_ip`` specification. If no matching
+certificates are found, a default certificate is chosen. The final
+certificate selection is made during the SSL handshake. At this
+point, the client may use `Server Name Indication
+<http://en.wikipedia.org/wiki/Server_Name_Indication>`_ to request
+a specific hostname. Traffic Server will use this request to select
+a certificate with a matching subject or subject alternative name.
+Failing that, a wildcard certificate match is attempted. If no match
+can be made, the initial certificate selection remains in force.
+
+In all cases, Traffic Server attempts to select the most specific
+match. An address specification that contains a port number will
+take precedence over a specification that does not contain a port
+number. A specific certificate subject will take precedence over a
+wildcard certificate. In the case of multiple matching certificates
+the first match will be returned to non-SNI capable clients.
+
+Examples
+========
+
+The following example configures Traffic Server to use the SSL
+certificate ``server.pem`` for all requests to the IP address
+111.11.11.1 and the SSL certificate ``server1.pem`` for all requests
+to the IP address 11.1.1.1. Connections from all other IP addresses
+are terminated with the ``default.pem`` certificate.
+Since the private key is included in the certificate files, no
+private key name is specified.
+
+::
+
+ dest_ip=111.11.11.1 ssl_cert_name=server.pem
+ dest_ip=11.1.1.1 ssl_cert_name=server1.pem
+ dest_ip=* ssl_cert_name=default.pem
+
+The following example configures Traffic Server to use the ECDSA
+certificate chain ``ecdsa.pem`` or RSA certificate chain ``rsa.pem``
+for all requests.
+
+::
+
+ dest_ip=* ssl_cert_name=ecdsa.pem,rsa.pem
+
+The following example configures Traffic Server to use the ECDSA
+certificate chain ``ecdsa.pem`` or RSA certificate chain ``rsa.pem``
+for all requests, the public key and private key are in separate PEM files.
+Note that the number of files in ssl_key_name must match the files in
ssl_cert_name,
+and they should be presented in the same order.
+
+::
+
+ dest_ip=* ssl_cert_name=ecdsa_pub.pem,rsa_pub.pem
ssl_key_name=ecdsa_private.pem,rsa_private.pem
+
+The following example configures Traffic Server to use the SSL
+certificate ``server.pem`` and the private key ``serverKey.pem``
+for all requests to port 8443 on IP address 111.11.11.1. The
+``general.pem`` certificate is used for server name matches.
+
+::
+
+ dest_ip=111.11.11.1:8443 ssl_cert_name=server.pem
ssl_key_name=serverKey.pem ssl_cert_name=general.pem
Review Comment:
This example line repeats `ssl_cert_name` twice, which doesn’t match the
documented legacy format (`ssl_cert_name=FILENAME[,FILENAME ...]`) and also
disagrees with the equivalent YAML example (which uses
`server.pem,general.pem`). Please update the example to show multiple certs
using the comma-separated `ssl_cert_name=server.pem,general.pem` form (and keep
`ssl_key_name` once).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]