Copilot commented on code in PR #13548:
URL: https://github.com/apache/trafficserver/pull/13548#discussion_r4420543622
##########
src/proxy/http/HttpSessionManager.cc:
##########
@@ -163,18 +174,36 @@ ServerSessionPool::validate_sni(HttpSM *sm,
NetVConnection *netvc)
std::string_view proposed_sni = sm->get_outbound_sni();
Dbg(dbg_ctl_http_ss, "validate_sni proposed_sni=%.*s, sni=%s",
static_cast<int>(proposed_sni.length()), proposed_sni.data(),
session_sni);
- if (!session_sni || session_sni[0] == '\0' || proposed_sni.length() ==
0) {
- retval = session_sni == nullptr && proposed_sni.length() == 0;
- } else {
- retval = proposed_sni.compare(session_sni) == 0;
- }
+ retval = sni_matches(proposed_sni, session_sni);
Review Comment:
The empty-SNI match in `sni_matches()` applies to every pooled TLS origin
session, not only raw public key ones. Before this change, X.509 sessions to
IP-literal origins were never reused; now they are, and the PR description does
not mention this. Either limit the empty-SNI match to sessions where
`origin_pinned_raw_public_key()` is true, or describe this broader reuse change
in the PR description and add an X.509 reuse test.
##########
include/config/ssl_multicert.h:
##########
@@ -37,16 +37,18 @@ namespace config
* Represents a single certificate entry in ssl_multicert configuration.
*/
struct SSLMultiCertEntry {
- std::string ssl_cert_name; ///< Certificate file name (required
unless action is tunnel).
- std::string dest_ip; ///< IP address to match (empty means
no binding). Only "*" sets default.
- std::string ssl_key_name; ///< Private key file name (optional).
- std::string ssl_ca_name; ///< CA certificate file name
(optional).
- std::string ssl_ocsp_name; ///< OCSP response file name
(optional).
- std::string ssl_key_dialog; ///< Passphrase dialog method
(optional).
- std::string dest_fqdn; ///< Destination FQDN (optional).
- std::string action; ///< Action (e.g., "tunnel").
- std::optional<int> ssl_ticket_enabled; ///< Session ticket enabled
(optional).
- std::optional<int> ssl_ticket_number; ///< Number of session tickets
(optional).
+ std::string ssl_cert_name; ///< Certificate file name
(required unless action is tunnel).
+ std::string dest_ip; ///< IP address to match (empty
means no binding). Only "*" sets default.
+ std::string ssl_key_name; ///< Private key file name
(optional).
+ std::string ssl_ca_name; ///< CA certificate file name
(optional).
+ std::string ssl_ocsp_name; ///< OCSP response file name
(optional).
+ std::string ssl_key_dialog; ///< Passphrase dialog method
(optional).
+ std::string dest_fqdn; ///< Destination FQDN (optional).
+ std::string action; ///< Action (e.g., "tunnel").
+ std::optional<int> ssl_ticket_enabled; ///< Session ticket enabled
(optional).
+ std::optional<int> ssl_ticket_number; ///< Number of session tickets
(optional).
+ std::optional<int> ssl_rpk_enabled; ///< Offer a RFC 7250 raw public
key alongside X.509 (optional).
Review Comment:
Grammar: "a RFC 7250" should be "an RFC 7250", because RFC is read as
"ar-eff-see". Same issue in `include/iocore/net/SSLSNIConfig.h:55`.
##########
include/iocore/net/SSLSNIConfig.h:
##########
@@ -42,15 +42,24 @@
#include "iocore/eventsystem/ConfigProcessor.h"
#include "iocore/net/SNIActionItem.h"
+#include "iocore/net/SSLTypes.h"
#include "iocore/net/YamlSNIConfig.h"
#include "mgmt/config/ConfigContext.h"
#include <functional>
// Properties for the next hop server
struct NextHopProperty {
- std::string client_cert_file;
// full path to client cert file for lookup
- std::string client_key_file;
// full path to client key file for lookup
+ std::string client_cert_file; // full path to client cert file for
lookup
+ std::string client_key_file; // full path to client key file for
lookup
+ bool client_rpk_enabled = false; // offer a RFC 7250 raw public key
(derived from the configured client
Review Comment:
Grammar: "a RFC" should be "an RFC", because RFC is read as "ar-eff-see".
Same issue in `include/config/ssl_multicert.h:50`.
--
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]
