From e22ca065d6ced6c9cab7d13d453b8b2d706fa25b Mon Sep 17 00:00:00 2001 From: sigiesec Date: Thu, 17 Aug 2017 18:32:44 +0200 Subject: [PATCH] Problem: curve_client_t does not emit handshake failure events Solution: add handshake failure events to curve_client_t --- include/zmq.h | 3 +++ src/curve_client.cpp | 52 +++++++++++++++++++++++++++++++++++++++++-- src/curve_client.hpp | 4 +++- src/stream_engine.cpp | 3 ++- src/zmq_draft.h | 3 +++ 5 files changed, 61 insertions(+), 4 deletions(-) diff --git a/include/zmq.h b/include/zmq.h index 9477575a..71cc111d 100644 --- a/include/zmq.h +++ b/include/zmq.h @@ -589,6 +589,9 @@ ZMQ_EXPORT void zmq_threadclose (void* thread); #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO 0x10000013 #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE 0x10000014 #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_ERROR 0x10000015 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_READY 0x10000016 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_WELCOME 0x10000017 +#define ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_METADATA 0x10000018 // the following two may be due to erroneous configuration of a peer #define ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC 0x11000001 diff --git a/src/curve_client.cpp b/src/curve_client.cpp index 2d31c37a..09107654 100644 --- a/src/curve_client.cpp +++ b/src/curve_client.cpp @@ -39,8 +39,10 @@ #include "wire.hpp" #include "curve_client_tools.hpp" -zmq::curve_client_t::curve_client_t (const options_t &options_) : +zmq::curve_client_t::curve_client_t (session_base_t *session_, + const options_t &options_) : mechanism_t (options_), + session (session_), state (send_hello), tools (options_.curve_public_key, options_.curve_secret_key, @@ -92,6 +94,9 @@ int zmq::curve_client_t::process_handshake_command (msg_t *msg_) msg_size)) rc = process_error (msg_data, msg_size); else { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; rc = -1; } @@ -163,12 +168,18 @@ int zmq::curve_client_t::decode (msg_t *msg_) zmq_assert (state == connected); if (msg_->size () < 33) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); // TODO message may not be a MESSAGE at all errno = EPROTO; return -1; } const uint8_t *message = static_cast (msg_->data ()); if (memcmp (message, "\x07MESSAGE", 8)) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } @@ -178,6 +189,9 @@ int zmq::curve_client_t::decode (msg_t *msg_) memcpy (message_nonce + 16, message + 8, 8); uint64_t nonce = get_uint64(message + 8); if (nonce <= cn_peer_nonce) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_SEQUENCE); errno = EPROTO; return -1; } @@ -214,8 +228,12 @@ int zmq::curve_client_t::decode (msg_t *msg_) message_plaintext + crypto_box_ZEROBYTES + 1, msg_->size ()); } - else + else { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; + } free (message_plaintext); free (message_box); @@ -241,6 +259,10 @@ int zmq::curve_client_t::produce_hello (msg_t *msg_) rc = tools.produce_hello (msg_->data (), cn_nonce); if (rc == -1) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); + // TODO this is somewhat inconsistent: we call init_size, but we may // not close msg_; i.e. we assume that msg_ is initialized but empty // (if it were non-empty, calling init_size might cause a leak!) @@ -260,6 +282,10 @@ int zmq::curve_client_t::process_welcome (const uint8_t *msg_data, int rc = tools.process_welcome (msg_data, msg_size); if (rc == -1) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); + errno = EPROTO; return -1; } @@ -288,6 +314,10 @@ int zmq::curve_client_t::produce_initiate (msg_t *msg_) free (metadata_plaintext); if (-1 == rc) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); + // TODO see comment in produce_hello return -1; } @@ -301,6 +331,9 @@ int zmq::curve_client_t::process_ready ( const uint8_t *msg_data, size_t msg_size) { if (msg_size < 30) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_READY); errno = EPROTO; return -1; } @@ -327,6 +360,9 @@ int zmq::curve_client_t::process_ready ( free (ready_box); if (rc != 0) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; return -1; } @@ -337,6 +373,12 @@ int zmq::curve_client_t::process_ready ( if (rc == 0) state = connected; + else + { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_METADATA); + errno = EPROTO; + } return rc; } @@ -345,15 +387,21 @@ int zmq::curve_client_t::process_error ( const uint8_t *msg_data, size_t msg_size) { if (state != expect_welcome && state != expect_ready) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } if (msg_size < 7) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_ERROR); errno = EPROTO; return -1; } const size_t error_reason_len = static_cast (msg_data [6]); if (error_reason_len > msg_size - 7) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_ERROR); errno = EPROTO; return -1; } diff --git a/src/curve_client.hpp b/src/curve_client.hpp index 066cd36e..9b2d2175 100644 --- a/src/curve_client.hpp +++ b/src/curve_client.hpp @@ -46,7 +46,7 @@ namespace zmq { public: - curve_client_t (const options_t &options_); + curve_client_t (session_base_t *session_, const options_t &options_); virtual ~curve_client_t (); // mechanism implementation @@ -67,6 +67,8 @@ namespace zmq connected }; + session_base_t *session; + // Current FSM state state_t state; diff --git a/src/stream_engine.cpp b/src/stream_engine.cpp index bbe232a2..d1731bbe 100644 --- a/src/stream_engine.cpp +++ b/src/stream_engine.cpp @@ -685,7 +685,8 @@ bool zmq::stream_engine_t::handshake () mechanism = new (std::nothrow) curve_server_t (session, peer_address, options); else - mechanism = new (std::nothrow) curve_client_t (options); + mechanism = + new (std::nothrow) curve_client_t (session, options); alloc_assert (mechanism); } #endif diff --git a/src/zmq_draft.h b/src/zmq_draft.h index 89386ea6..a4df3db1 100644 --- a/src/zmq_draft.h +++ b/src/zmq_draft.h @@ -71,6 +71,9 @@ #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO 0x10000013 #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE 0x10000014 #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_ERROR 0x10000015 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_READY 0x10000016 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_WELCOME 0x10000017 +#define ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_METADATA 0x10000018 // the following two may be due to erroneous configuration of a peer #define ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC 0x11000001