List:Commits« Previous MessageNext Message »
From:Rafal Somla Date:March 29 2011 9:55pm
Subject:bzr commit into mysql-5.5 branch (rafal.somla:3375) Bug#11879051
View as plain text  
#At D:/source/bzr2/mysql-5.5 based on revid:rafal.somla@stripped

 3375 Rafal Somla	2011-03-29
      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. This also simplified logic for handling first client reply.

    modified:
      libmysql/authentication_win/common.h
      libmysql/authentication_win/handshake.cc
      libmysql/authentication_win/handshake.h
      libmysql/authentication_win/handshake_client.cc
=== modified file 'libmysql/authentication_win/common.h'
--- a/libmysql/authentication_win/common.h	2011-03-28 16:20:59 +0000
+++ b/libmysql/authentication_win/common.h	2011-03-29 21:55:14 +0000
@@ -167,6 +167,11 @@ public:
   {
     return m_ptr == NULL;
   }
+
+  void trim(size_t l)
+  {
+    m_len= l;
+  }
 };
 
 

=== modified file 'libmysql/authentication_win/handshake.cc'
--- a/libmysql/authentication_win/handshake.cc	2011-03-28 16:20:59 +0000
+++ b/libmysql/authentication_win/handshake.cc	2011-03-29 21:55:14 +0000
@@ -90,17 +90,18 @@ 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= 1;
 
   do {
     // 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 +114,7 @@ int Handshake::packet_processing_loop(Co
 
     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 +125,14 @@ int Handshake::packet_processing_loop(Co
 
     if (!new_data.is_null())
     {
-      ++round;
-      DBUG_PRINT("info", ("Round %d started", round));
+      ++m_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(Co
     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(Co
       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;
 }
 

=== modified file 'libmysql/authentication_win/handshake.h'
--- a/libmysql/authentication_win/handshake.h	2011-03-28 16:20:59 +0000
+++ b/libmysql/authentication_win/handshake.h	2011-03-29 21:55:14 +0000
@@ -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
 

=== modified file 'libmysql/authentication_win/handshake_client.cc'
--- a/libmysql/authentication_win/handshake_client.cc	2011-03-28 16:20:59 +0000
+++ b/libmysql/authentication_win/handshake_client.cc	2011-03-29 21:55:14 +0000
@@ -33,21 +33,27 @@ 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 target    name of the target service with which we will authenticate
-                   (can be NULL if not used)
+  @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)
 
   Some security packages (like Kerberos) require providing explicit name
   of the service with which a client wants to authenticate. The server-side
@@ -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,70 @@ 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)
+
+  return m_error ? Blob() : packet;
+}
 
-Blob Handshake_client::first_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, 
+   sencond part will be sent in the follwing packet. Byte 255 of the payload 
+   is repeated in the second packet to ensure that it is not empty.
+
+   Server's logic for reading first client's payload is as follows:
+   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.
+  */
 
-  m_output.free();
+  size_t len2= 0;   // length of the second part of first data payload
 
-  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;
+    data.trim(255);
+  }
 
-  if (process_result(ret))
+  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();
+    Blob data2(data.ptr() + 254, len2);
+    ret= m_con.write(data2);
   }
 
-  return m_output.as_blob();
+  return ret;
 }
 
 
@@ -139,6 +173,10 @@ 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.
 */
@@ -152,12 +190,12 @@ Blob Handshake_client::process_data(cons
 
   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
@@ -234,9 +272,10 @@ int win_auth_handshake_client(MYSQL_PLUG
   }
   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())
   {
@@ -244,40 +283,16 @@ int win_auth_handshake_client(MYSQL_PLUG
     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_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())
+    return CR_ERROR;
 
   DBUG_ASSERT(!hndshk.error() && hndshk.is_complete());
 


Attachment: [text/bzr-bundle] bzr/rafal.somla@oracle.com-20110329215514-vv4k9vv444dn8sy0.bundle
Thread
bzr commit into mysql-5.5 branch (rafal.somla:3375) Bug#11879051Rafal Somla29 Mar