Fix RSA-AES state machines

If there isn't enough data, then the client reading the hash will fall
down and try to read the subtype instead, which isn't correct.

Invert the logic so we get a more consistent way through where we only
break out when there is insufficient data.

Do the same for the server code, for consistency.

diff --git a/common/rfb/CSecurityRSAAES.cxx b/common/rfb/CSecurityRSAAES.cxx
index db6e43ad10973acdb113f0be8de3d81eb5b001d6..208bcc2f4e876639d21d25a4c289c4d3d57d8784 100644 (file)
--- a/common/rfb/CSecurityRSAAES.cxx
+++ b/common/rfb/CSecurityRSAAES.cxx
@@ -100,31 +100,28 @@ bool CSecurityRSAAES::processMsg()
 {
   switch (state) {
     case ReadPublicKey:
-      if (readPublicKey()) {
-        verifyServer();
-        writePublicKey();
-        writeRandom();
-        state = ReadRandom;
-      }
-      return false;
+      if (!readPublicKey())
+        return false;
+      verifyServer();
+      writePublicKey();
+      writeRandom();
+      state = ReadRandom;
     case ReadRandom:
-      if (readRandom()) {
-        setCipher();
-        writeHash();
-        state = ReadHash;
-      }
-      return false;
+      if (!readRandom())
+        return false;
+      setCipher();
+      writeHash();
+      state = ReadHash;
     case ReadHash:
-      if (readHash()) {
-        clearSecrets();
-        state = ReadSubtype;
-      }
+      if (!readHash())
+        return false;
+      clearSecrets();
+      state = ReadSubtype;
     case ReadSubtype:
-      if (readSubtype()) {
-        writeCredentials();
-        return true;
-      }
-      return false;
+      if (!readSubtype())
+        return false;
+      writeCredentials();
+      return true;
   }
   assert(!"unreachable");
   return false;

diff --git a/common/rfb/SSecurityRSAAES.cxx b/common/rfb/SSecurityRSAAES.cxx
index 15d2e97b1d82c5403181f6b74484fafb5daa0125..3211f12f82ca50bf1559ace3b40302b7541649f2 100644 (file)
--- a/common/rfb/SSecurityRSAAES.cxx
+++ b/common/rfb/SSecurityRSAAES.cxx
@@ -238,36 +238,35 @@ bool SSecurityRSAAES::processMsg()
       loadPrivateKey();
       writePublicKey();
       state = ReadPublicKey;
-      // fall through
+      /* fall through */
     case ReadPublicKey:
-      if (readPublicKey()) {
-        writeRandom();
-        state = ReadRandom;
-      }
-      return false;
+      if (!readPublicKey())
+        return false;
+      writeRandom();
+      state = ReadRandom;
+      /* fall through */
     case ReadRandom:
-      if (readRandom()) {
-        setCipher();
-        writeHash();
-        state = ReadHash;
-      }
-      return false;
+      if (!readRandom())
+        return false;
+      setCipher();
+      writeHash();
+      state = ReadHash;
+      /* fall through */
     case ReadHash:
-      if (readHash()) {
-        clearSecrets();
-        writeSubtype();
-        state = ReadCredentials;
-      }
-      return false;
+      if (!readHash())
+        return false;
+      clearSecrets();
+      writeSubtype();
+      state = ReadCredentials;
+      /* fall through */
     case ReadCredentials:
-      if (readCredentials()) {
-        if (requireUsername)
-          verifyUserPass();
-        else
-          verifyPass();
-        return true;
-      }
-      return false;
+      if (!readCredentials())
+        return false;
+      if (requireUsername)
+        verifyUserPass();
+      else
+        verifyPass();
+      return true;
   }
   assert(!"unreachable");
   return false;