From 11b3c938521064c4d08393d06d73890b37ead8b1 Mon Sep 17 00:00:00 2001 From: sigiesec Date: Thu, 17 Aug 2017 18:16:31 +0200 Subject: [PATCH] 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;