BUG#11879051: FIRST REPLY LENGTH LIMIT (255) CAN BE VIOLATED

BEFORE: First packet sent by client-side plugin (generated by Windows
function InitializeSecurityContext()) could be longer than 255 bytes 
violating the limitation imposed by authentication protocol.

AFTER: Handshake protocol is  changed so that if first client's reply is 
longer than 254 bytes then  it is be sent in 2 parts. However, for replies
shorter than 255 bytes nothing changes.

ADDITIONAL CHANGES: 
- The generic packet processing loop  (Handshake::packet_processing_loop) 
has been refactored. Communication with the peer has been abstracted
into virtual methods read/write_packet() which are implemented in client 
and server and transparently do the required splitting and gluing of packets.
- Make it possible to optionally use dbug library in the plugin.
- Add code for testing splitting of long first client reply.

libmysql/authentication_win/CMakeLists.txt

@@ -19,6 +19,8 @@
 
 ADD_DEFINITIONS(-DSECURITY_WIN32)
 ADD_DEFINITIONS(-DDEBUG_ERRROR_LOG)  # no error logging in production builds
+ADD_DEFINITIONS(-DWINAUTH_USE_DBUG_LIB)      # it is OK to use dbug library in statically
+                                             # linked plugin
 
 SET(HEADERS common.h handshake.h)
 SET(PLUGIN_SOURCES plugin_client.cc handshake_client.cc log_client.cc common.cc handshake.cc)

libmysql/authentication_win/common.h

@@ -41,54 +41,65 @@ struct error_log_level
   typedef enum {INFO, WARNING, ERROR}  type;
 };
 
-#undef  DBUG_ASSERT
-#ifndef DBUG_OFF
-#define DBUG_ASSERT(X)  assert(X)
+
+/*
+  If DEBUG_ERROR_LOG is defined then error logging happens only
+  in debug-copiled code. Otherwise ERROR_LOG() expands to 
+  error_log_print() even in production code. Note that in client
+  plugin, error_log_print() will print nothing if opt_auth_win_clinet_log
+  is 0.
+
+  Note: Macro ERROR_LOG() can use printf-like format string like this:
+
+    ERROR_LOG(Level, ("format string", args));
+
+  The implementation should handle it correctly. Currently it is passed 
+  to fprintf() (see error_log_vprint() function).
+*/
+
+extern "C" int opt_auth_win_client_log;
+
+#if defined(DEBUG_ERROR_LOG) && defined(DBUG_OFF)
+#define ERROR_LOG(Level, Msg)     do {} while (0)
 #else
-#define DBUG_ASSERT(X)  do {} while (0)
+#define ERROR_LOG(Level, Msg)     error_log_print< error_log_level::Level > Msg
 #endif
 
-extern "C" int opt_auth_win_client_log;
 
-/*
-  Note1: Double level of indirection in definition of DBUG_PRINT allows to
-  temporary redefine or disable DBUG_PRINT macro and then easily return to
-  the original definition (in terms of DBUG_PRINT_DO).
+void error_log_vprint(error_log_level::type level,
+                        const char *fmt, va_list args);
 
-  Note2: DBUG_PRINT() can use printf-like format string like this:
+template <error_log_level::type Level>
+void error_log_print(const char *fmt, ...)
+{
+  va_list args;
+  va_start(args, fmt);
+  error_log_vprint(Level, fmt, args);
+  va_end(args);
+}
 
-    DBUG_PRINT(Keyword, ("format string", args));
+typedef char Error_message_buf[1024];
+const char* get_last_error_message(Error_message_buf);
 
-  The implementation should handle it correctly. Currently it is passed 
-  to fprintf() (see debug_msg() function).
+
+/*
+  Internal implementation of debug message printing which does not use
+  dbug library. This is invoked via macro:
+
+    DBUG_PRINT_DO(Keyword, ("format string", args));
+
+  This is supposed to be used as an implementation of DBUG_PRINT() macro,
+  unless the dbug library implementation is used or debug messages are disabled.
 */
 
 #ifndef DBUG_OFF
+
 #define DBUG_PRINT_DO(Keyword, Msg) \
   do { \
     if (2 > opt_auth_win_client_log) break; \
     fprintf(stderr, "winauth: %s: ", Keyword); \
     debug_msg Msg; \
   } while (0)
-#else
-#define DBUG_PRINT_DO(K, M)  do {} while (0)
-#endif
-
-#undef  DBUG_PRINT
-#define DBUG_PRINT(Keyword, Msg)  DBUG_PRINT_DO(Keyword, Msg)
-
-/*
-  If DEBUG_ERROR_LOG is defined then error logging happens only
-  in debug-copiled code. Otherwise ERROR_LOG() expands to 
-  error_log_print() even in production code. Note that in client
-  plugin, error_log_print() will print nothing if opt_auth_win_clinet_log
-  is 0.
-*/
-#if defined(DEBUG_ERROR_LOG) && defined(DBUG_OFF)
-#define ERROR_LOG(Level, Msg)     do {} while (0)
-#else
-#define ERROR_LOG(Level, Msg)     error_log_print< error_log_level::Level > Msg
-#endif
 
 inline
 void debug_msg(const char *fmt, ...)
@@ -101,21 +112,38 @@ void debug_msg(const char *fmt, ...)
   va_end(args);
 }
 
+#else
+#define DBUG_PRINT_DO(K, M)  do {} while (0)
+#endif
 
-void error_log_vprint(error_log_level::type level,
-                        const char *fmt, va_list args);
 
-template <error_log_level::type Level>
-void error_log_print(const char *fmt, ...)
-{
-  va_list args;
-  va_start(args, fmt);
-  error_log_vprint(Level, fmt, args);
-  va_end(args);
-}
+#ifndef WINAUTH_USE_DBUG_LIB
 
-typedef char Error_message_buf[1024];
-const char* get_last_error_message(Error_message_buf);
+#undef  DBUG_PRINT
+#define DBUG_PRINT(Keyword, Msg)  DBUG_PRINT_DO(Keyword, Msg)
+
+/*
+  Redefine few more debug macros to make sure that no symbols from
+  dbug library are used.
+*/
+
+#undef DBUG_ENTER
+#define DBUG_ENTER(X)  do {} while (0)
+
+#undef DBUG_RETURN
+#define DBUG_RETURN(X) return (X)
+
+#undef DBUG_ASSERT
+#ifndef DBUG_OFF
+#define DBUG_ASSERT(X) assert (X)
+#else
+#define DBUG_ASSERT(X) do {} while (0)
+#endif
+
+#undef DBUG_DUMP
+#define DBUG_DUMP(A,B,C) do {} while (0)
+
+#endif
 
 
 /** Blob class *************************************************************/
@@ -158,15 +186,21 @@ public:
     return m_len;
   }
 
-  byte operator[](unsigned pos) const
+  byte& operator[](unsigned pos) const
   {
-    return pos < len() ? m_ptr[pos] : 0x00;
+    static byte out_of_range= 0;  // alas, no exceptions...
+    return pos < len() ? m_ptr[pos] : out_of_range;
   }
 
   bool is_null() const
   {
     return m_ptr == NULL;
   }
+
+  void trim(size_t l)
+  {
+    m_len= l;
+  }
 };
 
 

libmysql/authentication_win/handshake.cc

@@ -90,17 +90,19 @@ Handshake::~Handshake()
 
   @note In case of error, appropriate error message is logged.
 */
-int Handshake::packet_processing_loop(Connection &con)
+int Handshake::packet_processing_loop()
 {
-  unsigned round= 1;
+  m_round= 0;
 
   do {
+    ++m_round;
     // Read packet send by the peer
+
     DBUG_PRINT("info", ("Waiting for packet"));
-    Blob packet= con.read();
-    if (con.error() || packet.is_null())
+    Blob packet= read_packet();
+    if (error())
     {
-      ERROR_LOG(ERROR, ("Error reading packet in round %d", round));
+      ERROR_LOG(ERROR, ("Error reading packet in round %d", m_round));
       return 1;
     }
     DBUG_PRINT("info", ("Got packet of length %d", packet.len()));
@@ -113,7 +115,7 @@ int Handshake::packet_processing_loop(Connection &con)
 
     if (error())
     {
-      ERROR_LOG(ERROR, ("Error processing packet in round %d", round));
+      ERROR_LOG(ERROR, ("Error processing packet in round %d", m_round));
       return 1;
     }
 
@@ -124,14 +126,13 @@ int Handshake::packet_processing_loop(Connection &con)
 
     if (!new_data.is_null())
     {
-      ++round;
-      DBUG_PRINT("info", ("Round %d started", round));
+      DBUG_PRINT("info", ("Round %d started", m_round));
 
       DBUG_PRINT("info", ("Sending packet of length %d", new_data.len()));
-      int ret= con.write(new_data);
+      int ret= write_packet(new_data);
       if (ret)
       {
-        ERROR_LOG(ERROR, ("Error writing packet in round %d", round));
+        ERROR_LOG(ERROR, ("Error writing packet in round %d", m_round));
         return 1;
       }
       DBUG_PRINT("info", ("Data sent"));
@@ -139,7 +140,7 @@ int Handshake::packet_processing_loop(Connection &con)
     else if (!is_complete())
     {
       ERROR_LOG(ERROR, ("No data to send in round %d"
-                        " but handshake is not complete", round));
+                        " but handshake is not complete", m_round));
       return 1;
     }
 
@@ -148,16 +149,16 @@ int Handshake::packet_processing_loop(Connection &con)
       too many rounds.
     */
 
-    if (round > MAX_HANDSHAKE_ROUNDS)
+    if (m_round > MAX_HANDSHAKE_ROUNDS)
     {
       ERROR_LOG(ERROR, ("Authentication handshake could not be completed"
-                        " after %d rounds", round));
+                        " after %d rounds", m_round));
       return 1;
     }
 
   } while(!is_complete());
 
-  ERROR_LOG(INFO, ("Handshake completed after %d rounds", round));
+  ERROR_LOG(INFO, ("Handshake completed after %d rounds", m_round));
   return 0;
 }
 

libmysql/authentication_win/handshake.h

@@ -100,7 +100,7 @@ public:
   Handshake(const char *ssp, side_t side);
   virtual ~Handshake();
 
-  int Handshake::packet_processing_loop(Connection &con);
+  int Handshake::packet_processing_loop();
 
   bool virtual is_complete() const
   {
@@ -126,6 +126,13 @@ protected:
   /// Stores attributes of the created security context.
   ULONG  m_atts;
 
+  /**
+    Round of the handshake (starting from round 1). One round
+    consist of reading packet from the other side, processing it and
+    optionally sending a reply (see @c packet_processing_loop()).
+  */
+  unsigned int m_round;
+
   /// If non-zero, stores error code of the last failed operation.
   int  m_error;
 
@@ -152,7 +159,13 @@ protected:
     @return A blob with data to be sent to the other end or null blob if
     no more data needs to be exchanged.
   */
-  virtual Blob process_data(const Blob &data)= 0;
+  virtual Blob process_data(const Blob &data) =0;
+
+  /// Read packet from the other end.
+  virtual Blob read_packet()  =0;
+
+  /// Write packet to the other end.
+  virtual int  write_packet(Blob &data) =0;
 
 #ifndef DBUG_OFF
 

libmysql/authentication_win/handshake_client.cc

@@ -33,19 +33,25 @@ class Handshake_client: public Handshake
   /// Buffer for storing service name obtained from server.
   SEC_WCHAR   m_service_name_buf[MAX_SERVICE_NAME_LENGTH];
 
+  Connection &m_con;
+
 public:
 
-  Handshake_client(const char *target, size_t len);
+  Handshake_client(Connection &con, const char *target, size_t len);
   ~Handshake_client();
 
   Blob  first_packet();
   Blob  process_data(const Blob&);
+
+  Blob read_packet();
+  int write_packet(Blob &data);
 };
 
 
 /**
   Create authentication handshake context for client.
 
+  @param con     connection for communication with the peer 
   @param target  name of the target service with which we will authenticate
                  (can be NULL if not used)
 
@@ -55,8 +61,9 @@ public:
   (see @c win_auth_handshake_{server,client}() functions).
 */
 
-Handshake_client::Handshake_client(const char *target, size_t len)
-: Handshake(SSP_NAME, CLIENT), m_service_name(NULL)
+Handshake_client::Handshake_client(Connection &con, 
+                                   const char *target, size_t len)
+: Handshake(SSP_NAME, CLIENT), m_service_name(NULL), m_con(con)
 {
   if (!target || 0 == len)
     return;
@@ -88,43 +95,97 @@ Handshake_client::~Handshake_client()
 }
 
 
-/**
-  Generate first packet to be sent to the server during packet exchange.
+Blob Handshake_client::read_packet()
+{
+  /*
+    We do a fake read in the first round because first
+    packet from the server containing UPN must be read
+    before the handshake context is created and the packet
+    processing loop starts. We return an empty blob here
+    and process_data() function will ignore it.
+  */
+  if (m_round == 1)
+    return Blob();
 
-  This first packet should contain some data. In case of error a null blob
-  is returned and @c error() gives non-zero error code.
+  // Otherwise we read packet from the connection.
 
-  @return Data to be sent in the first packet or null blob in case of error.
-*/
+  Blob packet= m_con.read();
+  m_error= m_con.error();
+  if (!m_error && packet.is_null())
+    m_error= true;  // (no specific error code assigned)
+
+  if (m_error)
+    return Blob();
 
-Blob Handshake_client::first_packet()
+  DBUG_PRINT("dump", ("Got the following bytes"));
+  DBUG_DUMP("dump", packet.ptr(), packet.len());
+  return packet;
+}
+
+
+
+int Handshake_client::write_packet(Blob &data)
 {
-  SECURITY_STATUS ret;
+  /*
+   Length of the first data payload send by client authentication plugin is
+   limited to 255 bytes (because it is wrapped inside client authentication
+   packet and is length-encoded with 1 byte for the length).
+
+   If the data payload is longer than 254 bytes, then it is sent in two parts:
+   first part of length 255 will be embedded in the authentication packet, 
+   second part will be sent in the following packet. Byte 255 of the first 
+   part contains information about the total length of the payload. It is a
+   number of blocks of size 512 bytes which is sufficient to store the
+   combined packets.
+
+   Server's logic for reading first client's payload is as follows
+   (see Handshake_server::read_packet()):
+   1. Read data from the authentication packet, if it is shorter than 255 bytes 
+      then that is all data sent by client.
+   2. If there is 255 bytes of data in the authentication packet, read another
+      packet and append it to the data, skipping byte 255 of the first packet
+      which can be used to allocate buffer of appropriate size.
+  */
 
-  m_output.free();
+  size_t len2= 0;   // length of the second part of first data payload
+  byte saved_byte;  // for saving byte 255 in which data length is stored
 
-  ret= InitializeSecurityContextW(
-         &m_cred,
-         NULL,                                 // partial context
-         m_service_name,                       // service name
-         ASC_REQ_ALLOCATE_MEMORY,              // requested attributes
-         0,                                    // reserved
-         SECURITY_NETWORK_DREP,                // data representation
-         NULL,                                 // input data
-         0,                                    // reserved
-         &m_sctx,                              // context
-         &m_output,                            // output data
-         &m_atts,                              // attributes
-         &m_expire);                           // expire date
+  if (m_round == 1 && data.len() > 254)
+  {
+    len2= data.len() - 254;
+    DBUG_PRINT("info", ("Splitting first packet of length %lu"
+                        ", %lu bytes will be sent in a second part", 
+                        data.len(), len2));
+    /* 
+      Store in byte 255 the number of 512b blocks that are needed to
+      keep all the data.
+    */
+    unsigned block_count= data.len()/512 + ((data.len() % 512) ? 1 : 0);
+    DBUG_ASSERT(block_count < (unsigned)0x100);
+    saved_byte= data[254];
+    data[254] = block_count;
 
-  if (process_result(ret))
+    data.trim(255);
+  }
+
+  DBUG_PRINT("dump", ("Sending the following data"));
+  DBUG_DUMP("dump", data.ptr(), data.len());
+  int ret= m_con.write(data);
+
+  if (ret)
+    return ret;
+
+  // Write second part if it is present.
+  if (len2)
   {
-    DBUG_PRINT("error",
-               ("InitializeSecurityContext() failed with error %X", ret));
-    return Blob();
+    data[254]= saved_byte;
+    Blob data2(data.ptr() + 254, len2);
+    DBUG_PRINT("info", ("Sending second part of data"));
+    DBUG_DUMP("info", data2.ptr(), data2.len());
+    ret= m_con.write(data2);
   }
 
-  return m_output.as_blob();
+  return ret;
 }
 
 
@@ -139,12 +200,66 @@ Blob Handshake_client::first_packet()
   empty blob is returned and @c is_complete() is @c true. In case of error
   an empty blob is returned and @c error() gives non-zero error code.
 
+  When invoked for the first time (in the first round of the handshake)
+  there is no data from the server (data blob is null) and the intial
+  packet is generated without an input.
+
   @return Data to be sent to the server next or null blob if no more data
   needs to be exchanged or in case of error.
 */
 
 Blob Handshake_client::process_data(const Blob &data)
 {
+#if !defined(DBUG_OFF) && defined(WINAUTH_USE_DBUG_LIB)
+  /*
+    Code for testing the logic for sending the first client payload.
+
+    A fake data of length given by environment variable TEST_PACKET_LENGTH
+    (or default 255 bytes) is sent to the server. First 2 bytes of the
+    payload contain its total length (LSB first). The length of test data
+    is limited to 2048 bytes.
+
+    Upon receiving test data, server will check that data is correct and
+    refuse connection. If server detects data errors it will crash on 
+    assertion.
+
+    This code is executed if debug flag "winauth_first_packet_test" is
+    set, e.g. using client option:
+
+     --debug="d,winauth_first_packet_test"
+
+     The same debug flag must be enabled in the server, e.g. using 
+     statement:
+
+     SET GLOBAL debug= '+d,winauth_first_packet_test'; 
+  */
+
+  static byte test_buf[2048];
+
+  if (m_round == 1 
+      && DBUG_EVALUATE_IF("winauth_first_packet_test", true, false))
+  {
+    const char *env= getenv("TEST_PACKET_LENGTH");
+    size_t len= env ? atoi(env) : 0;
+    if (!len)
+      len= 255;
+    if (len > sizeof(test_buf))
+      len= sizeof(test_buf);
+
+    // Store data length in first 2 bytes.
+    byte *ptr= test_buf;
+    *ptr++= len & 0xFF;
+    *ptr++= len >> 8;
+
+    // Fill remaining bytes with known values.
+    for (byte b= 0; ptr < test_buf + len; ++ptr, ++b)
+      *ptr= b;
+
+    return Blob(test_buf, len);
+  };
+
+#endif
+
   Security_buffer  input(data);
   SECURITY_STATUS  ret;
 
@@ -152,12 +267,12 @@ Blob Handshake_client::process_data(const Blob &data)
 
   ret= InitializeSecurityContextW(
          &m_cred,
-         &m_sctx,                              // partial context
+         m_round == 1 ? NULL : &m_sctx,        // partial context
          m_service_name,                       // service name
          ASC_REQ_ALLOCATE_MEMORY,              // requested attributes
          0,                                    // reserved
          SECURITY_NETWORK_DREP,                // data representation
-         &input,                               // input data
+         m_round == 1 ? NULL : &input,         // input data
          0,                                    // reserved
          &m_sctx,                              // context
          &m_output,                            // output data
@@ -198,6 +313,8 @@ Blob Handshake_client::process_data(const Blob &data)
 
 int win_auth_handshake_client(MYSQL_PLUGIN_VIO *vio, MYSQL *mysql)
 {
+  DBUG_ENTER("win_auth_handshake_client");
+
   /*
     Check if we should enable logging.
   */
@@ -224,62 +341,38 @@ int win_auth_handshake_client(MYSQL_PLUGIN_VIO *vio, MYSQL *mysql)
 
   // Read initial packet from server containing service name.
 
-  int ret;
   Blob service_name= con.read();
 
   if (con.error() || service_name.is_null())
   {
     ERROR_LOG(ERROR, ("Error reading initial packet"));
-    return CR_ERROR;
+    DBUG_RETURN(CR_ERROR);
   }
   DBUG_PRINT("info", ("Got initial packet of length %d", service_name.len()));
 
-  // Create authentication handsake context using the given service name.
+  // Create authentication handshake context using the given service name.
 
-  Handshake_client hndshk(service_name[0] ? (char *)service_name.ptr() : NULL,
+  Handshake_client hndshk(con,
+                          service_name[0] ? (char *)service_name.ptr() : NULL,
                           service_name.len());
   if (hndshk.error())
   {
     ERROR_LOG(ERROR, ("Could not create authentication handshake context"));
-    return CR_ERROR;
-  }
-
-  /*
-    The following packet exchange always starts with a packet sent by
-    the client. Send this first packet now.
-  */
-
-  {
-    Blob packet= hndshk.first_packet();
-    if (hndshk.error() || packet.is_null())
-    {
-      ERROR_LOG(ERROR, ("Could not generate first packet"));
-      return CR_ERROR;
-    }
-    DBUG_PRINT("info", ("Sending first packet of length %d", packet.len()));
-
-    ret= con.write(packet);
-    if (ret)
-    {
-      ERROR_LOG(ERROR, ("Error writing first packet"));
-      return CR_ERROR;
-    }
-    DBUG_PRINT("info", ("First packet sent"));
+    DBUG_RETURN(CR_ERROR);
   }
 
   DBUG_ASSERT(!hndshk.error());
 
   /*
-    If handshake is not yet complete and client expects a reply, 
-    read and process packets from server until handshake is complete. 
+    Read and process packets from server until handshake is complete.
+    Note that the first read from server is dummy 
+    (see Handshake_client::read_packet()) as we already have read the 
+    first packet to establish service name.
   */
-  if (!hndshk.is_complete())
-  {
-    if (hndshk.packet_processing_loop(con))
-      return CR_ERROR;
-  }
+  if (hndshk.packet_processing_loop())
+    DBUG_RETURN(CR_ERROR);
 
   DBUG_ASSERT(!hndshk.error() && hndshk.is_complete());
 
-  return CR_OK;
+  DBUG_RETURN(CR_OK);
 }