[ble][tcat] improve error handling for BLE packets and TCAT TLVs (#12029)

Updates in error handling to ensure that recoverable BLE errors are
logged, failures don't lead to partial TLVs being generated or
processed, and internal allocation failures lead to a GeneralError
status response back to the TCAT Commissioner. In case of likely
unrecoverable TLV damage, or cases where the GeneralError status
response even couldn't be generated, the secure TLS connection is
closed (Disconnect).

BLE ATT MTU handling is updated to use default MTU 23 if fetching the
MTU fails; and more clear check on min MTU. If the platform reports a
very high MTU (above max) this is not seen as an error, but BleSecure
will simply uses the highest usable MTU that it has configured,
whether the platform's report is correct or not.

Close #11536
This commit is contained in:
Esko Dijk
2025-11-22 08:41:40 -05:00
committed by GitHub
parent b83f7b5813
commit 4f38928041
6 changed files with 114 additions and 64 deletions
+1 -1
View File
@@ -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
+15 -3
View File
@@ -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);
+8 -2
View File
@@ -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.
+18 -10
View File
@@ -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<ResponseWithStatusTlv>(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<ResponseWithStatusTlv>(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;
}
+52 -38
View File
@@ -284,9 +284,10 @@ Error BleSecure::SendApplicationTlv(MeshCoP::TcatAgent::TcatApplicationProtocol
{
Error error = kErrorNone;
VerifyOrExit((aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolStatus &&
aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolResponse) ||
Get<MeshCoP::TcatAgent>().GetApplicationResponsePending(),
VerifyOrExit(aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolNone &&
((aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolStatus &&
aTcatApplicationProtocol != MeshCoP::TcatAgent::kApplicationProtocolResponse) ||
Get<MeshCoP::TcatAgent>().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<MessagePool>().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<uint16_t>(missingBytes)));
SuccessOrExit(error = mReceivedMessage->AppendBytes(aBuf, static_cast<uint16_t>(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<MeshCoP::TcatAgent>().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<MeshCoP::TcatAgent>().HandleSingleTlv(*mReceivedMessage, *mSendMessage);
IgnoreError(Flush());
errorTcatAgent = Get<MeshCoP::TcatAgent>().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<uint16_t>(offset));
mReceiveCallback.InvokeIfSet(&GetInstance(), mReceivedMessage, static_cast<int32_t>(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<Ble::BleSecure>().HandleBleReceive(aPacket->mValue, aPacket->mLength));
AsCoreType(aInstance).Get<Ble::BleSecure>().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<Ble::BleSecure>().HandleBleMtuUpdate(aMtu));
AsCoreType(aInstance).Get<Ble::BleSecure>().HandleBleMtuUpdate(aMtu);
}
#endif // OPENTHREAD_CONFIG_BLE_TCAT_ENABLE
+20 -10
View File
@@ -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<MeshCoP::TcatAgent>().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(..)