Problem: curve encoding and decoding are not easily testable

Solution: extract into separate class curve_encoding_t

src/curve_client.cpp

@@ -137,7 +137,7 @@ int zmq::curve_client_t::produce_hello (msg_t *msg_)
     int rc = msg_->init_size (200);
     errno_assert (rc == 0);
 
-    rc = _tools.produce_hello (msg_->data (), _cn_nonce);
+    rc = _tools.produce_hello (msg_->data (), get_and_inc_nonce ());
     if (rc == -1) {
         session->get_socket ()->event_handshake_failed_protocol (
           session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC);
@@ -150,15 +150,14 @@ int zmq::curve_client_t::produce_hello (msg_t *msg_)
         return -1;
     }
 
-    _cn_nonce++;
-
     return 0;
 }
 
 int zmq::curve_client_t::process_welcome (const uint8_t *msg_data_,
                                           size_t msg_size_)
 {
-    const int rc = _tools.process_welcome (msg_data_, msg_size_, _cn_precom);
+    const int rc = _tools.process_welcome (msg_data_, msg_size_,
+                                           get_writable_precom_buffer ());
 
     if (rc == -1) {
         session->get_socket ()->event_handshake_failed_protocol (
@@ -186,7 +185,7 @@ int zmq::curve_client_t::produce_initiate (msg_t *msg_)
     int rc = msg_->init_size (msg_size);
     errno_assert (rc == 0);
 
-    rc = _tools.produce_initiate (msg_->data (), msg_size, _cn_nonce,
+    rc = _tools.produce_initiate (msg_->data (), msg_size, get_and_inc_nonce (),
                                   &metadata_plaintext[0], metadata_length);
 
     if (-1 == rc) {
@@ -197,8 +196,6 @@ int zmq::curve_client_t::produce_initiate (msg_t *msg_)
         return -1;
     }
 
-    _cn_nonce++;
-
     return 0;
 }
 
@@ -227,10 +224,10 @@ int zmq::curve_client_t::process_ready (const uint8_t *msg_data_,
 
     memcpy (ready_nonce, "CurveZMQREADY---", 16);
     memcpy (ready_nonce + 16, msg_data_ + 6, 8);
-    _cn_peer_nonce = get_uint64 (msg_data_ + 6);
+    set_peer_nonce (get_uint64 (msg_data_ + 6));
 
     int rc = crypto_box_open_afternm (&ready_plaintext[0], &ready_box[0], clen,
-                                      ready_nonce, _cn_precom);
+                                      ready_nonce, get_precom_buffer ());
 
     if (rc != 0) {
         session->get_socket ()->event_handshake_failed_protocol (

src/curve_mechanism_base.cpp

@@ -42,6 +42,33 @@ zmq::curve_mechanism_base_t::curve_mechanism_base_t (
   const char *encode_nonce_prefix_,
   const char *decode_nonce_prefix_) :
     mechanism_base_t (session_, options_),
+    curve_encoding_t (encode_nonce_prefix_, decode_nonce_prefix_)
+{
+}
+
+int zmq::curve_mechanism_base_t::encode (msg_t *msg_)
+{
+    return curve_encoding_t::encode (msg_);
+}
+
+int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
+{
+    int rc = check_basic_command_structure (msg_);
+    if (rc == -1)
+        return -1;
+
+    int error_event_code;
+    rc = curve_encoding_t::decode (msg_, &error_event_code);
+    if (-1 == rc) {
+        session->get_socket ()->event_handshake_failed_protocol (
+          session->get_endpoint (), error_event_code);
+    }
+
+    return rc;
+}
+
+zmq::curve_encoding_t::curve_encoding_t (const char *encode_nonce_prefix_,
+                                         const char *decode_nonce_prefix_) :
     _encode_nonce_prefix (encode_nonce_prefix_),
     _decode_nonce_prefix (decode_nonce_prefix_),
     _cn_nonce (1),
@@ -49,7 +76,7 @@ zmq::curve_mechanism_base_t::curve_mechanism_base_t (
 {
 }
 
-int zmq::curve_mechanism_base_t::encode (msg_t *msg_)
+int zmq::curve_encoding_t::encode (msg_t *msg_)
 {
     const size_t mlen = crypto_box_ZEROBYTES + 1 + msg_->size ();
 
@@ -98,26 +125,19 @@ int zmq::curve_mechanism_base_t::encode (msg_t *msg_)
     return 0;
 }
 
-int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
+int zmq::curve_encoding_t::decode (msg_t *msg_, int *error_event_code_)
 {
-    int rc = check_basic_command_structure (msg_);
-    if (rc == -1)
-        return -1;
-
     const size_t size = msg_->size ();
     const uint8_t *message = static_cast<uint8_t *> (msg_->data ());
 
-    if (size < 8 || memcmp (message, "\x07MESSAGE", 8)) {
+    if (size < 8 || 0 != memcmp (message, "\x07MESSAGE", 8)) {
-        session->get_socket ()->event_handshake_failed_protocol (
+        *error_event_code_ = ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND;
-          session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_UNEXPECTED_COMMAND);
         errno = EPROTO;
         return -1;
     }
 
     if (size < 33) {
-        session->get_socket ()->event_handshake_failed_protocol (
+        *error_event_code_ = ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE;
-          session->get_endpoint (),
-          ZMQ_PROTOCOL_ERROR_ZMTP_MALFORMED_COMMAND_MESSAGE);
         errno = EPROTO;
         return -1;
     }
@@ -127,8 +147,7 @@ int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
     memcpy (message_nonce + 16, message + 8, 8);
     const uint64_t nonce = get_uint64 (message + 8);
     if (nonce <= _cn_peer_nonce) {
-        session->get_socket ()->event_handshake_failed_protocol (
+        *error_event_code_ = ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_SEQUENCE;
-          session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_INVALID_SEQUENCE);
         errno = EPROTO;
         return -1;
     }
@@ -144,8 +163,8 @@ int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
     memcpy (&message_box[crypto_box_BOXZEROBYTES], message + 16,
             msg_->size () - 16);
 
-    rc = crypto_box_open_afternm (&message_plaintext[0], &message_box[0], clen,
+    int rc = crypto_box_open_afternm (&message_plaintext[0], &message_box[0],
-                                  message_nonce, _cn_precom);
+                                      clen, message_nonce, _cn_precom);
     if (rc == 0) {
         rc = msg_->close ();
         zmq_assert (rc == 0);
@@ -166,8 +185,7 @@ int zmq::curve_mechanism_base_t::decode (msg_t *msg_)
                     msg_->size ());
     } else {
         // CURVE I : connection key used for MESSAGE is wrong
-        session->get_socket ()->event_handshake_failed_protocol (
+        *error_event_code_ = ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC;
-          session->get_endpoint (), ZMQ_PROTOCOL_ERROR_ZMTP_CRYPTOGRAPHIC);
         errno = EPROTO;
     }
 

src/curve_mechanism_base.hpp

@@ -52,7 +52,39 @@
 
 namespace zmq
 {
-class curve_mechanism_base_t : public virtual mechanism_base_t
+class curve_encoding_t
+{
+  public:
+    curve_encoding_t (const char *encode_nonce_prefix_,
+                      const char *decode_nonce_prefix_);
+
+    int encode (msg_t *msg_);
+    int decode (msg_t *msg_, int *error_event_code_);
+
+    uint8_t *get_writable_precom_buffer () { return _cn_precom; }
+    const uint8_t *get_precom_buffer () const { return _cn_precom; }
+
+    uint64_t get_and_inc_nonce () { return _cn_nonce++; }
+    void set_peer_nonce (uint64_t peer_nonce_)
+    {
+        _cn_peer_nonce = peer_nonce_;
+    };
+
+  private:
+    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];
+
+    ZMQ_NON_COPYABLE_NOR_MOVABLE (curve_encoding_t)
+};
+
+class curve_mechanism_base_t : public virtual mechanism_base_t,
+                               public curve_encoding_t
 {
   public:
     curve_mechanism_base_t (session_base_t *session_,
@@ -63,17 +95,6 @@ class curve_mechanism_base_t : public virtual mechanism_base_t
     // mechanism implementation
     int encode (msg_t *msg_) ZMQ_OVERRIDE;
     int decode (msg_t *msg_) ZMQ_OVERRIDE;
-
-  private:
-    const char *_encode_nonce_prefix;
-    const char *_decode_nonce_prefix;
-
-  protected:
-    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];
 };
 }
 

src/curve_server.cpp

@@ -181,7 +181,7 @@ int zmq::curve_server_t::process_hello (msg_t *msg_)
 
     memcpy (hello_nonce, "CurveZMQHELLO---", 16);
     memcpy (hello_nonce + 16, hello + 112, 8);
-    _cn_peer_nonce = get_uint64 (hello + 112);
+    set_peer_nonce (get_uint64 (hello + 112));
 
     memset (hello_box, 0, crypto_box_BOXZEROBYTES);
     memcpy (hello_box + crypto_box_BOXZEROBYTES, hello + 120, 80);
@@ -345,7 +345,7 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
 
     memcpy (initiate_nonce, "CurveZMQINITIATE", 16);
     memcpy (initiate_nonce + 16, initiate + 105, 8);
-    _cn_peer_nonce = get_uint64 (initiate + 105);
+    set_peer_nonce (get_uint64 (initiate + 105));
 
     const uint8_t *client_key = &initiate_plaintext[crypto_box_ZEROBYTES];
 
@@ -396,7 +396,8 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
     }
 
     //  Precompute connection secret from client key
-    rc = crypto_box_beforenm (_cn_precom, _cn_client, _cn_secret);
+    rc = crypto_box_beforenm (get_writable_precom_buffer (), _cn_client,
+                              _cn_secret);
     zmq_assert (rc == 0);
 
     //  Given this is a backward-incompatible change, it's behind a socket
@@ -449,13 +450,13 @@ int zmq::curve_server_t::produce_ready (msg_t *msg_)
     const size_t mlen = ptr - &ready_plaintext[0];
 
     memcpy (ready_nonce, "CurveZMQREADY---", 16);
-    put_uint64 (ready_nonce + 16, _cn_nonce);
+    put_uint64 (ready_nonce + 16, get_and_inc_nonce ());
 
     std::vector<uint8_t> ready_box (crypto_box_BOXZEROBYTES + 16
                                     + metadata_length);
 
     int rc = crypto_box_afternm (&ready_box[0], &ready_plaintext[0], mlen,
-                                 ready_nonce, _cn_precom);
+                                 ready_nonce, get_precom_buffer ());
     zmq_assert (rc == 0);
 
     rc = msg_->init_size (14 + mlen - crypto_box_BOXZEROBYTES);
@@ -470,8 +471,6 @@ int zmq::curve_server_t::produce_ready (msg_t *msg_)
     memcpy (ready + 14, &ready_box[crypto_box_BOXZEROBYTES],
             mlen - crypto_box_BOXZEROBYTES);
 
-    _cn_nonce++;
-
     return 0;
 }