From be55c5ac3d2b4d66abb18ff139dec172c0249209 Mon Sep 17 00:00:00 2001 From: Pieter Hintjens Date: Fri, 31 Oct 2014 11:38:01 +0100 Subject: [PATCH 1/5] Fixed .gitignore for mains in root --- .gitignore | 166 ++++++++++++++++++++++++++--------------------------- 1 file changed, 83 insertions(+), 83 deletions(-) diff --git a/.gitignore b/.gitignore index ef73164a..f93b4281 100644 --- a/.gitignore +++ b/.gitignore @@ -21,93 +21,93 @@ autom4te.cache .* *~ .*~ -tools/curve_keygen -tests/test_resource -tests/test_ipc_wildcard -tests/test_stream_empty -tests/test_stream_timeout -tests/test_issue_566 -tests/test_ctx_destroy -tests/test_term_endpoint -tests/test_system -tests/test_monitor -tests/test_last_endpoint -tests/test_pair_inproc -tests/test_pair_ipc -tests/test_pair_tcp -tests/test_reqrep_inproc -tests/test_reqrep_ipc -tests/test_reqrep_tcp -tests/test_shutdown_stress -tests/test_hwm -tests/test_timeo -tests/test_reqrep_device -tests/test_reqrep_drop -tests/test_sub_forward -tests/test_invalid_rep -tests/test_msg_flags -tests/test_ts_context -tests/test_connect_resolve -tests/test_immediate -tests/test_term_endpoint -tests/test_router_mandatory -tests/test_disconnect_inproc -tests/test_raw_sock -tests/test_disconnect_inproc -tests/test_ctx_options -tests/test_iov -tests/test_security -tests/test_security_curve -tests/test_probe_router -tests/test_stream -tests/test_spec_dealer -tests/test_spec_pushpull -tests/test_spec_rep -tests/test_spec_req -tests/test_spec_router -tests/test_req_correlate -tests/test_req_relaxed -tests/test_fork -tests/test_conflate -tests/test_inproc_connect -tests/test_linger -tests/test_security_null -tests/test_security_plain -tests/test_proxy -tests/test_abstract_ipc -tests/test_filter_ipc -tests/test_connect_delay_tipc -tests/test_pair_tipc -tests/test_reqrep_device_tipc -tests/test_reqrep_tipc -tests/test_router_handover -tests/test_router_mandatory_tipc -tests/test_shutdown_stress_tipc -tests/test_sub_forward_tipc -tests/test_term_endpoint_tipc -tests/test_many_sockets -tests/test_diffserv -tests/test_connect_rid -tests/test_srcfd -tests/test_stream_disconnect -tests/test_proxy_chain -tests/test_bind_src_address -tests/test_metadata -tests/test_id2fd -tests/test_capabilities -tests/test_hwm_pubsub -tests/test_router_mandatory_hwm -tests/test_xpub_nodrop +curve_keygen +test_resource +test_ipc_wildcard +test_stream_empty +test_stream_timeout +test_issue_566 +test_ctx_destroy +test_term_endpoint +test_system +test_monitor +test_last_endpoint +test_pair_inproc +test_pair_ipc +test_pair_tcp +test_reqrep_inproc +test_reqrep_ipc +test_reqrep_tcp +test_shutdown_stress +test_hwm +test_timeo +test_reqrep_device +test_reqrep_drop +test_sub_forward +test_invalid_rep +test_msg_flags +test_ts_context +test_connect_resolve +test_immediate +test_term_endpoint +test_router_mandatory +test_disconnect_inproc +test_raw_sock +test_disconnect_inproc +test_ctx_options +test_iov +test_security +test_security_curve +test_probe_router +test_stream +test_spec_dealer +test_spec_pushpull +test_spec_rep +test_spec_req +test_spec_router +test_req_correlate +test_req_relaxed +test_fork +test_conflate +test_inproc_connect +test_linger +test_security_null +test_security_plain +test_proxy +test_abstract_ipc +test_filter_ipc +test_connect_delay_tipc +test_pair_tipc +test_reqrep_device_tipc +test_reqrep_tipc +test_router_handover +test_router_mandatory_tipc +test_shutdown_stress_tipc +test_sub_forward_tipc +test_term_endpoint_tipc +test_many_sockets +test_diffserv +test_connect_rid +test_srcfd +test_stream_disconnect +test_proxy_chain +test_bind_src_address +test_metadata +test_id2fd +test_capabilities +test_hwm_pubsub +test_router_mandatory_hwm +test_xpub_nodrop tests/test*.log tests/test*.trs src/platform.hpp* src/stamp-h1 -perf/local_lat -perf/local_thr -perf/remote_lat -perf/remote_thr -perf/inproc_lat -perf/inproc_thr +local_lat +local_thr +remote_lat +remote_thr +inproc_lat +inproc_thr doc/*.1 doc/*.3 doc/*.7 From ea9f7acce3b8a8258f42aae23de398eb2e3cba5b Mon Sep 17 00:00:00 2001 From: Pieter Hintjens Date: Thu, 6 Nov 2014 10:55:26 +0100 Subject: [PATCH 2/5] Problem: zmq_ctx_term has insane behavior by default Solution: document this with a clear warning. It would be nicer perhaps to change the default LINGER to e.g. a few seconds. However this could break existing applications. --- doc/zmq_ctx_term.txt | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/doc/zmq_ctx_term.txt b/doc/zmq_ctx_term.txt index 50951cd1..44c667bc 100644 --- a/doc/zmq_ctx_term.txt +++ b/doc/zmq_ctx_term.txt @@ -23,8 +23,8 @@ Context termination is performed in the following steps: exception of _zmq_close()_, any further operations on sockets open within 'context' shall fail with an error code of ETERM. -2. After interrupting all blocking calls, _zmq_ctx_term()_ shall _block_ until the - following conditions are satisfied: +2. After interrupting all blocking calls, _zmq_ctx_term()_ shall _block_ until + the following conditions are satisfied: * All sockets open within 'context' have been closed with _zmq_close()_. @@ -39,6 +39,15 @@ option in linkzmq:zmq_setsockopt[3]. This function replaces the deprecated function linkzmq:zmq_term[3]. +WARNING +------- + +As _ZMQ_LINGER_ defaults to "infinite", by default this function will block +indefinitely if there are any pending connects or sends. We strongly +recommend to (a) set _ZMQ_LINGER_ to zero on all sockets and (b) close all +sockets, before calling this function. + + RETURN VALUE ------------ The _zmq_ctx_term()_ function shall return zero if successful. Otherwise From 7781375adf5baf6d810ebe291d46a77146c62fb3 Mon Sep 17 00:00:00 2001 From: Pieter Hintjens Date: Thu, 6 Nov 2014 15:30:04 +0100 Subject: [PATCH 3/5] Problem: default LINGER value is insane Solution: use a sane value, e.g. 2 seconds Fixes #1247 --- doc/zmq_ctx_term.txt | 9 --------- doc/zmq_setsockopt.txt | 4 ++-- src/options.cpp | 2 +- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/doc/zmq_ctx_term.txt b/doc/zmq_ctx_term.txt index 44c667bc..2e3ca08d 100644 --- a/doc/zmq_ctx_term.txt +++ b/doc/zmq_ctx_term.txt @@ -39,15 +39,6 @@ option in linkzmq:zmq_setsockopt[3]. This function replaces the deprecated function linkzmq:zmq_term[3]. -WARNING -------- - -As _ZMQ_LINGER_ defaults to "infinite", by default this function will block -indefinitely if there are any pending connects or sends. We strongly -recommend to (a) set _ZMQ_LINGER_ to zero on all sockets and (b) close all -sockets, before calling this function. - - RETURN VALUE ------------ The _zmq_ctx_term()_ function shall return zero if successful. Otherwise diff --git a/doc/zmq_setsockopt.txt b/doc/zmq_setsockopt.txt index 0025ef5f..1f80584a 100644 --- a/doc/zmq_setsockopt.txt +++ b/doc/zmq_setsockopt.txt @@ -306,7 +306,7 @@ linkzmq:zmq_disconnect[3] or closed with linkzmq:zmq_close[3], and further affects the termination of the socket's context with linkzmq:zmq_term[3]. The following outlines the different behaviours: -* The default value of '-1' specifies an infinite linger period. Pending +* A value of '-1' specifies an infinite linger period. Pending messages shall not be discarded after a call to _zmq_disconnect()_ or _zmq_close()_; attempting to terminate the socket's context with _zmq_term()_ shall block until all pending messages have been sent to a peer. @@ -323,7 +323,7 @@ following outlines the different behaviours: [horizontal] Option value type:: int Option value unit:: milliseconds -Default value:: -1 (infinite) +Default value:: 2000 (two seconds) Applicable socket types:: all diff --git a/src/options.cpp b/src/options.cpp index 84b2a7b1..caf3587f 100644 --- a/src/options.cpp +++ b/src/options.cpp @@ -35,7 +35,7 @@ zmq::options_t::options_t () : rcvbuf (0), tos (0), type (-1), - linger (-1), + linger (2000), reconnect_ivl (100), reconnect_ivl_max (0), backlog (100), From 1844fc3284d76a4a788a7489fdb2a5aedde9ccd8 Mon Sep 17 00:00:00 2001 From: Constantin Rack Date: Fri, 7 Nov 2014 16:56:49 +0100 Subject: [PATCH 4/5] Problem: No error-checking of setsockopt ZMQ_CURVE_* z85 keys. Solves #1094. --- src/options.cpp | 52 +++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/src/options.cpp b/src/options.cpp index 84b2a7b1..5450dbf2 100644 --- a/src/options.cpp +++ b/src/options.cpp @@ -376,19 +376,21 @@ int zmq::options_t::setsockopt (int option_, const void *optval_, } else if (optvallen_ == CURVE_KEYSIZE_Z85 + 1) { - zmq_z85_decode (curve_public_key, (char *) optval_); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_public_key, (char *) optval_)) { + mechanism = ZMQ_CURVE; + return 0; + } } else // Deprecated, not symmetrical with zmq_getsockopt if (optvallen_ == CURVE_KEYSIZE_Z85) { - char z85_key [41]; + char z85_key [CURVE_KEYSIZE_Z85 + 1]; memcpy (z85_key, (char *) optval_, CURVE_KEYSIZE_Z85); z85_key [CURVE_KEYSIZE_Z85] = 0; - zmq_z85_decode (curve_public_key, z85_key); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_public_key, z85_key)) { + mechanism = ZMQ_CURVE; + return 0; + } } break; @@ -400,19 +402,21 @@ int zmq::options_t::setsockopt (int option_, const void *optval_, } else if (optvallen_ == CURVE_KEYSIZE_Z85 + 1) { - zmq_z85_decode (curve_secret_key, (char *) optval_); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_secret_key, (char *) optval_)) { + mechanism = ZMQ_CURVE; + return 0; + } } else // Deprecated, not symmetrical with zmq_getsockopt if (optvallen_ == CURVE_KEYSIZE_Z85) { - char z85_key [41]; + char z85_key [CURVE_KEYSIZE_Z85 + 1]; memcpy (z85_key, (char *) optval_, CURVE_KEYSIZE_Z85); z85_key [CURVE_KEYSIZE_Z85] = 0; - zmq_z85_decode (curve_secret_key, z85_key); - mechanism = ZMQ_CURVE; - return 0; + if (zmq_z85_decode (curve_secret_key, z85_key)) { + mechanism = ZMQ_CURVE; + return 0; + } } break; @@ -425,21 +429,23 @@ int zmq::options_t::setsockopt (int option_, const void *optval_, } else if (optvallen_ == CURVE_KEYSIZE_Z85 + 1) { - zmq_z85_decode (curve_server_key, (char *) optval_); - mechanism = ZMQ_CURVE; - as_server = 0; - return 0; + if (zmq_z85_decode (curve_server_key, (char *) optval_)) { + mechanism = ZMQ_CURVE; + as_server = 0; + return 0; + } } else // Deprecated, not symmetrical with zmq_getsockopt if (optvallen_ == CURVE_KEYSIZE_Z85) { - char z85_key [41]; + char z85_key [CURVE_KEYSIZE_Z85 + 1]; memcpy (z85_key, (char *) optval_, CURVE_KEYSIZE_Z85); z85_key [CURVE_KEYSIZE_Z85] = 0; - zmq_z85_decode (curve_server_key, z85_key); - mechanism = ZMQ_CURVE; - as_server = 0; - return 0; + if (zmq_z85_decode (curve_server_key, z85_key)) { + mechanism = ZMQ_CURVE; + as_server = 0; + return 0; + } } break; # endif From e00ea532df2a8957150e5e37f4886d7abe049af1 Mon Sep 17 00:00:00 2001 From: Constantin Rack Date: Fri, 7 Nov 2014 17:35:41 +0100 Subject: [PATCH 5/5] Add tests for issue #1094. --- tests/test_security_curve.cpp | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/test_security_curve.cpp b/tests/test_security_curve.cpp index af3925ab..75ca8466 100644 --- a/tests/test_security_curve.cpp +++ b/tests/test_security_curve.cpp @@ -217,7 +217,22 @@ int main (void) assert (rc == 0); expect_bounce_fail (server, client); close_zero_linger (client); - + + // Check return codes for invalid buffer sizes + client = zmq_socket (ctx, ZMQ_DEALER); + assert (client); + errno = 0; + rc = zmq_setsockopt (client, ZMQ_CURVE_SERVERKEY, server_public, 123); + assert (rc == -1 && errno == EINVAL); + errno = 0; + rc = zmq_setsockopt (client, ZMQ_CURVE_PUBLICKEY, client_public, 123); + assert (rc == -1 && errno == EINVAL); + errno = 0; + rc = zmq_setsockopt (client, ZMQ_CURVE_SECRETKEY, client_secret, 123); + assert (rc == -1 && errno == EINVAL); + rc = zmq_close (client); + assert (rc == 0); + // Shutdown rc = zmq_close (server); assert (rc == 0);