diff --git a/include/openthread/instance.h b/include/openthread/instance.h index df1920b5e..0e92d9387 100644 --- a/include/openthread/instance.h +++ b/include/openthread/instance.h @@ -52,7 +52,7 @@ extern "C" { * * @note This number versions both OpenThread platform and user APIs. */ -#define OPENTHREAD_API_VERSION (552) +#define OPENTHREAD_API_VERSION (553) /** * @addtogroup api-instance diff --git a/include/openthread/platform/ble.h b/include/openthread/platform/ble.h index a37aa078e..1285fa1a1 100644 --- a/include/openthread/platform/ble.h +++ b/include/openthread/platform/ble.h @@ -86,11 +86,23 @@ extern "C" { #define OT_BLE_ADV_INTERVAL_UNIT OT_BLE_TIMESLOT_UNIT /** - * Maximum allowed ATT MTU size (must be >= 23). + * Minimum allowed ATT MTU size (MUST be 23). + */ + +#define OT_BLE_ATT_MTU_MIN 23 + +/** + * Maximum allowed ATT MTU size (MUST be >= OT_BLE_ATT_MTU_MIN). */ #define OT_BLE_ATT_MTU_MAX 67 +/** + * Default ATT MTU size (used if no MTU indication is given yet, or none can be given). It MUST be 23. + */ + +#define OT_BLE_ATT_MTU_DEFAULT 23 + /** * Default power value for BLE. */ @@ -274,7 +286,7 @@ otError otPlatBleGapDisconnect(otInstance *aInstance); *******************************************************************************/ /** - * Reads currently use value of ATT_MTU. + * Reads currently used value of ATT_MTU from the platform BLE driver. * * @param[in] aInstance The OpenThread instance structure. * @param[out] aMtu A pointer to output the current ATT_MTU value. @@ -288,7 +300,7 @@ otError otPlatBleGattMtuGet(otInstance *aInstance, uint16_t *aMtu); * The BLE driver calls this method to notify OpenThread that ATT_MTU has been updated. * * @param[in] aInstance The OpenThread instance structure. - * @param[in] aMtu The updated ATT_MTU value. + * @param[in] aMtu The updated ATT_MTU value. It MUST be >=OT_BLE_ATT_MTU_MIN. */ extern void otPlatBleGattOnMtuUpdate(otInstance *aInstance, uint16_t aMtu); diff --git a/src/core/meshcop/secure_transport.hpp b/src/core/meshcop/secure_transport.hpp index bc868bc78..ee7afff89 100644 --- a/src/core/meshcop/secure_transport.hpp +++ b/src/core/meshcop/secure_transport.hpp @@ -326,11 +326,17 @@ public: static constexpr uint8_t kPskMaxLength = 32; ///< Maximum PSK length. /** - * Pointer is called to send encrypted message. + * Pointer to function that is called to send an encrypted message. * * @param[in] aContext A pointer to arbitrary context information. * @param[in] aMessage A reference to the message to send. * @param[in] aMessageInfo A reference to the message info associated with @p aMessage. + * + * @retval kErrorNone Successfully sent message. + * @retval kErrorNoBufs Message not sent: signal to SecureTransport that the sending operation blocks. + * In this case, the stack will retry the sending later. + * @retval kErrorFailed Failure to send, for other reasons. Note that any other errors not listed here + * will map to kErrorFailed. */ typedef Error (*TransportCallback)(void *aContext, ot::Message &aMessage, const Ip6::MessageInfo &aMessageInfo); @@ -351,7 +357,7 @@ public: * * `nullptr` can be returned to reject the new session connection request. * - * @param[in] aContex A pointer to arbitrary context information. + * @param[in] aContext A pointer to arbitrary context information. * @param[in] aMessageInfo The message info from the new session connection request message. * * @returns A pointer to `SecureSession` to use for new session or `nullptr` if new connection is rejected. diff --git a/src/core/meshcop/tcat_agent.cpp b/src/core/meshcop/tcat_agent.cpp index 3064d8b2c..4b522e73f 100644 --- a/src/core/meshcop/tcat_agent.cpp +++ b/src/core/meshcop/tcat_agent.cpp @@ -398,11 +398,13 @@ bool TcatAgent::IsCommandClassAuthorized(CommandClass aCommandClass) const Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutgoingMessage) { - Error error = kErrorParse; - ot::Tlv tlv; - uint16_t offset = aIncomingMessage.GetOffset(); - uint16_t length; - bool response = false; + Error error; + StatusCode statusCode = kStatusGeneralError; + ot::Tlv tlv; + uint16_t offset = aIncomingMessage.GetOffset(); + const uint16_t initialOutgoingMsgLength = aOutgoingMessage.GetLength(); + uint16_t length; + bool response = false; VerifyOrExit(IsConnected(), error = kErrorInvalidState); SuccessOrExit(error = aIncomingMessage.Read(offset, tlv)); @@ -514,8 +516,6 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg if (!response) { - StatusCode statusCode; - switch (error) { case kErrorNone: @@ -557,14 +557,21 @@ Error TcatAgent::HandleSingleTlv(const Message &aIncomingMessage, Message &aOutg break; default: - statusCode = kStatusGeneralError; + // remains kStatusGeneralError break; } - - SuccessOrExit(error = ot::Tlv::Append(aOutgoingMessage, statusCode)); + error = kErrorNone; // reset the error - as it's now converted to statusCode } exit: + if (!response && error == kErrorNone) // skip initial-check error cases, where no TLV response must be sent. + { + // reset any partial TLV content that may have been appended already by failed Handle...() methods. + IgnoreError(aOutgoingMessage.SetLength(initialOutgoingMsgLength)); + // Append a single Response with Status TLV to the response message; and ensure to only + // return error != kErrorNone if there was an issue appending this TLV. + error = ot::Tlv::Append(aOutgoingMessage, statusCode); + } return error; } @@ -887,6 +894,7 @@ Error TcatAgent::HandleRequestRandomNumberChallenge(Message &aOutgoingMessage, b SuccessOrExit( error = Tlv::AppendTlv(aOutgoingMessage, kTlvResponseWithPayload, &mRandomChallenge, sizeof(mRandomChallenge))); aResponse = true; + exit: return error; } diff --git a/src/core/radio/ble_secure.cpp b/src/core/radio/ble_secure.cpp index 3842a6b71..ced802e1b 100644 --- a/src/core/radio/ble_secure.cpp +++ b/src/core/radio/ble_secure.cpp @@ -284,9 +284,10 @@ Error BleSecure::SendApplicationTlv(MeshCoP::TcatAgent::TcatApplicationProtocol { Error error = kErrorNone; - VerifyOrExit((aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolStatus && - aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolResponse) || - Get().GetApplicationResponsePending(), + VerifyOrExit(aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolNone && + ((aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolStatus && + aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolResponse) || + Get().GetApplicationResponsePending()), error = kErrorRejected); if (aLength > Tlv::kBaseTlvMaxLength) @@ -339,7 +340,7 @@ Error BleSecure::Flush(void) length -= kTlsDataMaxSize; // Should never fail since we are decreasing the length of the message - SuccessOrAssert(error = mSendMessage->SetLength(length)); + IgnoreError(mSendMessage->SetLength(length)); mTransmitQueue.Enqueue(*message); mTransmitTask.Post(); message = nullptr; @@ -362,7 +363,7 @@ exit: return error; } -Error BleSecure::HandleBleReceive(uint8_t *aBuf, uint16_t aLength) +void BleSecure::HandleBleReceive(uint8_t *aBuf, uint16_t aLength) { ot::Message *message = nullptr; Ip6::MessageInfo messageInfo; @@ -370,8 +371,7 @@ Error BleSecure::HandleBleReceive(uint8_t *aBuf, uint16_t aLength) if ((message = Get().Allocate(Message::kTypeBle, 0)) == nullptr) { - error = kErrorNoBufs; - ExitNow(); + ExitNow(error = kErrorNoBufs); } SuccessOrExit(error = message->AppendBytes(aBuf, aLength)); @@ -379,18 +379,19 @@ Error BleSecure::HandleBleReceive(uint8_t *aBuf, uint16_t aLength) mTls.HandleReceive(*message, messageInfo); exit: + // if BLE packets go missing, the TLS layer will catch the damaged records - so we just warn here. + LogWarnOnError(error, "HandleBleReceive"); FreeMessage(message); - return error; } void BleSecure::HandleBleConnected(uint16_t aConnectionId) { OT_UNUSED_VARIABLE(aConnectionId); - mBleState = kConnected; - + // if getting ATT MTU size fails, it stays at the default + mMtuSize = kInitialMtuSize; IgnoreError(otPlatBleGattMtuGet(&GetInstance(), &mMtuSize)); - + mBleState = kConnected; mConnectCallback.InvokeIfSet(&GetInstance(), IsConnected(), true); } @@ -402,28 +403,22 @@ void BleSecure::HandleBleDisconnected(uint16_t aConnectionId) mBleState = kAdvertising; mMtuSize = kInitialMtuSize; - Disconnect(); // Stop TLS connection + Disconnect(); // Stop TLS connection and update advertisement data // if a different BLE advertising state was requested earlier while a BLE client was connected, // then now's the time to fulfill the request. IgnoreError(SetRequestedBleAdvertisementsState()); } -Error BleSecure::HandleBleMtuUpdate(uint16_t aMtu) +void BleSecure::HandleBleMtuUpdate(uint16_t aMtu) { - Error error = kErrorNone; - - if (aMtu <= OT_BLE_ATT_MTU_MAX) + OT_ASSERT(aMtu >= kMinMtuSize); + // if higher MTU is available than configured, we still obey our configured maximum. + if (aMtu > kMaxMtuSize) { - mMtuSize = aMtu; + aMtu = kMaxMtuSize; } - else - { - mMtuSize = OT_BLE_ATT_MTU_MAX; - error = kErrorInvalidArgs; - } - - return error; + mMtuSize = aMtu; } void BleSecure::HandleTlsConnectEvent(MeshCoP::Tls::ConnectEvent aEvent, void *aContext) @@ -479,12 +474,14 @@ void BleSecure::HandleTlsReceive(void *aContext, uint8_t *aBuf, uint16_t aLength void BleSecure::HandleTlsReceive(uint8_t *aBuf, uint16_t aLength) { + otError error = kErrorNone; + VerifyOrExit(mReceivedMessage != nullptr); DumpDebg("Rx", aBuf, aLength); if (!mTlvMode) { - SuccessOrExit(mReceivedMessage->AppendBytes(aBuf, aLength)); + SuccessOrExit(error = mReceivedMessage->AppendBytes(aBuf, aLength)); mReceiveCallback.InvokeIfSet(&GetInstance(), mReceivedMessage, 0, OT_TCAT_APPLICATION_PROTOCOL_NONE); IgnoreError(mReceivedMessage->SetLength(0)); } @@ -502,12 +499,12 @@ void BleSecure::HandleTlsReceive(uint8_t *aBuf, uint16_t aLength) if (missingBytes > aLength) { - SuccessOrExit(mReceivedMessage->AppendBytes(aBuf, aLength)); + SuccessOrExit(error = mReceivedMessage->AppendBytes(aBuf, aLength)); break; } else { - SuccessOrExit(mReceivedMessage->AppendBytes(aBuf, static_cast(missingBytes))); + SuccessOrExit(error = mReceivedMessage->AppendBytes(aBuf, static_cast(missingBytes))); aLength -= missingBytes; aBuf += missingBytes; } @@ -543,7 +540,7 @@ void BleSecure::HandleTlsReceive(uint8_t *aBuf, uint16_t aLength) // TLV fully loaded - let TCAT agent handle it, if connected if (Get().IsConnected()) { - Error error = kErrorNone; + Error errorTcatAgent = kErrorNone; IgnoreError(Flush()); @@ -553,33 +550,50 @@ void BleSecure::HandleTlsReceive(uint8_t *aBuf, uint16_t aLength) VerifyOrExit(mSendMessage != nullptr, error = kErrorNoBufs); } - error = Get().HandleSingleTlv(*mReceivedMessage, *mSendMessage); - IgnoreError(Flush()); + errorTcatAgent = Get().HandleSingleTlv(*mReceivedMessage, *mSendMessage); - if (error == kErrorAbort) + if (errorTcatAgent != kErrorNone) { - LogInfo("Disconnecting TCAT client."); // kErrorAbort indicates that a Disconnect command TLV has been received. + if (errorTcatAgent == kErrorAbort) + { + LogInfo("Disconnecting TCAT client."); + } + else + { + // fatal error: TLV integrity and order can't be guaranteed anymore for this connection. + LogWarnOnError(errorTcatAgent, "HandleSingleTlv"); + } Disconnect(); // BleSecure is not stopped here, it must remain active in advertising state and // must be ready to receive a next TCAT commissioner. ExitNow(); } + SuccessOrExit(error = Flush()); // send out the response generated by the TcatAgent } else { + // if the TCAT agent is not connected - do callback using the TLV's value as the message mReceivedMessage->SetOffset(static_cast(offset)); mReceiveCallback.InvokeIfSet(&GetInstance(), mReceivedMessage, static_cast(offset), OT_TCAT_APPLICATION_PROTOCOL_NONE); } - SuccessOrExit(mReceivedMessage->SetLength(0)); // also sets the offset to 0 - requiredBytes = sizeof(Tlv); - } + IgnoreError(mReceivedMessage->SetLength(0)); // also sets the offset to 0 + requiredBytes = sizeof(Tlv); // set for the next TLV + } // while (aLength > 0) } exit: - return; + if (error != kErrorNone) + { + // in this very rare case, a partial TLV is received, or a TLV has been fully dropped, or Flush() failed. + // mSendMessage is most likely not initialized; so appending a GeneralError status TLV to mSendMessage would + // fail also. In this case it's not possible to recover TLV integrity and client/server sync. + // It's handled by logging the error and (necessarily) closing the secure connection. + LogCrit("HandleTlsReceive: %s", ErrorToString(error)); + Disconnect(); + } } void BleSecure::HandleTransmit(void) @@ -664,7 +678,7 @@ void otPlatBleGattServerOnWriteRequest(otInstance *aInstance, uint16_t aHandle, OT_UNUSED_VARIABLE(aHandle); // Only a single handle is expected for RX VerifyOrExit(aPacket != nullptr); - IgnoreError(AsCoreType(aInstance).Get().HandleBleReceive(aPacket->mValue, aPacket->mLength)); + AsCoreType(aInstance).Get().HandleBleReceive(aPacket->mValue, aPacket->mLength); exit: return; } @@ -681,7 +695,7 @@ void otPlatBleGapOnDisconnected(otInstance *aInstance, uint16_t aConnectionId) void otPlatBleGattOnMtuUpdate(otInstance *aInstance, uint16_t aMtu) { - IgnoreError(AsCoreType(aInstance).Get().HandleBleMtuUpdate(aMtu)); + AsCoreType(aInstance).Get().HandleBleMtuUpdate(aMtu); } #endif // OPENTHREAD_CONFIG_BLE_TCAT_ENABLE diff --git a/src/core/radio/ble_secure.hpp b/src/core/radio/ble_secure.hpp index 6d7872d0a..6cc59dcae 100644 --- a/src/core/radio/ble_secure.hpp +++ b/src/core/radio/ble_secure.hpp @@ -154,7 +154,12 @@ public: /** * Initializes TLS session with a peer using an already open BLE connection. * - * @retval kErrorNone Successfully started TLS connection. + * @retval kErrorNone Successfully started TLS connection. + * @retval kErrorSecurity TLS security error during session establishment. + * @retval kErrorInvalidState BLE link is not active. + * @retval kErrorNoBufs Failure in buffer memory allocation. + * @retval kErrorInvalidArgs Internal TLS stack error. + * @retval kErrorFailed General TLS error. */ Error Connect(void); @@ -278,7 +283,7 @@ public: * @param[in] aBuf A pointer to the data received. * @param[in] aLength A number indicating the length of the data buffer. */ - Error HandleBleReceive(uint8_t *aBuf, uint16_t aLength); + void HandleBleReceive(uint8_t *aBuf, uint16_t aLength); /** * Used to notify the secure BLE server that a BLE Device has been connected. @@ -297,15 +302,16 @@ public: /** * Used to notify the secure BLE server that the BLE Device has updated ATT_MTU size. * - * @param[in] aMtu The updated ATT_MTU value. + * @param[in] aMtu The updated ATT_MTU value. Note that values above OT_BLE_ATT_MTU_MAX + * will be clamped to OT_BLE_ATT_MTU_MAX. */ - Error HandleBleMtuUpdate(uint16_t aMtu); + void HandleBleMtuUpdate(uint16_t aMtu); /** * @brief Gets the Install Code Verify Status during the current session. * - * @return TRUE The install code was correctly verified. - * @return FALSE The install code was not verified. + * @retval TRUE The install code was correctly verified. + * @retval FALSE The install code was not verified. */ bool GetInstallCodeVerifyStatus(void) const { return Get().GetInstallCodeVerifyStatus(); } @@ -313,8 +319,10 @@ public: * @brief Notifies the BLE layer that the TCAT advertisement data was changed, so * BLE advertisement message content should be updated. * - * @retval kErrorNone Successfully updated using the new data. - * @return kErrorFailed Update failed. + * @retval kErrorNone Successfully updated using the new data. + * @retval kErrorNoBufs Updated failed due to buffer allocation failure. + * @retval kErrorInvalidArgs Updated failed due to invalid advertisement data. + * @retval kErrorFailed Update failed. */ Error NotifyAdvertisementChanged(); @@ -338,8 +346,10 @@ private: kNotAdvertising = 3, // Ble secure is started but not advertising. }; - static constexpr uint8_t kInitialMtuSize = 23; // ATT_MTU - static constexpr uint8_t kGattOverhead = 3; // BLE GATT payload fits MTU size - 3 bytes + static constexpr uint8_t kInitialMtuSize = OT_BLE_ATT_MTU_DEFAULT; + static constexpr uint8_t kMinMtuSize = OT_BLE_ATT_MTU_MIN; + static constexpr uint8_t kMaxMtuSize = OT_BLE_ATT_MTU_MAX; + static constexpr uint8_t kGattOverhead = 3; // BLE GATT payload fits (MTU size - 3 bytes) static constexpr uint8_t kPacketBufferSize = OT_BLE_ATT_MTU_MAX - kGattOverhead; static constexpr uint16_t kTxBleHandle = 0; // Characteristics Handle for TX (not used) static constexpr uint16_t kTlsDataMaxSize = 800; // Maximum size of data chunks sent with mTls.Send(..)