Fix up the negotiation, eliminating a failure mode with old clients.

If we send a "wrong version number" error message to an old client
(before the changes to support version negotiation) with a version
number other than 0, it'll treat it as a protocol error and return the
wrong error message to the caller.  Instead, if the client's version
number is too old for the server, send back the version number they sent
us; they'll try it again and get the same error, and give up.  (This
would happen only if there's a server that doesn't support version 0;
let's not create so many protocol versions that there's a temptation to
drop older ones.)

Also, add details on protocol versioning and protocol negotiation (the
construction of which pointed out the failure mode).

diff --git a/pcap-rpcap.c b/pcap-rpcap.c
index 2687745a133de0b626fe7b7a786f35e8e5023110..d420cf3d54ed2bf831134efcfd36aac3d2f3ec99 100644 (file)
--- a/pcap-rpcap.c
+++ b/pcap-rpcap.c
@@ -1652,10 +1652,11 @@ static int rpcap_doauth(SOCKET sockctrl, uint8 *ver, struct pcap_rmtauth *auth,
        }
 
        /*
        }
 
        /*

-        * THe server doesn't support the version we used in the initial
-        * message, and it sent us back a reply with the maximum version
-        * they do support, and we also support it.  *ver has been set to
-        * that version; try authenticating again with that version.
+        * The server doesn't support the version we used in the initial
+        * message, and it sent us back a reply either with the maximum
+        * version they do support, or with the version we sent, and we
+        * support that version.  *ver has been set to that version; try
+        * authenticating again with that version.

         */
        status = rpcap_sendauth(sockctrl, ver, auth, errbuf);
        if (status == -1)
         */
        status = rpcap_sendauth(sockctrl, ver, auth, errbuf);
        if (status == -1)


@@ -1666,13 +1667,11 @@ static int rpcap_doauth(SOCKET sockctrl, uint8 *ver, struct pcap_rmtauth *auth,
        if (status == -2)
        {
                /*
        if (status == -2)
        {
                /*

-                * The server doesn't support the version
-                * it told us was the maximum version it
-                * supported, so this is a fatal error.
-                *
-                * XXX - should we overwrite the error
-                * string it gave us?
+                * The server doesn't support that version, which
+                * means there is no version we both support, so
+                * this is a fatal error.

                 */
                 */

+               pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "The server doesn't support any protocol version that we support");

                return -1;
        }
        return 0;
                return -1;
        }
        return 0;


@@ -1696,8 +1695,8 @@ static int rpcap_doauth(SOCKET sockctrl, uint8 *ver, struct pcap_rmtauth *auth,
  * is one). It could be a network problem or the fact that the authorization
  * failed.
  *
  * is one). It could be a network problem or the fact that the authorization
  * failed.
  *

- * \return '0' if everything is fine, '-2' if the server doesn't support
- * the protocol version we requested but does support a version we also
+ * \return '0' if everything is fine, '-2' if the server didn't reply with
+ * the protocol version we requested but replied with a version we do

  * support, or '-1' for other errors.  For errors, an error message string
  * is returned in the 'errbuf' variable.
  */
  * support, or '-1' for other errors.  For errors, an error message string
  * is returned in the 'errbuf' variable.
  */


@@ -1815,14 +1814,17 @@ static int rpcap_sendauth(SOCKET sock, uint8 *ver, struct pcap_rmtauth *auth, ch
                {
                        /*
                         * The server didn't support the version we sent,
                {
                        /*
                         * The server didn't support the version we sent,

-                        * and replied with the maximum version it supports.
+                        * and replied with the maximum version it supports
+                        * if our version was too big or with the version
+                        * we sent if out version was too small.
+                        *

                         * Do we also support it?
                         */
                        if (header.ver < RPCAP_MIN_VERSION ||
                            header.ver > RPCAP_MAX_VERSION)
                        {
                                /*
                         * Do we also support it?
                         */
                        if (header.ver < RPCAP_MIN_VERSION ||
                            header.ver > RPCAP_MAX_VERSION)
                        {
                                /*

-                                * No, so there's no version we support.
+                                * No, so there's no version we both support.

                                 * This is an unrecoverable error.
                                 */
                                pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "The server doesn't support any protocol version that we support");
                                 * This is an unrecoverable error.
                                 */
                                pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "The server doesn't support any protocol version that we support");

diff --git a/rpcap-protocol.h b/rpcap-protocol.h
index 48e8c821a8714afb75503c1805aa43629654abef..7523a9e44a51f56eea4c0e7a2ff819340760b073 100644 (file)
--- a/rpcap-protocol.h
+++ b/rpcap-protocol.h
@@ -41,6 +41,102 @@
 
 /*
  * Minimum and maximum supported versions of the protocol.
 
 /*
  * Minimum and maximum supported versions of the protocol.

+ *
+ * If new message types are added, the protocol version MUST be changed,
+ * so that a client knows, from the negotiated protocol version, what
+ * messages can be sent to the server.
+ *
+ * If the format of an existing message type is changed, the protocol
+ * version MUST be changed, so that each side knows, from the negotiated
+ * protocol version, what format should be used.
+ *
+ * The RPCAP_MSG_ERROR format MUST not change, as it's used to, among
+ * other things, report "incorrect version number" errors, where, if
+ * the format changed, the sender of the message might not know what
+ * versions the recipient would understand, or might know a version
+ * they support (the version number they sent) but might not know
+ * the format of the message in that version.
+ *
+ * Other message versions SHOULD not change, as that would complicate
+ * the process of interpreting the message, making it version-dependent.
+ * Introducing a new message with a new format is preferable.
+ *
+ * Version negotiation is done as part of the authentication process:
+ *
+ * The client sends an authentication request, with the version number
+ * in the request being the maximum version it supports.
+ *
+ * If the server supports that version, it attempts to authenticate the
+ * client, and replies as appropriate, with the version number in the
+ * reply being that version.
+ *
+ * If the server doesn't support that version because it's too large,
+ * it replies with a RPCAP_MSG_ERROR message, with the maximum version
+ * they support as the version number in the reply, and with the error
+ * code being PCAP_ERR_WRONGVER.
+ *
+ * If the server doesn't support that version because it's too small,
+ * it replies with a RPCAP_MSG_ERROR message, with that version as
+ * the version number in the reply, and with the error code being
+ * PCAP_ERR_WRONGVER.
+ *
+ * If the client supports that version, it retries the authentication
+ * with that version and, if that fails for any reason, including
+ * PCAP_ERR_WRONGVER, fails.  Otherwise, it fails, telling its caller
+ * that there's no version that both support.
+ *
+ * This requires that the set of versions supported by a client or
+ * server be a range of integers, with no gaps.  Thus:
+ *
+ * the client's version set is [Cmin, Cmax], with Cmin <= Cmax;
+ *
+ * the server's version set is [Smin, Smax], with Smin <= Smax;
+ *
+ * the client sends Cmax as the version number in the initial
+ * authentication request;
+ *
+ * if the server doesn't support the version sent by the client,
+ * either Smax < Cmax or Smin > Cmax (because the client sent Cmax
+ * to the server, and the server doesn't support it);
+ *
+ * if Smax < Cmax:
+ *
+ *    the server sends Smax as the version number in the RPCAP_MSG_ERROR/
+ *    PCAP_ERR_WRONGVER message - the client will accept this because
+ *    Cmax != 0, as these numbers are unsigned, and this means that
+ *    this isn't an old client that rejects all messages with a non-zero
+ *    version number, it's a new client that accepts RPCAP_MSG_ERROR
+ *    messages no matter what the version is;
+ *
+ *    if Smax >= Cmin, both the client and the server can use it, and
+ *    the client retries with Smax;
+ *
+ *    if Smax < Cmin, there is no version the client and server can
+ *    both support.
+ *
+ * if Smin > Cmax:
+ *
+ *    the server sends Cmax as the version number in the RPCAP_MSG_ERROR/
+ *    PCAP_ERR_WRONGVER message - the client will accept this because
+ *    Cmax is a valid client version number.
+ *
+ *    the client will retry with Cmax, get the same version failure,
+ *    and report that there is no version the client and server can
+ *    both support (as the version sets are disjoint).
+ *
+ * Old negotiation-unaware clients just send version 0 and, if they
+ * get back PCAP_ERR_WRONGVER, treat it as a fatal error.  This
+ * means they'll fail to talk to any server that can't handle
+ * version 0, which is the appropriate thing to do, as they can
+ * only use version 0.
+ *
+ * Old negotiation-unaware servers fail if they get a version other
+ * than 0, sending back PCAP_ERR_WRONGVER with version 0, which is
+ * the only version, and thus both the minimum and maximum version,
+ * they support.  The client will either fail if it doesn't support
+ * version 0, or will retry with version 0 and succeed, so it will
+ * fail with servers that can't handle version 0 or will negotiate
+ * version 0 with servers that can handle version 0.

  */
 #define RPCAP_MIN_VERSION 0
 #define RPCAP_MAX_VERSION 0
  */
 #define RPCAP_MIN_VERSION 0
 #define RPCAP_MAX_VERSION 0

diff --git a/rpcapd/daemon.c b/rpcapd/daemon.c
index 96540f9b4203985201f3197dbd65047fa9f4e675..71b88ab20b039c2ada577c29d59d8b46644bc46f 100755 (executable)
--- a/rpcapd/daemon.c
+++ b/rpcapd/daemon.c
@@ -210,15 +210,35 @@ void daemon_serviceloop(void *ptr)
                {
                        //
                        // Tell them it's not a valid protocol version.
                {
                        //
                        // Tell them it's not a valid protocol version.

-                       // Send our maximum supported version as the
-                       // version in the message.

                        //
                        //

-                       // XXX - if we ever refuse to support version
-                       // 0, for which older clients only handled
-                       // version 0 in error replies, will this cause
-                       // a problem?
-                       //
-                       if (rpcap_senderror(pars->sockctrl, RPCAP_MIN_VERSION,
+                       uint8 reply_version;
+
+                       if (header.ver < RPCAP_MIN_VERSION)
+                       {
+                               //
+                               // Their maximum version is too old;
+                               // there *is* no version we can both
+                               // handle, and they might reject
+                               // an error with a version they don't
+                               // understand, so reply with the
+                               // version they sent.  That may
+                               // make them retry with that version,
+                               // but they'll give up on that
+                               // failure.
+                               //
+                               reply_version = header.ver;
+                       }
+                       else
+                       {
+                               //
+                               // Their maximum version is too new,
+                               // but they might be able to handle
+                               // *our* maximum version, so reply
+                               // with that version.
+                               // 
+                               reply_version = RPCAP_MAX_VERSION;
+                       }
+                       if (rpcap_senderror(pars->sockctrl, reply_version,

                            PCAP_ERR_WRONGVER, "RPCAP version number mismatch",
                            errbuf) == -1)
                        {
                            PCAP_ERR_WRONGVER, "RPCAP version number mismatch",
                            errbuf) == -1)
                        {