[trafficserver] branch master updated: Fixed bug in the calculation of the header block fragment length (#6923)
This is an automated email from the ASF dual-hosted git repository.
bcall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 4817786 Fixed bug in the calculation of the header block fragment
length (#6923)
4817786 is described below
commit 481778679af293ff5558ed965481af683f703531
Author: Bryan Call
AuthorDate: Mon Jun 22 16:25:51 2020 -0700
Fixed bug in the calculation of the header block fragment length (#6923)
Co-authored-by: Masaori Koshiba
---
proxy/http2/HPACK.cc| 6 -
proxy/http2/Http2ConnectionState.cc | 49 +
proxy/http2/Http2Stream.h | 6 ++---
3 files changed, 35 insertions(+), 26 deletions(-)
diff --git a/proxy/http2/HPACK.cc b/proxy/http2/HPACK.cc
index 774ed2e..2680600 100644
--- a/proxy/http2/HPACK.cc
+++ b/proxy/http2/HPACK.cc
@@ -847,7 +847,11 @@ hpack_decode_header_block(HpackIndexingTable
_table, HTTPHdr *hdr, cons
field->name_get(_len);
field->value_get(_len);
-total_header_size += name_len + value_len;
+
+// [RFC 7540] 6.5.2. SETTINGS_MAX_HEADER_LIST_SIZE:
+// The value is based on the uncompressed size of header fields, including
the length of the name and value in octets plus an
+// overhead of 32 octets for each header field.
+total_header_size += name_len + value_len + ADDITIONAL_OCTETS;
if (total_header_size > max_header_size) {
return HPACK_ERROR_SIZE_EXCEEDED_ERROR;
diff --git a/proxy/http2/Http2ConnectionState.cc
b/proxy/http2/Http2ConnectionState.cc
index d2a1cc2..5b2e6d5 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -260,13 +260,6 @@ rcv_headers_frame(Http2ConnectionState , const
Http2Frame )
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_NONE);
}
- // keep track of how many bytes we get in the frame
- stream->request_header_length += payload_length;
- if (stream->request_header_length > Http2::max_header_list_size) {
-return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_STREAM,
Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
- "recv headers payload for headers greater than header
length");
- }
-
Http2HeadersParameter params;
uint32_t header_block_fragment_offset = 0;
uint32_t header_block_fragment_length = payload_length;
@@ -285,7 +278,8 @@ rcv_headers_frame(Http2ConnectionState , const
Http2Frame )
"recv headers failed to parse");
}
-if (params.pad_length > payload_length) {
+// Payload length can't be smaller than the pad length
+if ((params.pad_length + HTTP2_HEADERS_PADLEN_LEN) >
header_block_fragment_length) {
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION,
Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
"recv headers pad > payload length");
}
@@ -301,7 +295,7 @@ rcv_headers_frame(Http2ConnectionState , const
Http2Frame )
frame.reader()->memcpy(buf, HTTP2_PRIORITY_LEN,
header_block_fragment_offset);
if (!http2_parse_priority_parameter(make_iovec(buf, HTTP2_PRIORITY_LEN),
params.priority)) {
return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION,
Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
-"recv headers prioirity parameters failed parse");
+"recv headers priority parameters failed parse");
}
// Protocol error if the stream depends on itself
if (stream_id == params.priority.stream_dependency) {
@@ -309,6 +303,12 @@ rcv_headers_frame(Http2ConnectionState , const
Http2Frame )
"recv headers self dependency");
}
+// Payload length can't be smaller than the priority length
+if (HTTP2_PRIORITY_LEN > header_block_fragment_length) {
+ return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION,
Http2ErrorCode::HTTP2_ERROR_PROTOCOL_ERROR,
+"recv priority length > payload length");
+}
+
header_block_fragment_offset += HTTP2_PRIORITY_LEN;
header_block_fragment_length -= HTTP2_PRIORITY_LEN;
}
@@ -328,11 +328,19 @@ rcv_headers_frame(Http2ConnectionState , const
Http2Frame )
}
}
+ stream->header_blocks_length = header_block_fragment_length;
+
+ // ATS advertises SETTINGS_MAX_HEADER_LIST_SIZE as a limit of total header
blocks length. (Details in [RFC 7560] 10.5.1.)
+ // Make it double to relax the limit in cases of 1) HPACK is used naively,
or 2) Huffman Encoding generates large header blocks.
+ // The total "decoded" header length is strictly checked by
hpack_decode_header_block().
+ if (stream->header_blocks_length > std::max(Http2::max_header_list_size,
Http2::max_header_list_size * 2)) {
+return Http2Error(Http2ErrorClass::HTTP2_ERROR_CLASS_CONNECTION,
Http2ErrorCode::HTTP2_ERROR_ENHANCE_YOUR_CALM,
+
[trafficserver] branch master updated: Disable max_connections_active_in default now that featur works (#6903)
This is an automated email from the ASF dual-hosted git repository.
shinrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 54cff07 Disable max_connections_active_in default now that featur
works (#6903)
54cff07 is described below
commit 54cff079598f67e01a80a3badb9f7c772562df74
Author: Susan Hinrichs
AuthorDate: Mon Jun 22 17:54:13 2020 -0500
Disable max_connections_active_in default now that featur works (#6903)
Co-authored-by: Susan Hinrichs
---
iocore/net/P_UnixNet.h | 2 +-
iocore/net/UnixNet.cc | 11 ---
mgmt/RecordsConfig.cc | 2 +-
proxy/http2/Http2ClientSession.cc | 1 +
tests/gold_tests/h2/h2active_timeout.py | 29 +
tests/gold_tests/h2/http2.test.py | 2 +-
6 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/iocore/net/P_UnixNet.h b/iocore/net/P_UnixNet.h
index 1e61672..d369284 100644
--- a/iocore/net/P_UnixNet.h
+++ b/iocore/net/P_UnixNet.h
@@ -319,7 +319,7 @@ public:
void process_enabled_list();
void process_ready_list();
void manage_keep_alive_queue();
- bool manage_active_queue(bool ignore_queue_size);
+ bool manage_active_queue(NetEvent *ne, bool ignore_queue_size);
void add_to_keep_alive_queue(NetEvent *ne);
void remove_from_keep_alive_queue(NetEvent *ne);
bool add_to_active_queue(NetEvent *ne);
diff --git a/iocore/net/UnixNet.cc b/iocore/net/UnixNet.cc
index 653b675..f368e07 100644
--- a/iocore/net/UnixNet.cc
+++ b/iocore/net/UnixNet.cc
@@ -111,7 +111,7 @@ public:
// Therefore we don't have to check all the NetEvents as much as open_list.
// Cleanup the active and keep-alive queues periodically
-nh.manage_active_queue(true); // close any connections over the active
timeout
+nh.manage_active_queue(nullptr, true); // close any connections over the
active timeout
nh.manage_keep_alive_queue();
return 0;
@@ -565,7 +565,7 @@ NetHandler::signalActivity()
}
bool
-NetHandler::manage_active_queue(bool ignore_queue_size = false)
+NetHandler::manage_active_queue(NetEvent *enabling_ne, bool ignore_queue_size
= false)
{
const int total_connections_in = active_queue_size + keep_alive_queue_size;
Debug("v_net_queue",
@@ -594,6 +594,11 @@ NetHandler::manage_active_queue(bool ignore_queue_size =
false)
int total_idle_count = 0;
for (; ne != nullptr; ne = ne_next) {
ne_next = ne->active_queue_link.next;
+// It seems dangerous closing the current ne at this point
+// Let the activity_cop deal with it
+if (ne == enabling_ne) {
+ continue;
+}
if ((ne->inactivity_timeout_in && ne->next_inactivity_timeout_at <= now) ||
(ne->active_timeout_in && ne->next_activity_timeout_at <= now)) {
_close_ne(ne, now, handle_event, closed, total_idle_time,
total_idle_count);
@@ -741,7 +746,7 @@ NetHandler::add_to_active_queue(NetEvent *ne)
bool active_queue_full = false;
// if active queue is over size then close inactive connections
- if (manage_active_queue() == false) {
+ if (manage_active_queue(ne) == false) {
active_queue_full = true;
}
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index 325cbf1..23cb774 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -400,7 +400,7 @@ static const RecordElement RecordsConfig[] =
,
{RECT_CONFIG, "proxy.config.net.max_connections_in", RECD_INT, "3",
RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
- {RECT_CONFIG, "proxy.config.net.max_connections_active_in", RECD_INT,
"1", RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
+ {RECT_CONFIG, "proxy.config.net.max_connections_active_in", RECD_INT, "0",
RECU_DYNAMIC, RR_NULL, RECC_STR, "^[0-9]+$", RECA_NULL}
,
// ###
diff --git a/proxy/http2/Http2ClientSession.cc
b/proxy/http2/Http2ClientSession.cc
index 638679f..8045883 100644
--- a/proxy/http2/Http2ClientSession.cc
+++ b/proxy/http2/Http2ClientSession.cc
@@ -346,6 +346,7 @@ Http2ClientSession::main_event_handler(int event, void
*edata)
case VC_EVENT_INACTIVITY_TIMEOUT:
case VC_EVENT_ERROR:
case VC_EVENT_EOS:
+Http2SsnDebug("Closing event %d", event);
this->set_dying_event(event);
this->do_io_close();
if (_vc != nullptr) {
diff --git a/tests/gold_tests/h2/h2active_timeout.py
b/tests/gold_tests/h2/h2active_timeout.py
index aa79cb1..d2f47f2 100644
--- a/tests/gold_tests/h2/h2active_timeout.py
+++ b/tests/gold_tests/h2/h2active_timeout.py
@@ -22,23 +22,25 @@ import argparse
import time
-def makerequest(port):
+def makerequest(port, active_timeout):
hyper.tls._context = hyper.tls.init_context()
hyper.tls._context.check_hostname = False
hyper.tls._context.verify_mode = hyper.compat.ssl.CERT_NONE
conn =
[trafficserver] branch master updated: Make compress Au test less flakey. (#6915)
This is an automated email from the ASF dual-hosted git repository. shinrich pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git The following commit(s) were added to refs/heads/master by this push: new 74366b9 Make compress Au test less flakey. (#6915) 74366b9 is described below commit 74366b963fd18e97d80e9c91681fe232678d6faf Author: Walt Karas AuthorDate: Mon Jun 22 17:51:23 2020 -0500 Make compress Au test less flakey. (#6915) --- tests/gold_tests/pluginTest/compress/compress.gold | 42 +++--- .../pluginTest/compress/compress.test.py | 11 -- tests/gold_tests/pluginTest/compress/greplog.sh| 2 +- 3 files changed, 30 insertions(+), 25 deletions(-) diff --git a/tests/gold_tests/pluginTest/compress/compress.gold b/tests/gold_tests/pluginTest/compress/compress.gold index ff02ed6..05c8eb5 100644 --- a/tests/gold_tests/pluginTest/compress/compress.gold +++ b/tests/gold_tests/pluginTest/compress/compress.gold @@ -6,7 +6,7 @@ < Content-Encoding: br < Vary: Accept-Encoding < Content-Length: 46 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip > Accept-Encoding: gzip @@ -15,7 +15,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/br > Accept-Encoding: br @@ -24,14 +24,14 @@ < Content-Encoding: br < Vary: Accept-Encoding < Content-Length: 46 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/deflate > Accept-Encoding: deflate < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 - +=== > GET http://ae-1/obj1 HTTP/1.1 > X-Ats-Compress-Test: 1/gzip, deflate, sdch, br > Accept-Encoding: gzip, deflate, sdch, br @@ -40,7 +40,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-1/obj1 HTTP/1.1 > X-Ats-Compress-Test: 1/gzip > Accept-Encoding: gzip @@ -49,21 +49,21 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-1/obj1 HTTP/1.1 > X-Ats-Compress-Test: 1/br > Accept-Encoding: br < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 - +=== > GET http://ae-1/obj1 HTTP/1.1 > X-Ats-Compress-Test: 1/deflate > Accept-Encoding: deflate < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 - +=== > GET http://ae-2/obj2 HTTP/1.1 > X-Ats-Compress-Test: 2/gzip, deflate, sdch, br > Accept-Encoding: gzip, deflate, sdch, br @@ -72,7 +72,7 @@ < Content-Encoding: br < Vary: Accept-Encoding < Content-Length: 46 - +=== > GET http://ae-2/obj2 HTTP/1.1 > X-Ats-Compress-Test: 2/gzip > Accept-Encoding: gzip @@ -81,7 +81,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-2/obj2 HTTP/1.1 > X-Ats-Compress-Test: 2/br > Accept-Encoding: br @@ -90,14 +90,14 @@ < Content-Encoding: br < Vary: Accept-Encoding < Content-Length: 46 - +=== > GET http://ae-2/obj2 HTTP/1.1 > X-Ats-Compress-Test: 2/deflate > Accept-Encoding: deflate < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=0.666 > Accept-Encoding: gzip;q=0.666 @@ -106,7 +106,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=0.666x > Accept-Encoding: gzip;q=0.666x @@ -115,7 +115,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=#0.666 > Accept-Encoding: gzip;q=#0.666 @@ -124,7 +124,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip; Q = 0.666 > Accept-Encoding: gzip; Q = 0.666 @@ -133,14 +133,14 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=0.0 > Accept-Encoding: gzip;q=0.0 < HTTP/1.1 200 OK < Content-Type: text/javascript < Content-Length: 1049 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/gzip;q=-0.1 > Accept-Encoding: gzip;q=-0.1 @@ -149,7 +149,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/aaa, gzip;q=0.666, bbb > Accept-Encoding: aaa, gzip;q=0.666, bbb @@ -158,7 +158,7 @@ < Content-Encoding: gzip < Vary: Accept-Encoding < Content-Length: 71 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/ br ; q=0.666, bbb > Accept-Encoding: br ; q=0.666, bbb @@ -167,7 +167,7 @@ < Content-Encoding: br < Vary: Accept-Encoding < Content-Length: 46 - +=== > GET http://ae-0/obj0 HTTP/1.1 > X-Ats-Compress-Test: 0/aaa, gzip;q=0.666 , > Accept-Encoding: aaa, gzip;q=0.666 , @@ -176,4 +176,4 @@ <
[trafficserver] branch master updated (9ae1a76 -> bfffcf0)
This is an automated email from the ASF dual-hosted git repository. bcall pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git. from 9ae1a76 example: Move to blocklists and allowlists add bfffcf0 Enable only squash and merge for GitHub No new revisions were added by this update. Summary of changes: .asf.yaml | 5 + 1 file changed, 5 insertions(+)
[trafficserver] branch master updated: example: Move to blocklists and allowlists
This is an automated email from the ASF dual-hosted git repository.
rrm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 9ae1a76 example: Move to blocklists and allowlists
9ae1a76 is described below
commit 9ae1a7666ae0419173894b1a11c16f74f0541878
Author: Randall Meyer
AuthorDate: Thu Jun 18 09:04:37 2020 -0700
example: Move to blocklists and allowlists
This change renames the blacklist* to blocklist* and ssl_sni_whitelist
to ssl_sni_allowlist
---
example/plugins/c-api/Makefile.am | 12
example/plugins/c-api/blacklist_1/readme.txt | 17 --
.../blacklist_0.c => blocklist_0/blocklist_0.c}| 12
.../blacklist.txt => blocklist_1/blocklist.txt}| 0
.../blacklist_1.c => blocklist_1/blocklist_1.c}| 36 +++---
example/plugins/c-api/blocklist_1/readme.txt | 17 ++
.../ssl_sni_allowlist.cc} | 8 ++---
7 files changed, 51 insertions(+), 51 deletions(-)
diff --git a/example/plugins/c-api/Makefile.am
b/example/plugins/c-api/Makefile.am
index b63f81e..97abebb 100644
--- a/example/plugins/c-api/Makefile.am
+++ b/example/plugins/c-api/Makefile.am
@@ -26,8 +26,8 @@ example_Plugins = \
add_header.la \
append_transform.la \
basic_auth.la \
- blacklist_0.la \
- blacklist_1.la \
+ blocklist_0.la \
+ blocklist_1.la \
bnull_transform.la \
cert_update.la \
request_buffer.la \
@@ -54,7 +54,7 @@ example_Plugins = \
server_transform.la \
session_hooks.la \
ssl_preaccept.la \
- ssl_sni_whitelist.la \
+ ssl_sni_allowlist.la \
ssl_sni.la \
statistic.la \
thread_1.la \
@@ -71,8 +71,8 @@ endif
add_header_la_SOURCES = add_header/add_header.c
append_transform_la_SOURCES = append_transform/append_transform.c
basic_auth_la_SOURCES = basic_auth/basic_auth.c
-blacklist_0_la_SOURCES = blacklist_0/blacklist_0.c
-blacklist_1_la_SOURCES = blacklist_1/blacklist_1.c
+blocklist_0_la_SOURCES = blocklist_0/blocklist_0.c
+blocklist_1_la_SOURCES = blocklist_1/blocklist_1.c
bnull_transform_la_SOURCES = bnull_transform/bnull_transform.c
cert_update_la_SOURCES = cert_update/cert_update.cc
request_buffer_la_SOURCES = request_buffer/request_buffer.c
@@ -98,7 +98,7 @@ server_push_la_SOURCES = server_push/server_push.c
server_transform_la_SOURCES = server_transform/server_transform.c
ssl_preaccept_la_SOURCES = ssl_preaccept/ssl_preaccept.cc
ssl_sni_la_SOURCES = ssl_sni/ssl_sni.cc
-ssl_sni_whitelist_la_SOURCES = ssl_sni_whitelist/ssl_sni_whitelist.cc
+ssl_sni_allowlist_la_SOURCES = ssl_sni_allowlist/ssl_sni_allowlist.cc
disable_http2_la_SOURCES = disable_http2/disable_http2.cc
verify_cert_la_SOURCES = verify_cert/verify_cert.cc
statistic_la_SOURCES = statistic/statistic.cc
diff --git a/example/plugins/c-api/blacklist_1/readme.txt
b/example/plugins/c-api/blacklist_1/readme.txt
deleted file mode 100644
index 0e7cf27..000
--- a/example/plugins/c-api/blacklist_1/readme.txt
+++ /dev/null
@@ -1,17 +0,0 @@
-How to run the blacklist plugin
-===
-
-1. Modify blacklist.cgi to specify the location of perl and traffic server.
-2. Copy blacklist.cgi, blacklist_1.so, PoweredByInktomi.gif to the directory
- specified by the variable proxy.config.plugin.plugin_dir.
-3. Modify plugin.config to load the blacklist plugin.
-
-
-
-About the blacklist plugin
-==
-
-The blacklist plugin allows Traffic Server to compare all incoming request
-origin servers with a blacklisted set of web servers. If the requested origin
-server is blacklisted, Traffic Server sends the client a message saying that
-access is denied.
diff --git a/example/plugins/c-api/blacklist_0/blacklist_0.c
b/example/plugins/c-api/blocklist_0/blocklist_0.c
similarity index 93%
rename from example/plugins/c-api/blacklist_0/blacklist_0.c
rename to example/plugins/c-api/blocklist_0/blocklist_0.c
index 5ca5179..a7da67c 100644
--- a/example/plugins/c-api/blacklist_0/blacklist_0.c
+++ b/example/plugins/c-api/blocklist_0/blocklist_0.c
@@ -22,8 +22,8 @@
*/
/*
- * blacklist_0.c:
- * original version of blacklist-1, now used for internal testing
+ * blocklist_0.c:
+ * original version of blocklist-1, now used for internal testing
*
*
* Usage:
@@ -34,7 +34,7 @@
#include
#include
-#define PLUGIN_NAME "blacklist_0"
+#define PLUGIN_NAME "blocklist_0"
static char **sites;
static int nsites;
@@ -69,7 +69,7 @@ handle_dns(TSHttpTxn txnp, TSCont contp)
}
for (i = 0; i < nsites; i++) {
if (strncmp(host, sites[i], host_length) == 0) {
- printf("blacklisting site: %s\n", sites[i]);
+ printf("blocklisting site: %s\n", sites[i]);
TSHttpTxnHookAdd(txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK, contp);
[trafficserver] branch master updated: Fix dual_cert_select test to run with older openssl binary (#6896)
This is an automated email from the ASF dual-hosted git repository.
shinrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new ae9af29 Fix dual_cert_select test to run with older openssl binary
(#6896)
ae9af29 is described below
commit ae9af297279f2c386d550f69fb731ff9fc43ef3a
Author: Susan Hinrichs
AuthorDate: Mon Jun 22 08:53:25 2020 -0500
Fix dual_cert_select test to run with older openssl binary (#6896)
Co-authored-by: Susan Hinrichs
---
.../tls/tls_check_dual_cert_selection.test.py | 46 ++
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/tests/gold_tests/tls/tls_check_dual_cert_selection.test.py
b/tests/gold_tests/tls/tls_check_dual_cert_selection.test.py
index bc2682a..633144a 100644
--- a/tests/gold_tests/tls/tls_check_dual_cert_selection.test.py
+++ b/tests/gold_tests/tls/tls_check_dual_cert_selection.test.py
@@ -20,7 +20,8 @@ Test.Summary = '''
Test ATS offering both RSA and EC certificates
'''
-Test.SkipUnless(Condition.HasOpenSSLVersion('1.1.1'))
+import os
+import re
# Define default ATS
ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
@@ -59,7 +60,7 @@ ts.Disk.ssl_multicert_config.AddLines([
ts.Disk.records_config.update({
'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
'proxy.config.ssl.server.private_key.path':
'{0}'.format(ts.Variables.SSLDir),
-'proxy.config.ssl.server.cipher_suite':
'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384:AES128-GCM-SHA256:AES256-GCM-SHA384:ECDHE-RSA-RC4-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:RC4-SHA:RC4-MD5:AES128-SHA:AES256-SHA:DES-CBC3-SHA!SRP:!DSS:!PSK:!aNULL:!eNULL:!SSLv2',
+'proxy.config.ssl.server.cipher_suite':
'ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256',
'proxy.config.url_remap.pristine_host_hdr': 1,
'proxy.config.dns.nameservers': '127.0.0.1:{0}'.format(dns.Variables.Port),
'proxy.config.exec_thread.autoconfig.scale': 1.0,
@@ -71,59 +72,72 @@ ts.Disk.records_config.update({
dns.addRecords(records={"foo.com.": ["127.0.0.1"]})
dns.addRecords(records={"bar.com.": ["127.0.0.1"]})
-# Should receive a EC cert
+foo_ec_string = ""
+foo_rsa_string = ""
+san_ec_string = ""
+san_rsa_string = ""
+with open(os.path.join(Test.TestDirectory,'ssl', 'signed-foo-ec.pem'), 'r') as
myfile:
+foo_ec_string = re.escape(myfile.read())
+with open(os.path.join(Test.TestDirectory,'ssl', 'signed-foo.pem'), 'r') as
myfile:
+foo_rsa_string = re.escape(myfile.read())
+with open(os.path.join(Test.TestDirectory,'ssl', 'signed-san-ec.pem'), 'r') as
myfile:
+san_ec_string = re.escape(myfile.read())
+with open(os.path.join(Test.TestDirectory,'ssl', 'signed-san.pem'), 'r') as
myfile:
+san_rsa_string = re.escape(myfile.read())
+
+# Should receive a EC cert since ATS cipher list prefers EC
tr = Test.AddTestRun("Default for foo should return EC cert")
tr.Setup.Copy("ssl/signer.pem")
-tr.Processes.Default.Command = "echo foo | openssl s_client -servername
foo.com -connect 127.0.0.1:{0}".format(ts.Variables.ssl_port)
+tr.Processes.Default.Command = "echo foo | openssl s_client -tls1_2
-servername foo.com -connect 127.0.0.1:{0}".format(ts.Variables.ssl_port,
foo_ec_string)
tr.ReturnCode = 0
tr.Processes.Default.StartBefore(server)
tr.Processes.Default.StartBefore(dns)
tr.Processes.Default.StartBefore(Test.Processes.ts,
ready=When.PortOpen(ts.Variables.ssl_port))
tr.StillRunningAfter = server
tr.StillRunningAfter = ts
-tr.Processes.Default.Streams.All += Testers.ContainsExpression("Peer signature
type: ECDSA", "Should select EC cert")
+tr.Processes.Default.Streams.All += Testers.ContainsExpression(foo_ec_string,
"Should select EC cert",reflags=re.S | re.M)
# Should receive a RSA cert
tr = Test.AddTestRun("Only offer RSA ciphers, should receive RSA cert")
-tr.Processes.Default.Command = "echo foo | openssl s_client -servername
foo.com -sigalgs 'RSA-PSS+SHA256' -connect
127.0.0.1:{0}".format(ts.Variables.ssl_port)
+tr.Processes.Default.Command = "echo foo | openssl s_client -tls1_2
-servername foo.com -cipher 'ECDHE-RSA-AES128-GCM-SHA256' -connect
127.0.0.1:{0}".format(ts.Variables.ssl_port)
tr.ReturnCode = 0
tr.StillRunningAfter = server
tr.StillRunningAfter = ts
-tr.Processes.Default.Streams.All += Testers.ContainsExpression("Peer signature
type: RSA-PSS", "Should select RSA cert")
+tr.Processes.Default.Streams.All += Testers.ContainsExpression(foo_rsa_string,
"Should select RSA cert",reflags=re.S | re.M)
# Should receive a EC cert
-tr = Test.AddTestRun("Default for one.com should return EC cert")
-tr.Processes.Default.Command = "echo foo | openssl s_client -servername
one.com -connect 127.0.0.1:{0}".format(ts.Variables.ssl_port)
+tr = Test.AddTestRun("Default for two.com should return EC
[trafficserver] branch master updated: Prevent stale netvc access on SSL Callbacks (#6925)
This is an automated email from the ASF dual-hosted git repository.
sudheerv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new c7e8054 Prevent stale netvc access on SSL Callbacks (#6925)
c7e8054 is described below
commit c7e80542aa1a5323399226f636ef196955f60791
Author: Sudheer Vinukonda
AuthorDate: Mon Jun 22 05:44:47 2020 -0700
Prevent stale netvc access on SSL Callbacks (#6925)
Since SSL Callbacks are asynchronous in nature, it's possible the
associated NetVC is already freed causing a potential use-after-free
problem.
---
iocore/net/SSLNetVConnection.cc | 4 ++--
iocore/net/SSLUtils.cc | 36 +++-
2 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index 8d19880..5209a93 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -1505,7 +1505,7 @@ SSLNetVConnection::advertise_next_protocol(SSL *ssl,
const unsigned char **out,
{
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
- ink_release_assert(netvc != nullptr);
+ ink_release_assert(netvc && netvc->ssl == ssl);
if (netvc->getNPN(out, outlen)) {
// Successful return tells OpenSSL to advertise.
@@ -1522,7 +1522,7 @@ SSLNetVConnection::select_next_protocol(SSL *ssl, const
unsigned char **out, uns
{
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
- ink_release_assert(netvc != nullptr);
+ ink_release_assert(netvc && netvc->ssl == ssl);
const unsigned char *npnptr = nullptr;
unsigned int npnsize= 0;
if (netvc->getNPN(, )) {
diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
index 9387a65..56fae1d 100644
--- a/iocore/net/SSLUtils.cc
+++ b/iocore/net/SSLUtils.cc
@@ -250,6 +250,12 @@ set_context_cert(SSL *ssl)
bool found = true;
int retval = 1;
+ if (!netvc || netvc->ssl != ssl) {
+Debug("ssl.error", "set_context_cert call back on stale netvc");
+retval = 0; // Error
+goto done;
+ }
+
Debug("ssl", "set_context_cert ssl=%p server=%s handshake_complete=%d", ssl,
servername, netvc->getSSLHandShakeComplete());
// catch the client renegotiation early on
@@ -317,6 +323,11 @@ ssl_verify_client_callback(int preverify_ok,
X509_STORE_CTX *ctx)
auto *ssl= static_cast(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
+ if (!netvc || netvc->ssl != ssl) {
+Debug("ssl.error", "ssl_verify_client_callback call back on stale netvc");
+return false;
+ }
+
netvc->set_verify_cert(ctx);
netvc->callHooks(TS_EVENT_SSL_VERIFY_CLIENT);
netvc->set_verify_cert(nullptr);
@@ -355,6 +366,12 @@ ssl_client_hello_callback(SSL *s, int *al, void *arg)
const char *servername = nullptr;
const unsigned char *p;
size_t remaining, len;
+
+ if (!netvc || netvc->ssl != s) {
+Debug("ssl.error", "ssl_client_hello_callback call back on stale netvc");
+return SSL_CLIENT_HELLO_ERROR;
+ }
+
// Parse the server name if the get extension call succeeds and there are
more than 2 bytes to parse
if (SSL_client_hello_get0_ext(s, TLSEXT_TYPE_server_name, , ) &&
remaining > 2) {
// Parse to get to the name, originally from test/handshake_helper.c in
openssl tree
@@ -414,6 +431,11 @@ ssl_cert_callback(SSL *ssl, void * /*arg*/)
bool reenabled;
int retval = 1;
+ if (!netvc || netvc->ssl != ssl) {
+Debug("ssl.error", "ssl_cert_callback call back on stale netvc");
+return 0;
+ }
+
// If we are in tunnel mode, don't select a cert. Pause!
if (HttpProxyPort::TRANSPORT_BLIND_TUNNEL == netvc->attributes) {
return -1; // Pause
@@ -447,6 +469,12 @@ static int
ssl_servername_callback(SSL *ssl, int * /* ad */, void * /*arg*/)
{
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
+
+ if (!netvc || netvc->ssl != ssl) {
+Debug("ssl.error", "ssl_servername_callback call back on stale netvc");
+return SSL_TLSEXT_ERR_ALERT_FATAL;
+ }
+
netvc->callHooks(TS_EVENT_SSL_SERVERNAME);
const char *name = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
@@ -1019,7 +1047,12 @@ ssl_callback_info(const SSL *ssl, int where, int ret)
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
- if (netvc && (where & SSL_CB_ACCEPT_LOOP) &&
netvc->getSSLHandShakeComplete() == true &&
+ if (!netvc || netvc->ssl != ssl) {
+Debug("ssl.error", "ssl_callback_info call back on stale netvc");
+return;
+ }
+
+ if ((where & SSL_CB_ACCEPT_LOOP) && netvc->getSSLHandShakeComplete() == true
&&
SSLConfigParams::ssl_allow_client_renegotiation == false) {
int state = SSL_get_state(ssl);
@@ -1790,6 +1823,7 @@ SSLAccept(SSL *ssl)
#if TS_HAS_TLS_EARLY_DATA
SSLNetVConnection *netvc = SSLNetVCAccess(ssl);
+
if
