Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems: free of stack variable, TODO left to evaluate #2700

Merged
merged 9 commits into from
Aug 19, 2017
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,7 @@ if (MSVC)
else ()
if (BUILD_SHARED)
add_library (libzmq SHARED ${sources} ${public_headers} ${html-docs} ${readme-docs} ${zmq-pkgconfig})
target_link_libraries (libzmq ${OPTIONAL_LIBRARIES})
# NOTE: the SOVERSION MUST be the same as the one generated by libtool!
set_target_properties (libzmq PROPERTIES
COMPILE_DEFINITIONS "DLL_EXPORT"
Expand Down
5 changes: 5 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,11 @@ src_libzmq_la_CPPFLAGS += ${pgm_CFLAGS}
src_libzmq_la_LIBADD += ${pgm_LIBS}
endif

if BUILD_GSSAPI
src_libzmq_la_CPPFLAGS += ${gssapi_krb5_CFLAGS}
src_libzmq_la_LIBADD += ${gssapi_krb5_LIBS}
endif

if ENABLE_PERF
noinst_PROGRAMS = \
perf/local_lat \
Expand Down
1 change: 1 addition & 0 deletions builds/cmake/platform.hpp.in
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#cmakedefine ZMQ_HAVE_EVENTFD
#cmakedefine ZMQ_HAVE_EVENTFD_CLOEXEC
#cmakedefine ZMQ_HAVE_IFADDRS
#cmakedefine ZMQ_HAVE_SO_BINDTODEVICE

#cmakedefine ZMQ_HAVE_SO_PEERCRED
#cmakedefine ZMQ_HAVE_LOCAL_PEERCRED
Expand Down
10 changes: 6 additions & 4 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -442,10 +442,12 @@ AC_ARG_WITH([libgssapi_krb5], [AS_HELP_STRING([--with-libgssapi_krb5],

# conditionally require libgssapi_krb5
if test "x$require_libgssapi_krb5_ext" != "xno"; then
AC_CHECK_HEADERS(gssapi/gssapi_generic.h)
AC_SEARCH_LIBS([gss_init_sec_context], [gssapi_krb5 gssapi],
AC_DEFINE(HAVE_LIBGSSAPI_KRB5, [1], [Enabled GSSAPI security]),
AC_MSG_ERROR(libgssapi_krb5 is needed for GSSAPI security))
PKG_CHECK_MODULES([gssapi_krb5], [krb5-gssapi], [], [
AC_CHECK_HEADERS(gssapi/gssapi_generic.h)
AC_SEARCH_LIBS([gss_init_sec_context], [gssapi_krb5 gssapi],
AC_DEFINE(HAVE_LIBGSSAPI_KRB5, [1], [Enabled GSSAPI security]),
AC_MSG_ERROR(libgssapi_krb5 is needed for GSSAPI security))
])
fi
AM_CONDITIONAL(BUILD_GSSAPI, test "x$require_libgssapi_krb5_ext" != "xno")

Expand Down
9 changes: 0 additions & 9 deletions doc/zmq_socket.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ after which either peer can send messages asynchronously, to the other.

The client-server pattern is formally defined by http://rfc.zeromq.org/spec:41.

Note: this pattern is meant to eventually deprecate the use of 'ZMQ_DEALER' and
'ZMQ_ROUTER' to build client-server architectures, as well as 'ZMQ_REP' and
'ZMQ_REQ' for request-reply.

ZMQ_CLIENT
^^^^^^^^^^
A 'ZMQ_CLIENT' socket talks to a 'ZMQ_SERVER' socket. Either peer can connect,
Expand Down Expand Up @@ -166,9 +162,6 @@ Groups are matched using exact matching (vs prefix matching of PubSub).

NOTE: Radio-dish is still in draft phase.

Note: this pattern is meant to eventually deprecate the use of 'ZMQ_PUB' and
'ZMQ_SUB' to build pub-sub architectures.

ZMQ_RADIO
^^^^^^^
A socket of type 'ZMQ_RADIO' is used by a _publisher_ to distribute data.
Expand Down Expand Up @@ -429,8 +422,6 @@ request sent.

The request-reply pattern is formally defined by http://rfc.zeromq.org/spec:28.

Note: this pattern will be deprecated in favor of the client-server pattern.

ZMQ_REQ
^^^^^^^
A socket of type 'ZMQ_REQ' is used by a _client_ to send requests to and
Expand Down
4 changes: 1 addition & 3 deletions src/gssapi_mechanism_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,8 @@ int zmq::gssapi_mechanism_base_t::decode_message (msg_t *msg_)

if (maj_stat != GSS_S_COMPLETE)
{
// TODO is it correct to release the plaintext buffer if gss_unwrap
// did not succeed?
gss_release_buffer (&min_stat, &plaintext);
free (wrapped);
free (wrapped.value);
session->get_socket ()->event_handshake_failed_protocol (
session->get_endpoint (),
ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC);
Expand Down
3 changes: 3 additions & 0 deletions src/ip.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,5 +228,8 @@ void zmq::bind_to_device (fd_t s_, std::string &bound_device_)
#else
errno_assert (rc == 0);
#endif
#else
LIBZMQ_UNUSED (s_);
LIBZMQ_UNUSED (bound_device_);
#endif
}
1 change: 1 addition & 0 deletions src/pgm_receiver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ int zmq::pgm_receiver_t::init (bool udp_encapsulation_, const char *network_)
void zmq::pgm_receiver_t::plug (io_thread_t *io_thread_,
session_base_t *session_)
{
LIBZMQ_UNUSED (io_thread_);
// Retrieve PGM fds and start polling.
fd_t socket_fd = retired_fd;
fd_t waiting_pipe_fd = retired_fd;
Expand Down
2 changes: 2 additions & 0 deletions src/pgm_sender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "err.hpp"
#include "wire.hpp"
#include "stdint.hpp"
#include "macros.hpp"

zmq::pgm_sender_t::pgm_sender_t (io_thread_t *parent_,
const options_t &options_) :
Expand Down Expand Up @@ -73,6 +74,7 @@ int zmq::pgm_sender_t::init (bool udp_encapsulation_, const char *network_)

void zmq::pgm_sender_t::plug (io_thread_t *io_thread_, session_base_t *session_)
{
LIBZMQ_UNUSED (io_thread_);
// Allocate 2 fds for PGM socket.
fd_t downlink_socket_fd = retired_fd;
fd_t uplink_socket_fd = retired_fd;
Expand Down
21 changes: 21 additions & 0 deletions tests/test_security_curve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ void test_curve_security_with_valid_credentials (
void test_curve_security_with_bogus_client_credentials (
void *ctx, char *my_endpoint, void *server, void *server_mon, int timeout)
{
LIBZMQ_UNUSED (timeout);

// This must be caught by the ZAP handler
char bogus_public [41];
char bogus_secret [41];
Expand Down Expand Up @@ -278,6 +280,9 @@ void test_curve_security_invalid_hello_wrong_length (char *my_endpoint,
void *server_mon,
int timeout)
{
LIBZMQ_UNUSED (server);
LIBZMQ_UNUSED (timeout);

int s = connect_vanilla_socket (my_endpoint);

// send GREETING
Expand Down Expand Up @@ -352,6 +357,9 @@ void test_curve_security_invalid_hello_command_name (char *my_endpoint,
void *server_mon,
int timeout)
{
LIBZMQ_UNUSED (server);
LIBZMQ_UNUSED (timeout);

int s = connect_vanilla_socket (my_endpoint);

send_greeting (s);
Expand Down Expand Up @@ -380,6 +388,9 @@ void test_curve_security_invalid_hello_version (char *my_endpoint,
void *server_mon,
int timeout)
{
LIBZMQ_UNUSED (server);
LIBZMQ_UNUSED (timeout);

int s = connect_vanilla_socket (my_endpoint);

send_greeting (s);
Expand Down Expand Up @@ -455,6 +466,8 @@ void test_curve_security_invalid_initiate_length (char *my_endpoint,
void *server_mon,
int timeout)
{
LIBZMQ_UNUSED (server);

zmq::curve_client_tools_t tools = make_curve_client_tools ();

int s = connect_exchange_greeting_and_send_hello (my_endpoint, tools);
Expand All @@ -465,6 +478,8 @@ void test_curve_security_invalid_initiate_length (char *my_endpoint,
#ifdef ZMQ_BUILD_DRAFT_API
int res = get_monitor_event_with_timeout (server_mon, NULL, NULL, timeout);
assert (res == -1);
#else
LIBZMQ_UNUSED (timeout);
#endif

send(s, "\x04\x09\x08INITIATE");
Expand Down Expand Up @@ -508,6 +523,8 @@ void test_curve_security_invalid_initiate_command_name (char *my_endpoint,
void *server_mon,
int timeout)
{
LIBZMQ_UNUSED (server);

zmq::curve_client_tools_t tools = make_curve_client_tools ();
int s = connect_exchange_greeting_and_hello_welcome (
my_endpoint, server_mon, timeout, tools);
Expand All @@ -531,6 +548,8 @@ void test_curve_security_invalid_initiate_command_name (char *my_endpoint,
void test_curve_security_invalid_initiate_command_encrypted_cookie (
char *my_endpoint, void *server, void *server_mon, int timeout)
{
LIBZMQ_UNUSED (server);

zmq::curve_client_tools_t tools = make_curve_client_tools ();
int s = connect_exchange_greeting_and_hello_welcome (
my_endpoint, server_mon, timeout, tools);
Expand All @@ -554,6 +573,8 @@ void test_curve_security_invalid_initiate_command_encrypted_cookie (
void test_curve_security_invalid_initiate_command_encrypted_content (
char *my_endpoint, void *server, void *server_mon, int timeout)
{
LIBZMQ_UNUSED (server);

zmq::curve_client_tools_t tools = make_curve_client_tools ();
int s = connect_exchange_greeting_and_hello_welcome (
my_endpoint, server_mon, timeout, tools);
Expand Down
33 changes: 27 additions & 6 deletions tests/test_security_gssapi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ static volatile int zap_deny_all = 0;
// by reference, if not null, and event number by value. Returns -1
// in case of error.

#ifdef ZMQ_BUILD_DRAFT_API
static int
get_monitor_event (void *monitor, int *value, char **address)
{
Expand Down Expand Up @@ -89,6 +90,7 @@ get_monitor_event (void *monitor, int *value, char **address)
}
return event;
}
#endif

// --------------------------------------------------------------------------
// This methods receives and validates ZAP requestes (allowing or denying
Expand Down Expand Up @@ -151,19 +153,23 @@ void test_valid_creds (void *ctx, void *server, void *server_mon, char *endpoint
rc = zmq_setsockopt (client, ZMQ_GSSAPI_PRINCIPAL,
name, strlen (name) + 1);
assert (rc == 0);
#ifdef ZMQ_BUILD_DRAFT_API
int name_type = ZMQ_GSSAPI_NT_HOSTBASED;
rc = zmq_setsockopt (client, ZMQ_GSSAPI_PRINCIPAL_NAMETYPE,
&name_type, sizeof (name_type));
assert (rc == 0);
#endif
rc = zmq_connect (client, endpoint);
assert (rc == 0);

bounce (server, client);
rc = zmq_close (client);
assert (rc == 0);

#ifdef ZMQ_BUILD_DRAFT_API
int event = get_monitor_event (server_mon, NULL, NULL);
assert (event == ZMQ_EVENT_HANDSHAKE_SUCCEED);
assert (event == ZMQ_EVENT_HANDSHAKE_SUCCEEDED);
#endif
}

// Check security with valid but unauthorized credentials
Expand All @@ -179,19 +185,23 @@ void test_unauth_creds (void *ctx, void *server, void *server_mon, char *endpoin
rc = zmq_setsockopt (client, ZMQ_GSSAPI_PRINCIPAL,
name, strlen (name) + 1);
assert (rc == 0);
#ifdef ZMQ_BUILD_DRAFT_API
int name_type = ZMQ_GSSAPI_NT_HOSTBASED;
rc = zmq_setsockopt (client, ZMQ_GSSAPI_PRINCIPAL_NAMETYPE,
&name_type, sizeof (name_type));
assert (rc == 0);
#endif
zap_deny_all = 1;
rc = zmq_connect (client, endpoint);
assert (rc == 0);

expect_bounce_fail (server, client);
close_zero_linger (client);

#ifdef ZMQ_BUILD_DRAFT_API
int event = get_monitor_event (server_mon, NULL, NULL);
assert (event == ZMQ_EVENT_HANDSHAKE_FAILED);
assert (event == ZMQ_EVENT_HANDSHAKE_FAILED_AUTH);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sigiesec I took a very wild guess here and in the next events, is this correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one looks good.

#endif
}

// Check GSSAPI security with NULL client credentials
Expand All @@ -205,8 +215,10 @@ void test_null_creds (void *ctx, void *server, void *server_mon, char *endpoint)
expect_bounce_fail (server, client);
close_zero_linger (client);

#ifdef ZMQ_BUILD_DRAFT_API
int event = get_monitor_event (server_mon, NULL, NULL);
assert (event == ZMQ_EVENT_HANDSHAKE_FAILED);
assert (event == ZMQ_EVENT_HANDSHAKE_FAILED_AUTH);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL with value ZMQ_PROTOCOL_ERROR_ZMTP_MECHANISM_MISMATCH as in

ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL,

Maybe expect_zmtp_mechanism_mismatch should be moved to testutil_security.hpp and reused here (and for the PLAIN client test as well)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, sent a quick fix here: #2706

#endif
}

// Check GSSAPI security with PLAIN client credentials
Expand Down Expand Up @@ -242,7 +254,7 @@ void test_vanilla_socket (void *ctx, void *server, void *server_mon, char *endpo
#endif

s = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
int rc = connect (s, (struct sockaddr*) &ip4addr, sizeof (ip4addr));
rc = connect (s, (struct sockaddr*) &ip4addr, sizeof (ip4addr));
assert (rc > -1);
// send anonymous ZMTP/1.0 greeting
send (s, "\x01\x00", 2, 0);
Expand Down Expand Up @@ -292,23 +304,30 @@ int main (void)
rc = zmq_setsockopt (server, ZMQ_GSSAPI_PRINCIPAL,
name, strlen (name) + 1);
assert (rc == 0);
#ifdef ZMQ_BUILD_DRAFT_API
int name_type = ZMQ_GSSAPI_NT_HOSTBASED;
rc = zmq_setsockopt (server, ZMQ_GSSAPI_PRINCIPAL_NAMETYPE,
&name_type, sizeof (name_type));
assert (rc == 0);
#endif
rc = zmq_bind (server, "tcp://127.0.0.1:*");
assert (rc == 0);
rc = zmq_getsockopt (server, ZMQ_LAST_ENDPOINT, my_endpoint, &len);
assert (rc == 0);

#ifdef ZMQ_BUILD_DRAFT_API
// Monitor handshake events on the server
rc = zmq_socket_monitor (server, "inproc://monitor-server",
ZMQ_EVENT_HANDSHAKE_SUCCEED | ZMQ_EVENT_HANDSHAKE_FAILED);
ZMQ_EVENT_HANDSHAKE_SUCCEEDED | ZMQ_EVENT_HANDSHAKE_FAILED_AUTH);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL must also be enabled.

assert (rc == 0);
#endif

// Create socket for collecting monitor events
void *server_mon = zmq_socket (ctx, ZMQ_PAIR);
void *server_mon = NULL;
#ifdef ZMQ_BUILD_DRAFT_API
server_mon = zmq_socket (ctx, ZMQ_PAIR);
assert (server_mon);
#endif

// Connect it to the inproc endpoints so they'll get events
rc = zmq_connect (server_mon, "inproc://monitor-server");
Expand All @@ -322,7 +341,9 @@ int main (void)
test_unauth_creds (ctx, server, server_mon, my_endpoint);

// Shutdown
#ifdef ZMQ_BUILD_DRAFT_API
close_zero_linger (server_mon);
#endif
rc = zmq_close (server);
assert (rc == 0);
rc = zmq_ctx_term (ctx);
Expand Down
2 changes: 2 additions & 0 deletions tests/testutil.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@
# endif
#endif

#define LIBZMQ_UNUSED(object) (void)object

// Bounce a message from client to server and back
// For REQ/REP or DEALER/DEALER pairs only
void
Expand Down
8 changes: 8 additions & 0 deletions tests/testutil_security.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,14 @@ const char *test_zap_domain = "ZAPTEST";
// NULL specific functions
void socket_config_null_client (void *server, void *server_secret)
{
LIBZMQ_UNUSED (server);
LIBZMQ_UNUSED (server_secret);
}

void socket_config_null_server (void *server, void *server_secret)
{
LIBZMQ_UNUSED (server_secret);

int rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, test_zap_domain, 7);
assert (rc == 0);
}
Expand All @@ -55,6 +59,8 @@ const char *test_plain_password = "testpass";

void socket_config_plain_client (void *server, void *server_secret)
{
LIBZMQ_UNUSED (server_secret);

int rc = zmq_setsockopt (server, ZMQ_PLAIN_PASSWORD, test_plain_password, 8);
assert (rc == 0);

Expand All @@ -64,6 +70,8 @@ void socket_config_plain_client (void *server, void *server_secret)

void socket_config_plain_server (void *server, void *server_secret)
{
LIBZMQ_UNUSED (server_secret);

int as_server = 1;
int rc = zmq_setsockopt (server, ZMQ_PLAIN_SERVER, &as_server, sizeof (int));
assert (rc == 0);
Expand Down