Copilot commented on code in PR #13284:
URL: https://github.com/apache/trafficserver/pull/13284#discussion_r3422249281
##########
src/iocore/net/TLSCertCompression.cc:
##########
@@ -97,8 +97,105 @@ find_algorithm(std::string const &name)
} // end anonymous namespace
#endif
+#if HAVE_SSL_CTX_ADD_CERT_COMPRESSION_ALG
+static int cert_compress_cache_index = -1;
+
+static void
+cert_compress_cache_free_cb(void * /* parent */, void *ptr, CRYPTO_EX_DATA *
/* ad */, int /* idx */, long /* argl */,
+ void * /* argp */)
+{
+ auto *cache = static_cast<CertCompressionCache *>(ptr);
+ if (cache) {
+ for (auto &slot : cache->slots) {
+ delete slot.live.load(std::memory_order_acquire);
+ delete slot.retired.load(std::memory_order_acquire);
+ }
+ delete cache;
+ }
+}
+
+void
+cert_compress_cache_init()
+{
+ if (cert_compress_cache_index != -1) {
+ return;
+ }
+ cert_compress_cache_index = SSL_CTX_get_ex_new_index(0, nullptr, nullptr,
nullptr, cert_compress_cache_free_cb);
+}
+
+CertCompressionCache *
+cert_compress_cache_get(SSL_CTX *ctx)
+{
+ if (cert_compress_cache_index < 0) {
+ return nullptr;
+ }
+ return static_cast<CertCompressionCache *>(SSL_CTX_get_ex_data(ctx,
cert_compress_cache_index));
+}
+
+static void
+cert_compress_cache_attach(SSL_CTX *ctx)
+{
+ if (cert_compress_cache_index < 0) {
+ return;
+ }
+ auto *cache = new CertCompressionCache();
+ SSL_CTX_set_ex_data(ctx, cert_compress_cache_index, cache);
+}
+
+void
+cert_compress_cache_try_publish(CertCompressionCache::Slot &slot,
CertCompressionCache::Entry const *fresh)
+{
+ CertCompressionCache::Entry const *expected = nullptr;
+ if (!slot.live.compare_exchange_strong(expected, fresh,
std::memory_order_acq_rel)) {
+ delete fresh;
+ }
+}
+
+void
+cert_compress_cache_invalidate(SSL_CTX *ctx)
+{
+ auto *cache = cert_compress_cache_get(ctx);
+ if (!cache) {
+ return;
+ }
+ for (auto &slot : cache->slots) {
+ auto const *prev = slot.live.exchange(nullptr,
std::memory_order_acq_rel);
+ auto const *to_free = slot.retired.exchange(prev,
std::memory_order_acq_rel);
+ delete to_free;
+ }
Review Comment:
cert_compress_cache_invalidate() deletes the previously-retired Entry
immediately on the next invalidation. A compression callback can still be
memcpy'ing from that Entry if it loaded the pointer shortly before the prior
invalidation, so a second invalidation (which can happen back-to-back in
ocsp_update() for multiple certs on the same SSL_CTX) can cause a
use-after-free. Consider using an RCU-style reclamation (e.g., store
std::shared_ptr<const Entry> in the slot and atomic_load it in the callback) or
add explicit refcounting/hazard tracking so entries are only freed after all
concurrent readers have finished.
##########
tests/gold_tests/tls/tls_cert_comp_cache.test.py:
##########
@@ -0,0 +1,172 @@
+'''
+Verify TLS Certificate Compression cache behavior.
+'''
+# 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.
+
+Test.Summary = '''
+Verify that the certificate compression cache works correctly. When caching
+is enabled, the compressed result is reused across handshakes. When disabled,
+each handshake compresses independently and the cache_hit metric stays at 0.
+'''
+
+Test.SkipUnless(Condition.HasATSFeature('TS_HAS_CERT_COMPRESSION'))
Review Comment:
This test is intended to validate the new cache_hit metric, which is only
available when the cert compression *callbacks* are supported. Gating on
TS_HAS_CERT_COMPRESSION will also run the test on OpenSSL builds where
callbacks (and cache_hit) may be unavailable, causing spurious failures.
--
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]
