address review comments

project-chip · Sep 24, 2021 · 6dc0d2a · 6dc0d2a
1 parent 0d4b2b9
commit 6dc0d2a
Show file tree

Hide file tree

Showing 8 changed files with 160 additions and 108 deletions.
diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h
@@ -2181,6 +2181,15 @@ using CHIP_ERROR = ::chip::ChipError;
  */
 #define CHIP_ERROR_MESSAGE_COUNTER_OUT_OF_WINDOW               CHIP_CORE_ERROR(0xc7)
 
+/**
+ * @def CHIP_ERROR_NO_SHARED_TRUSTED_ROOT
+ *
+ * @brief
+ *   The CASE session could not be established as peer's credentials do not have
+ *   a common root of trust.
+ */
+#define CHIP_ERROR_NO_SHARED_TRUSTED_ROOT                      CHIP_CORE_ERROR(0xc8)
+
 /**
  *  @}
  */

diff --git a/src/messaging/ApplicationExchangeDispatch.cpp b/src/messaging/ApplicationExchangeDispatch.cpp
@@ -56,7 +56,7 @@ bool ApplicationExchangeDispatch::MessagePermitted(uint16_t protocol, uint8_t ty
         case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma1):
         case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma2):
         case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma3):
-        case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_SigmaErr):
+        case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma2Resume):
             return false;
 
         default:

diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp
diff --git a/src/protocols/secure_channel/CASESession.h b/src/protocols/secure_channel/CASESession.h
@@ -65,13 +65,14 @@ struct CASESessionSerialized;
 
 struct CASESessionSerializable
 {
+    uint8_t mVersion;
+    uint8_t mPairingComplete;
     uint16_t mSharedSecretLen;
     uint8_t mSharedSecret[Crypto::kMax_ECDH_Secret_Length];
     uint16_t mMessageDigestLen;
     uint8_t mMessageDigest[Crypto::kSHA256_Hash_Length];
     uint16_t mIPKLen;
     uint8_t mIPK[kIPKSize];
-    uint8_t mPairingComplete;
     NodeId mPeerNodeId;
     uint16_t mLocalSessionId;
     uint16_t mPeerSessionId;
@@ -120,7 +121,7 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
                                 SessionEstablishmentDelegate * delegate);
 
     /**
-     *   Parse the message to check if it is a valid session resumption request.
+     *   Parse the message to check if it has a session resumption request.
      *   A valid session resumption request must have Resumption ID, and InitiationResumeMIC.
      *
      *   If the message is a valid session resumption request, the output parameter resumptionRequested is set to true,
@@ -132,8 +133,8 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
      *   If the message doesn't contain either Resumption ID or InitiationResumeMIC (i.e. contains only one of these fields), the
      *   function returns CHIP_ERROR_INVALID_ARGUMENT.
      */
-    static CHIP_ERROR ParseSessionResumptionRequest(const System::PacketBufferHandle & message, bool & resumptionRequested,
-                                                    MutableByteSpan & resumptionID, MutableByteSpan & resume1MIC);
+    static CHIP_ERROR IsResumptionRequestPresent(const System::PacketBufferHandle & message, bool & resumptionRequested,
+                                                 ByteSpan & resumptionID, ByteSpan & resume1MIC);
 
     /**
      * @brief
@@ -273,6 +274,7 @@ class DLL_EXPORT CASESession : public Messaging::ExchangeDelegate, public Pairin
     Transport::FabricInfo * mFabricInfo    = nullptr;
 
     uint8_t mResumptionId[kCASEResumptionIDSize];
+    // Sigma1 initiator random, maintained to be reused post-Sigma1, such as when generating Sigma2 S2RK key
     uint8_t mInitiatorRandom[kSigmaParamRandomNumberSize];
 
     State mState;

diff --git a/src/protocols/secure_channel/Constants.h b/src/protocols/secure_channel/Constants.h
@@ -66,9 +66,7 @@ enum class MsgType : uint8_t
     CASE_Sigma1       = 0x30,
     CASE_Sigma2       = 0x31,
     CASE_Sigma3       = 0x32,
-    CASE_Sigma1Resume = 0x33,
-    CASE_Sigma2Resume = 0x34,
-    CASE_SigmaErr     = 0x3F,
+    CASE_Sigma2Resume = 0x33,
 
     StatusReport = 0x40,
 };

diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp
@@ -811,7 +811,7 @@ CHIP_ERROR PASESession::OnFailureStatusReport(Protocols::SecureChannel::GeneralS
         err = CHIP_ERROR_INTERNAL;
         break;
     };
-    ChipLogError(SecureChannel, "Received error (protocol code %d) during pairing process. %s", protocolCode, ErrorStr(err));
+    ChipLogError(SecureChannel, "Received error (protocol code %d) during PASE process. %s", protocolCode, ErrorStr(err));
     return err;
 }
 
@@ -872,7 +872,7 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl
         break;
 
     case MsgType::StatusReport:
-        err = HandleStatusReport(std::move(msg));
+        err = HandleStatusReport(std::move(msg), mNextExpectedMsg == MsgType::StatusReport);
         break;
 
     default:

diff --git a/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.cpp b/src/protocols/secure_channel/SessionEstablishmentExchangeDispatch.cpp
@@ -66,7 +66,7 @@ bool SessionEstablishmentExchangeDispatch::MessagePermitted(uint16_t protocol, u
         case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma1):
         case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma2):
         case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma3):
-        case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_SigmaErr):
+        case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::CASE_Sigma2Resume):
         case static_cast<uint8_t>(Protocols::SecureChannel::MsgType::StatusReport):
             return true;
 

diff --git a/src/transport/PairingSession.h b/src/transport/PairingSession.h
@@ -122,7 +122,7 @@ class DLL_EXPORT PairingSession
         }
     }
 
-    CHIP_ERROR HandleStatusReport(System::PacketBufferHandle && msg)
+    CHIP_ERROR HandleStatusReport(System::PacketBufferHandle && msg, bool successExpected)
     {
         Protocols::SecureChannel::StatusReport report;
         CHIP_ERROR err = report.Parse(std::move(msg));
@@ -131,7 +131,7 @@ class DLL_EXPORT PairingSession
                             CHIP_ERROR_INVALID_ARGUMENT);
 
         if (report.GetGeneralCode() == Protocols::SecureChannel::GeneralStatusCode::kSuccess &&
-            report.GetProtocolCode() == Protocols::SecureChannel::kProtocolCodeSuccess)
+            report.GetProtocolCode() == Protocols::SecureChannel::kProtocolCodeSuccess && successExpected)
         {
             OnSuccessStatusReport();
         }