You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/02/23 19:48:39 UTC

[GitHub] [trafficserver] duke8253 opened a new pull request #7552: Use SSL_CTX address as part of the lookup key

duke8253 opened a new pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552


   This fixes the issue found in PR #7537, as described in this comment https://github.com/apache/trafficserver/pull/7537#issuecomment-783658010
   
   Linking to #7479 in case we backport.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] maskit commented on a change in pull request #7552: Use SSL_CTX address as part of the lookup key

Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552#discussion_r581619998



##########
File path: iocore/net/SSLClientUtils.cc
##########
@@ -159,32 +159,21 @@ ssl_client_cert_callback(SSL *ssl, void * /*arg*/)
 static int
 ssl_new_session_callback(SSL *ssl, SSL_SESSION *sess)
 {
-  const char *tlsext_host_name = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
-  if (tlsext_host_name) {
-    std::string sni(tlsext_host_name);
-    origin_sess_cache->insert_session(sni, sess);
+  std::string sni_addr = get_sni_addr(ssl);
+  if (!sni_addr.empty()) {
+    SSL_CTX *ctx = SSL_get_SSL_CTX(ssl);
+    std::stringstream ctx_ss;
+    ctx_ss << static_cast<const void *>(ctx);
+    std::string lookup_key;
+    ts::bwprint(lookup_key, "{}:{}", sni_addr.c_str(), ctx_ss.str().c_str());

Review comment:
       We can't do `%p` with bwprint?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] zwoop commented on pull request #7552: Use SSL_CTX address as part of the lookup key

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552#issuecomment-800597963


   Moving to 9.2.x.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] duke8253 commented on pull request #7552: Use SSL_CTX address as part of the lookup key

Posted by GitBox <gi...@apache.org>.
duke8253 commented on pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552#issuecomment-784762852


   > Seems reasonable.
   > 
   > 
   > 
   > We might want to use configuration id or something that we can set by `SSL_CTX_set_ex_data ` instead of SSL_CTX address. If we optimize code for OpenSSL 3, we would create a SSL_CTX per thread (and configuration differences), and that means we would be unable to reuse sessions created by different threads.
   
   I actually had code using the key file and cert file paths as part of the lookup key for sessions, but decided using the SSL_CTX was sufficient enough for now, I can switch it back to do that so it'll be a bit more future proof.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] maskit commented on pull request #7552: Use SSL_CTX address as part of the lookup key

Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552#issuecomment-784847295


   I'm fine with either way for now since this is a bug fix. And OpenSSL 3 *beta* is still months away.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] duke8253 merged pull request #7552: Use SSL_CTX address as part of the lookup key

Posted by GitBox <gi...@apache.org>.
duke8253 merged pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] duke8253 commented on pull request #7552: Use SSL_CTX address as part of the lookup key

Posted by GitBox <gi...@apache.org>.
duke8253 commented on pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552#issuecomment-784564341


   [approve ci autest]


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] duke8253 commented on pull request #7552: Use SSL_CTX address as part of the lookup key

Posted by GitBox <gi...@apache.org>.
duke8253 commented on pull request #7552:
URL: https://github.com/apache/trafficserver/pull/7552#issuecomment-785188960


   I'll leave it as is then, we can change it when we start using OpenSSL 3.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org