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 2020/04/22 15:27:34 UTC
[GitHub] [trafficserver] shinrich opened a new pull request #6699: Log whether client certs were exchanged in TLS handshake
shinrich opened a new pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699
Added docs about the two new logging formats. Augmented existing autests to exercise them.
Useful to verify that your mTLS policy is operating as you expect.
----------------------------------------------------------------
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] shinrich edited a comment on pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich edited a comment on pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#issuecomment-642252716
To merge this in to another branch you also need to merge in 15e749ea4cf3b5e66ffd0ad53d20fa4c611e2ca5. The later test fixups rely on the file there.
----------------------------------------------------------------
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] bneradt commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r438178153
##########
File path: iocore/net/SSLClientUtils.cc
##########
@@ -138,6 +138,20 @@ verify_callback(int signature_ok, X509_STORE_CTX *ctx)
return true;
}
+static int
+ssl_client_cert_callback(SSL *ssl, void * /*arg*/)
+{
+ SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
+ SSL_CTX *ctx = SSL_get_SSL_CTX(ssl);
+ if (ctx) {
+ // Do not need to free either the cert or the ssl_ctx
+ // both are internal pointers
+ X509 *cert = SSL_CTX_get0_certificate(ctx);
+ netvc->set_sent_cert(cert != nullptr ? 2 : 1);
Review comment:
It would be helpful to name the 0, 1, 2 values in some way. Perhaps an enum?
##########
File path: tests/gold_tests/autest-site/when.test.ext
##########
@@ -34,3 +35,20 @@ def FileContains(haystack, needle):
AddWhenFunction(FileContains)
+
+def FilePresent(tsenv, file):
+ cmd = "test -f {0}".format(file)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ return retval.returncode == 0
+
+AddWhenFunction(FilePresent)
+
+def SNIReloadDone(tsenv, diags_file):
+ cmd = "grep 'sni.yaml finished loading' {0} | wc -l | sed -e 's/ //g'> ./test.out".format(diags_file)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ if retval.returncode == 0:
+ cmd ="if [ -f ./test.out -a \"`cat ./test.out`\" = \"2\" ] ; then true; else false; fi"
Review comment:
To make this a bit easier to read, the outer double quotes could be replaced with single quotes, and the escaped inner double quotes could just be unescaped double quotes.
----------------------------------------------------------------
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] shinrich commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r444455950
##########
File path: iocore/net/SSLClientUtils.cc
##########
@@ -138,6 +138,20 @@ verify_callback(int signature_ok, X509_STORE_CTX *ctx)
return true;
}
+static int
+ssl_client_cert_callback(SSL *ssl, void * /*arg*/)
+{
+ SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
Review comment:
Sorry I don't see where to put this in the QUIC classes. Filed issue #6939.
----------------------------------------------------------------
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 a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
zwoop commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r437689327
##########
File path: proxy/http/HttpSM.h
##########
@@ -523,25 +523,26 @@ class HttpSM : public Continuation, public PluginUserArgs<TS_USER_ARGS_TXN>
public:
// TODO: Now that bodies can be empty, should the body counters be set to -1 ? TS-2213
// Stats & Logging Info
- int client_request_hdr_bytes = 0;
- int64_t client_request_body_bytes = 0;
- int server_request_hdr_bytes = 0;
- int64_t server_request_body_bytes = 0;
- int server_response_hdr_bytes = 0;
- int64_t server_response_body_bytes = 0;
- int client_response_hdr_bytes = 0;
- int64_t client_response_body_bytes = 0;
- int cache_response_hdr_bytes = 0;
- int64_t cache_response_body_bytes = 0;
- int pushed_response_hdr_bytes = 0;
- int64_t pushed_response_body_bytes = 0;
- bool client_tcp_reused = false;
- bool client_ssl_reused = false;
- bool client_connection_is_ssl = false;
- bool is_internal = false;
- bool server_connection_is_ssl = false;
- bool is_waiting_for_full_body = false;
- bool is_using_post_buffer = false;
+ int client_request_hdr_bytes = 0;
+ int64_t client_request_body_bytes = 0;
+ int server_request_hdr_bytes = 0;
+ int64_t server_request_body_bytes = 0;
+ int server_response_hdr_bytes = 0;
+ int64_t server_response_body_bytes = 0;
+ int client_response_hdr_bytes = 0;
+ int64_t client_response_body_bytes = 0;
+ int cache_response_hdr_bytes = 0;
+ int64_t cache_response_body_bytes = 0;
+ int pushed_response_hdr_bytes = 0;
+ int64_t pushed_response_body_bytes = 0;
+ bool client_tcp_reused = false;
+ bool client_ssl_reused = false;
+ bool client_connection_is_ssl = false;
+ bool is_internal = false;
+ bool server_connection_is_ssl = false;
+ int server_connection_provided_cert = 0;
Review comment:
This likely introduces unnecessary padding. It'd be much better to move the to be before, or after, all the other bool's.
----------------------------------------------------------------
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] shinrich commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r415927293
##########
File path: tests/gold_tests/tls/tls_client_cert.test.py
##########
@@ -293,3 +306,21 @@ def done_reload(process, hasRunFor, **kw):
tr4fail.Processes.Default.Command = 'curl -H host:example.com http://127.0.0.1:{0}/case2'.format(ts.Variables.port)
tr4fail.Processes.Default.ReturnCode = 0
tr4fail.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Check response")
+
+# Parking this as a ready tester on a meaningless process
+# Stall the test runs until the squid.log file has appeared
+def access_log_ready(tsenv):
+ def done_access_log(process, hasRunFor, **kw):
+ cmd = "test -f {0}".format(ts.Disk.squid_log)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ return retval.returncode == 0
+
+ return done_access_log
+
+tr = Test.AddTestRun("Wait for the access log to write out")
+tr.Processes.Default.StartBefore(server4, ready=access_log_ready(ts.Env))
+tr.StillRunningAfter = ts
+tr.Processes.Default.Command = 'ls'
Review comment:
Given the size of the test and the number of log entries, it seems likely likely that this will get pushed out in one flush.
----------------------------------------------------------------
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] randall commented on pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
randall commented on pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#issuecomment-648438407
Thanks for the test code de-duplication!
----------------------------------------------------------------
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 a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
zwoop commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r437689327
##########
File path: proxy/http/HttpSM.h
##########
@@ -523,25 +523,26 @@ class HttpSM : public Continuation, public PluginUserArgs<TS_USER_ARGS_TXN>
public:
// TODO: Now that bodies can be empty, should the body counters be set to -1 ? TS-2213
// Stats & Logging Info
- int client_request_hdr_bytes = 0;
- int64_t client_request_body_bytes = 0;
- int server_request_hdr_bytes = 0;
- int64_t server_request_body_bytes = 0;
- int server_response_hdr_bytes = 0;
- int64_t server_response_body_bytes = 0;
- int client_response_hdr_bytes = 0;
- int64_t client_response_body_bytes = 0;
- int cache_response_hdr_bytes = 0;
- int64_t cache_response_body_bytes = 0;
- int pushed_response_hdr_bytes = 0;
- int64_t pushed_response_body_bytes = 0;
- bool client_tcp_reused = false;
- bool client_ssl_reused = false;
- bool client_connection_is_ssl = false;
- bool is_internal = false;
- bool server_connection_is_ssl = false;
- bool is_waiting_for_full_body = false;
- bool is_using_post_buffer = false;
+ int client_request_hdr_bytes = 0;
+ int64_t client_request_body_bytes = 0;
+ int server_request_hdr_bytes = 0;
+ int64_t server_request_body_bytes = 0;
+ int server_response_hdr_bytes = 0;
+ int64_t server_response_body_bytes = 0;
+ int client_response_hdr_bytes = 0;
+ int64_t client_response_body_bytes = 0;
+ int cache_response_hdr_bytes = 0;
+ int64_t cache_response_body_bytes = 0;
+ int pushed_response_hdr_bytes = 0;
+ int64_t pushed_response_body_bytes = 0;
+ bool client_tcp_reused = false;
+ bool client_ssl_reused = false;
+ bool client_connection_is_ssl = false;
+ bool is_internal = false;
+ bool server_connection_is_ssl = false;
+ int server_connection_provided_cert = 0;
Review comment:
This likely introduces unnecessary padding. It'd be much better to move the int member to be before, or after, all the other bool's.
----------------------------------------------------------------
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] ywkaras commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r414059248
##########
File path: proxy/logging/LogAccess.cc
##########
@@ -1298,6 +1298,44 @@ LogAccess::marshal_client_sni_server_name(char *buf)
return len;
}
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+int
+LogAccess::marshal_client_provided_cert(char *buf)
+{
+ int provided_cert = 0;
+ if (m_http_sm) {
+ auto txn = m_http_sm->get_ua_txn();
+ if (txn) {
+ auto ssn = txn->get_proxy_ssn();
+ if (ssn) {
+ auto ssl = ssn->ssl();
+ if (ssl) {
+ provided_cert = ssl->client_provided_certificate();
Review comment:
In the past, code similar to this has caused segment violations, due to race conditions between the destruction times of associated objects.
----------------------------------------------------------------
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] ywkaras commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r414090649
##########
File path: tests/gold_tests/tls/tls_client_cert.test.py
##########
@@ -293,3 +306,21 @@ def done_reload(process, hasRunFor, **kw):
tr4fail.Processes.Default.Command = 'curl -H host:example.com http://127.0.0.1:{0}/case2'.format(ts.Variables.port)
tr4fail.Processes.Default.ReturnCode = 0
tr4fail.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Check response")
+
+# Parking this as a ready tester on a meaningless process
+# Stall the test runs until the squid.log file has appeared
+def access_log_ready(tsenv):
+ def done_access_log(process, hasRunFor, **kw):
+ cmd = "test -f {0}".format(ts.Disk.squid_log)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ return retval.returncode == 0
+
+ return done_access_log
+
+tr = Test.AddTestRun("Wait for the access log to write out")
+tr.Processes.Default.StartBefore(server4, ready=access_log_ready(ts.Env))
+tr.StillRunningAfter = ts
+tr.Processes.Default.Command = 'ls'
Review comment:
Nitpick: I'd use 'echo "log file exists"' or such instead of 'ls'.
----------------------------------------------------------------
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 #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#issuecomment-655171182
Cherry-picked to v9.0.x branch.
----------------------------------------------------------------
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] shinrich commented on pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#issuecomment-641657565
Pushed a new commit to address comments from @zwoop @randall and @ywkaras
----------------------------------------------------------------
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 #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r437806428
##########
File path: iocore/net/SSLClientUtils.cc
##########
@@ -138,6 +138,20 @@ verify_callback(int signature_ok, X509_STORE_CTX *ctx)
return true;
}
+static int
+ssl_client_cert_callback(SSL *ssl, void * /*arg*/)
+{
+ SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
Review comment:
A new callback function just for SSLNetVC 😢 Please create an issue for QUIC.
----------------------------------------------------------------
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] randall commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r420169114
##########
File path: tests/gold_tests/tls/tls_client_cert2.test.py
##########
@@ -170,3 +185,21 @@
trfail.Processes.Default.Command = 'curl -H host:random.foo.com http://127.0.0.1:{0}/case1'.format(ts.Variables.port)
trfail.Processes.Default.ReturnCode = 0
trfail.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Check response")
+
+# Parking this as a ready tester on a meaningless process
+# Stall the test runs until the squid.log file has appeared
+def access_log_ready(tsenv):
Review comment:
indentation is not a multiple of four
##########
File path: tests/gold_tests/tls/tls_client_cert.test.py
##########
@@ -293,3 +306,21 @@ def done_reload(process, hasRunFor, **kw):
tr4fail.Processes.Default.Command = 'curl -H host:example.com http://127.0.0.1:{0}/case2'.format(ts.Variables.port)
tr4fail.Processes.Default.ReturnCode = 0
tr4fail.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Check response")
+
+# Parking this as a ready tester on a meaningless process
+# Stall the test runs until the squid.log file has appeared
+def access_log_ready(tsenv):
+ def done_access_log(process, hasRunFor, **kw):
Review comment:
indentation is not a multiple of four
##########
File path: tests/gold_tests/tls/tls_client_verify.test.py
##########
@@ -168,19 +184,39 @@
tr.Setup.Copy("ssl/server.key")
tr.StillRunningAfter = ts
tr.StillRunningAfter = server
-tr.Processes.Default.Command = "curl --tls-max 1.2 -k --cert ./server.pem --key ./server.key --resolve 'bar.com:{0}:127.0.0.1' https://bar.com:{0}/case1".format(ts.Variables.ssl_port)
+tr.Processes.Default.Command = "curl --tls-max 1.2 -k --cert ./server.pem --key ./server.key --resolve 'bar.com:{0}:127.0.0.1' https://bar.com:{0}/case12".format(ts.Variables.ssl_port)
tr.Processes.Default.ReturnCode = 35
# Test that the fqdn's match completely. bob.com should require client certificate. bob.com.com should not
tr = Test.AddTestRun("Connect to bob.com without cert, should fail")
tr.StillRunningAfter = ts
tr.StillRunningAfter = server
-tr.Processes.Default.Command = "curl --tls-max 1.2 -k --resolve 'bob.com:{0}:127.0.0.1' https://bob.com:{0}/case1".format(ts.Variables.ssl_port)
+tr.Processes.Default.Command = "curl --tls-max 1.2 -k --resolve 'bob.com:{0}:127.0.0.1' https://bob.com:{0}/case13".format(ts.Variables.ssl_port)
tr.Processes.Default.ReturnCode = 35
tr = Test.AddTestRun("Connect to bob.com.com without cert, should succeed")
tr.StillRunningAfter = ts
tr.StillRunningAfter = server
-tr.Processes.Default.Command = "curl --tls-max 1.2 -k --resolve 'bob.com.com:{0}:127.0.0.1' https://bob.com.com:{0}/case1".format(ts.Variables.ssl_port)
+tr.Processes.Default.Command = "curl --tls-max 1.2 -k --resolve 'bob.com.com:{0}:127.0.0.1' https://bob.com.com:{0}/case14".format(ts.Variables.ssl_port)
tr.Processes.Default.ReturnCode = 0
+
+# Parking this as a ready tester on a meaningless process
+# Stall the test runs until the squid.log file has appeared
+def access_log_ready(tsenv):
Review comment:
indentation is not a multiple of four.
also, can this be moved into a separate file for use in the 3 places that use it?
----------------------------------------------------------------
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] ywkaras commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r414092885
##########
File path: tests/gold_tests/tls/tls_client_cert.test.py
##########
@@ -293,3 +306,21 @@ def done_reload(process, hasRunFor, **kw):
tr4fail.Processes.Default.Command = 'curl -H host:example.com http://127.0.0.1:{0}/case2'.format(ts.Variables.port)
tr4fail.Processes.Default.ReturnCode = 0
tr4fail.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Check response")
+
+# Parking this as a ready tester on a meaningless process
+# Stall the test runs until the squid.log file has appeared
+def access_log_ready(tsenv):
+ def done_access_log(process, hasRunFor, **kw):
+ cmd = "test -f {0}".format(ts.Disk.squid_log)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ return retval.returncode == 0
+
+ return done_access_log
+
+tr = Test.AddTestRun("Wait for the access log to write out")
+tr.Processes.Default.StartBefore(server4, ready=access_log_ready(ts.Env))
+tr.StillRunningAfter = ts
+tr.Processes.Default.Command = 'ls'
Review comment:
Or actually how about 'sleep 1 ; echo "log file ready"' as a failsafe to ensure that TS has finished writing the log file before Au test compares it to the gold file.
----------------------------------------------------------------
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] ywkaras commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r415939863
##########
File path: tests/gold_tests/tls/tls_client_cert.test.py
##########
@@ -293,3 +306,21 @@ def done_reload(process, hasRunFor, **kw):
tr4fail.Processes.Default.Command = 'curl -H host:example.com http://127.0.0.1:{0}/case2'.format(ts.Variables.port)
tr4fail.Processes.Default.ReturnCode = 0
tr4fail.Processes.Default.Streams.stdout = Testers.ContainsExpression("Could Not Connect", "Check response")
+
+# Parking this as a ready tester on a meaningless process
+# Stall the test runs until the squid.log file has appeared
+def access_log_ready(tsenv):
+ def done_access_log(process, hasRunFor, **kw):
+ cmd = "test -f {0}".format(ts.Disk.squid_log)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ return retval.returncode == 0
+
+ return done_access_log
+
+tr = Test.AddTestRun("Wait for the access log to write out")
+tr.Processes.Default.StartBefore(server4, ready=access_log_ready(ts.Env))
+tr.StillRunningAfter = ts
+tr.Processes.Default.Command = 'ls'
Review comment:
Hmm seems questionable that creating a directory entry and writing the file would be a single atomic filesystem op. But, since this is Python, I suppose the delay between seeing the file exists and actually reading it should be sufficient for the writing to be complete.
----------------------------------------------------------------
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] shinrich edited a comment on pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich edited a comment on pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#issuecomment-642252716
To merge this in to another branch you also need to merge in 15e749ea4cf3b5e66ffd0ad53d20fa4c611e2ca5 (PR #6616). The later test fixups rely on the file there.
----------------------------------------------------------------
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] shinrich merged pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich merged pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699
----------------------------------------------------------------
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] randall commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r444518072
##########
File path: tests/gold_tests/autest-site/when.test.ext
##########
@@ -34,3 +35,20 @@ def FileContains(haystack, needle):
AddWhenFunction(FileContains)
+
+def FilePresent(tsenv, file):
+ cmd = "test -f {0}".format(file)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ return retval.returncode == 0
+
+AddWhenFunction(FilePresent)
+
+def SNIReloadDone(tsenv, diags_file):
+ cmd = "grep 'sni.yaml finished loading' {0} | wc -l | sed -e 's/ //g'> ./test.out".format(diags_file)
+ retval = subprocess.run(cmd, shell=True, env=tsenv)
+ if retval.returncode == 0:
+ cmd ="if [ -f ./test.out -a \"`cat ./test.out`\" = \"2\" ] ; then true; else false; fi"
Review comment:
like ?
```
cmd = 'if [ -f ./test.out -a "`cat ./test.out`" = "2" ] ; then true; else false; fi'
```
----------------------------------------------------------------
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] ywkaras commented on a change in pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#discussion_r414059248
##########
File path: proxy/logging/LogAccess.cc
##########
@@ -1298,6 +1298,44 @@ LogAccess::marshal_client_sni_server_name(char *buf)
return len;
}
+/*-------------------------------------------------------------------------
+ -------------------------------------------------------------------------*/
+int
+LogAccess::marshal_client_provided_cert(char *buf)
+{
+ int provided_cert = 0;
+ if (m_http_sm) {
+ auto txn = m_http_sm->get_ua_txn();
+ if (txn) {
+ auto ssn = txn->get_proxy_ssn();
+ if (ssn) {
+ auto ssl = ssn->ssl();
+ if (ssl) {
+ provided_cert = ssl->client_provided_certificate();
Review comment:
In the past, code similar to this has caused segment violations, due to race conditions between the destruction times of associated objects.
----------------------------------------------------------------
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] shinrich commented on pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#issuecomment-651160379
Rebased to fix conflicts and used the FileContains added in PR #6931 instead of the SNIReload
----------------------------------------------------------------
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] shinrich commented on pull request #6699: Log whether client certs were exchanged in TLS handshake
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #6699:
URL: https://github.com/apache/trafficserver/pull/6699#issuecomment-642252716
To merge this in you also need to merge in 15e749ea4cf3b5e66ffd0ad53d20fa4c611e2ca5. The later test fixups rely on the file there.
----------------------------------------------------------------
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