Problem: unreachable code in zap_client_t

Solution: replaced unreachable code by assertions and adapted uses
This commit is contained in:
sigiesec 2017-08-17 12:54:05 +02:00
parent 7f15e6c868
commit f9985708b7
10 changed files with 45 additions and 74 deletions

View File

@ -488,9 +488,7 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
// not work properly the program will abort. // not work properly the program will abort.
rc = session->zap_connect (); rc = session->zap_connect ();
if (rc == 0) { if (rc == 0) {
rc = send_zap_request (client_key); send_zap_request (client_key);
if (rc != 0)
return -1;
rc = receive_and_process_zap_reply (); rc = receive_and_process_zap_reply ();
if (rc == -1) if (rc == -1)
return -1; return -1;
@ -561,10 +559,9 @@ int zmq::curve_server_t::produce_error (msg_t *msg_) const
return 0; return 0;
} }
int zmq::curve_server_t::send_zap_request (const uint8_t *key) void zmq::curve_server_t::send_zap_request (const uint8_t *key)
{ {
return zap_client_t::send_zap_request ("CURVE", 5, key, zap_client_t::send_zap_request ("CURVE", 5, key, crypto_box_PUBLICKEYBYTES);
crypto_box_PUBLICKEYBYTES);
} }
#endif #endif

View File

@ -103,7 +103,7 @@ namespace zmq
int produce_ready (msg_t *msg_); int produce_ready (msg_t *msg_);
int produce_error (msg_t *msg_) const; int produce_error (msg_t *msg_) const;
int send_zap_request (const uint8_t *key); void send_zap_request (const uint8_t *key);
}; };
} }

View File

@ -125,9 +125,7 @@ int zmq::gssapi_server_t::process_handshake_command (msg_t *msg_)
bool expecting_zap_reply = false; bool expecting_zap_reply = false;
int rc = session->zap_connect (); int rc = session->zap_connect ();
if (rc == 0) { if (rc == 0) {
rc = send_zap_request (); send_zap_request ();
if (rc != 0)
return -1;
rc = receive_and_process_zap_reply (); rc = receive_and_process_zap_reply ();
if (rc != 0) { if (rc != 0) {
if (rc == -1) if (rc == -1)
@ -151,16 +149,14 @@ int zmq::gssapi_server_t::process_handshake_command (msg_t *msg_)
return 0; return 0;
} }
int zmq::gssapi_server_t::send_zap_request () void zmq::gssapi_server_t::send_zap_request ()
{ {
gss_buffer_desc principal; gss_buffer_desc principal;
gss_display_name (&min_stat, target_name, &principal, NULL); gss_display_name (&min_stat, target_name, &principal, NULL);
int rc = zap_client_t::send_zap_request ("GSSAPI", 6, principal.value, zap_client_t::send_zap_request ("GSSAPI", 6, principal.value,
principal.length); principal.length);
gss_release_buffer (&min_stat, &principal); gss_release_buffer (&min_stat, &principal);
return rc;
} }
int zmq::gssapi_server_t::encode (msg_t *msg_) int zmq::gssapi_server_t::encode (msg_t *msg_)

View File

@ -88,7 +88,7 @@ namespace zmq
void accept_context (); void accept_context ();
int produce_next_token (msg_t *msg_); int produce_next_token (msg_t *msg_);
int process_next_token (msg_t *msg_); int process_next_token (msg_t *msg_);
int send_zap_request (); void send_zap_request ();
}; };
} }

View File

@ -74,11 +74,9 @@ int zmq::null_mechanism_t::next_handshake_command (msg_t *msg_)
errno = EAGAIN; errno = EAGAIN;
return -1; return -1;
} }
int rc = send_zap_request (); send_zap_request ();
if (rc != 0)
return -1;
zap_request_sent = true; zap_request_sent = true;
rc = receive_and_process_zap_reply (); int rc = receive_and_process_zap_reply ();
if (rc == -1 || rc == 1) if (rc == -1 || rc == 1)
return -1; return -1;
zap_reply_received = true; zap_reply_received = true;
@ -189,8 +187,8 @@ zmq::mechanism_t::status_t zmq::null_mechanism_t::status () const
return handshaking; return handshaking;
} }
int zmq::null_mechanism_t::send_zap_request () void zmq::null_mechanism_t::send_zap_request ()
{ {
return zap_client_t::send_zap_request ("NULL", 4, NULL, NULL, 0); zap_client_t::send_zap_request ("NULL", 4, NULL, NULL, 0);
} }

View File

@ -70,7 +70,7 @@ namespace zmq
int process_error_command ( int process_error_command (
const unsigned char *cmd_data, size_t data_size); const unsigned char *cmd_data, size_t data_size);
int send_zap_request (); void send_zap_request ();
}; };
} }

View File

@ -169,9 +169,7 @@ int zmq::plain_server_t::process_hello (msg_t *msg_)
int rc = session->zap_connect (); int rc = session->zap_connect ();
if (rc != 0) if (rc != 0)
return -1; return -1;
rc = send_zap_request (username, password); send_zap_request (username, password);
if (rc != 0)
return -1;
return receive_and_process_zap_reply () == -1 ? -1 : 0; return receive_and_process_zap_reply () == -1 ? -1 : 0;
} }
@ -219,13 +217,13 @@ int zmq::plain_server_t::produce_error (msg_t *msg_) const
return 0; return 0;
} }
int zmq::plain_server_t::send_zap_request (const std::string &username, void zmq::plain_server_t::send_zap_request (const std::string &username,
const std::string &password) const std::string &password)
{ {
const uint8_t *credentials[] = { const uint8_t *credentials[] = {
reinterpret_cast<const uint8_t *> (username.c_str ()), reinterpret_cast<const uint8_t *> (username.c_str ()),
reinterpret_cast<const uint8_t *> (password.c_str ())}; reinterpret_cast<const uint8_t *> (password.c_str ())};
size_t credentials_sizes[] = {username.size (), password.size ()}; size_t credentials_sizes[] = {username.size (), password.size ()};
return zap_client_t::send_zap_request ("PLAIN", 5, credentials, zap_client_t::send_zap_request ("PLAIN", 5, credentials, credentials_sizes,
credentials_sizes, 2); 2);
} }

View File

@ -62,8 +62,8 @@ namespace zmq
int process_hello (msg_t *msg_); int process_hello (msg_t *msg_);
int process_initiate (msg_t *msg_); int process_initiate (msg_t *msg_);
int send_zap_request(const std::string &username, void send_zap_request (const std::string &username,
const std::string &password); const std::string &password);
}; };
} }

View File

@ -45,25 +45,23 @@ zap_client_t::zap_client_t (session_base_t *const session_,
{ {
} }
int zap_client_t::send_zap_request (const char *mechanism, void zap_client_t::send_zap_request (const char *mechanism,
size_t mechanism_length, size_t mechanism_length,
const uint8_t *credentials, const uint8_t *credentials,
size_t credentials_size) size_t credentials_size)
{ {
return send_zap_request (mechanism, mechanism_length, &credentials, send_zap_request (mechanism, mechanism_length, &credentials,
&credentials_size, 1); &credentials_size, 1);
} }
int zap_client_t::send_zap_request (const char *mechanism, void zap_client_t::send_zap_request (const char *mechanism,
size_t mechanism_length, size_t mechanism_length,
const uint8_t **credentials, const uint8_t **credentials,
size_t *credentials_sizes, size_t *credentials_sizes,
size_t credentials_count) size_t credentials_count)
{ {
// TODO I don't think the rc can be -1 anywhere below. // write_zap_msg cannot fail. It could only fail if the HWM was exceeded,
// It might only be -1 if the HWM was exceeded, but on the ZAP socket, // but on the ZAP socket, the HWM is disabled.
// the HWM is disabled. They should be changed to zmq_assert (rc == 0);
// The method's return type can be changed to void then.
int rc; int rc;
msg_t msg; msg_t msg;
@ -73,8 +71,7 @@ int zap_client_t::send_zap_request (const char *mechanism,
errno_assert (rc == 0); errno_assert (rc == 0);
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
// Version frame // Version frame
rc = msg.init_size (3); rc = msg.init_size (3);
@ -82,8 +79,7 @@ int zap_client_t::send_zap_request (const char *mechanism,
memcpy (msg.data (), "1.0", 3); memcpy (msg.data (), "1.0", 3);
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
// Request ID frame // Request ID frame
rc = msg.init_size (1); rc = msg.init_size (1);
@ -91,8 +87,7 @@ int zap_client_t::send_zap_request (const char *mechanism,
memcpy (msg.data (), "1", 1); memcpy (msg.data (), "1", 1);
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
// Domain frame // Domain frame
rc = msg.init_size (options.zap_domain.length ()); rc = msg.init_size (options.zap_domain.length ());
@ -101,8 +96,7 @@ int zap_client_t::send_zap_request (const char *mechanism,
options.zap_domain.length ()); options.zap_domain.length ());
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
// Address frame // Address frame
rc = msg.init_size (peer_address.length ()); rc = msg.init_size (peer_address.length ());
@ -110,8 +104,7 @@ int zap_client_t::send_zap_request (const char *mechanism,
memcpy (msg.data (), peer_address.c_str (), peer_address.length ()); memcpy (msg.data (), peer_address.c_str (), peer_address.length ());
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
// Identity frame // Identity frame
rc = msg.init_size (options.identity_size); rc = msg.init_size (options.identity_size);
@ -119,8 +112,7 @@ int zap_client_t::send_zap_request (const char *mechanism,
memcpy (msg.data (), options.identity, options.identity_size); memcpy (msg.data (), options.identity, options.identity_size);
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
// Mechanism frame // Mechanism frame
rc = msg.init_size (mechanism_length); rc = msg.init_size (mechanism_length);
@ -129,8 +121,7 @@ int zap_client_t::send_zap_request (const char *mechanism,
if (credentials_count) if (credentials_count)
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
// Credentials frames // Credentials frames
for (size_t i = 0; i < credentials_count; ++i) { for (size_t i = 0; i < credentials_count; ++i) {
@ -140,11 +131,8 @@ int zap_client_t::send_zap_request (const char *mechanism,
msg.set_flags (msg_t::more); msg.set_flags (msg_t::more);
memcpy (msg.data (), credentials[i], credentials_sizes[i]); memcpy (msg.data (), credentials[i], credentials_sizes[i]);
rc = session->write_zap_msg (&msg); rc = session->write_zap_msg (&msg);
if (rc != 0) errno_assert (rc == 0);
return close_and_return (&msg, -1);
} }
return 0;
} }
int zap_client_t::receive_and_process_zap_reply () int zap_client_t::receive_and_process_zap_reply ()
@ -288,13 +276,7 @@ zmq::mechanism_t::status_t zap_client_common_handshake_t::status () const
int zap_client_common_handshake_t::zap_msg_available () int zap_client_common_handshake_t::zap_msg_available ()
{ {
// TODO I don't think that it is possible that this is called in any zmq_assert (state == waiting_for_zap_reply);
// state other than expect_zap_reply. It should be changed to
// zmq_assert (state == expect_zap_reply);
if (state != waiting_for_zap_reply) {
errno = EFSM;
return -1;
}
return receive_and_process_zap_reply () == -1 ? -1 : 0; return receive_and_process_zap_reply () == -1 ? -1 : 0;
} }

View File

@ -43,16 +43,16 @@ class zap_client_t : public virtual mechanism_t
const std::string &peer_address_, const std::string &peer_address_,
const options_t &options_); const options_t &options_);
int send_zap_request (const char *mechanism, void send_zap_request (const char *mechanism,
size_t mechanism_length, size_t mechanism_length,
const uint8_t *credentials, const uint8_t *credentials,
size_t credentials_size); size_t credentials_size);
int send_zap_request (const char *mechanism, void send_zap_request (const char *mechanism,
size_t mechanism_length, size_t mechanism_length,
const uint8_t **credentials, const uint8_t **credentials,
size_t *credentials_sizes, size_t *credentials_sizes,
size_t credentials_count); size_t credentials_count);
virtual int receive_and_process_zap_reply (); virtual int receive_and_process_zap_reply ();
virtual void handle_zap_status_code (); virtual void handle_zap_status_code ();