From e2d3ba9c62c780e0fb5974f7a1dcff7a8d4184c9 Mon Sep 17 00:00:00 2001 From: sigiesec Date: Thu, 17 Aug 2017 17:54:07 +0200 Subject: [PATCH 1/9] Problem: classification ZMQ_HANDSHAKE_FAILED_* events is coarse-grained and partially misleading Solution: redesign ZMQ_HANDSHAKE_FAILED_* events, introduce new class of ZMQ_HANDSHAKE_FAILED_AUTH events --- include/zmq.h | 34 ++++++++-- src/curve_server.cpp | 115 ++++++++++++++++++++++------------ src/curve_server.hpp | 1 + src/mechanism.hpp | 12 ---- src/socket_base.cpp | 22 +++---- src/socket_base.hpp | 5 +- src/stream_engine.cpp | 23 +++---- src/zap_client.cpp | 46 +++++++------- src/zap_client.hpp | 7 --- src/zmq_draft.h | 34 ++++++++-- tests/test_security_curve.cpp | 60 ++++++++++-------- tests/test_security_zap.cpp | 41 +++++++++--- tests/testutil_security.hpp | 16 +++-- 13 files changed, 253 insertions(+), 163 deletions(-) diff --git a/include/zmq.h b/include/zmq.h index 0fd4ef71..47dff04a 100644 --- a/include/zmq.h +++ b/include/zmq.h @@ -568,11 +568,37 @@ ZMQ_EXPORT void zmq_threadclose (void* thread); #define ZMQ_BINDTODEVICE 90 /* DRAFT 0MQ socket events and monitoring */ -#define ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL 0x0800 +/* Unspecified system errors during handshake. Event value is an errno. */ +#define ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL 0x0800 +/* Handshake complete successfully with successful authentication (if * + * enabled). Event value is unused. */ #define ZMQ_EVENT_HANDSHAKE_SUCCEEDED 0x1000 -#define ZMQ_EVENT_HANDSHAKE_FAILED_ENCRYPTION 0x2000 -#define ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP 0x4000 -#define ZMQ_EVENT_HANDSHAKE_FAILED_ZAP 0x8000 +/* Protocol errors between ZMTP peers or between server and ZAP handler. * + * Event value is one of ZMQ_PROTOCOL_ERROR_* */ +#define ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL 0x2000 +/* Failed authentication requests. Event value is the numeric ZAP status * + * code, i.e. 300, 400 or 500. */ +#define ZMQ_EVENT_HANDSHAKE_FAILED_AUTH 0x4000 + +#define ZMQ_PROTOCOL_ERROR_ZMTP_UNSPECIFIED 0x10000000 +#define ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND 0x10000001 +#define ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_SEQUENCE 0x10000002 +#define ZMQ_PROTOCOL_ERROR_ZMTP_KEY_EXCHANGE 0x10000003 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED 0x10000011 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE 0x10000012 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO 0x10000013 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE 0x10000014 + +// the following two may be due to erroneous configuration of a peer +#define ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC 0x11000001 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MECHANISM_MISMATCH 0x11000002 + +#define ZMQ_PROTOCOL_ERROR_ZAP_UNSPECIFIED 0x20000000 +#define ZMQ_PROTOCOL_ERROR_ZAP_MALFORMED_REPLY 0x20000001 +#define ZMQ_PROTOCOL_ERROR_ZAP_BAD_REQUEST_ID 0x20000002 +#define ZMQ_PROTOCOL_ERROR_ZAP_BAD_VERSION 0x20000003 +#define ZMQ_PROTOCOL_ERROR_ZAP_INVALID_STATUS_CODE 0x20000004 +#define ZMQ_PROTOCOL_ERROR_ZAP_INVALID_METADATA 0x20000005 /* DRAFT Context options */ #define ZMQ_MSG_T_SIZE 6 diff --git a/src/curve_server.cpp b/src/curve_server.cpp index 0e210ba8..af18acc5 100644 --- a/src/curve_server.cpp +++ b/src/curve_server.cpp @@ -107,7 +107,8 @@ int zmq::curve_server_t::process_handshake_command (msg_t *msg_) // Therefore, it should be changed to zmq_assert (false); // CURVE I: invalid handshake command - current_error_detail = zmtp; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNSPECIFIED); errno = EPROTO; rc = -1; break; @@ -177,17 +178,22 @@ int zmq::curve_server_t::decode (msg_t *msg_) { zmq_assert (state == ready); - if (msg_->size () < 33) { - // CURVE I : invalid CURVE client, sent malformed command - current_error_detail = zmtp; + int rc = check_basic_command_structure (msg_); + if (rc == -1) + 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; } - const uint8_t *message = static_cast (msg_->data ()); - if (memcmp (message, "\x07MESSAGE", 8)) { - // CURVE I: invalid CURVE client, did not send MESSAGE - current_error_detail = zmtp; + if (msg_->size () < 33) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); errno = EPROTO; return -1; } @@ -197,6 +203,8 @@ int zmq::curve_server_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 +222,8 @@ int zmq::curve_server_t::decode (msg_t *msg_) memcpy (message_box + crypto_box_BOXZEROBYTES, message + 16, msg_->size () - 16); - int rc = crypto_box_open_afternm (message_plaintext, message_box, - clen, message_nonce, cn_precom); + rc = crypto_box_open_afternm (message_plaintext, message_box, clen, + message_nonce, cn_precom); if (rc == 0) { rc = msg_->close (); zmq_assert (rc == 0); @@ -235,7 +243,8 @@ int zmq::curve_server_t::decode (msg_t *msg_) } else { // CURVE I : connection key used for MESSAGE is wrong - current_error_detail = encryption; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; } free (message_plaintext); @@ -246,17 +255,21 @@ int zmq::curve_server_t::decode (msg_t *msg_) int zmq::curve_server_t::process_hello (msg_t *msg_) { - if (msg_->size () != 200) { - // CURVE I: client HELLO is not correct size - current_error_detail = zmtp; + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return -1; + + const uint8_t * const hello = static_cast (msg_->data ()); + if (memcmp (hello, "\x05HELLO", 6)) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } - const uint8_t * const hello = static_cast (msg_->data ()); - if (memcmp (hello, "\x05HELLO", 6)) { - // CURVE I: client HELLO has invalid command name - current_error_detail = zmtp; + if (msg_->size () != 200) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; return -1; } @@ -266,7 +279,8 @@ int zmq::curve_server_t::process_hello (msg_t *msg_) if (major != 1 || minor != 0) { // CURVE I: client HELLO has unknown version number - current_error_detail = zmtp; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; return -1; } @@ -286,12 +300,12 @@ int zmq::curve_server_t::process_hello (msg_t *msg_) memcpy (hello_box + crypto_box_BOXZEROBYTES, hello + 120, 80); // Open Box [64 * %x0](C'->S) - int rc = crypto_box_open (hello_plaintext, hello_box, - sizeof hello_box, - hello_nonce, cn_client, secret_key); + rc = crypto_box_open (hello_plaintext, hello_box, sizeof hello_box, + hello_nonce, cn_client, secret_key); if (rc != 0) { // CURVE I: cannot open client HELLO -- wrong server key? - current_error_detail = encryption; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; return -1; } @@ -345,8 +359,8 @@ int zmq::curve_server_t::produce_welcome (msg_t *msg_) cookie_ciphertext + crypto_secretbox_BOXZEROBYTES, 80); rc = crypto_box (welcome_ciphertext, welcome_plaintext, - sizeof welcome_plaintext, - welcome_nonce, cn_client, secret_key); + sizeof welcome_plaintext, welcome_nonce, cn_client, + secret_key); // TODO I think we should change this back to zmq_assert (rc == 0); // as it was before https://github.com/zeromq/libzmq/pull/1832 @@ -370,17 +384,22 @@ int zmq::curve_server_t::produce_welcome (msg_t *msg_) int zmq::curve_server_t::process_initiate (msg_t *msg_) { - if (msg_->size () < 257) { - // CURVE I: client INITIATE is not correct size - current_error_detail = zmtp; + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return -1; + + const uint8_t *initiate = static_cast (msg_->data ()); + if (memcmp (initiate, "\x08INITIATE", 9)) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } - const uint8_t *initiate = static_cast (msg_->data ()); - if (memcmp (initiate, "\x08INITIATE", 9)) { - // CURVE I: client INITIATE has invalid command name - current_error_detail = zmtp; + if (msg_->size () < 257) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE); errno = EPROTO; return -1; } @@ -396,12 +415,12 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_) memcpy (cookie_nonce, "COOKIE--", 8); memcpy (cookie_nonce + 8, initiate + 9, 16); - int rc = crypto_secretbox_open (cookie_plaintext, cookie_box, - sizeof cookie_box, - cookie_nonce, cookie_key); + rc = crypto_secretbox_open (cookie_plaintext, cookie_box, sizeof cookie_box, + cookie_nonce, cookie_key); if (rc != 0) { // CURVE I: cannot open client INITIATE cookie - current_error_detail = encryption; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; return -1; } @@ -413,7 +432,8 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_) // client that knows the server's secret temporary cookie key // CURVE I: client INITIATE cookie is not valid - current_error_detail = encryption; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; return -1; } @@ -437,7 +457,8 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_) clen, initiate_nonce, cn_client, cn_secret); if (rc != 0) { // CURVE I: cannot open client INITIATE - current_error_detail = encryption; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; return -1; } @@ -462,7 +483,8 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_) vouch_nonce, client_key, cn_secret); if (rc != 0) { // CURVE I: cannot open client INITIATE vouch - current_error_detail = encryption; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); errno = EPROTO; return -1; } @@ -473,7 +495,8 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_) // client that knows the server's secret short-term key // CURVE I: invalid handshake from client (public key) - current_error_detail = encryption; + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_KEY_EXCHANGE); errno = EPROTO; return -1; } @@ -564,4 +587,16 @@ void zmq::curve_server_t::send_zap_request (const uint8_t *key) zap_client_t::send_zap_request ("CURVE", 5, key, crypto_box_PUBLICKEYBYTES); } +int zmq::curve_server_t::check_basic_command_structure (msg_t *msg_) +{ + if (msg_->size () <= 1 || msg_->size () <= ((uint8_t *) msg_->data ())[0]) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED); + errno = EPROTO; + return -1; + } + return 0; +} + #endif diff --git a/src/curve_server.hpp b/src/curve_server.hpp index 44562716..935bb3bc 100644 --- a/src/curve_server.hpp +++ b/src/curve_server.hpp @@ -104,6 +104,7 @@ namespace zmq int produce_error (msg_t *msg_) const; void send_zap_request (const uint8_t *key); + int check_basic_command_structure (msg_t *msg_); }; } diff --git a/src/mechanism.hpp b/src/mechanism.hpp index 1e38e45e..8b47d963 100644 --- a/src/mechanism.hpp +++ b/src/mechanism.hpp @@ -53,14 +53,6 @@ namespace zmq error }; - // Provides more details when in status_t::error - enum error_detail_t { - no_detail, - zmtp, - zap, - encryption - }; - mechanism_t (const options_t &options_); virtual ~mechanism_t (); @@ -81,10 +73,6 @@ namespace zmq // Returns the status of this mechanism. virtual status_t status () const = 0; - // Returns details about of the current error of the mechanism. - // Returned value does not makes sense if the current status is not error. - virtual error_detail_t error_detail () const { return no_detail; } - void set_peer_identity (const void *id_ptr, size_t id_size); void peer_identity (msg_t *msg_); diff --git a/src/socket_base.cpp b/src/socket_base.cpp index 5cb2c085..44f904fd 100644 --- a/src/socket_base.cpp +++ b/src/socket_base.cpp @@ -1692,22 +1692,16 @@ void zmq::socket_base_t::event_handshake_failed_no_detail ( event (addr_, err_, ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL); } -void zmq::socket_base_t::event_handshake_failed_zmtp (const std::string &addr_, - int err_) -{ - event (addr_, err_, ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP); -} - -void zmq::socket_base_t::event_handshake_failed_zap (const std::string &addr_, - int err_) -{ - event (addr_, err_, ZMQ_EVENT_HANDSHAKE_FAILED_ZAP); -} - -void zmq::socket_base_t::event_handshake_failed_encryption ( +void zmq::socket_base_t::event_handshake_failed_protocol ( const std::string &addr_, int err_) { - event (addr_, err_, ZMQ_EVENT_HANDSHAKE_FAILED_ENCRYPTION); + event (addr_, err_, ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL); +} + +void zmq::socket_base_t::event_handshake_failed_auth (const std::string &addr_, + int err_) +{ + event (addr_, err_, ZMQ_EVENT_HANDSHAKE_FAILED_AUTH); } void zmq::socket_base_t::event_handshake_succeeded (const std::string &addr_, diff --git a/src/socket_base.hpp b/src/socket_base.hpp index f953a9a9..a8b100e1 100644 --- a/src/socket_base.hpp +++ b/src/socket_base.hpp @@ -134,9 +134,8 @@ namespace zmq void event_close_failed (const std::string &addr_, int err_); void event_disconnected (const std::string &addr_, zmq::fd_t fd_); void event_handshake_failed_no_detail(const std::string &addr_, int err_); - void event_handshake_failed_zmtp(const std::string &addr_, int err_); - void event_handshake_failed_zap(const std::string &addr_, int err_); - void event_handshake_failed_encryption(const std::string &addr_, int err_); + void event_handshake_failed_protocol(const std::string &addr_, int err_); + void event_handshake_failed_auth(const std::string &addr_, int err_); void event_handshake_succeeded(const std::string &addr_, int err_); protected: diff --git a/src/stream_engine.cpp b/src/stream_engine.cpp index 7cf13f21..bbe232a2 100644 --- a/src/stream_engine.cpp +++ b/src/stream_engine.cpp @@ -702,6 +702,8 @@ bool zmq::stream_engine_t::handshake () } #endif else { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MECHANISM_MISMATCH); error (protocol_error); return false; } @@ -979,21 +981,12 @@ void zmq::stream_engine_t::error (error_reason_t reason) } zmq_assert (session); #ifdef ZMQ_BUILD_DRAFT_API - int err = errno; - if (mechanism == NULL) { - if (reason == protocol_error) - socket->event_handshake_failed_zmtp (endpoint, err); - else - socket->event_handshake_failed_no_detail (endpoint, err); - } else if (mechanism->status () == mechanism_t::handshaking) { - if (mechanism->error_detail () == mechanism_t::zmtp) - socket->event_handshake_failed_zmtp (endpoint, err); - else if (mechanism->error_detail () == mechanism_t::zap) - socket->event_handshake_failed_zap (endpoint, err); - else if (mechanism->error_detail () == mechanism_t::encryption) - socket->event_handshake_failed_encryption (endpoint, err); - else - socket->event_handshake_failed_no_detail (endpoint, err); + // protocol errors have been signaled already at the point where they occurred + if (reason != protocol_error + && (mechanism == NULL + || mechanism->status () == mechanism_t::handshaking)) { + int err = errno; + socket->event_handshake_failed_no_detail (endpoint, err); } #endif socket->event_disconnected (endpoint, (int) s); diff --git a/src/zap_client.cpp b/src/zap_client.cpp index d535ebb3..396880f1 100644 --- a/src/zap_client.cpp +++ b/src/zap_client.cpp @@ -40,8 +40,7 @@ zap_client_t::zap_client_t (session_base_t *const session_, const options_t &options_) : mechanism_t (options_), session (session_), - peer_address (peer_address_), - current_error_detail (no_detail) + peer_address (peer_address_) { } @@ -155,34 +154,35 @@ int zap_client_t::receive_and_process_zap_reply () return close_and_return (msg, -1); } if ((msg[i].flags () & msg_t::more) == (i < 6 ? 0 : msg_t::more)) { - // CURVE I : ZAP handler sent incomplete reply message + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZAP_MALFORMED_REPLY); errno = EPROTO; - current_error_detail = mechanism_t::zap; return close_and_return (msg, -1); } } // Address delimiter frame if (msg[0].size () > 0) { - // CURVE I: ZAP handler sent malformed reply message + // TODO can a ZAP handler produce such a message at all? + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZAP_UNSPECIFIED); errno = EPROTO; - current_error_detail = mechanism_t::zap; return close_and_return (msg, -1); } // Version frame if (msg[1].size () != 3 || memcmp (msg[1].data (), "1.0", 3)) { - // CURVE I: ZAP handler sent bad version number + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZAP_BAD_VERSION); errno = EPROTO; - current_error_detail = mechanism_t::zap; return close_and_return (msg, -1); } // Request id frame if (msg[2].size () != 1 || memcmp (msg[2].data (), "1", 1)) { - // CURVE I: ZAP handler sent bad request ID + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZAP_BAD_REQUEST_ID); errno = EPROTO; - current_error_detail = mechanism_t::zap; return close_and_return (msg, -1); } @@ -191,9 +191,9 @@ int zap_client_t::receive_and_process_zap_reply () if (msg[3].size () != 3 || status_code_data[0] < '2' || status_code_data[0] > '5' || status_code_data[1] != '0' || status_code_data[2] != '0') { - // CURVE I: ZAP handler sent invalid status code + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZAP_INVALID_STATUS_CODE); errno = EPROTO; - current_error_detail = mechanism_t::zap; return close_and_return (msg, -1); } @@ -208,6 +208,9 @@ int zap_client_t::receive_and_process_zap_reply () msg[6].size (), true); if (rc != 0) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZAP_INVALID_METADATA); + errno = EPROTO; return close_and_return (msg, -1); } @@ -226,30 +229,23 @@ void zap_client_t::handle_zap_status_code () { // we can assume here that status_code is a valid ZAP status code, // i.e. 200, 300, 400 or 500 - int err = 0; + int status_code_numeric = 0; switch (status_code[0]) { case '2': return; case '3': - err = EAGAIN; + status_code_numeric = 300; break; case '4': - err = EACCES; + status_code_numeric = 400; break; case '5': - err = EFAULT; + status_code_numeric = 500; break; } - // TODO use event_handshake_failed_zap here? but this is not a ZAP - // protocol error - session->get_socket ()->event_handshake_failed_no_detail ( - session->get_endpoint (), err); -} - -mechanism_t::error_detail_t zap_client_t::error_detail () const -{ - return current_error_detail; + session->get_socket ()->event_handshake_failed_auth ( + session->get_endpoint (), status_code_numeric); } zap_client_common_handshake_t::zap_client_common_handshake_t ( diff --git a/src/zap_client.hpp b/src/zap_client.hpp index 5f67ea29..d86c10ee 100644 --- a/src/zap_client.hpp +++ b/src/zap_client.hpp @@ -57,19 +57,12 @@ class zap_client_t : public virtual mechanism_t virtual int receive_and_process_zap_reply (); virtual void handle_zap_status_code (); - // methods from mechanism_t - error_detail_t error_detail () const; - protected: session_base_t *const session; const std::string peer_address; // Status code as received from ZAP handler std::string status_code; - - // Details about the current error state - // TODO remove this - error_detail_t current_error_detail; }; class zap_client_common_handshake_t : public zap_client_t diff --git a/src/zmq_draft.h b/src/zmq_draft.h index e4aefe7f..2c82d174 100644 --- a/src/zmq_draft.h +++ b/src/zmq_draft.h @@ -50,11 +50,37 @@ #define ZMQ_BINDTODEVICE 90 /* DRAFT 0MQ socket events and monitoring */ -#define ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL 0x0800 +/* Unspecified system errors during handshake. Event value is an errno. */ +#define ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL 0x0800 +/* Handshake complete successfully with successful authentication (if * + * enabled). Event value is unused. */ #define ZMQ_EVENT_HANDSHAKE_SUCCEEDED 0x1000 -#define ZMQ_EVENT_HANDSHAKE_FAILED_ENCRYPTION 0x2000 -#define ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP 0x4000 -#define ZMQ_EVENT_HANDSHAKE_FAILED_ZAP 0x8000 +/* Protocol errors between ZMTP peers or between server and ZAP handler. * + * Event value is one of ZMQ_PROTOCOL_ERROR_* */ +#define ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL 0x2000 +/* Failed authentication requests. Event value is the numeric ZAP status * + * code, i.e. 300, 400 or 500. */ +#define ZMQ_EVENT_HANDSHAKE_FAILED_AUTH 0x4000 + +#define ZMQ_PROTOCOL_ERROR_ZMTP_UNSPECIFIED 0x10000000 +#define ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND 0x10000001 +#define ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_SEQUENCE 0x10000002 +#define ZMQ_PROTOCOL_ERROR_ZMTP_KEY_EXCHANGE 0x10000003 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED 0x10000011 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE 0x10000012 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO 0x10000013 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE 0x10000014 + +// the following two may be due to erroneous configuration of a peer +#define ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC 0x11000001 +#define ZMQ_PROTOCOL_ERROR_ZMTP_MECHANISM_MISMATCH 0x11000002 + +#define ZMQ_PROTOCOL_ERROR_ZAP_UNSPECIFIED 0x20000000 +#define ZMQ_PROTOCOL_ERROR_ZAP_MALFORMED_REPLY 0x20000001 +#define ZMQ_PROTOCOL_ERROR_ZAP_BAD_REQUEST_ID 0x20000002 +#define ZMQ_PROTOCOL_ERROR_ZAP_BAD_VERSION 0x20000003 +#define ZMQ_PROTOCOL_ERROR_ZAP_INVALID_STATUS_CODE 0x20000004 +#define ZMQ_PROTOCOL_ERROR_ZAP_INVALID_METADATA 0x20000005 /* DRAFT Context options */ #define ZMQ_MSG_T_SIZE 6 diff --git a/tests/test_security_curve.cpp b/tests/test_security_curve.cpp index 222cdce2..04ed9bfd 100644 --- a/tests/test_security_curve.cpp +++ b/tests/test_security_curve.cpp @@ -103,7 +103,8 @@ void test_garbage_key(void *ctx, #ifdef ZMQ_BUILD_DRAFT_API int handshake_failed_encryption_event_count = expect_monitor_event_multiple (server_mon, - ZMQ_EVENT_HANDSHAKE_FAILED_ENCRYPTION); + ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); // handshake_failed_encryption_event_count should be at least two because // expect_bounce_fail involves two exchanges @@ -149,9 +150,8 @@ void test_curve_security_with_bogus_client_credentials ( int event_count = 0; #ifdef ZMQ_BUILD_DRAFT_API - // TODO add another event type ZMQ_EVENT_HANDSHAKE_FAILED_AUTH for this case? event_count = expect_monitor_event_multiple ( - server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL, EACCES); + server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_AUTH, 400); assert (event_count <= 1); #endif @@ -160,7 +160,10 @@ void test_curve_security_with_bogus_client_credentials ( || 1 <= zmq_atomic_counter_value (zap_requests_handled)); } -void expect_zmtp_failure (void *client, char *my_endpoint, void *server, void *server_mon) +void expect_zmtp_mechanism_mismatch (void *client, + char *my_endpoint, + void *server, + void *server_mon) { // This must be caught by the curve_server class, not passed to ZAP int rc = zmq_connect (client, my_endpoint); @@ -169,7 +172,9 @@ void expect_zmtp_failure (void *client, char *my_endpoint, void *server, void *s close_zero_linger (client); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple (server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP); + expect_monitor_event_multiple (server_mon, + ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_MECHANISM_MISMATCH); #endif assert (0 == zmq_atomic_counter_value (zap_requests_handled)); @@ -183,7 +188,7 @@ void test_curve_security_with_null_client_credentials (void *ctx, void *client = zmq_socket (ctx, ZMQ_DEALER); assert (client); - expect_zmtp_failure (client, my_endpoint, server, server_mon); + expect_zmtp_mechanism_mismatch (client, my_endpoint, server, server_mon); } void test_curve_security_with_plain_client_credentials (void *ctx, @@ -198,7 +203,7 @@ void test_curve_security_with_plain_client_credentials (void *ctx, rc = zmq_setsockopt (client, ZMQ_PLAIN_PASSWORD, "password", 8); assert (rc == 0); - expect_zmtp_failure (client, my_endpoint, server, server_mon); + expect_zmtp_mechanism_mismatch (client, my_endpoint, server, server_mon); } int connect_vanilla_socket (char *my_endpoint) @@ -279,11 +284,12 @@ void test_curve_security_invalid_hello_wrong_length (char *my_endpoint, send_greeting (s); // send CURVE HELLO of wrong size - send(s, "\x04\x05HELLO"); + send(s, "\x04\x06\x05HELLO"); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple (server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP, - EPROTO); + expect_monitor_event_multiple ( + server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); #endif close (s); @@ -361,8 +367,9 @@ void test_curve_security_invalid_hello_command_name (char *my_endpoint, send_command(s, hello); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple (server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP, - EPROTO); + expect_monitor_event_multiple (server_mon, + ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); #endif close (s); @@ -388,8 +395,9 @@ void test_curve_security_invalid_hello_version (char *my_endpoint, send_command (s, hello); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple (server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP, - EPROTO); + expect_monitor_event_multiple ( + server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); #endif close (s); @@ -459,11 +467,12 @@ void test_curve_security_invalid_initiate_length (char *my_endpoint, assert (res == -1); #endif - send(s, "\x04\x08INITIATE"); + send(s, "\x04\x09\x08INITIATE"); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple (server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP, - EPROTO); + expect_monitor_event_multiple ( + server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE); #endif close (s); @@ -510,8 +519,9 @@ void test_curve_security_invalid_initiate_command_name (char *my_endpoint, send_command (s, initiate); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple (server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP, - EPROTO); + expect_monitor_event_multiple (server_mon, + ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); #endif close (s); @@ -532,8 +542,9 @@ void test_curve_security_invalid_initiate_command_encrypted_cookie ( send_command (s, initiate); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple ( - server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ENCRYPTION, EPROTO); + expect_monitor_event_multiple (server_mon, + ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); #endif close (s); @@ -554,8 +565,9 @@ void test_curve_security_invalid_initiate_command_encrypted_content ( send_command (s, initiate); #ifdef ZMQ_BUILD_DRAFT_API - expect_monitor_event_multiple ( - server_mon, ZMQ_EVENT_HANDSHAKE_FAILED_ENCRYPTION, EPROTO); + expect_monitor_event_multiple (server_mon, + ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, + ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); #endif close (s); @@ -648,7 +660,6 @@ int main (void) server_mon, timeout); shutdown_context_and_server_side (ctx, zap_thread, server, server_mon, handler); - fprintf (stderr, "test_curve_security_with_null_client_credentials\n"); setup_context_and_server_side (&ctx, &handler, &zap_thread, &server, &server_mon, my_endpoint); @@ -664,7 +675,6 @@ int main (void) server_mon); shutdown_context_and_server_side (ctx, zap_thread, server, server_mon, handler); - fprintf (stderr, "test_curve_security_unauthenticated_message\n"); setup_context_and_server_side (&ctx, &handler, &zap_thread, &server, &server_mon, my_endpoint); diff --git a/tests/test_security_zap.cpp b/tests/test_security_zap.cpp index 292f4ea7..b07b2492 100644 --- a/tests/test_security_zap.cpp +++ b/tests/test_security_zap.cpp @@ -87,11 +87,12 @@ void test_zap_protocol_error (void *ctx, void *server, void *server_mon, socket_config_fn socket_config_, - void *socket_config_data_) + void *socket_config_data_, + int expected_error) { test_zap_unsuccessful (ctx, my_endpoint, server, server_mon, #ifdef ZMQ_BUILD_DRAFT_API - ZMQ_EVENT_HANDSHAKE_FAILED_ZAP, EPROTO, + ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL, expected_error, #else 0, 0, #endif @@ -119,7 +120,13 @@ void test_zap_errors (socket_config_fn server_socket_config_, &zap_handler_wrong_version, server_socket_config_, server_socket_config_data_); test_zap_protocol_error (ctx, my_endpoint, server, server_mon, - client_socket_config_, client_socket_config_data_); + client_socket_config_, client_socket_config_data_, +#ifdef ZMQ_BUILD_DRAFT_API + ZMQ_PROTOCOL_ERROR_ZAP_BAD_VERSION +#else + 0 +#endif + ); shutdown_context_and_server_side (ctx, zap_thread, server, server_mon, handler); @@ -130,7 +137,13 @@ void test_zap_errors (socket_config_fn server_socket_config_, &zap_handler_wrong_request_id, server_socket_config_, server_socket_config_data_); test_zap_protocol_error (ctx, my_endpoint, server, server_mon, - client_socket_config_, client_socket_config_data_); + client_socket_config_, client_socket_config_data_, +#ifdef ZMQ_BUILD_DRAFT_API + ZMQ_PROTOCOL_ERROR_ZAP_BAD_REQUEST_ID +#else + 0 +#endif + ); shutdown_context_and_server_side (ctx, zap_thread, server, server_mon, handler); @@ -141,7 +154,13 @@ void test_zap_errors (socket_config_fn server_socket_config_, &zap_handler_wrong_status_invalid, server_socket_config_, server_socket_config_data_); test_zap_protocol_error (ctx, my_endpoint, server, server_mon, - client_socket_config_, client_socket_config_data_); + client_socket_config_, client_socket_config_data_, +#ifdef ZMQ_BUILD_DRAFT_API + ZMQ_PROTOCOL_ERROR_ZAP_INVALID_STATUS_CODE +#else + 0 +#endif + ); shutdown_context_and_server_side (ctx, zap_thread, server, server_mon, handler); @@ -152,7 +171,13 @@ void test_zap_errors (socket_config_fn server_socket_config_, &zap_handler_too_many_parts, server_socket_config_, server_socket_config_data_); test_zap_protocol_error (ctx, my_endpoint, server, server_mon, - client_socket_config_, client_socket_config_data_); + client_socket_config_, client_socket_config_data_, +#ifdef ZMQ_BUILD_DRAFT_API + ZMQ_PROTOCOL_ERROR_ZAP_MALFORMED_REPLY +#else + 0 +#endif + ); shutdown_context_and_server_side (ctx, zap_thread, server, server_mon, handler); @@ -169,7 +194,7 @@ void test_zap_errors (socket_config_fn server_socket_config_, server_socket_config_data_); test_zap_unsuccessful (ctx, my_endpoint, server, server_mon, #ifdef ZMQ_BUILD_DRAFT_API - ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL, EAGAIN, + ZMQ_EVENT_HANDSHAKE_FAILED_AUTH, 300, #else 0, 0, #endif @@ -184,7 +209,7 @@ void test_zap_errors (socket_config_fn server_socket_config_, &zap_handler_wrong_status_internal_error, server_socket_config_); test_zap_unsuccessful (ctx, my_endpoint, server, server_mon, #ifdef ZMQ_BUILD_DRAFT_API - ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL, EFAULT, + ZMQ_EVENT_HANDSHAKE_FAILED_AUTH, 500, #else 0, 0, #endif diff --git a/tests/testutil_security.hpp b/tests/testutil_security.hpp index 96300eac..811c14d3 100644 --- a/tests/testutil_security.hpp +++ b/tests/testutil_security.hpp @@ -331,11 +331,11 @@ void setup_context_and_server_side ( char monitor_endpoint [] = "inproc://monitor-server"; // Monitor handshake events on the server - rc = zmq_socket_monitor ( - *server, monitor_endpoint, - ZMQ_EVENT_HANDSHAKE_SUCCEEDED | ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL - | ZMQ_EVENT_HANDSHAKE_FAILED_ZAP | ZMQ_EVENT_HANDSHAKE_FAILED_ZMTP - | ZMQ_EVENT_HANDSHAKE_FAILED_ENCRYPTION); + rc = zmq_socket_monitor (*server, monitor_endpoint, + ZMQ_EVENT_HANDSHAKE_SUCCEEDED + | ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL + | ZMQ_EVENT_HANDSHAKE_FAILED_AUTH + | ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL); assert (rc == 0); // Create socket for collecting monitor events @@ -514,7 +514,11 @@ int expect_monitor_event_multiple (void *server_mon, } if (event != expected_event || (-1 != expected_err && err != expected_err)) { - fprintf (stderr, "Unexpected event: %x (err = %i)\n", event, err); + fprintf ( + stderr, + "Unexpected event: 0x%x, value = %i/0x%x (expected: 0x%x, value " + "= %i/0x%x)\n", + event, err, err, expected_event, expected_err, expected_err); assert (false); } ++count_of_expected_events; From 11b3c938521064c4d08393d06d73890b37ead8b1 Mon Sep 17 00:00:00 2001 From: sigiesec Date: Thu, 17 Aug 2017 18:16:31 +0200 Subject: [PATCH 2/9] Problem: console output for PLAIN protocol errors Solution: emit socket monitor events for PLAIN protocol errors (like CURVE) --- src/curve_server.cpp | 32 +++++++++++--------------- src/curve_server.hpp | 1 - src/plain_server.cpp | 54 ++++++++++++++++++++++++++------------------ src/zap_client.cpp | 12 ++++++++++ src/zap_client.hpp | 1 + 5 files changed, 58 insertions(+), 42 deletions(-) diff --git a/src/curve_server.cpp b/src/curve_server.cpp index af18acc5..ec0d1b49 100644 --- a/src/curve_server.cpp +++ b/src/curve_server.cpp @@ -182,15 +182,17 @@ int zmq::curve_server_t::decode (msg_t *msg_) if (rc == -1) return -1; + const size_t size = msg_->size (); const uint8_t *message = static_cast (msg_->data ()); - if (memcmp (message, "\x07MESSAGE", 8)) { + + if (size < 8 || memcmp (message, "\x07MESSAGE", 8)) { session->get_socket ()->event_handshake_failed_protocol ( session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } - if (msg_->size () < 33) { + if (size < 33) { session->get_socket ()->event_handshake_failed_protocol ( session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); @@ -259,15 +261,17 @@ int zmq::curve_server_t::process_hello (msg_t *msg_) if (rc == -1) return -1; + const size_t size = msg_->size (); const uint8_t * const hello = static_cast (msg_->data ()); - if (memcmp (hello, "\x05HELLO", 6)) { + + if (size < 6 || memcmp (hello, "\x05HELLO", 6)) { session->get_socket ()->event_handshake_failed_protocol ( session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } - if (msg_->size () != 200) { + if (size != 200) { session->get_socket ()->event_handshake_failed_protocol ( session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; @@ -388,15 +392,17 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_) if (rc == -1) return -1; + const size_t size = msg_->size (); const uint8_t *initiate = static_cast (msg_->data ()); - if (memcmp (initiate, "\x08INITIATE", 9)) { + + if (size < 9 || memcmp (initiate, "\x08INITIATE", 9)) { session->get_socket ()->event_handshake_failed_protocol ( session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } - if (msg_->size () < 257) { + if (size < 257) { session->get_socket ()->event_handshake_failed_protocol ( session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE); @@ -438,7 +444,7 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_) return -1; } - const size_t clen = (msg_->size () - 113) + crypto_box_BOXZEROBYTES; + const size_t clen = (size - 113) + crypto_box_BOXZEROBYTES; uint8_t initiate_nonce [crypto_box_NONCEBYTES]; uint8_t initiate_plaintext [crypto_box_ZEROBYTES + 128 + 256]; @@ -587,16 +593,4 @@ void zmq::curve_server_t::send_zap_request (const uint8_t *key) zap_client_t::send_zap_request ("CURVE", 5, key, crypto_box_PUBLICKEYBYTES); } -int zmq::curve_server_t::check_basic_command_structure (msg_t *msg_) -{ - if (msg_->size () <= 1 || msg_->size () <= ((uint8_t *) msg_->data ())[0]) { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED); - errno = EPROTO; - return -1; - } - return 0; -} - #endif diff --git a/src/curve_server.hpp b/src/curve_server.hpp index 935bb3bc..44562716 100644 --- a/src/curve_server.hpp +++ b/src/curve_server.hpp @@ -104,7 +104,6 @@ namespace zmq int produce_error (msg_t *msg_) const; void send_zap_request (const uint8_t *key); - int check_basic_command_structure (msg_t *msg_); }; } diff --git a/src/plain_server.cpp b/src/plain_server.cpp index 27bf7db3..f205c1ac 100644 --- a/src/plain_server.cpp +++ b/src/plain_server.cpp @@ -89,8 +89,9 @@ int zmq::plain_server_t::process_handshake_command (msg_t *msg_) rc = process_initiate (msg_); break; default: - // Temporary support for security debugging - puts ("PLAIN I: invalid handshake command"); + // TODO see comment in curve_server_t::process_handshake_command + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNSPECIFIED); errno = EPROTO; rc = -1; break; @@ -106,12 +107,16 @@ int zmq::plain_server_t::process_handshake_command (msg_t *msg_) int zmq::plain_server_t::process_hello (msg_t *msg_) { + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return -1; + const unsigned char *ptr = static_cast (msg_->data ()); - size_t bytes_left = msg_->size (); + int bytes_left = msg_->size (); if (bytes_left < 6 || memcmp (ptr, "\x05HELLO", 6)) { - // Temporary support for security debugging - puts ("PLAIN I: invalid PLAIN client, did not send HELLO"); + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } @@ -119,17 +124,19 @@ int zmq::plain_server_t::process_hello (msg_t *msg_) bytes_left -= 6; if (bytes_left < 1) { - // Temporary support for security debugging - puts ("PLAIN I: invalid PLAIN client, did not send username"); + // PLAIN I: invalid PLAIN client, did not send username + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; return -1; } - const size_t username_length = static_cast (*ptr++); + const uint8_t username_length = *ptr++; bytes_left -= 1; - if (bytes_left < username_length) { - // Temporary support for security debugging - puts ("PLAIN I: invalid PLAIN client, sent malformed username"); + if (bytes_left < (int)username_length) { + // PLAIN I: invalid PLAIN client, sent malformed username + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; return -1; } @@ -137,17 +144,19 @@ int zmq::plain_server_t::process_hello (msg_t *msg_) ptr += username_length; bytes_left -= username_length; if (bytes_left < 1) { - // Temporary support for security debugging - puts ("PLAIN I: invalid PLAIN client, did not send password"); + // PLAIN I: invalid PLAIN client, did not send password + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; return -1; } - const size_t password_length = static_cast (*ptr++); + const uint8_t password_length = *ptr++; bytes_left -= 1; - if (bytes_left < password_length) { - // Temporary support for security debugging - puts ("PLAIN I: invalid PLAIN client, sent malformed password"); + if (bytes_left < (int)password_length) { + // PLAIN I: invalid PLAIN client, sent malformed password + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; return -1; } @@ -156,8 +165,9 @@ int zmq::plain_server_t::process_hello (msg_t *msg_) ptr += password_length; bytes_left -= password_length; if (bytes_left > 0) { - // Temporary support for security debugging - puts ("PLAIN I: invalid PLAIN client, sent extraneous data"); + // PLAIN I: invalid PLAIN client, sent extraneous data + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_HELLO); errno = EPROTO; return -1; } @@ -166,7 +176,7 @@ int zmq::plain_server_t::process_hello (msg_t *msg_) // Note that there is no point to PLAIN if ZAP is not set up to handle the // username and password, so if ZAP is not configured it is considered a // failure. - int rc = session->zap_connect (); + rc = session->zap_connect (); if (rc != 0) return -1; send_zap_request (username, password); @@ -187,8 +197,8 @@ int zmq::plain_server_t::process_initiate (msg_t *msg_) const size_t bytes_left = msg_->size (); if (bytes_left < 9 || memcmp (ptr, "\x08INITIATE", 9)) { - // Temporary support for security debugging - puts ("PLAIN I: invalid PLAIN client, did not send INITIATE"); + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } diff --git a/src/zap_client.cpp b/src/zap_client.cpp index 396880f1..9a5b7145 100644 --- a/src/zap_client.cpp +++ b/src/zap_client.cpp @@ -248,6 +248,18 @@ void zap_client_t::handle_zap_status_code () session->get_endpoint (), status_code_numeric); } +int zap_client_t::check_basic_command_structure (msg_t *msg_) +{ + if (msg_->size () <= 1 || msg_->size () <= ((uint8_t *) msg_->data ())[0]) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED); + errno = EPROTO; + return -1; + } + return 0; +} + zap_client_common_handshake_t::zap_client_common_handshake_t ( session_base_t *const session_, const std::string &peer_address_, diff --git a/src/zap_client.hpp b/src/zap_client.hpp index d86c10ee..cb2837ed 100644 --- a/src/zap_client.hpp +++ b/src/zap_client.hpp @@ -57,6 +57,7 @@ class zap_client_t : public virtual mechanism_t virtual int receive_and_process_zap_reply (); virtual void handle_zap_status_code (); + int check_basic_command_structure (msg_t *msg_); protected: session_base_t *const session; const std::string peer_address; From 9bec68354c2b7d16f9be16c726aae4ce6df36e3e Mon Sep 17 00:00:00 2001 From: sigiesec Date: Thu, 17 Aug 2017 18:20:45 +0200 Subject: [PATCH 3/9] Problem: console output for NULL protocol errors Solution: emit socket monitor events for NULL protocol errors (like CURVE) --- include/zmq.h | 1 + src/null_mechanism.cpp | 19 ++++++++++++++----- src/zmq_draft.h | 1 + 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/include/zmq.h b/include/zmq.h index 47dff04a..9477575a 100644 --- a/include/zmq.h +++ b/include/zmq.h @@ -588,6 +588,7 @@ ZMQ_EXPORT void zmq_threadclose (void* thread); #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE 0x10000012 #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 // the following two may be due to erroneous configuration of a peer #define ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC 0x11000001 diff --git a/src/null_mechanism.cpp b/src/null_mechanism.cpp index cca61482..c765bdee 100644 --- a/src/null_mechanism.cpp +++ b/src/null_mechanism.cpp @@ -104,8 +104,9 @@ int zmq::null_mechanism_t::next_handshake_command (msg_t *msg_) int zmq::null_mechanism_t::process_handshake_command (msg_t *msg_) { if (ready_command_received || error_command_received) { - // Temporary support for security debugging - puts ("NULL I: client sent invalid NULL handshake (duplicate READY)"); + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } @@ -121,8 +122,9 @@ int zmq::null_mechanism_t::process_handshake_command (msg_t *msg_) if (data_size >= 6 && !memcmp (cmd_data, "\5ERROR", 6)) rc = process_error_command (cmd_data, data_size); else { - // Temporary support for security debugging - puts ("NULL I: client sent invalid NULL handshake (not READY)"); + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; rc = -1; } @@ -147,11 +149,19 @@ int zmq::null_mechanism_t::process_error_command ( const unsigned char *cmd_data, size_t data_size) { if (data_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 (cmd_data [6]); if (error_reason_len > data_size - 7) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_ERROR); + errno = EPROTO; return -1; } @@ -191,4 +201,3 @@ void zmq::null_mechanism_t::send_zap_request () { zap_client_t::send_zap_request ("NULL", 4, NULL, NULL, 0); } - diff --git a/src/zmq_draft.h b/src/zmq_draft.h index 2c82d174..89386ea6 100644 --- a/src/zmq_draft.h +++ b/src/zmq_draft.h @@ -70,6 +70,7 @@ #define ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE 0x10000012 #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 // the following two may be due to erroneous configuration of a peer #define ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC 0x11000001 From e22ca065d6ced6c9cab7d13d453b8b2d706fa25b Mon Sep 17 00:00:00 2001 From: sigiesec Date: Thu, 17 Aug 2017 18:32:44 +0200 Subject: [PATCH 4/9] 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 From bdd0f3b18bf4e326495581a2c8def38471d745e1 Mon Sep 17 00:00:00 2001 From: sigiesec Date: Fri, 18 Aug 2017 09:35:13 +0200 Subject: [PATCH 5/9] Problem: documentation on zmq_socket_monitor out-of-sync with current state of ZMQ_EVENT_HANDSHAKE_FAILED_* events Solution: update documentation --- doc/zmq_socket_monitor.txt | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/doc/zmq_socket_monitor.txt b/doc/zmq_socket_monitor.txt index 71d664e7..29125d16 100644 --- a/doc/zmq_socket_monitor.txt +++ b/doc/zmq_socket_monitor.txt @@ -99,18 +99,29 @@ ZMQ_EVENT_MONITOR_STOPPED ~~~~~~~~~~~~~~~~~~~~~~~~~ Monitoring on this socket ended. -ZMQ_EVENT_HANDSHAKE_FAILED -~~~~~~~~~~~~~~~~~~~~~~~~~~ -The ZMTP security mechanism handshake failed. -The event value is unspecified. -NOTE: in DRAFT state, not yet available in stable releases. - -ZMQ_EVENT_HANDSHAKE_SUCCEED +ZMQ_EVENT_HANDSHAKE_SUCCEEDED ~~~~~~~~~~~~~~~~~~~~~~~~~~~ The ZMTP security mechanism handshake succeeded. The event value is unspecified. NOTE: in DRAFT state, not yet available in stable releases. +ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The ZMTP security mechanism handshake failed due to some mechanism protocol +error, either between the ZMTP mechanism peers, or between the mechanism +server and the ZAP handler. This indicates a configuration or implementation +error in either peer resp. the ZAP handler. +The event value is one of the ZMQ_PROTOCOL_ERROR_* values. +NOTE: in DRAFT state, not yet available in stable releases. + +ZMQ_EVENT_HANDSHAKE_FAILED_AUTH +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The ZMTP security mechanism handshake failed due to an authentication failure. +The event value is the status code returned by the ZAP handler (i.e. 300, +400 or 500). +NOTE: in DRAFT state, not yet available in stable releases. + + RETURN VALUE ------------ From c66ae4656fae001318de922760884678712b86d5 Mon Sep 17 00:00:00 2001 From: sigiesec Date: Fri, 18 Aug 2017 10:04:58 +0200 Subject: [PATCH 6/9] Problem: curve_client_t may emit misleading event on bad data processed by curve_client_t::decode Solution: use check_basic_command_structure in curve_client_t::decode, also prepare other client mechanisms to use that method by rearranging inheritance hierarchy --- src/curve_client.cpp | 20 +++++++++++--------- src/curve_client.hpp | 4 +--- src/curve_server.cpp | 2 +- src/gssapi_client.cpp | 4 +++- src/gssapi_client.hpp | 9 ++++----- src/gssapi_mechanism_base.cpp | 7 +++++-- src/gssapi_mechanism_base.hpp | 11 ++++++----- src/gssapi_server.cpp | 2 +- src/gssapi_server.hpp | 23 +++++++++++------------ src/mechanism.cpp | 22 ++++++++++++++++++++++ src/mechanism.hpp | 17 ++++++++++++++--- src/null_mechanism.cpp | 2 +- src/plain_server.cpp | 2 +- src/stream_engine.cpp | 3 ++- src/zap_client.cpp | 17 ++--------------- src/zap_client.hpp | 7 +------ 16 files changed, 86 insertions(+), 66 deletions(-) diff --git a/src/curve_client.cpp b/src/curve_client.cpp index 09107654..49e7c471 100644 --- a/src/curve_client.cpp +++ b/src/curve_client.cpp @@ -41,8 +41,7 @@ zmq::curve_client_t::curve_client_t (session_base_t *session_, const options_t &options_) : - mechanism_t (options_), - session (session_), + mechanism_base_t (session_, options_), state (send_hello), tools (options_.curve_public_key, options_.curve_secret_key, @@ -166,20 +165,23 @@ int zmq::curve_client_t::encode (msg_t *msg_) int zmq::curve_client_t::decode (msg_t *msg_) { zmq_assert (state == connected); + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return rc; - if (msg_->size () < 33) { + const uint8_t *message = static_cast (msg_->data ()); + if (msg_->size() < 8 || memcmp (message, "\x07MESSAGE", 8)) { 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 + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } - const uint8_t *message = static_cast (msg_->data ()); - if (memcmp (message, "\x07MESSAGE", 8)) { + if (msg_->size () < 33) { session->get_socket ()->event_handshake_failed_protocol ( session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); errno = EPROTO; return -1; } @@ -209,8 +211,8 @@ int zmq::curve_client_t::decode (msg_t *msg_) memcpy (message_box + crypto_box_BOXZEROBYTES, message + 16, msg_->size () - 16); - int rc = crypto_box_open_afternm (message_plaintext, message_box, clen, - message_nonce, tools.cn_precom); + rc = crypto_box_open_afternm (message_plaintext, message_box, clen, + message_nonce, tools.cn_precom); if (rc == 0) { rc = msg_->close (); zmq_assert (rc == 0); diff --git a/src/curve_client.hpp b/src/curve_client.hpp index 9b2d2175..f5be67e7 100644 --- a/src/curve_client.hpp +++ b/src/curve_client.hpp @@ -42,7 +42,7 @@ namespace zmq class msg_t; class session_base_t; - class curve_client_t : public mechanism_t + class curve_client_t : public mechanism_base_t { public: @@ -67,8 +67,6 @@ namespace zmq connected }; - session_base_t *session; - // Current FSM state state_t state; diff --git a/src/curve_server.cpp b/src/curve_server.cpp index ec0d1b49..aa9f5425 100644 --- a/src/curve_server.cpp +++ b/src/curve_server.cpp @@ -41,7 +41,7 @@ zmq::curve_server_t::curve_server_t (session_base_t *session_, const std::string &peer_address_, const options_t &options_) : - mechanism_t (options_), + mechanism_base_t (session_, options_), zap_client_common_handshake_t ( session_, peer_address_, options_, sending_ready), cn_nonce (1), diff --git a/src/gssapi_client.cpp b/src/gssapi_client.cpp index 0f73db3c..942c25fa 100644 --- a/src/gssapi_client.cpp +++ b/src/gssapi_client.cpp @@ -40,7 +40,9 @@ #include "gssapi_client.hpp" #include "wire.hpp" -zmq::gssapi_client_t::gssapi_client_t (const options_t &options_) : +zmq::gssapi_client_t::gssapi_client_t (session_base_t *session_, + const options_t &options_) : + mechanism_base_t (session_, options_), gssapi_mechanism_base_t (options_), state (call_next_init), token_ptr (GSS_C_NO_BUFFER), diff --git a/src/gssapi_client.hpp b/src/gssapi_client.hpp index 0a89e149..6bfdf50c 100644 --- a/src/gssapi_client.hpp +++ b/src/gssapi_client.hpp @@ -38,13 +38,12 @@ namespace zmq { class msg_t; + class session_base_t; - class gssapi_client_t : - public gssapi_mechanism_base_t + class gssapi_client_t : public gssapi_mechanism_base_t { - public: - - gssapi_client_t (const options_t &options_); + public: + gssapi_client_t (session_base_t *session_, const options_t &options_); virtual ~gssapi_client_t (); // mechanism implementation diff --git a/src/gssapi_mechanism_base.cpp b/src/gssapi_mechanism_base.cpp index 78106e7b..65d9b124 100644 --- a/src/gssapi_mechanism_base.cpp +++ b/src/gssapi_mechanism_base.cpp @@ -40,8 +40,11 @@ #include "gssapi_mechanism_base.hpp" #include "wire.hpp" -zmq::gssapi_mechanism_base_t::gssapi_mechanism_base_t (const options_t & options_) : - mechanism_t(options_), +zmq::gssapi_mechanism_base_t::gssapi_mechanism_base_t ( + session_base_t *session_, + const std::string &peer_address_, + const options_t &options_) : + mechanism_base_t (session_, options_), send_tok (), recv_tok (), /// FIXME remove? in_buf (), diff --git a/src/gssapi_mechanism_base.hpp b/src/gssapi_mechanism_base.hpp index b8d157b8..03c947bf 100644 --- a/src/gssapi_mechanism_base.hpp +++ b/src/gssapi_mechanism_base.hpp @@ -49,14 +49,15 @@ namespace zmq /// For example, clients and servers both need to produce and /// process context-level GSSAPI tokens (via INITIATE commands) /// and per-message GSSAPI tokens (via MESSAGE commands). - class gssapi_mechanism_base_t: - public virtual mechanism_t + class gssapi_mechanism_base_t : public virtual mechanism_base_t { - public: - gssapi_mechanism_base_t (const options_t &options_); + public: + gssapi_mechanism_base_t (session_base_t *session_, + const std::string &peer_address_, + const options_t &options_); virtual ~gssapi_mechanism_base_t () = 0; - protected: + protected: // Produce a context-level GSSAPI token (INITIATE command) // during security context initialization. int produce_initiate (msg_t *msg_, void *data_, size_t data_len_); diff --git a/src/gssapi_server.cpp b/src/gssapi_server.cpp index d665c7e8..936c4499 100644 --- a/src/gssapi_server.cpp +++ b/src/gssapi_server.cpp @@ -45,7 +45,7 @@ zmq::gssapi_server_t::gssapi_server_t (session_base_t *session_, const std::string &peer_address_, const options_t &options_) : - mechanism_t (options_), + mechanism_base_t (session_, options_), gssapi_mechanism_base_t (options_), zap_client_t (session_, peer_address_, options_), state (recv_next_token), diff --git a/src/gssapi_server.hpp b/src/gssapi_server.hpp index 62a4ff66..5aa272da 100644 --- a/src/gssapi_server.hpp +++ b/src/gssapi_server.hpp @@ -45,19 +45,18 @@ namespace zmq : public gssapi_mechanism_base_t, public zap_client_t { public: + gssapi_server_t (session_base_t *session_, + const std::string &peer_address, + const options_t &options_); + virtual ~gssapi_server_t (); - gssapi_server_t (session_base_t *session_, - const std::string &peer_address, - const options_t &options_); - virtual ~gssapi_server_t (); - - // mechanism implementation - virtual int next_handshake_command (msg_t *msg_); - virtual int process_handshake_command (msg_t *msg_); - virtual int encode (msg_t *msg_); - virtual int decode (msg_t *msg_); - virtual int zap_msg_available (); - virtual status_t status () const; + // mechanism implementation + virtual int next_handshake_command (msg_t *msg_); + virtual int process_handshake_command (msg_t *msg_); + virtual int encode (msg_t *msg_); + virtual int decode (msg_t *msg_); + virtual int zap_msg_available (); + virtual status_t status () const; private: diff --git a/src/mechanism.cpp b/src/mechanism.cpp index a7b17d92..39117359 100644 --- a/src/mechanism.cpp +++ b/src/mechanism.cpp @@ -35,6 +35,7 @@ #include "msg.hpp" #include "err.hpp" #include "wire.hpp" +#include "session_base.hpp" zmq::mechanism_t::mechanism_t (const options_t &options_) : options (options_) @@ -281,3 +282,24 @@ bool zmq::mechanism_t::check_socket_type (const std::string& type_) const } return false; } + +zmq::mechanism_base_t::mechanism_base_t (session_base_t *const session_, + const options_t &options_) : + mechanism_t (options_), + session (session_) +{ + +} + +int zmq::mechanism_base_t::check_basic_command_structure (msg_t *msg_) +{ + if (msg_->size () <= 1 || msg_->size () <= ((uint8_t *) msg_->data ())[0]) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED); + errno = EPROTO; + return -1; + } + return 0; +} + diff --git a/src/mechanism.hpp b/src/mechanism.hpp index 8b47d963..e07ce567 100644 --- a/src/mechanism.hpp +++ b/src/mechanism.hpp @@ -38,11 +38,12 @@ namespace zmq { + class msg_t; + class session_base_t; + // Abstract class representing security mechanism. // Different mechanism extends this class. - class msg_t; - class mechanism_t { public: @@ -146,6 +147,16 @@ namespace zmq bool check_socket_type (const std::string& type_) const; }; -} + class mechanism_base_t : public mechanism_t + { + protected: + mechanism_base_t (session_base_t *const session_, + const options_t &options_); + + session_base_t *const session; + + int check_basic_command_structure (msg_t *msg_); + }; + } #endif diff --git a/src/null_mechanism.cpp b/src/null_mechanism.cpp index c765bdee..8f181920 100644 --- a/src/null_mechanism.cpp +++ b/src/null_mechanism.cpp @@ -42,7 +42,7 @@ zmq::null_mechanism_t::null_mechanism_t (session_base_t *session_, const std::string &peer_address_, const options_t &options_) : - mechanism_t (options_), + mechanism_base_t (session_, options_), zap_client_t (session_, peer_address_, options_), ready_command_sent (false), error_command_sent (false), diff --git a/src/plain_server.cpp b/src/plain_server.cpp index f205c1ac..5eaf58aa 100644 --- a/src/plain_server.cpp +++ b/src/plain_server.cpp @@ -40,7 +40,7 @@ zmq::plain_server_t::plain_server_t (session_base_t *session_, const std::string &peer_address_, const options_t &options_) : - mechanism_t (options_), + mechanism_base_t (session_, options_), zap_client_common_handshake_t ( session_, peer_address_, options_, sending_welcome) { diff --git a/src/stream_engine.cpp b/src/stream_engine.cpp index d1731bbe..80f0e487 100644 --- a/src/stream_engine.cpp +++ b/src/stream_engine.cpp @@ -698,7 +698,8 @@ bool zmq::stream_engine_t::handshake () mechanism = new (std::nothrow) gssapi_server_t (session, peer_address, options); else - mechanism = new (std::nothrow) gssapi_client_t (options); + mechanism = + new (std::nothrow) gssapi_client_t (session, options); alloc_assert (mechanism); } #endif diff --git a/src/zap_client.cpp b/src/zap_client.cpp index 9a5b7145..1d820d37 100644 --- a/src/zap_client.cpp +++ b/src/zap_client.cpp @@ -38,8 +38,7 @@ namespace zmq zap_client_t::zap_client_t (session_base_t *const session_, const std::string &peer_address_, const options_t &options_) : - mechanism_t (options_), - session (session_), + mechanism_base_t (session_, options_), peer_address (peer_address_) { } @@ -248,24 +247,12 @@ void zap_client_t::handle_zap_status_code () session->get_endpoint (), status_code_numeric); } -int zap_client_t::check_basic_command_structure (msg_t *msg_) -{ - if (msg_->size () <= 1 || msg_->size () <= ((uint8_t *) msg_->data ())[0]) { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED); - errno = EPROTO; - return -1; - } - return 0; -} - zap_client_common_handshake_t::zap_client_common_handshake_t ( session_base_t *const session_, const std::string &peer_address_, const options_t &options_, state_t zap_reply_ok_state_) : - mechanism_t (options_), + mechanism_base_t (session_, options_), zap_client_t (session_, peer_address_, options_), state (waiting_for_hello), zap_reply_ok_state (zap_reply_ok_state_) diff --git a/src/zap_client.hpp b/src/zap_client.hpp index cb2837ed..c2652bb8 100644 --- a/src/zap_client.hpp +++ b/src/zap_client.hpp @@ -34,9 +34,7 @@ namespace zmq { -class session_base_t; - -class zap_client_t : public virtual mechanism_t +class zap_client_t : public virtual mechanism_base_t { public: zap_client_t (session_base_t *const session_, @@ -56,10 +54,7 @@ class zap_client_t : public virtual mechanism_t virtual int receive_and_process_zap_reply (); virtual void handle_zap_status_code (); - - int check_basic_command_structure (msg_t *msg_); protected: - session_base_t *const session; const std::string peer_address; // Status code as received from ZAP handler From ca7eee357e6e1bc76e8f0f38ae37cc14f70ab0ef Mon Sep 17 00:00:00 2001 From: sigiesec Date: Fri, 18 Aug 2017 10:15:44 +0200 Subject: [PATCH 7/9] Problem: no ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL events emitted in plain_client_t Solution: emit events at appropriate places --- src/plain_client.cpp | 30 ++++++++++++++++++++++++++---- src/plain_client.hpp | 10 +++++----- src/stream_engine.cpp | 2 +- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/plain_client.cpp b/src/plain_client.cpp index 59ffec15..43918cab 100644 --- a/src/plain_client.cpp +++ b/src/plain_client.cpp @@ -35,9 +35,11 @@ #include "msg.hpp" #include "err.hpp" #include "plain_client.hpp" +#include "session_base.hpp" -zmq::plain_client_t::plain_client_t (const options_t &options_) : - mechanism_t (options_), +zmq::plain_client_t::plain_client_t (session_base_t *const session_, + const options_t &options_) : + mechanism_base_t (session_, options_), state (sending_hello) { } @@ -84,8 +86,9 @@ int zmq::plain_client_t::process_handshake_command (msg_t *msg_) if (data_size >= 6 && !memcmp (cmd_data, "\5ERROR", 6)) rc = process_error (cmd_data, data_size); else { - // Temporary support for security debugging - puts ("PLAIN I: invalid handshake command"); + // TODO see comment in curve_server_t::process_handshake_command + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNSPECIFIED); errno = EPROTO; rc = -1; } @@ -146,10 +149,15 @@ int zmq::plain_client_t::process_welcome ( LIBZMQ_UNUSED (cmd_data); if (state != waiting_for_welcome) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } if (data_size != 8) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_WELCOME); errno = EPROTO; return -1; } @@ -168,12 +176,18 @@ int zmq::plain_client_t::process_ready ( const unsigned char *cmd_data, size_t data_size) { if (state != waiting_for_ready) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } const int rc = parse_metadata (cmd_data + 6, data_size - 6); if (rc == 0) state = ready; + else + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_METADATA); + return rc; } @@ -181,15 +195,23 @@ int zmq::plain_client_t::process_error ( const unsigned char *cmd_data, size_t data_size) { if (state != waiting_for_welcome && state != waiting_for_ready) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } if (data_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 (cmd_data [6]); if (error_reason_len > data_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/plain_client.hpp b/src/plain_client.hpp index b6e42b4d..6e5b5586 100644 --- a/src/plain_client.hpp +++ b/src/plain_client.hpp @@ -38,11 +38,11 @@ namespace zmq class msg_t; - class plain_client_t : public mechanism_t + class plain_client_t : public mechanism_base_t { - public: - - plain_client_t (const options_t &options_); + public: + plain_client_t (session_base_t *const session_, + const options_t &options_); virtual ~plain_client_t (); // mechanism implementation @@ -50,7 +50,7 @@ namespace zmq virtual int process_handshake_command (msg_t *msg_); virtual status_t status () const; - private: + private: enum state_t { sending_hello, diff --git a/src/stream_engine.cpp b/src/stream_engine.cpp index 80f0e487..0a7b9961 100644 --- a/src/stream_engine.cpp +++ b/src/stream_engine.cpp @@ -674,7 +674,7 @@ bool zmq::stream_engine_t::handshake () plain_server_t (session, peer_address, options); else mechanism = new (std::nothrow) - plain_client_t (options); + plain_client_t (session, options); alloc_assert (mechanism); } #ifdef ZMQ_HAVE_CURVE From 44f6aa3de6a0895091a1475bc2106e9889c6dd9e Mon Sep 17 00:00:00 2001 From: sigiesec Date: Fri, 18 Aug 2017 10:30:48 +0200 Subject: [PATCH 8/9] Problem: gssapi_* do not emit ZMQ_EVENT_HANDSHAKE_FAILED_PROTOCOL events Solution: emit appropriate events --- src/gssapi_client.cpp | 3 ++ src/gssapi_mechanism_base.cpp | 55 +++++++++++++++++++++++++++++++++-- src/gssapi_server.cpp | 3 ++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/gssapi_client.cpp b/src/gssapi_client.cpp index 942c25fa..68e33d9b 100644 --- a/src/gssapi_client.cpp +++ b/src/gssapi_client.cpp @@ -125,6 +125,9 @@ int zmq::gssapi_client_t::process_handshake_command (msg_t *msg_) } if (state != recv_next_token) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } diff --git a/src/gssapi_mechanism_base.cpp b/src/gssapi_mechanism_base.cpp index 65d9b124..43d4f79b 100644 --- a/src/gssapi_mechanism_base.cpp +++ b/src/gssapi_mechanism_base.cpp @@ -128,8 +128,15 @@ int zmq::gssapi_mechanism_base_t::decode_message (msg_t *msg_) const uint8_t *ptr = static_cast (msg_->data ()); size_t bytes_left = msg_->size (); + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return rc; + // Get command string if (bytes_left < 8 || memcmp (ptr, "\x07MESSAGE", 8)) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } @@ -138,6 +145,9 @@ int zmq::gssapi_mechanism_base_t::decode_message (msg_t *msg_) // Get token length if (bytes_left < 4) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); errno = EPROTO; return -1; } @@ -148,6 +158,9 @@ int zmq::gssapi_mechanism_base_t::decode_message (msg_t *msg_) // Get token value if (bytes_left < wrapped.length) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); errno = EPROTO; return -1; } @@ -168,11 +181,16 @@ int zmq::gssapi_mechanism_base_t::decode_message (msg_t *msg_) maj_stat = gss_unwrap(&min_stat, context, &wrapped, &plaintext, &state, (gss_qop_t *) NULL); + // TODO I don't think it is a good idea to use zmq_assert here. If + // decryption fails, gss_unwrap returns GSS_S_BAD_SIG. This opens up + // to DoS attacks by clients! Instead, a + // ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC event should be emitted. + zmq_assert(maj_stat == GSS_S_COMPLETE); zmq_assert(state); // Re-initialize msg_ for plaintext - int rc = msg_->close (); + rc = msg_->close (); zmq_assert (rc == 0); rc = msg_->init_size (plaintext.length-1); @@ -190,6 +208,9 @@ int zmq::gssapi_mechanism_base_t::decode_message (msg_t *msg_) free(wrapped.value); if (bytes_left > 0) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); errno = EPROTO; return -1; } @@ -231,8 +252,15 @@ int zmq::gssapi_mechanism_base_t::process_initiate (msg_t *msg_, void **token_va const uint8_t *ptr = static_cast (msg_->data ()); size_t bytes_left = msg_->size (); + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return rc; + // Get command string if (bytes_left < 9 || memcmp (ptr, "\x08INITIATE", 9)) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } @@ -241,6 +269,9 @@ int zmq::gssapi_mechanism_base_t::process_initiate (msg_t *msg_, void **token_va // Get token length if (bytes_left < 4) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE); errno = EPROTO; return -1; } @@ -250,6 +281,9 @@ int zmq::gssapi_mechanism_base_t::process_initiate (msg_t *msg_, void **token_va // Get token value if (bytes_left < token_length_) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE); errno = EPROTO; return -1; } @@ -264,6 +298,9 @@ int zmq::gssapi_mechanism_base_t::process_initiate (msg_t *msg_, void **token_va } if (bytes_left > 0) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_INITIATE); errno = EPROTO; return -1; } @@ -317,14 +354,28 @@ int zmq::gssapi_mechanism_base_t::process_ready (msg_t *msg_) const unsigned char *ptr = static_cast (msg_->data ()); size_t bytes_left = msg_->size (); + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return rc; + if (bytes_left < 6 || memcmp (ptr, "\x05READY", 6)) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } ptr += 6; bytes_left -= 6; - return parse_metadata (ptr, bytes_left); + rc = parse_metadata (ptr, bytes_left); + if (rc == -1) + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_METADATA); + + return rc; } + const gss_OID zmq::gssapi_mechanism_base_t::convert_nametype (int zmq_nametype) { switch (zmq_nametype) { diff --git a/src/gssapi_server.cpp b/src/gssapi_server.cpp index 936c4499..dbb065d7 100644 --- a/src/gssapi_server.cpp +++ b/src/gssapi_server.cpp @@ -114,6 +114,9 @@ int zmq::gssapi_server_t::process_handshake_command (msg_t *msg_) } if (state != recv_next_token) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); errno = EPROTO; return -1; } From 301f3c70c2fe4b86e9e0e60ff233f6ca36a33769 Mon Sep 17 00:00:00 2001 From: sigiesec Date: Fri, 18 Aug 2017 11:34:22 +0200 Subject: [PATCH 9/9] Problem: code duplication between curve_client_t and curve_server_t decode and encode Solution: extracted common base class curve_mechanism_base_t --- CMakeLists.txt | 2 + Makefile.am | 4 + builds/gyp/project.gyp | 5 + builds/msvc/vs2008/libzmq/libzmq.vcproj | 2 + builds/msvc/vs2010/libzmq/libzmq.vcxproj | 3 + builds/msvc/vs2012/libzmq/libzmq.vcxproj | 5 +- builds/msvc/vs2013/libzmq/libzmq.vcxproj | 5 +- builds/msvc/vs2015/libzmq/libzmq.vcxproj | 5 +- builds/msvc/vs2015_xp/libzmq.vcxproj | 5 +- builds/msvc/vs2017/libzmq/libzmq.vcxproj | 5 +- src/curve_client.cpp | 135 +---------------- src/curve_client.hpp | 10 +- src/curve_client_tools.hpp | 8 +- src/curve_mechanism_base.cpp | 181 +++++++++++++++++++++++ src/curve_mechanism_base.hpp | 79 ++++++++++ src/curve_server.cpp | 129 +--------------- src/curve_server.hpp | 39 ++--- src/mechanism.cpp | 21 --- src/mechanism.hpp | 12 +- src/mechanism_base.cpp | 54 +++++++ src/mechanism_base.hpp | 49 ++++++ src/plain_client.hpp | 2 +- src/zap_client.hpp | 2 +- tests/test_security_curve.cpp | 3 +- 24 files changed, 431 insertions(+), 334 deletions(-) create mode 100644 src/curve_mechanism_base.cpp create mode 100644 src/curve_mechanism_base.hpp create mode 100644 src/mechanism_base.cpp create mode 100644 src/mechanism_base.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index c3eadb21..9af932d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -484,6 +484,7 @@ set (cxx-sources client.cpp clock.cpp ctx.cpp + curve_mechanism_base.cpp curve_client.cpp curve_server.cpp dealer.cpp @@ -504,6 +505,7 @@ set (cxx-sources mailbox.cpp mailbox_safe.cpp mechanism.cpp + mechanism_base.cpp metadata.cpp msg.cpp mtrie.cpp diff --git a/Makefile.am b/Makefile.am index 671640f3..bd971d49 100644 --- a/Makefile.am +++ b/Makefile.am @@ -38,6 +38,8 @@ src_libzmq_la_SOURCES = \ src/curve_client.cpp \ src/curve_client.hpp \ src/curve_client_tools.hpp \ + src/curve_mechanism_base.cpp \ + src/curve_mechanism_base.hpp \ src/curve_server.cpp \ src/curve_server.hpp \ src/dbuffer.hpp \ @@ -97,6 +99,8 @@ src_libzmq_la_SOURCES = \ src/mailbox_safe.hpp \ src/mechanism.cpp \ src/mechanism.hpp \ + src/mechanism_base.cpp \ + src/mechanism_base.hpp \ src/metadata.cpp \ src/metadata.hpp \ src/msg.cpp \ diff --git a/builds/gyp/project.gyp b/builds/gyp/project.gyp index 73fa3bbb..a89366c0 100644 --- a/builds/gyp/project.gyp +++ b/builds/gyp/project.gyp @@ -80,6 +80,9 @@ '../../src/ctx.hpp', '../../src/curve_client.cpp', '../../src/curve_client.hpp', + '../../src/curve_client_tools.hpp', + '../../src/curve_mechanism_base.cpp', + '../../src/curve_mechanism_base.hpp', '../../src/curve_server.cpp', '../../src/curve_server.hpp', '../../src/dbuffer.hpp', @@ -136,6 +139,8 @@ '../../src/mailbox_safe.hpp', '../../src/mechanism.cpp', '../../src/mechanism.hpp ', + '../../src/mechanism_base.cpp', + '../../src/mechanism_base.hpp ', '../../src/metadata.cpp', '../../src/metadata.hpp', '../../src/msg.cpp', diff --git a/builds/msvc/vs2008/libzmq/libzmq.vcproj b/builds/msvc/vs2008/libzmq/libzmq.vcproj index 2077bbce..18c5521e 100644 --- a/builds/msvc/vs2008/libzmq/libzmq.vcproj +++ b/builds/msvc/vs2008/libzmq/libzmq.vcproj @@ -121,6 +121,7 @@ + @@ -173,6 +174,7 @@ + diff --git a/builds/msvc/vs2010/libzmq/libzmq.vcxproj b/builds/msvc/vs2010/libzmq/libzmq.vcxproj index 603dec3a..bc07be2b 100644 --- a/builds/msvc/vs2010/libzmq/libzmq.vcxproj +++ b/builds/msvc/vs2010/libzmq/libzmq.vcxproj @@ -180,6 +180,7 @@ + @@ -205,6 +206,7 @@ + @@ -266,6 +268,7 @@ + diff --git a/builds/msvc/vs2012/libzmq/libzmq.vcxproj b/builds/msvc/vs2012/libzmq/libzmq.vcxproj index c5c52f5c..cc0d84fd 100644 --- a/builds/msvc/vs2012/libzmq/libzmq.vcxproj +++ b/builds/msvc/vs2012/libzmq/libzmq.vcxproj @@ -1,4 +1,4 @@ - + {641C5F36-32EE-4323-B740-992B651CF9D6} @@ -180,6 +180,7 @@ + @@ -205,6 +206,7 @@ + @@ -266,6 +268,7 @@ + diff --git a/builds/msvc/vs2013/libzmq/libzmq.vcxproj b/builds/msvc/vs2013/libzmq/libzmq.vcxproj index 6957c364..3d42eaa6 100644 --- a/builds/msvc/vs2013/libzmq/libzmq.vcxproj +++ b/builds/msvc/vs2013/libzmq/libzmq.vcxproj @@ -1,4 +1,4 @@ - + {641C5F36-32EE-4323-B740-992B651CF9D6} @@ -180,6 +180,7 @@ + @@ -205,6 +206,7 @@ + @@ -266,6 +268,7 @@ + diff --git a/builds/msvc/vs2015/libzmq/libzmq.vcxproj b/builds/msvc/vs2015/libzmq/libzmq.vcxproj index 14070e0a..1b56266d 100644 --- a/builds/msvc/vs2015/libzmq/libzmq.vcxproj +++ b/builds/msvc/vs2015/libzmq/libzmq.vcxproj @@ -1,4 +1,4 @@ - + {641C5F36-32EE-4323-B740-992B651CF9D6} @@ -180,6 +180,7 @@ + @@ -205,6 +206,7 @@ + @@ -266,6 +268,7 @@ + diff --git a/builds/msvc/vs2015_xp/libzmq.vcxproj b/builds/msvc/vs2015_xp/libzmq.vcxproj index 36ee6431..69623b5a 100644 --- a/builds/msvc/vs2015_xp/libzmq.vcxproj +++ b/builds/msvc/vs2015_xp/libzmq.vcxproj @@ -1,4 +1,4 @@ - + @@ -152,6 +152,7 @@ + @@ -177,6 +178,7 @@ + @@ -242,6 +244,7 @@ + diff --git a/builds/msvc/vs2017/libzmq/libzmq.vcxproj b/builds/msvc/vs2017/libzmq/libzmq.vcxproj index 23491d12..8f3eee8d 100644 --- a/builds/msvc/vs2017/libzmq/libzmq.vcxproj +++ b/builds/msvc/vs2017/libzmq/libzmq.vcxproj @@ -1,4 +1,4 @@ - + {641C5F36-32EE-4323-B740-992B651CF9D6} @@ -180,6 +180,7 @@ + @@ -205,6 +206,7 @@ + @@ -266,6 +268,7 @@ + diff --git a/src/curve_client.cpp b/src/curve_client.cpp index 49e7c471..e1012e7c 100644 --- a/src/curve_client.cpp +++ b/src/curve_client.cpp @@ -42,12 +42,12 @@ zmq::curve_client_t::curve_client_t (session_base_t *session_, const options_t &options_) : mechanism_base_t (session_, options_), + curve_mechanism_base_t ( + session_, options_, "CurveZMQMESSAGEC", "CurveZMQMESSAGES"), state (send_hello), tools (options_.curve_public_key, options_.curve_secret_key, - options_.curve_server_key), - cn_nonce (1), - cn_peer_nonce (1) + options_.curve_server_key) { } @@ -113,134 +113,13 @@ int zmq::curve_client_t::process_handshake_command (msg_t *msg_) int zmq::curve_client_t::encode (msg_t *msg_) { zmq_assert (state == connected); - - uint8_t flags = 0; - if (msg_->flags () & msg_t::more) - flags |= 0x01; - if (msg_->flags () & msg_t::command) - flags |= 0x02; - - uint8_t message_nonce [crypto_box_NONCEBYTES]; - memcpy (message_nonce, "CurveZMQMESSAGEC", 16); - put_uint64 (message_nonce + 16, cn_nonce); - - const size_t mlen = crypto_box_ZEROBYTES + 1 + msg_->size (); - - uint8_t *message_plaintext = static_cast (malloc (mlen)); - alloc_assert (message_plaintext); - - memset (message_plaintext, 0, crypto_box_ZEROBYTES); - message_plaintext [crypto_box_ZEROBYTES] = flags; - memcpy (message_plaintext + crypto_box_ZEROBYTES + 1, - msg_->data (), msg_->size ()); - - uint8_t *message_box = static_cast (malloc (mlen)); - alloc_assert (message_box); - - int rc = crypto_box_afternm (message_box, message_plaintext, mlen, - message_nonce, tools.cn_precom); - zmq_assert (rc == 0); - - rc = msg_->close (); - zmq_assert (rc == 0); - - rc = msg_->init_size (16 + mlen - crypto_box_BOXZEROBYTES); - zmq_assert (rc == 0); - - uint8_t *message = static_cast (msg_->data ()); - - memcpy (message, "\x07MESSAGE", 8); - memcpy (message + 8, message_nonce + 16, 8); - memcpy (message + 16, message_box + crypto_box_BOXZEROBYTES, - mlen - crypto_box_BOXZEROBYTES); - - free (message_plaintext); - free (message_box); - - cn_nonce++; - - return 0; + return curve_mechanism_base_t::encode (msg_); } int zmq::curve_client_t::decode (msg_t *msg_) { zmq_assert (state == connected); - int rc = check_basic_command_structure (msg_); - if (rc == -1) - return rc; - - const uint8_t *message = static_cast (msg_->data ()); - if (msg_->size() < 8 || memcmp (message, "\x07MESSAGE", 8)) { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); - errno = EPROTO; - return -1; - } - - if (msg_->size () < 33) { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); - errno = EPROTO; - return -1; - } - - uint8_t message_nonce [crypto_box_NONCEBYTES]; - memcpy (message_nonce, "CurveZMQMESSAGES", 16); - 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; - } - cn_peer_nonce = nonce; - - const size_t clen = crypto_box_BOXZEROBYTES + (msg_->size () - 16); - - uint8_t *message_plaintext = static_cast (malloc (clen)); - alloc_assert (message_plaintext); - - uint8_t *message_box = static_cast (malloc (clen)); - alloc_assert (message_box); - - memset (message_box, 0, crypto_box_BOXZEROBYTES); - memcpy (message_box + crypto_box_BOXZEROBYTES, - message + 16, msg_->size () - 16); - - rc = crypto_box_open_afternm (message_plaintext, message_box, clen, - message_nonce, tools.cn_precom); - if (rc == 0) { - rc = msg_->close (); - zmq_assert (rc == 0); - - rc = msg_->init_size (clen - 1 - crypto_box_ZEROBYTES); - zmq_assert (rc == 0); - - const uint8_t flags = message_plaintext [crypto_box_ZEROBYTES]; - if (flags & 0x01) - msg_->set_flags (msg_t::more); - if (flags & 0x02) - msg_->set_flags (msg_t::command); - - memcpy (msg_->data (), - message_plaintext + crypto_box_ZEROBYTES + 1, - msg_->size ()); - } - else { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); - errno = EPROTO; - } - - free (message_plaintext); - free (message_box); - - return rc; + return curve_mechanism_base_t::decode (msg_); } zmq::mechanism_t::status_t zmq::curve_client_t::status () const @@ -281,7 +160,7 @@ int zmq::curve_client_t::produce_hello (msg_t *msg_) int zmq::curve_client_t::process_welcome (const uint8_t *msg_data, size_t msg_size) { - int rc = tools.process_welcome (msg_data, msg_size); + int rc = tools.process_welcome (msg_data, msg_size, cn_precom); if (rc == -1) { session->get_socket ()->event_handshake_failed_protocol ( @@ -358,7 +237,7 @@ int zmq::curve_client_t::process_ready ( cn_peer_nonce = get_uint64(msg_data + 6); int rc = crypto_box_open_afternm (ready_plaintext, ready_box, - clen, ready_nonce, tools.cn_precom); + clen, ready_nonce, cn_precom); free (ready_box); if (rc != 0) { diff --git a/src/curve_client.hpp b/src/curve_client.hpp index f5be67e7..7693ac64 100644 --- a/src/curve_client.hpp +++ b/src/curve_client.hpp @@ -32,7 +32,7 @@ #ifdef ZMQ_HAVE_CURVE -#include "mechanism.hpp" +#include "curve_mechanism_base.hpp" #include "options.hpp" #include "curve_client_tools.hpp" @@ -42,7 +42,7 @@ namespace zmq class msg_t; class session_base_t; - class curve_client_t : public mechanism_base_t + class curve_client_t : public curve_mechanism_base_t { public: @@ -72,11 +72,7 @@ namespace zmq // CURVE protocol tools curve_client_tools_t tools; - - // Nonce - uint64_t cn_nonce; - uint64_t cn_peer_nonce; - + int produce_hello (msg_t *msg_); int process_welcome (const uint8_t *cmd_data, size_t data_size); int produce_initiate (msg_t *msg_); diff --git a/src/curve_client_tools.hpp b/src/curve_client_tools.hpp index 7ce35270..cb162115 100644 --- a/src/curve_client_tools.hpp +++ b/src/curve_client_tools.hpp @@ -251,7 +251,9 @@ struct curve_client_tools_t return produce_hello (data, server_key, cn_nonce, cn_public, cn_secret); } - int process_welcome (const uint8_t *msg_data, size_t msg_size) + int process_welcome (const uint8_t *msg_data, + size_t msg_size, + uint8_t *cn_precom) { return process_welcome (msg_data, msg_size, server_key, cn_secret, cn_server, cn_cookie, cn_precom); @@ -289,10 +291,6 @@ struct curve_client_tools_t // Cookie received from server uint8_t cn_cookie[16 + 80]; - // Intermediary buffer used to speed up boxing and unboxing. - uint8_t cn_precom[crypto_box_BEFORENMBYTES]; - - private: template static bool is_handshake_command (const uint8_t *msg_data, diff --git a/src/curve_mechanism_base.cpp b/src/curve_mechanism_base.cpp new file mode 100644 index 00000000..b61edd2e --- /dev/null +++ b/src/curve_mechanism_base.cpp @@ -0,0 +1,181 @@ +/* + Copyright (c) 2007-2016 Contributors as noted in the AUTHORS file + + This file is part of libzmq, the ZeroMQ core engine in C++. + + libzmq is free software; you can redistribute it and/or modify it under + the terms of the GNU Lesser General Public License (LGPL) as published + by the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + As a special exception, the Contributors give you permission to link + this library with independent modules to produce an executable, + regardless of the license terms of these independent modules, and to + copy and distribute the resulting executable under terms of your choice, + provided that you also meet, for each linked independent module, the + terms and conditions of the license of that module. An independent + module is a module which is not derived from or based on this library. + If you modify this library, you must extend this exception to your + version of the library. + + libzmq is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program. If not, see . +*/ + + +#include "precompiled.hpp" +#include "curve_mechanism_base.hpp" +#include "msg.hpp" +#include "wire.hpp" +#include "session_base.hpp" + +#ifdef ZMQ_HAVE_CURVE + +zmq::curve_mechanism_base_t::curve_mechanism_base_t ( + session_base_t *session_, + const options_t &options_, + const char *encode_nonce_prefix_, + const char *decode_nonce_prefix_) : + mechanism_base_t (session_, options_), + encode_nonce_prefix (encode_nonce_prefix_), + decode_nonce_prefix (decode_nonce_prefix_), + cn_nonce (1), + cn_peer_nonce (1) +{ +} + +int zmq::curve_mechanism_base_t::encode (msg_t *msg_) +{ + const size_t mlen = crypto_box_ZEROBYTES + 1 + msg_->size (); + + uint8_t message_nonce [crypto_box_NONCEBYTES]; + memcpy (message_nonce, encode_nonce_prefix, 16); + put_uint64 (message_nonce + 16, cn_nonce); + + uint8_t flags = 0; + if (msg_->flags () & msg_t::more) + flags |= 0x01; + if (msg_->flags () & msg_t::command) + flags |= 0x02; + + uint8_t *message_plaintext = static_cast (malloc (mlen)); + alloc_assert (message_plaintext); + + memset (message_plaintext, 0, crypto_box_ZEROBYTES); + message_plaintext [crypto_box_ZEROBYTES] = flags; + memcpy (message_plaintext + crypto_box_ZEROBYTES + 1, + msg_->data (), msg_->size ()); + + uint8_t *message_box = static_cast (malloc (mlen)); + alloc_assert (message_box); + + int rc = crypto_box_afternm (message_box, message_plaintext, + mlen, message_nonce, cn_precom); + zmq_assert (rc == 0); + + rc = msg_->close (); + zmq_assert (rc == 0); + + rc = msg_->init_size (16 + mlen - crypto_box_BOXZEROBYTES); + zmq_assert (rc == 0); + + uint8_t *message = static_cast (msg_->data ()); + + memcpy (message, "\x07MESSAGE", 8); + memcpy (message + 8, message_nonce + 16, 8); + memcpy (message + 16, message_box + crypto_box_BOXZEROBYTES, + mlen - crypto_box_BOXZEROBYTES); + + free (message_plaintext); + free (message_box); + + cn_nonce++; + + return 0; +} + +int zmq::curve_mechanism_base_t::decode (msg_t *msg_) +{ + int rc = check_basic_command_structure (msg_); + if (rc == -1) + return -1; + + const size_t size = msg_->size (); + const uint8_t *message = static_cast (msg_->data ()); + + if (size < 8 || memcmp (message, "\x07MESSAGE", 8)) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); + errno = EPROTO; + return -1; + } + + if (size < 33) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); + errno = EPROTO; + return -1; + } + + uint8_t message_nonce [crypto_box_NONCEBYTES]; + memcpy (message_nonce, decode_nonce_prefix, 16); + 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; + } + cn_peer_nonce = nonce; + + const size_t clen = crypto_box_BOXZEROBYTES + msg_->size () - 16; + + uint8_t *message_plaintext = static_cast (malloc (clen)); + alloc_assert (message_plaintext); + + uint8_t *message_box = static_cast (malloc (clen)); + alloc_assert (message_box); + + memset (message_box, 0, crypto_box_BOXZEROBYTES); + memcpy (message_box + crypto_box_BOXZEROBYTES, + message + 16, msg_->size () - 16); + + rc = crypto_box_open_afternm (message_plaintext, message_box, clen, + message_nonce, cn_precom); + if (rc == 0) { + rc = msg_->close (); + zmq_assert (rc == 0); + + rc = msg_->init_size (clen - 1 - crypto_box_ZEROBYTES); + zmq_assert (rc == 0); + + const uint8_t flags = message_plaintext [crypto_box_ZEROBYTES]; + if (flags & 0x01) + msg_->set_flags (msg_t::more); + if (flags & 0x02) + msg_->set_flags (msg_t::command); + + memcpy (msg_->data (), + message_plaintext + crypto_box_ZEROBYTES + 1, + msg_->size ()); + } + else { + // CURVE I : connection key used for MESSAGE is wrong + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); + errno = EPROTO; + } + free (message_plaintext); + free (message_box); + + return rc; +} + +#endif diff --git a/src/curve_mechanism_base.hpp b/src/curve_mechanism_base.hpp new file mode 100644 index 00000000..98cfd03d --- /dev/null +++ b/src/curve_mechanism_base.hpp @@ -0,0 +1,79 @@ +/* + Copyright (c) 2007-2016 Contributors as noted in the AUTHORS file + + This file is part of libzmq, the ZeroMQ core engine in C++. + + libzmq is free software; you can redistribute it and/or modify it under + the terms of the GNU Lesser General Public License (LGPL) as published + by the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + As a special exception, the Contributors give you permission to link + this library with independent modules to produce an executable, + regardless of the license terms of these independent modules, and to + copy and distribute the resulting executable under terms of your choice, + provided that you also meet, for each linked independent module, the + terms and conditions of the license of that module. An independent + module is a module which is not derived from or based on this library. + If you modify this library, you must extend this exception to your + version of the library. + + libzmq is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program. If not, see . +*/ + +#ifndef __ZMQ_CURVE_MECHANISM_BASE_HPP_INCLUDED__ +#define __ZMQ_CURVE_MECHANISM_BASE_HPP_INCLUDED__ + +#ifdef ZMQ_HAVE_CURVE + +#if defined(ZMQ_USE_TWEETNACL) +#include "tweetnacl.h" +#elif defined(ZMQ_USE_LIBSODIUM) +#include "sodium.h" +#endif + +#if crypto_box_NONCEBYTES != 24 || crypto_box_PUBLICKEYBYTES != 32 \ + || crypto_box_SECRETKEYBYTES != 32 || crypto_box_ZEROBYTES != 32 \ + || crypto_box_BOXZEROBYTES != 16 || crypto_secretbox_NONCEBYTES != 24 \ + || crypto_secretbox_ZEROBYTES != 32 || crypto_secretbox_BOXZEROBYTES != 16 +#error "CURVE library not built properly" +#endif + +#include "mechanism_base.hpp" +#include "options.hpp" + +namespace zmq +{ +class curve_mechanism_base_t : public virtual mechanism_base_t +{ + public: + curve_mechanism_base_t (session_base_t *session_, + const options_t &options_, + const char *encode_nonce_prefix_, + const char *decode_nonce_prefix_); + + // mechanism implementation + virtual int encode (msg_t *msg_); + virtual int decode (msg_t *msg_); + + protected: + const char *encode_nonce_prefix; + const char *decode_nonce_prefix; + + uint64_t cn_nonce; + uint64_t cn_peer_nonce; + + // Intermediary buffer used to speed up boxing and unboxing. + uint8_t cn_precom [crypto_box_BEFORENMBYTES]; +}; +} + +#endif + +#endif diff --git a/src/curve_server.cpp b/src/curve_server.cpp index aa9f5425..bbcec99b 100644 --- a/src/curve_server.cpp +++ b/src/curve_server.cpp @@ -44,8 +44,8 @@ zmq::curve_server_t::curve_server_t (session_base_t *session_, mechanism_base_t (session_, options_), zap_client_common_handshake_t ( session_, peer_address_, options_, sending_ready), - cn_nonce (1), - cn_peer_nonce (1) + curve_mechanism_base_t ( + session_, options_, "CurveZMQMESSAGES", "CurveZMQMESSAGEC") { int rc; // Fetch our secret key from socket options @@ -125,134 +125,13 @@ int zmq::curve_server_t::process_handshake_command (msg_t *msg_) int zmq::curve_server_t::encode (msg_t *msg_) { zmq_assert (state == ready); - - const size_t mlen = crypto_box_ZEROBYTES + 1 + msg_->size (); - - uint8_t message_nonce [crypto_box_NONCEBYTES]; - memcpy (message_nonce, "CurveZMQMESSAGES", 16); - put_uint64 (message_nonce + 16, cn_nonce); - - uint8_t flags = 0; - if (msg_->flags () & msg_t::more) - flags |= 0x01; - if (msg_->flags () & msg_t::command) - flags |= 0x02; - - uint8_t *message_plaintext = static_cast (malloc (mlen)); - alloc_assert (message_plaintext); - - memset (message_plaintext, 0, crypto_box_ZEROBYTES); - message_plaintext [crypto_box_ZEROBYTES] = flags; - memcpy (message_plaintext + crypto_box_ZEROBYTES + 1, - msg_->data (), msg_->size ()); - - uint8_t *message_box = static_cast (malloc (mlen)); - alloc_assert (message_box); - - int rc = crypto_box_afternm (message_box, message_plaintext, - mlen, message_nonce, cn_precom); - zmq_assert (rc == 0); - - rc = msg_->close (); - zmq_assert (rc == 0); - - rc = msg_->init_size (16 + mlen - crypto_box_BOXZEROBYTES); - zmq_assert (rc == 0); - - uint8_t *message = static_cast (msg_->data ()); - - memcpy (message, "\x07MESSAGE", 8); - memcpy (message + 8, message_nonce + 16, 8); - memcpy (message + 16, message_box + crypto_box_BOXZEROBYTES, - mlen - crypto_box_BOXZEROBYTES); - - free (message_plaintext); - free (message_box); - - cn_nonce++; - - return 0; + return curve_mechanism_base_t::encode (msg_); } int zmq::curve_server_t::decode (msg_t *msg_) { zmq_assert (state == ready); - - int rc = check_basic_command_structure (msg_); - if (rc == -1) - return -1; - - const size_t size = msg_->size (); - const uint8_t *message = static_cast (msg_->data ()); - - if (size < 8 || memcmp (message, "\x07MESSAGE", 8)) { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND); - errno = EPROTO; - return -1; - } - - if (size < 33) { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE); - errno = EPROTO; - return -1; - } - - uint8_t message_nonce [crypto_box_NONCEBYTES]; - memcpy (message_nonce, "CurveZMQMESSAGEC", 16); - 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; - } - cn_peer_nonce = nonce; - - const size_t clen = crypto_box_BOXZEROBYTES + msg_->size () - 16; - - uint8_t *message_plaintext = static_cast (malloc (clen)); - alloc_assert (message_plaintext); - - uint8_t *message_box = static_cast (malloc (clen)); - alloc_assert (message_box); - - memset (message_box, 0, crypto_box_BOXZEROBYTES); - memcpy (message_box + crypto_box_BOXZEROBYTES, - message + 16, msg_->size () - 16); - - rc = crypto_box_open_afternm (message_plaintext, message_box, clen, - message_nonce, cn_precom); - if (rc == 0) { - rc = msg_->close (); - zmq_assert (rc == 0); - - rc = msg_->init_size (clen - 1 - crypto_box_ZEROBYTES); - zmq_assert (rc == 0); - - const uint8_t flags = message_plaintext [crypto_box_ZEROBYTES]; - if (flags & 0x01) - msg_->set_flags (msg_t::more); - if (flags & 0x02) - msg_->set_flags (msg_t::command); - - memcpy (msg_->data (), - message_plaintext + crypto_box_ZEROBYTES + 1, - msg_->size ()); - } - else { - // CURVE I : connection key used for MESSAGE is wrong - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC); - errno = EPROTO; - } - free (message_plaintext); - free (message_box); - - return rc; + return curve_mechanism_base_t::decode (msg_); } int zmq::curve_server_t::process_hello (msg_t *msg_) diff --git a/src/curve_server.hpp b/src/curve_server.hpp index 44562716..30efda5d 100644 --- a/src/curve_server.hpp +++ b/src/curve_server.hpp @@ -32,34 +32,18 @@ #ifdef ZMQ_HAVE_CURVE -#if defined (ZMQ_USE_TWEETNACL) -# include "tweetnacl.h" -#elif defined (ZMQ_USE_LIBSODIUM) -# include "sodium.h" -#endif - -#if crypto_box_NONCEBYTES != 24 \ -|| crypto_box_PUBLICKEYBYTES != 32 \ -|| crypto_box_SECRETKEYBYTES != 32 \ -|| crypto_box_ZEROBYTES != 32 \ -|| crypto_box_BOXZEROBYTES != 16 \ -|| crypto_secretbox_NONCEBYTES != 24 \ -|| crypto_secretbox_ZEROBYTES != 32 \ -|| crypto_secretbox_BOXZEROBYTES != 16 -# error "CURVE library not built properly" -#endif - -#include "mechanism.hpp" +#include "curve_mechanism_base.hpp" #include "options.hpp" #include "zap_client.hpp" namespace zmq { - - class msg_t; - class session_base_t; - - class curve_server_t : public zap_client_common_handshake_t +#ifdef _MSC_VER +#pragma warning (push) +#pragma warning (disable: 4250) +#endif + class curve_server_t : public zap_client_common_handshake_t, + public curve_mechanism_base_t { public: @@ -76,9 +60,6 @@ namespace zmq private: - uint64_t cn_nonce; - uint64_t cn_peer_nonce; - // Our secret key (s) uint8_t secret_key [crypto_box_SECRETKEYBYTES]; @@ -94,9 +75,6 @@ namespace zmq // Key used to produce cookie uint8_t cookie_key [crypto_secretbox_KEYBYTES]; - // Intermediary buffer used to speed up boxing and unboxing. - uint8_t cn_precom [crypto_box_BEFORENMBYTES]; - int process_hello (msg_t *msg_); int produce_welcome (msg_t *msg_); int process_initiate (msg_t *msg_); @@ -105,6 +83,9 @@ namespace zmq void send_zap_request (const uint8_t *key); }; +#ifdef _MSC_VER +#pragma warning (pop) +#endif } diff --git a/src/mechanism.cpp b/src/mechanism.cpp index 39117359..54c36bc3 100644 --- a/src/mechanism.cpp +++ b/src/mechanism.cpp @@ -282,24 +282,3 @@ bool zmq::mechanism_t::check_socket_type (const std::string& type_) const } return false; } - -zmq::mechanism_base_t::mechanism_base_t (session_base_t *const session_, - const options_t &options_) : - mechanism_t (options_), - session (session_) -{ - -} - -int zmq::mechanism_base_t::check_basic_command_structure (msg_t *msg_) -{ - if (msg_->size () <= 1 || msg_->size () <= ((uint8_t *) msg_->data ())[0]) { - session->get_socket ()->event_handshake_failed_protocol ( - session->get_endpoint (), - ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED); - errno = EPROTO; - return -1; - } - return 0; -} - diff --git a/src/mechanism.hpp b/src/mechanism.hpp index e07ce567..f8c05b66 100644 --- a/src/mechanism.hpp +++ b/src/mechanism.hpp @@ -147,16 +147,6 @@ namespace zmq bool check_socket_type (const std::string& type_) const; }; - class mechanism_base_t : public mechanism_t - { - protected: - mechanism_base_t (session_base_t *const session_, - const options_t &options_); - - session_base_t *const session; - - int check_basic_command_structure (msg_t *msg_); - }; - } +} #endif diff --git a/src/mechanism_base.cpp b/src/mechanism_base.cpp new file mode 100644 index 00000000..e255cb01 --- /dev/null +++ b/src/mechanism_base.cpp @@ -0,0 +1,54 @@ +/* + Copyright (c) 2007-2016 Contributors as noted in the AUTHORS file + + This file is part of libzmq, the ZeroMQ core engine in C++. + + libzmq is free software; you can redistribute it and/or modify it under + the terms of the GNU Lesser General Public License (LGPL) as published + by the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + As a special exception, the Contributors give you permission to link + this library with independent modules to produce an executable, + regardless of the license terms of these independent modules, and to + copy and distribute the resulting executable under terms of your choice, + provided that you also meet, for each linked independent module, the + terms and conditions of the license of that module. An independent + module is a module which is not derived from or based on this library. + If you modify this library, you must extend this exception to your + version of the library. + + libzmq is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program. If not, see . +*/ + +#include "precompiled.hpp" + +#include "mechanism_base.hpp" +#include "session_base.hpp" + +zmq::mechanism_base_t::mechanism_base_t (session_base_t *const session_, + const options_t &options_) : + mechanism_t (options_), + session (session_) +{ + +} + +int zmq::mechanism_base_t::check_basic_command_structure (msg_t *msg_) +{ + if (msg_->size () <= 1 || msg_->size () <= ((uint8_t *) msg_->data ())[0]) { + session->get_socket ()->event_handshake_failed_protocol ( + session->get_endpoint (), + ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_UNSPECIFIED); + errno = EPROTO; + return -1; + } + return 0; +} + diff --git a/src/mechanism_base.hpp b/src/mechanism_base.hpp new file mode 100644 index 00000000..e98a7f7e --- /dev/null +++ b/src/mechanism_base.hpp @@ -0,0 +1,49 @@ +/* + Copyright (c) 2007-2016 Contributors as noted in the AUTHORS file + + This file is part of libzmq, the ZeroMQ core engine in C++. + + libzmq is free software; you can redistribute it and/or modify it under + the terms of the GNU Lesser General Public License (LGPL) as published + by the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + As a special exception, the Contributors give you permission to link + this library with independent modules to produce an executable, + regardless of the license terms of these independent modules, and to + copy and distribute the resulting executable under terms of your choice, + provided that you also meet, for each linked independent module, the + terms and conditions of the license of that module. An independent + module is a module which is not derived from or based on this library. + If you modify this library, you must extend this exception to your + version of the library. + + libzmq is distributed in the hope that it will be useful, but WITHOUT + ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public + License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program. If not, see . +*/ + +#ifndef __ZMQ_MECHANISM_BASE_HPP_INCLUDED__ +#define __ZMQ_MECHANISM_BASE_HPP_INCLUDED__ + +#include "mechanism.hpp" + +namespace zmq +{ +class mechanism_base_t : public mechanism_t +{ + protected: + mechanism_base_t (session_base_t *const session_, + const options_t &options_); + + session_base_t *const session; + + int check_basic_command_structure (msg_t *msg_); +}; +} + +#endif diff --git a/src/plain_client.hpp b/src/plain_client.hpp index 6e5b5586..76789e09 100644 --- a/src/plain_client.hpp +++ b/src/plain_client.hpp @@ -30,7 +30,7 @@ #ifndef __ZMQ_PLAIN_CLIENT_HPP_INCLUDED__ #define __ZMQ_PLAIN_CLIENT_HPP_INCLUDED__ -#include "mechanism.hpp" +#include "mechanism_base.hpp" #include "options.hpp" namespace zmq diff --git a/src/zap_client.hpp b/src/zap_client.hpp index c2652bb8..557cab60 100644 --- a/src/zap_client.hpp +++ b/src/zap_client.hpp @@ -30,7 +30,7 @@ #ifndef __ZMQ_ZAP_CLIENT_HPP_INCLUDED__ #define __ZMQ_ZAP_CLIENT_HPP_INCLUDED__ -#include "mechanism.hpp" +#include "mechanism_base.hpp" namespace zmq { diff --git a/tests/test_security_curve.cpp b/tests/test_security_curve.cpp index 04ed9bfd..89bc3f05 100644 --- a/tests/test_security_curve.cpp +++ b/tests/test_security_curve.cpp @@ -491,7 +491,8 @@ int connect_exchange_greeting_and_hello_welcome ( uint8_t welcome[welcome_length + 2]; recv_all (s, welcome, welcome_length + 2); - int res = tools.process_welcome (welcome + 2, welcome_length); + uint8_t cn_precom [crypto_box_BEFORENMBYTES]; + int res = tools.process_welcome (welcome + 2, welcome_length, cn_precom); assert (res == 0); #ifdef ZMQ_BUILD_DRAFT_API