Copilot commented on code in PR #13199:
URL: https://github.com/apache/trafficserver/pull/13199#discussion_r3297156388
##########
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 Note message suggests running `traffic_ctl config convert ssl_multicert`
without required input/output arguments. This is misleading for operators;
update the example to include the expected `<input_file> <output_file>` (or `-`
for stdout) so the remediation guidance is actionable.
##########
src/iocore/net/SSLClientCoordinator.cc:
##########
@@ -65,9 +66,13 @@ SSLClientCoordinator::startup()
config::ConfigRegistry::Get_Instance().add_file_and_node_dependency(
"ssl_client_coordinator", "sni", "proxy.config.ssl.servername.filename",
ts::filename::SNI, false);
- // Track ssl_multicert.config — same pattern.
- config::ConfigRegistry::Get_Instance().add_file_and_node_dependency(
- "ssl_client_coordinator", "ssl_multicert",
"proxy.config.ssl.server.multicert.filename", ts::filename::SSL_MULTICERT,
false);
+ // Track ssl_multicert.yaml — same pattern.
+
config::ConfigRegistry::Get_Instance().add_file_and_node_dependency("ssl_client_coordinator",
"ssl_multicert",
+
"proxy.config.ssl.server.multicert.filename",
+
ts::filename::SSL_MULTICERT_YAML, false);
+
+ // Also register the legacy ssl_multicert.config so the
backward-compatibility fallback
Review Comment:
The comment is an incomplete sentence (ends after “backward-compatibility
fallback”), which makes it unclear what behavior is intended. Please finish the
sentence or remove it to keep the coordinator startup logic self-documenting.
##########
src/traffic_ctl/SSLMultiCertCommand.cc:
##########
@@ -26,12 +26,15 @@
#include "tscore/Layout.h"
#include "tscore/Filenames.h"
+#include "swoc/swoc_file.h"
+
#include <iostream>
namespace
{
-/// Get the default ssl_multicert.yaml file path.
+/// Get the active ssl_multicert file path. Prefers ssl_multicert.yaml; falls
+/// back to legacy ssl_multicert.config when only that exists.
std::string
get_default_ssl_multicert_path()
{
Review Comment:
The helper is now selecting the *active* multicert path (YAML preferred,
legacy fallback), but the function name still says
`get_default_ssl_multicert_path()`. Renaming it (or adjusting the
comment/semantics) would avoid confusion for future maintenance.
##########
doc/admin-guide/files/ssl_multicert.config.en.rst:
##########
@@ -0,0 +1,234 @@
+.. 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.
+
+====================
+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 able to configure a ECDSA certificate
+ chain and a RSA certificate chain and serve them simultaneously,
Review Comment:
Grammar issues in this new documentation section: “possible able” and the
articles “a ECDSA” / “a RSA” are incorrect. Please correct the wording (e.g.,
“possible to configure an ECDSA … and an RSA …”).
##########
doc/admin-guide/files/ssl_multicert.config.en.rst:
##########
@@ -0,0 +1,234 @@
+.. 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.
+
+====================
+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 able to configure a ECDSA certificate
+ chain and a 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
+ to match the client's SNI request.
+
+ You can also configure multiple leaf certificates in a same chain
Review Comment:
More wording typos here: “used to to match” has a duplicated “to”, and “in a
same chain” should be “in the same chain”.
--
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]
