From bbf807d630f0c7e21284259d965c9282ba31bdb8 Mon Sep 17 00:00:00 2001 From: Joey Grover Date: Tue, 23 Mar 2021 13:22:10 -0400 Subject: [PATCH 1/4] Fix sending encrypted packets Fixes both multiframe and single frame where single frame was actually over TLS max data size --- .../protocol/SdlProtocolBase.java | 103 +++++++++++------- 1 file changed, 64 insertions(+), 39 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java b/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java index d9e4375488..3d9a1dc21b 100644 --- a/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java +++ b/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java @@ -71,6 +71,10 @@ public class SdlProtocolBase { private final static String FailurePropagating_Msg = "Failure propagating "; private static final int TLS_MAX_RECORD_SIZE = 16384; + private final static int TLS_RECORD_HEADER_SIZE = 5; + private final static int TLS_RECORD_MES_AUTH_CDE_SIZE = 32; + private final static int TLS_MAX_RECORD_PADDING_SIZE = 256; + private final static int TLS_MAX_DATA_TO_ENCRYPT_SIZE = TLS_MAX_RECORD_SIZE - TLS_RECORD_HEADER_SIZE - TLS_RECORD_MES_AUTH_CDE_SIZE - TLS_MAX_RECORD_PADDING_SIZE; private static final int PRIMARY_TRANSPORT_ID = 1; private static final int SECONDARY_TRANSPORT_ID = 2; @@ -561,7 +565,8 @@ public void endSession(byte sessionID) { public void sendMessage(ProtocolMessage protocolMsg) { SessionType sessionType = protocolMsg.getSessionType(); byte sessionID = protocolMsg.getSessionID(); - + boolean requiresEncryption = protocolMsg.getPayloadProtected(); + SdlSecurityBase sdlSec = null; byte[] data; if (protocolVersion.getMajor() > 1 && sessionType != SessionType.NAV && sessionType != SessionType.PCM) { if (sessionType.eq(SessionType.CONTROL)) { @@ -590,21 +595,15 @@ public void sendMessage(ProtocolMessage protocolMsg) { data = protocolMsg.getData(); } - if (iSdlProtocol != null && protocolMsg.getPayloadProtected()) { - - if (data != null && data.length > 0) { - byte[] dataToRead = new byte[TLS_MAX_RECORD_SIZE]; - SdlSecurityBase sdlSec = iSdlProtocol.getSdlSecurity(); - if (sdlSec == null) - return; - - Integer iNumBytes = sdlSec.encryptData(data, dataToRead); - if ((iNumBytes == null) || (iNumBytes <= 0)) - return; - - byte[] encryptedData = new byte[iNumBytes]; - System.arraycopy(dataToRead, 0, encryptedData, 0, iNumBytes); - data = encryptedData; + if (requiresEncryption) { + if (iSdlProtocol == null) { + DebugTool.logError(TAG, "Unable to encrypt packet, protocol callback was null"); + return; + } + sdlSec = iSdlProtocol.getSdlSecurity(); + if (sdlSec == null) { + DebugTool.logError(TAG, "Unable to encrypt packet, security library not found"); + return; } } @@ -616,24 +615,25 @@ public void sendMessage(ProtocolMessage protocolMsg) { return; } - synchronized (messageLock) { - if (data != null && data.length > getMtu(sessionType)) { + //Set the MTU according to session MTU provided by the IVI or the max data size for encryption if required + final Long mtu = requiresEncryption ? TLS_MAX_DATA_TO_ENCRYPT_SIZE : getMtu(sessionType); + synchronized (messageLock) { + if (data != null && data.length > mtu) { + //Since the packet is larger than the mtu size, it will be sent as a multi-frame packet messageID++; // Assemble first frame. - Long mtu = getMtu(sessionType); - int frameCount = Long.valueOf(data.length / mtu).intValue(); - if (data.length % mtu > 0) { - frameCount++; - } + int frameCount = (int) Math.ceil(data.length / (double) mtu); + byte[] firstFrameData = new byte[8]; // First four bytes are data size. System.arraycopy(BitConverter.intToByteArray(data.length), 0, firstFrameData, 0, 4); // Second four bytes are frame count. System.arraycopy(BitConverter.intToByteArray(frameCount), 0, firstFrameData, 4, 4); - SdlPacket firstHeader = SdlPacketFactory.createMultiSendDataFirst(sessionType, sessionID, messageID, (byte) protocolVersion.getMajor(), firstFrameData, protocolMsg.getPayloadProtected()); + //NOTE: First frames cannot be encrypted because their payloads need to be exactly 8 bytes + SdlPacket firstHeader = SdlPacketFactory.createMultiSendDataFirst(sessionType, sessionID, messageID, (byte) protocolVersion.getMajor(), firstFrameData, false); firstHeader.setPriorityCoefficient(1 + protocolMsg.priorityCoefficient); firstHeader.setTransportRecord(activeTransports.get(sessionType)); //Send the first frame @@ -642,33 +642,58 @@ public void sendMessage(ProtocolMessage protocolMsg) { int currentOffset = 0; byte frameSequenceNumber = 0; + byte[] dataBuffer, encryptedData; for (int i = 0; i < frameCount; i++) { - if (i < (frameCount - 1)) { - ++frameSequenceNumber; - if (frameSequenceNumber == - SdlPacket.FRAME_INFO_FINAL_CONNESCUTIVE_FRAME) { - // we can't use 0x00 as frameSequenceNumber, because - // it's reserved for the last frame - ++frameSequenceNumber; - } - } else { + + frameSequenceNumber++; + if (i == frameCount - 1) { frameSequenceNumber = SdlPacket.FRAME_INFO_FINAL_CONNESCUTIVE_FRAME; - } // end-if + } int bytesToWrite = data.length - currentOffset; if (bytesToWrite > mtu) { bytesToWrite = mtu.intValue(); } - SdlPacket consecHeader = SdlPacketFactory.createMultiSendDataRest(sessionType, sessionID, bytesToWrite, frameSequenceNumber, messageID, (byte) protocolVersion.getMajor(), data, currentOffset, bytesToWrite, protocolMsg.getPayloadProtected()); - consecHeader.setTransportRecord(activeTransports.get(sessionType)); - consecHeader.setPriorityCoefficient(i + 2 + protocolMsg.priorityCoefficient); - handlePacketToSend(consecHeader); + + SdlPacket consecutiveFrame; + if (requiresEncryption) { + //Retrieve a chunk of the data into a temporary buffer to be encrypted + dataBuffer = new byte[bytesToWrite]; + System.arraycopy(data, currentOffset, dataBuffer, 0, bytesToWrite); + + encryptedData = new byte[TLS_MAX_RECORD_SIZE]; + Integer numberOfBytesEncrypted = sdlSec.encryptData(dataBuffer, encryptedData); + if (numberOfBytesEncrypted == null || (numberOfBytesEncrypted <= 0)) { + DebugTool.logError(TAG, "Unable to encrypt data"); + return; + } + + consecutiveFrame = SdlPacketFactory.createMultiSendDataRest(sessionType, sessionID, numberOfBytesEncrypted, frameSequenceNumber, messageID, (byte) protocolVersion.getMajor(), encryptedData, 0, numberOfBytesEncrypted, true); + } else { + consecutiveFrame = SdlPacketFactory.createMultiSendDataRest(sessionType, sessionID, bytesToWrite, frameSequenceNumber, messageID, (byte) protocolVersion.getMajor(), data, currentOffset, bytesToWrite, false); + } + + consecutiveFrame.setTransportRecord(activeTransports.get(sessionType)); + consecutiveFrame.setPriorityCoefficient(i + 2 + protocolMsg.priorityCoefficient); + handlePacketToSend(consecutiveFrame); currentOffset += bytesToWrite; } } else { messageID++; + if (requiresEncryption && data != null && data.length > 0) { + //Encrypt the data before sending + byte[] encryptedData = new byte[TLS_MAX_RECORD_SIZE]; + Integer numberOfBytesEncrypted = sdlSec.encryptData(data, encryptedData); + if (numberOfBytesEncrypted == null || (numberOfBytesEncrypted <= 0)) { + DebugTool.logError(TAG, "Unable to encrypt data"); + return; + } + //Put the encrypted bytes back into the data array + data = new byte[numberOfBytesEncrypted]; + System.arraycopy(encryptedData, 0, data, 0, numberOfBytesEncrypted); + } int dataLength = data != null ? data.length : 0; - SdlPacket header = SdlPacketFactory.createSingleSendData(sessionType, sessionID, dataLength, messageID, (byte) protocolVersion.getMajor(), data, protocolMsg.getPayloadProtected()); + SdlPacket header = SdlPacketFactory.createSingleSendData(sessionType, sessionID, dataLength, messageID, (byte) protocolVersion.getMajor(), data, requiresEncryption); header.setPriorityCoefficient(protocolMsg.priorityCoefficient); header.setTransportRecord(activeTransports.get(sessionType)); handlePacketToSend(header); From e21d19897f12fcdec71455c6c507b03003fbdff2 Mon Sep 17 00:00:00 2001 From: Joey Grover Date: Tue, 23 Mar 2021 13:29:00 -0400 Subject: [PATCH 2/4] Fix receiving encrypted multiframe packets --- .../com/smartdevicelink/protocol/SdlProtocolBase.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java b/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java index 3d9a1dc21b..e3621d598d 100644 --- a/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java +++ b/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java @@ -1399,16 +1399,17 @@ protected void handleFrame(SdlPacket packet) { if (packet.getPayload() != null && packet.getDataSize() > 0 && packet.isEncrypted()) { SdlSecurityBase sdlSec = iSdlProtocol.getSdlSecurity(); - byte[] dataToRead = new byte[4096]; + byte[] dataToRead = new byte[TLS_MAX_RECORD_SIZE]; - Integer iNumBytes = sdlSec.decryptData(packet.getPayload(), dataToRead); - if ((iNumBytes == null) || (iNumBytes <= 0)) { + Integer numberOfDecryptedBytes = sdlSec.decryptData(packet.getPayload(), dataToRead); + if ((numberOfDecryptedBytes == null) || (numberOfDecryptedBytes <= 0)) { return; } - byte[] decryptedData = new byte[iNumBytes]; - System.arraycopy(dataToRead, 0, decryptedData, 0, iNumBytes); + byte[] decryptedData = new byte[numberOfDecryptedBytes]; + System.arraycopy(dataToRead, 0, decryptedData, 0, numberOfDecryptedBytes); packet.payload = decryptedData; + packet.dataSize = numberOfDecryptedBytes; } if (packet.getFrameType().equals(FrameType.Control)) { From fa9301e8a1680fbcf9ce4d8dfccf63b10366263c Mon Sep 17 00:00:00 2001 From: Joey Grover Date: Tue, 23 Mar 2021 15:19:21 -0400 Subject: [PATCH 3/4] Add PSM fix for already released Core versions This is added to deal with the case where Core would incorrectly encrypt the first frame which would make the payload length greater than 8 which is a protocol spec violation. --- .../java/com/smartdevicelink/transport/SdlPsm.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/base/src/main/java/com/smartdevicelink/transport/SdlPsm.java b/base/src/main/java/com/smartdevicelink/transport/SdlPsm.java index a142cebadb..99510b0619 100644 --- a/base/src/main/java/com/smartdevicelink/transport/SdlPsm.java +++ b/base/src/main/java/com/smartdevicelink/transport/SdlPsm.java @@ -61,14 +61,14 @@ public class SdlPsm { private static final byte FIRST_FRAME_DATA_SIZE = 0x08; private static final int VERSION_MASK = 0xF0; //4 highest bits - private static final int COMPRESSION_MASK = 0x08; //4th lowest bit + private static final int ENCRYPTION_MASK = 0x08; //4th lowest bit private static final int FRAME_TYPE_MASK = 0x07; //3 lowest bits int state; int version; - boolean compression; + boolean encrypted; int frameType; int serviceType; int controlFrameInfo; @@ -97,7 +97,7 @@ private int transitionOnInput(byte rawByte, int state) { if (version == 0) { //It should never be 0 return ERROR_STATE; } - compression = (1 == ((rawByte & (byte) COMPRESSION_MASK) >> 3)); + encrypted = (1 == ((rawByte & (byte) ENCRYPTION_MASK) >> 3)); frameType = rawByte & (byte) FRAME_TYPE_MASK; @@ -194,7 +194,10 @@ private int transitionOnInput(byte rawByte, int state) { break; case SdlPacket.FRAME_TYPE_FIRST: - if (dataLength == FIRST_FRAME_DATA_SIZE) { + if (dataLength == FIRST_FRAME_DATA_SIZE || this.encrypted) { + //In a few production releases of core the first frame could be + //encrypted. Therefore it is not an error state if the first frame data + //length is greater than 8 in that case alone. break; } default: @@ -263,7 +266,7 @@ private int transitionOnInput(byte rawByte, int state) { public SdlPacket getFormedPacket() { if (state == FINISHED_STATE) { //Log.trace(TAG, "Finished packet."); - return new SdlPacket(version, compression, frameType, + return new SdlPacket(version, encrypted, frameType, serviceType, controlFrameInfo, sessionId, dataLength, messageId, payload); } else { From 385f31462beeb2514d606203f5e5dd756750dd87 Mon Sep 17 00:00:00 2001 From: Joey Grover Date: Wed, 24 Mar 2021 14:39:19 -0400 Subject: [PATCH 4/4] Add back frame sequence rollover fix In the clean up effort this was removed but needs to be added back --- .../java/com/smartdevicelink/protocol/SdlProtocolBase.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java b/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java index e3621d598d..64578e7ee9 100644 --- a/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java +++ b/base/src/main/java/com/smartdevicelink/protocol/SdlProtocolBase.java @@ -646,6 +646,13 @@ public void sendMessage(ProtocolMessage protocolMsg) { for (int i = 0; i < frameCount; i++) { frameSequenceNumber++; + + if (frameSequenceNumber == SdlPacket.FRAME_INFO_FINAL_CONNESCUTIVE_FRAME) { + //If sequence numbers roll over to 0, increment again to avoid + //using the reserved sequence value for the final frame + ++frameSequenceNumber; + } + if (i == frameCount - 1) { frameSequenceNumber = SdlPacket.FRAME_INFO_FINAL_CONNESCUTIVE_FRAME; }